From a6853bcae2dba3ac487a6695bec7c84fee783801 Mon Sep 17 00:00:00 2001 From: Magnus Enger Date: Wed, 4 Sep 2024 08:13:05 +0200 Subject: [PATCH] Bug 37816: Stop SIP2 from logging passwords MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Koha's SIP2 server does a lot of logging, including all incoming requests, in full. This means that passwords are logged, both for the user the SIP2 client uses for logging into Koha, as well as for the end users who provide a password to e.g. check something out. This patch replaces passwords with three asterisks in log strings, before they are written to the log. To test, in ktd: - Run the new tests: $ prove t/db_dependent/SIP/Sip.t - Tail the SIP2 logs: $ sudo tail -f /var/log/koha/kohadev/sip*.log - Telnet into the SIP2 server: $ telnet localhost 6001 - Try logging in by pasting this into the telnet session: "9300CNterm1|COmypassword|CPCPL|" - Verify that "mypassword" is replaced by "***" in the logs - Try different values for the password, including the correct password which is "term1" in ktd - Try other SIP2 messages that include password fields (AC, AD, CO) --- C4/SIP/Sip.pm | 36 +++++++++++++++++++++++++++ t/db_dependent/SIP/Sip.t | 53 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 t/db_dependent/SIP/Sip.t diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm index c29bff282f..b839614daa 100644 --- a/C4/SIP/Sip.pm +++ b/C4/SIP/Sip.pm @@ -231,8 +231,44 @@ sub siplog { my $message = @args ? sprintf($mask, @args) : $mask; + $message = remove_password_from_message( $message ); + my $logger = C4::SIP::Logger::get_logger(); $logger->$method($message) if $logger; } +# remove_password_from_message( $message ) +# +# Look for the fields AC, AD and CO, and replace contents with three asterisks. +# We do this by looking for: +# delimiter + AC/AD/CO + something + delimiter +# and replcing it with: +# delimiter + field name + asterisks + delimiter +# +# Messages and fields that can contain passwords: +# 23 Patron Status Request - AC terminal password - AD patron password +# 11 Checkout - AC terminal password - AD patron password +# 09 Checkin - AC terminal password +# 01 Block Patron - AC terminal password +# 93 Login - CO login password +# 63 Patron Information - AC terminal password - AD patron password +# 35 End Patron Session - AC terminal password - AD patron password +# 37 Fee Paid - AC terminal password - AD patron password +# 17 Item Information - AC terminal password +# 19 Item Status Update - AC terminal password +# 25 Patron Enable - AC terminal password- AD patron password +# 15 Hold - AC terminal password - AD patron password +# 29 Renew - AC terminal password - AD patron password +# 65 Renew All - AC terminal password - AD patron password + +sub remove_password_from_message { + my ( $message ) = @_; + + $message =~ s/\Q${field_delimiter}\EAC.*?\Q${field_delimiter}\E/${field_delimiter}AC***${field_delimiter}/g; + $message =~ s/\Q${field_delimiter}\EAD.*?\Q${field_delimiter}\E/${field_delimiter}AD***${field_delimiter}/g; + $message =~ s/\Q${field_delimiter}\ECO.*?\Q${field_delimiter}\E/${field_delimiter}CO***${field_delimiter}/g; + + return $message; +} + 1; diff --git a/t/db_dependent/SIP/Sip.t b/t/db_dependent/SIP/Sip.t new file mode 100644 index 0000000000..901d209779 --- /dev/null +++ b/t/db_dependent/SIP/Sip.t @@ -0,0 +1,53 @@ +#!/usr/bin/perl + +# This test is db dependent: SIPServer needs MsgType which needs Auth. +# And Auth needs config vars and prefences in its BEGIN block. + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 1; + +use C4::SIP::Sip; + +subtest 'remove_password_from_message' => sub { + plan tests => 8; + + is( C4::SIP::Sip::remove_password_from_message( "INPUT MSG: '9300CNterm1|COterm1|CPCPL|'" ), + "INPUT MSG: '9300CNterm1|CO***|CPCPL|'", "93 Login - password" ); + + is( C4::SIP::Sip::remove_password_from_message( "INPUT MSG: '9300CNterm1|COt|CPCPL|'" ), + "INPUT MSG: '9300CNterm1|CO***|CPCPL|'", "93 Login - short password" ); + + is( C4::SIP::Sip::remove_password_from_message( "INPUT MSG: '9300CNterm1|CO%&/()=+qwerty123456|CPCPL|'" ), + "INPUT MSG: '9300CNterm1|CO***|CPCPL|'", "93 Login - Complex password" ); + + is( C4::SIP::Sip::remove_password_from_message( "INPUT MSG: '9300CNterm1|CO1234|CPCPL|'" ), + "INPUT MSG: '9300CNterm1|CO***|CPCPL|'", "93 Login - PIN" ); + + is( C4::SIP::Sip::remove_password_from_message( "11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC1234|AD1234|BON" ), + '11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC***|AD***|BON', "11 Checkout" ); + + is( C4::SIP::Sip::remove_password_from_message( "11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC|AD1234|BON" ), + '11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC***|AD***|BON', "11 Checkout - empty AC" ); + + is( C4::SIP::Sip::remove_password_from_message( "11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC1234|AD|BON" ), + '11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC***|AD***|BON', "11 Checkout - empty AD" ); + + is( C4::SIP::Sip::remove_password_from_message( "11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC|AD|BON" ), + '11YN20240903 134450 AOCPL|AA23529000035676|AB39999000003697|AC***|AD***|BON', "11 Checkout - empty AC and AND" ); +}; -- 2.34.1