From 4f0fd3db556652f961710c64b9806512a491a7d3 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Mon, 4 Jul 2016 16:13:16 +0100 Subject: [PATCH] Bug 15006 Drop raw connection if login fails raw_connection was not behaving correctly if an invalid string was passed or a login failed. It was not checking that the login succeeded ( it checked that account existed not that it contained data and it existed even if login failed) and so failed logins instead of aborting immediately fell through into the sip_protocol_loop, forcing that to timeout invalid connections. It now checks that account has id set and returns if not. The timeout alarm is now set on the while loop, in normal running this should not be triggered as the socket is opened and the first data should be a login message and the while loop should only iterate once, but lets not go into an infinite loop due to unforeseen circumstances. I have reindented the routine as the flow was not clear (the while was not indented at all. Also if using Net::Server::PreFork when a new connection comes in you may be handed the the successful login parameters from a preceding call. Because of this you could successfully transmit transactions and Koha would carry them out without having received a valid login ( and possibly with the wrong account details!) We now delete any existing account for new connections. NB: This patch requires that the patch for bug 13807 has been applied Signed-off-by: Srdjan Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall --- C4/SIP/SIPServer.pm | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/C4/SIP/SIPServer.pm b/C4/SIP/SIPServer.pm index d556845a4b..b3f67d4ba8 100755 --- a/C4/SIP/SIPServer.pm +++ b/C4/SIP/SIPServer.pm @@ -124,25 +124,38 @@ sub process_request { sub raw_transport { my $self = shift; - my ($input); + my $input; my $service = $self->{service}; + # If using Net::Server::PreFork you may already have account set from a previous session + # Ensure you dont + if ($self->{account}) { + delete $self->{account}; + } + # Timeout the while loop if we get stuck in it + # In practice it should only iterate once but be prepared + local $SIG{ALRM} = sub { die 'raw transport Timed Out!' } + syslog('LOG_DEBUG', "raw_transport: timeout is $service->{timeout}"); + alarm $service->{timeout}; while (!$self->{account}) { - local $SIG{ALRM} = sub { die "raw_transport Timed Out!\n"; }; - syslog("LOG_DEBUG", "raw_transport: timeout is %d", $service->{timeout}); - $input = read_request(); - if (!$input) { - # EOF on the socket - syslog("LOG_INFO", "raw_transport: shutting down: EOF during login"); - return; - } - $input =~ s/[\r\n]+$//sm; # Strip off trailing line terminator(s) - last if C4::SIP::Sip::MsgType::handle($input, $self, LOGIN); + $input = read_request(); + if (!$input) { + # EOF on the socket + syslog("LOG_INFO", "raw_transport: shutting down: EOF during login"); + return; + } + $input =~ s/[\r\n]+$//sm; # Strip off trailing line terminator(s) + last if C4::SIP::Sip::MsgType::handle($input, $self, LOGIN); } + alarm 0; syslog("LOG_DEBUG", "raw_transport: uname/inst: '%s/%s'", - $self->{account}->{id}, - $self->{account}->{institution}); + $self->{account}->{id}, + $self->{account}->{institution}); + if (! $self->{account}->{id}) { + syslog("LOG_ERR","Login failed shutting down"); + return; + } $self->sip_protocol_loop(); syslog("LOG_INFO", "raw_transport: shutting down"); -- 2.39.5