Bugzilla – Attachment 175639 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.13 KB, created by
Victor Grousset/tuxayo
on 2024-12-18 08:21:01 UTC
(
hide
)
Description:
Bug 37816: Stop SIP2 from logging passwords
Filename:
MIME Type:
Creator:
Victor Grousset/tuxayo
Created:
2024-12-18 08:21:01 UTC
Size:
6.13 KB
patch
obsolete
>From cfd5f4b260e83ff53aefa8482e3bd3dd98e64e3f 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) > >Update 2024-12-03: Fix issues pointed out by QA. > >Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> >--- > C4/SIP/Sip.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++++++--- > t/SIP/Sip.t | 53 +++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 103 insertions(+), 4 deletions(-) > >diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm >index 667e0c83ac..189b8b54cd 100644 >--- a/C4/SIP/Sip.pm >+++ b/C4/SIP/Sip.pm >@@ -1,6 +1,9 @@ >-# >-# Sip.pm: General Sip utility functions >-# >+ >+=head1 NAME >+ >+C4::SIP::Sip - General Sip utility functions >+ >+=cut > > package C4::SIP::Sip; > >@@ -230,8 +233,53 @@ 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; > } > >+=head2 remove_password_from_message >+ >+ my $safe_message = 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 >+ >+=cut >+ >+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/SIP/Sip.t b/t/SIP/Sip.t >index 1dc7ed1684..8f58f46bb8 100755 >--- a/t/SIP/Sip.t >+++ b/t/SIP/Sip.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 9; >+use Test::More tests => 10; > use Test::Warn; > > BEGIN { >@@ -55,3 +55,54 @@ warning_is { $invalidTest = C4::SIP::Sip::Checksum::verify_cksum("1234567") } > 'verify_cksum prints the expected warning for an invalid checksum'; > is($invalidTest, 0, "Checksum: 1234567 is invalid as expected"); > >+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