Attachment 53355 Details for Bug 15006 – [SIGNED-OFF] Bug 15006 Drop raw connection if login fails

[patch] [SIGNED-OFF] Bug 15006 Drop raw connection if login fails

Bug-15006-Drop-raw-connection-if-login-fails.patch (text/plain), 3.63 KB, created by Srdjan Jankovic on 2016-07-13 00:55:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Srdjan Jankovic

Created: 2016-07-13 00:55:37 UTC

Size: 3.63 KB

patch

obsolete

>From 2fba32e912f0b4fffcd718e2ff87d4b98b1b790b Mon Sep 17 00:00:00 2001
>From: Colin Campbell <colin.campbell@ptfs-europe.com>
>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 <srdjan@catalyst.net.nz>
>---
> 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 d556845..b3f67d4 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.7.4

Actions: View | Diff | Splinter Review

Attachments on bug 15006: 45039 | 45583 | 45594 | 52477 | 52478 | 52479 | 52480 | 52784 | 52785 | 52786 | 52787 | 52849 | 52850 | 52851 | 52852 | 52853 | 53009 | 53010 | 53011 | 53012 | 53013 | 53014 | 53132 | 53133 | 53134 | 53135 | 53309 | 53310 | 53311 | 53312 | 53355 | 53356 | 53357 | 53358 | 53359 | 53360 | 53363 | 53364 | 53365 | 53366 | 53367 | 53368 | 53369