<p>Jaco Kroon <strong>uploaded patch set #6</strong> to this change.</p><p><a href="https://gerrit.asterisk.org/9858">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">chan_sip: tcp/tls honour insecure=port<br><br>Prior to b2c4e8660a9c89d07041271371151779b7ec75f6 (ASTERISK_27457)<br>insecure=port was the defacto standard. That commit also prevented<br>insecure=port from being applied for sip/tcp or sip/tls.<br><br>Into consideration there are three sets of behaviour:<br><br>1. "previous" - before the above commit.<br>2. "current" - post above commit, pre this one.<br>3. "new" - post this commit.<br><br>The problem that the above commit tried to address was guests over TCP.<br>It succeeded in doing that but broke transport!=udp with host!=dynamic.<br><br>This commit attempts to restore sane behaviour with respect to<br>transport!=udp for host!=dynamic whilst still retaining the guest users<br>over tcp.<br><br>It should be noted that when looking for a peer, two passes is made, the<br>first pass doesn't have SIP_INSECURE_PORT set for the searched-for peer,<br>thus looking for full matches (IP + Port), the second pass sets<br>SIP_INSECURE_PORT, thus expecting matches on IP only where the matched<br>peer allows for that (in the author's opinion: UDP with insecure=port,<br>or any TCP based, non-dynamic host).<br><br>In previous behaviour the there was special handling for<br>transport=tcp|tls whereby a peer would match during the first pass if<br>the utilized transport was TCP|TLS (and the peer allowed that specific<br>transport).<br><br>This behaviour was wrong, or dubious at best. Consider two dynamic tcp<br>peers, both registering from the same IP (NAT), in this case either peer<br>could match for connections from an IP. It's also this behaviour that<br>prevented SIP guests over tcp.<br><br>The above referenced commit removed this behaviour, but kept applying<br>the SIP_INSECURE_PORT only to WS|WSS|UDP. Since WS and WSS is also TCP<br>based, the logic here should fall into the TCP category.<br><br>This patch updates things such that the previously non-explicit (TCP<br>behaviour) transport test gets performed explicitly (ie, matched peer<br>must allow for the used transport), as well as the indeterministic<br>source-port nature of the TCP protocol is taken into account. The new<br>match algorithm now looks like:<br><br>1. As per previous behaviour, IP address is matched first.<br><br>2. Explicit filter with respect to transport protocol, previous<br> behaviour was semi-implied in the test for TCP pure IP match - this now<br> made explicit.<br><br>3. During first pass (without SIP_INSECURE_PORT), always match on port.<br><br>4. If doing TCP, match if matched against peer also has<br>SIP_INSECURE_PORT.<br><br>5. Match if not a dynamic host.<br><br>To logic-test this we need a few different scenarios. Towards this end,<br>I work with a set number of peers defined in sip.conf:<br><br>[peer1]<br>host=1.1.1.1<br>transport=tcp<br><br>[peer2]<br>host=1.1.1.1<br>transport=udp<br><br>[peer3]<br>host=1.1.1.1<br>port=5061<br>insecure=port<br>transport=udp<br><br>[peer4]<br>host=1.1.1.2<br>transport=udp,tcp<br><br>[peer5]<br>host=dynamic<br>transport=udp,tcp<br><br>Test cases for UDP:<br><br>1 - incoming UDP request from 1.1.1.1:<br> - previous:<br> * peer5 if registered from 1.1.1.1 and source port matches;<br> * peer1 or peer2 if from port 5060 (ordering); else<br> * peer3.<br> - current:<br> * peer5 if registered from 1.1.1.1 and source port matches;<br> * peer1 or peer2 if from port 5060 (ordering); else<br> * peer 3.<br> - new:<br> * peer5 if registered from 1.1.1.1 and source port matches;<br> * peer2 if from port 5060; else<br> * peer 3.<br><br>2 - incoming UDP request from 1.1.1.2:<br> - previous:<br> * peer5 if registered from 1.1.1.2 and port matches (irrespective of<br> transport protocol);<br> * peer4 if source port is 5060; else<br> * no match (guest).<br> - current: as previous.<br> - new:<br> * peer5 if registered from 1.1.1.2 and port matches (and udp<br> transport is allowed by the matched peer);<br> * peer4 if source port is 5060; else<br> * no match (guest).<br><br>3 - incoming UDP request from anywhere else:<br> - previous:<br> * peer5 if registered from that address (and port if not insecure=port).<br> * no match (guest)<br> - current - as per previous<br> - new - as per previous<br><br>Test cases for TCP based transports:<br><br>4 - incoming TCP request from 1.1.1.1<br> - previous:<br> * peer1 or peer5 if peer5 registered from 1.1.1.1;<br> * peer1 (unless the source port happens to be 5060 then there is a<br> possibility based on ordering of matching peer2); else<br> - current:<br> * peer5 if registered from 1.1.1.1 and source port matches;<br> * peer1 or peer2 if and only if source port happens to be 5060<br> (ordering);<br> * no match (guest)<br> - new:<br> * peer5 if registered from 1.1.1.1 and source port matches;<br> * peer1.<br><br>5 - incoming TCP request from 1.1.1.2<br> - previous:<br> * peer4 or peer5 if peer5 registered from 1.1.1.2<br> * peer4.<br> - current:<br> * peer5 if peer5 registered from 1.1.1.2 and source port matches;<br> * peer4 if source port happens to be port 5060; else<br> * no match (guest).<br> - new:<br> * peer5 if peer5 registered from 1.1.1.2 and source port matches;<br> * peer4.<br><br>6 - incoming TCP request from anywhere else:<br> - previous:<br> * peer5 if registered from that address; else<br> * no match (guest)<br> - current:<br> * peer5 if registered from that address + port; else<br> * no match (guest)<br> - new:<br> * peer5 if registered from that address + port; else<br> * no match (guest)<br><br>It should be noted the test cases doesn't make explicit mention of TLS,<br>WS or WSS. WS and WSS previously followed UDP semantics, they will now<br>follow TCP semantics (they are based on http and https respectively,<br>both also based on top of TCP like TLS).<br><br>The previous commit specifically tried to address test-case 6, but broke<br>test-cases 4 and 5 in the process.<br><br>ASTERISK-27881 #close<br><br>Change-Id: I61a9804e4feba9c7224c481f7a10bf7eb7c7f2a2<br>---<br>M channels/chan_sip.c<br>1 file changed, 22 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/58/9858/6</pre><p>To view, visit <a href="https://gerrit.asterisk.org/9858">change 9858</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/9858"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newpatchset </div>
<div style="display:none"> Gerrit-Change-Id: I61a9804e4feba9c7224c481f7a10bf7eb7c7f2a2 </div>
<div style="display:none"> Gerrit-Change-Number: 9858 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-Reviewer: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>