Bugzilla – Attachment 175039 Details for
Bug 37816
Stop SIP2 from logging passwords
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37816: Stop SIP2 from logging passwords
Bug-37816-Stop-SIP2-from-logging-passwords.patch (text/plain), 6.63 KB, created by
Victor Grousset/tuxayo
on 2024-12-01 05:31:03 UTC
(
hide
)
Description:
Bug 37816: Stop SIP2 from logging passwords
Filename:
MIME Type:
Creator:
Victor Grousset/tuxayo
Created:
2024-12-01 05:31:03 UTC
Size:
6.63 KB
patch
obsolete
>From eccbcdcf243b47cab5a699e4d5a1ba53d3be851b Mon Sep 17 00:00:00 2001 >From: Magnus Enger <magnus@libriotech.no> >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) > >Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> >--- > 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 667e0c83ac..a5b33b6d7b 100644 >--- a/C4/SIP/Sip.pm >+++ b/C4/SIP/Sip.pm >@@ -230,8 +230,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 <http://www.gnu.org/licenses>. >+ >+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.47.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37816
:
171011
|
175039
|
175079
|
175639
|
176717
|
176718