<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/1005/">https://reviewboard.asterisk.org/r/1005/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 9th, 2010, 12:33 p.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 think it's very dangerous to disable the verification of the auth. There has to be another way of fixing this. The retransmit should hit the same dialog and we still have the nonce. Why does the auth fail?</pre>
</blockquote>
<p>On November 9th, 2010, 1:27 p.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;">The auth fails because the nonce can only be responded to once... When a retransmission comes in check_auth() sees that the nonce has already been responded to and issues a new challenge. I don't think this is correct behavior. In the case of a retransmit the current nonce probably shouldn't be treated as a stale nonce.</pre>
</blockquote>
<p>On November 9th, 2010, 1:31 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;">We do the same thing in handle_request_invite.
if (!p->lastinvite && !ast_test_flag(req, SIP_PKT_IGNORE) && !p->owner) {
/* This is a new invite */
/* Handle authentication if this is our first invite */
res = check_user(p, req, SIP_INVITE, e, XMIT_RELIABLE, sin);
It is only disabling verification for retransmissions (unless I have messed up somewhere) and I checked that retransmissions that weren't authed aren't accepted.
It fails the verification with "Duplicate authentication received from ..." because the auth request has already been responded to and we treat the nonce as stale. We then re-generate a 401, which is what breaks the polycom implementation.</pre>
</blockquote>
<p>On November 9th, 2010, 2:24 p.m., <b>Klaus Darilion</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 agree disabling authentication sounds weird. The problem is on the transaction layer and thus the only clean solution would be to add a transaction layer - and all these problems will disappear.</pre>
</blockquote>
<p>On November 9th, 2010, 2:32 p.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;">Then I think we should fix the way we handle all retransmission of an authenticated message, regardless of type. We should not bypass verification of the auth at all. If we do, we need to consider if that's a security issue.</pre>
</blockquote>
<p>On November 9th, 2010, 2:33 p.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;">Klaus - yes, that is the religous correct issue but that's going to be very hard. We need a simple fix for 1.4 and that's not to rewrite the stack. But it's not to disable authentication either. </pre>
</blockquote>
<p>On November 9th, 2010, 9: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;">Klaus: Yes, having a transaction layer in chan_sip would be nice. Keep dreaming. :-)
Olle: Again, the method I used has been used for INVITES (which are far more dangerous than SUBSCRIBEs) for ages. That siad, I can "fix" the auth code by allowing the nonce to be re-used in the case of retransmitted packets, but I worry that it might open us up to some kind of replay attack because of some assumption buried in the code somewhere else. In fact, the patch is fairly simple. The important part is:
Index: channels/chan_sip.c
===================================================================
--- channels/chan_sip.c (revision 294499)
+++ channels/chan_sip.c (working copy)
@@ -9223,7 +9223,7 @@
/* Verify nonce from request matches our nonce, and the nonce has not already been responded to.
* If this check fails, send 401 with new nonce */
- if (strcasecmp(p->randdata, keys[K_NONCE].s) || p->stalenonce) { /* XXX it was 'n'casecmp ? */
+ if (strcasecmp(p->randdata, keys[K_NONCE].s) || (!ignore && p->stalenonce)) { /* XXX it was 'n'casecmp ? */
wrongnonce = TRUE;
usednonce = keys[K_NONCE].s;
} else {
and it solves the issue--it just scares me a bit. Opinions?</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;">As all code properly handles SIP_PKT_IGNOREd packets, this should be safe. I'm not sure how big an IF that is. I've tried breaking it with INVITE and SUBSCRIBE w/o much luck.</pre>
<br />
<p>- Terry</p>
<br />
<p>On November 9th, 2010, 12:21 p.m., Terry Wilson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/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 David Vossel.</div>
<div>By Terry Wilson.</div>
<p style="color: grey;"><i>Updated 2010-11-09 12:21:48</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;">From the issue:
0018075: Asterisk fails to recognize SUBSCRIBE retransmissions and tries to re-authenticate them, which breaks presence on polycom phones
Description Here's what happens from asterisk point of view:
-> SUBSCRIBE
<- 401 Unauthorized
-> SUBSCRIBE with auth
<- 200 OK (lost)
<- NOTIFY
-> 200 OK for NOTIFY
-> SUBSCRIBE with auth retransmission
<- 401 Unauthorized
At this point polycom phone received 2 "401 Unauthorized" in a row and it completely gives up on resubscribing to this hint until reboot. While polycom behavior is questionable, I believe asterisk should recognize retransmission and resubmit 200OK in this case</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;">I created a sipp scenario mimicking the issue, it now re-sends a 200 OK when an authed retransmission occurs. I also tested a scenario that sent a retransmission without authentication to ensure that we didn't accept un-authed retransmissions.</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/view.php?id=18075">18075</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>/branches/1.4/channels/chan_sip.c <span style="color: grey">(294120)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1005/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>