Bug 13252 - SIP2 server should accept IPv6 connections
Summary: SIP2 server should accept IPv6 connections
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Benjamin Rokseth
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-13 17:16 UTC by Benjamin Rokseth
Modified: 2016-06-21 21:37 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13252 - SIP2 server should accept IPv6 connections (1.67 KB, patch)
2014-11-13 19:45 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Signed off (1.73 KB, patch)
2014-11-17 10:43 UTC, Colin Campbell
Details | Diff | Splinter Review
Supplementary patch (1.53 KB, patch)
2014-11-27 16:29 UTC, Colin Campbell
Details | Diff | Splinter Review
[PASSED QA] Bug 13252 - SIP2 server should accept IPv6 connections (1.80 KB, patch)
2014-12-27 16:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13252 Allow for IPv6 formatted addresses in Port definition (1.60 KB, patch)
2014-12-27 17:11 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Rokseth 2014-11-13 17:16:01 UTC
Any IPv6 socket connection to SIP2 server will fail with


    Bad arg length for Socket::unpack_sockaddr_in, length is 28, should be 16 at /usr/lib/perl5/Socket.pm line 806.

This could be easily circumvented with a check on socket on connection as Socket already supports IPv6
Comment 1 Benjamin Rokseth 2014-11-13 19:45:37 UTC Comment hidden (obsolete)
Comment 2 Colin Campbell 2014-11-17 10:43:11 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2014-11-17 21:45:40 UTC
Could someone else take a look at this? My Ubuntu doesn't seem to show the problem.
Comment 4 Benjamin Rokseth 2014-11-18 12:48:13 UTC
(In reply to Katrin Fischer from comment #3)
> Could someone else take a look at this? My Ubuntu doesn't seem to show the
> problem.

Hi Katrin,
Actually it seems SIPServer takes any Net::Socket arguments, so you can override ipv setting in the SIPconfig.xml (I ended up setting mine to ipv4 until this patch is pushed)

in the server-params section, add:

ipv='6'

to force ipv6

Test plan:

before patch:
 - section server-params in SIPconfig.xml, add ipv='4'
 - start sip server and do a test login - success
 - modify ipv='4' to ipv='6'
 - restart sip server and do test login - failure, in sip-output.log 
   you should see error on bad login

after patch:
  - both scenarios should succeed
Comment 5 Katrin Fischer 2014-11-20 22:44:51 UTC
This doesn't work for me. As soon as I change to ipv='6' the sip server won't start. It works with 4?
Comment 6 Katrin Fischer 2014-11-23 12:10:05 UTC
I'd be happy if someone else could take a look.
Comment 7 Colin Campbell 2014-11-27 13:05:25 UTC
(In reply to Katrin Fischer from comment #5)
> This doesn't work for me. As soon as I change to ipv='6' the sip server
> won't start. It works with 4?

Do you have an ipv6 interface defined,? If in the config you have listeners defined on ip4 addresses the Net::Server will be unable to bind an ip6 connection to them.
Comment 8 Katrin Fischer 2014-11-27 13:10:04 UTC
Hi Colin, thx for taking a look!

I am quite sure it's something like that, but I don't understand what I'd need to do to make it work. Could you maybe continue with this patch? I think your understanding of how this all works is much better than mine :)
Comment 9 Colin Campbell 2014-11-27 14:40:36 UTC
(In reply to Katrin Fischer from comment #8)
> Hi Colin, thx for taking a look!
> 
> I am quite sure it's something like that, but I don't understand what I'd
> need to do to make it work. Could you maybe continue with this patch? I
> think your understanding of how this all works is much better than mine :)

Yes I'm trying to rig up a test scenario at the moment to confirm
Comment 10 Colin Campbell 2014-11-27 16:29:56 UTC Comment hidden (obsolete)
Comment 11 Colin Campbell 2014-11-27 16:47:41 UTC
I tested this and if no interface restriction (i.e specific ip address) is given on an ipv6 machine the server will handle connections using both ipv6 and ipv4. I did find in the course of testing that if you use the port attribute to specify an ip address you can fall foul of it failing to recognise an ipv6 address, I've added a supplementary patch that works around this. 

Otherwise with the patch installed it handles messages from clients runnng both protocols. The server should be logging the setting up of the ports heres what I get on my test machine 
Nov 27 16:04:11 zazou koha_sip[11385]: Resolved [*]:6003 to [::]:6003, IPv6
Nov 27 16:04:11 zazou koha_sip[11385]: Not including resolved host [0.0.0.0] IPv4 because it will be handled by [::] IPv6
Nov 27 16:04:11 zazou koha_sip[11385]: Binding to TCP port 6003 on host :: with IPv6
Comment 12 Katrin Fischer 2014-12-27 16:20:38 UTC
Created attachment 34726 [details] [review]
[PASSED QA] Bug 13252 - SIP2 server should accept IPv6 connections

This small patch adds a check on the SIP2 socket connection if it is
IPv6 and resolves socket address accordingly.

Any newer Debian distro would probably default to IPv6 so it would
eventually affect all SIP servers.

Tests against running SIP server on an IPv6 box:

http://wiki.koha-community.org/wiki/Koha_SIP2_server_setup#Testing_with_Telnet

before patch:
  disconnects immediately. Log output:
  Bad arg length for Socket::unpack_sockaddr_in, length is 28, should be 16

after patch:
  operates normally

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2014-12-27 16:21:40 UTC
This time around I could get this to work. With both ipv 6 and 4 the login succeeded.
Comment 14 Katrin Fischer 2014-12-27 17:11:48 UTC
Created attachment 34727 [details] [review]
[PASSED QA] Bug 13252 Allow for IPv6 formatted addresses in Port definition

The SIP config has allowed you to specify an interface ip as
part of the listeners/service/port attributei
 e. g.  as port="127.0.0.1:6001/tcp"
with IPv6 the equivalent would normally be
   as port="[::1]:5001/tcp"
However in this case incoming connections will get rejected because
Configuration constructs a string without the brackets
This patch makes tests both formats on incoming connections so that
they are accepted as they were previously

In future the best course is not to include a port identifier in the
port definition then if the server has ipv6 it will bind to all
interfaces and accept both IPv4 and IPv6 traffic

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Tomás Cohen Arazi 2014-12-28 23:55:24 UTC
Patch pushed to master.

Thanks Benjamin, and congratulations for your first patch pushed!