<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2851/">https://reviewboard.asterisk.org/r/2851/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 17th, 2013, 4:39 p.m. UTC, <b>Paul Belanger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So, had a chance to play more with this last night. FWIW, this patch broke my configuration, specifically if outboundproxy was setup, any SIP context with type=peer would break. IMO, this is the wrong functionality, what use case would setting a global outboundproxy be used? Additionally, I believe the patch should only affect outbound calls, since the name of the setting is outboundproxy.
Either way, after updating my configuration to work with the patch (create a peer specific outboundproxy), I actually didn't need to apply to patch at all. Either way, I believe there needs to be some more discussion about this scenario before 'ship it' can be applied.</pre>
</blockquote>
<p>On September 17th, 2013, 4:48 p.m. UTC, <b>Joshua Colp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just so other people understand why this broke his configuration:
Host based matching was being used, and as this change makes it so chan_sip does not do resolution when a global outbound proxy is specified chan_sip did not know the IP address and thus did not match it.</pre>
</blockquote>
<p>On September 17th, 2013, 5:52 p.m. UTC, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So, using Olle's example + 1 other peer:
[mypeer]
type=peer
host=oej.digium.com
outboundproxy=edvina.net
[otherpeer]
type=peer
host=oej.digium.com
Asterisk would resolve "oej.digium.com" for otherpeer, but would not for mypeer. That seems correct to me.
To Paul's point that a global outboundproxy would be used for all SIP peers, that seems intuitive, as it is in line with all other global settings. I think there is a valid use case for defining a global outboundproxy: all SIP traffic going out of your Asterisk system is relayed through a proxy.
Paul's other point is potentially a concern: I'm not sure how this should affect the inbound message traffic either. There are some times that an inbound request will attempt to look up a peer by address (inbound authentication for requests other than REGISTER/SUBSCRIBE, MWI NOTIFY requests). Since the host is now unresolvable, I'm not sure what this patch will do to that functionality. </pre>
</blockquote>
<p>On September 17th, 2013, 5:55 p.m. UTC, <b>Joshua Colp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">With the patch the peer will not be found in that scenario, since it is unresolved. We can't really have it both ways at the same time...</pre>
</blockquote>
<p>On September 17th, 2013, 6:20 p.m. UTC, <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Darn Peer to prove me fatally wrong here. I need to reconsider how we should solve this. Thank you Paul for finding the bug and everyone else involved in this mess. I need some time to go back and think about the architecture we need to solve this and it won't be tomorrow. I don't want to have one peer for the outbound proxy and another for if we receive calls from the peer. If we have peer in the general section - we might want to be able to disable it in a peer to enable bidirectional comm.
What a mess. </pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thinking about this some more.
When an outbound proxy is in use, the host is for informational purposes only, and the requests/responses from Asterisk to the proxy are sent directly to the address of the outbound proxy. I think we're all in agreement that this is a "good thing".
Requests from the outbound proxy are still relayed from a known endpoint to Asterisk. The problem is just that peers typically look things up by IP address, and we do not (and probably never will know) the actual address of the peer. It seems like we have a few options:
* If we receive something from something we don't know (proxy!), and we fail to look up the source by address, then:
* Look up by username. If we get a username match, and the address that sent the request matches the proxy, and the peer with that username has an outbound proxy, associate the request with that peer
* Otherwise, subject the request to the 'normal' rules
Josh/Olle: what do you guys think?
</pre>
<br />
<p>- Matt</p>
<br />
<p>On September 14th, 2013, 7:30 p.m. UTC, Joshua Colp wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Joshua Colp.</div>
<p style="color: grey;"><i>Updated Sept. 14, 2013, 7:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21231">ASTERISK-21231</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21694">ASTERISK-21694</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">During the development of 1.8 chan_sip was moved to using dnsmgr for host resolution. These changes introduced a regression where all hosts were looked up in DNS, including when an outbound proxy is in use. This is incorrect. The attached change removes this requirement.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Configured an outbound proxy at global and peer level, confirmed that no resolution occurs for the host and also that traffic goes to where it should. Removed outbound proxy and confirmed things returned to normal.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(398742)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2851/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>