[Asterisk-code-review] chan sip: tcp/tls honour insecure=port (asterisk[master])

[Jaco Kroon]

Hello Richard Mudgett, Jenkins2, 

I'd like you to reexamine a change. Please visit

    https://gerrit.asterisk.org/9858

to look at the new patch set (#5).

Change subject: chan_sip: tcp/tls honour insecure=port
......................................................................

chan_sip: tcp/tls honour insecure=port

Prior to b2c4e8660a9c89d07041271371151779b7ec75f6 (ASTERISK_27457)
insecure=port was the defacto standard.  That commit also prevented
insecure=port from being applied for sip/tcp or sip/tls.

This partially restores the previous behaviour:

For UDP, WS and WSS, insecure=port was required to match based only on
IP.
For TCP and TLS matching was always done only on IP.

Current behaviour is completely broken.

The new behaviour is a little bit more complex:

1.  Firstly the IP has to match (as per previous).
2.  The protocol has to match now (ie, if we're trying to find a UDP
peer, then the entry that we find needs to have UDP set as one of the
transports).
3.  If doing the first pass, then perform port-match irrespecive of
protocol.
4.  If doing the second pass (ie, with SIP_INSECURE_PORT), then handle
UDP as per previous (ie, if we have a matching peer with insecure=port,
match it).  For everything else, match only if we have a host= that is
not dynamic set for that peer.

This fully retains previous behaviour for UDP.

Sample config:

[peer1]
host=1.1.1.1
transport=tcp

[peer2]
host=1.1.1.1
transport=udp

[peer3]
host=1.1.1.2
transport=udp,tcp

[peer4]
host=dynamic
transport=udp,tcp

Use cases:

1 - Incoming TCP request from 1.1.1.1:
 - will match peer1 unless a more specific match is made for peer4 by
 way of port match (first pass, registration).  Previously this would
 match whichever peer was earlier in the peer list.

2 - Incoming UDP request from 1.1.1.1:
 - will match peer2 if the port is 5060; else
 - will match peer4 if there registered port matches; else
 - will match peer2 if if insecure=port is set for peer2.

For 1 and 2, would always match the first defined peer, possibly
resulting in peer not allowed to use transport during a later check.

3 - Incoming TCP/UDP from 1.1.1.2
 - combined as per 1 + 2, against peer3 instead of peer1 or peer2, as
 per previous.

Previously WS and WSS peers would need to be dynamic to match correctly,
or only have one registration from a given source IP.

Incoming UDP/TCP from anywhere else can only match peer4, if, and only
the source port matches the registered address, unless insecure=port is
set in which case incoming UDP could match (and will match the first
such peer) with insecure=port.

There is a possible problem here that for dynamic hosts we really should
match the registered protocol too, but I don't know how to do that, and
this wasn't previously the case anyway so should not be considered a
regression (and some existing use-case could conceivably conflict with
this).

ASTERISK-27881 #close

Change-Id: I61a9804e4feba9c7224c481f7a10bf7eb7c7f2a2
---
M channels/chan_sip.c
1 file changed, 23 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/58/9858/5
-- 
To view, visit https://gerrit.asterisk.org/9858
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a9804e4feba9c7224c481f7a10bf7eb7c7f2a2
Gerrit-Change-Number: 9858
Gerrit-PatchSet: 5
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180817/60946ea7/attachment-0001.html>