<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/1533/">https://reviewboard.asterisk.org/r/1533/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 20th, 2011, 11:13 a.m., <b>David Vossel</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;">I'm not sure we should make the registration change change for rejecting Registrations without a user name in the To header. We have probably treated the domain as the user name when no user name is present since the beginning of chan_sip. I certainly hope this logic isn't being depended on somewhere, but since we allow it, it probably is. Below is what I found on the user name in the To header URI. Unless there is some other verbiage I missed that uses the "MUST" key word in relation to this behavior, it seems to me like this falls in the category of being liberal about what we accept and strict about what we send. Does it cause any problems the way it is?
RFC 3261 section 10.2.
To: The To header field contains the address of record whose
registration is to be created, queried, or modified. The To
header field and the Request-URI field typically differ, as
the former contains a user name. This address-of-record MUST
be a SIP URI or SIPS URI.</pre>
</blockquote>
<p>On October 31st, 2011, 7:20 a.m., <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;">sip:digium.com is a valid SIP uri. The question here is if it's valid to register to it. I think it is.
Section 10.2 says this but is not normative:
"To: The To header field contains the address of record whose
registration is to be created, queried, or modified. The To
header field and the Request-URI field typically differ, as
the former contains a user name. This address-of-record MUST
be a SIP URI or SIPS URI."
The definition of address-of-record is:
" Address-of-Record: An address-of-record (AOR) is a SIP or SIPS URI
that points to a domain with a location service that can map
the URI to another URI where the user might be available.
Typically, the location service is populated through
registrations. An AOR is frequently thought of as the "public
address" of the user."
I e-mailed sip-implementors to see if someone can clear this up.</pre>
</blockquote>
<p>On November 9th, 2011, 4:56 p.m., <b>Terry Wilson</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;">> Unless there is some other verbiage I missed that uses the "MUST" key word in relation to this behavior, it seems to me like this falls in the category of being liberal about what we accept and strict about what we send. Does it cause any problems the way it is?
Since it says "This address-of-record MUST be a SIP URI or SIPS URI" and a SIP URI is defined as:
SIP-URI = "sip:" [ userinfo ] hostport uri-parameters [ headers ]
which shows that userinfo is optional, but hostport is required, then the RFC does say that we MUST treat the sip:blah as a domain (hostport) URI. As far as causing problems, it breaks the domain= setting in sip.conf for limiting requests by the domain they are addressed to. After staring at things for a while, I think fixing this would be a good idea.</pre>
</blockquote>
<p>On November 9th, 2011, 5:57 p.m., <b>Terry Wilson</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;">To clarify, though the hostport is required in the SIP URI, I don't think I would say that the username is. Just because the Request-URI and the AOR in the To: header *typically* differ, it does not seem that they are required to differ. I would keep the check on the empty domain name, but not reject an empty username. In fact, I would probably, just to make things consistent set an empty name to the domain and treat a REGISTER with To: <example.com> as a registration for peer [example.com].</pre>
</blockquote>
<p>On November 10th, 2011, 3:47 a.m., <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;">I would say that it's up to local policy on how to handle this. In Asterisk a INVITE to sip:terry.edv will look for extension "s".
Looking for a peer named "example.com" would be to implement a whole new level of peer matching, something I resist to do at this point. To be very clear: It would be very bad.
I would simply treat this case as "no match with existing peer" and continue along that code path.
</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;">Actually, looking for a peer named "example.com" is what we have done for most of Asterisk history. It is in 1.8 that parsing changed so that we didn't do this. So, I don't think this would be very bad. Also, this isn't about an INVITE to sip:terry.edv, it is about the From header of that request. Since it isn't the RURI we're talking about, the extension it goes to wouldn't come into play.
Since we didn't document that there was any change between versions, it was an unintended change and I think we should maintain consistency between versions when we can.</pre>
<br />
<p>- Terry</p>
<br />
<p>On October 19th, 2011, 12:30 p.m., wdoekes wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/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 and Paul Belanger.</div>
<div>By wdoekes.</div>
<p style="color: grey;"><i>Updated Oct. 19, 2011, 12:30 p.m.</i></p>
<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;">See the bug report: there were some XXX'es in the code about code that should be removed.
The patch does this:
(1) register_verify won't accept a To: without user-part anymore (illegal according to rfc3261, 10.2)
(2) check_user_full still doesn't require a user-part, but it won't match usernames by domain anymore. (i.e. it doesn't treat sip:domain as sip:domain@domain anymore)
(3) there was some freaky logic going on in get_msg_text, I had to rewrite it to make it make sense.
(4) in the reqresp parser there were lots of if (params) inside a big if (params) block. I scrapped the useless if's.</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;">It compiles.</pre>
</td>
</tr>
</table>
<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-18389">ASTERISK-18389</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/channels/chan_sip.c <span style="color: grey">(341249)</span></li>
<li>/trunk/channels/sip/reqresp_parser.c <span style="color: grey">(341249)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1533/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>