<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/2204/">https://reviewboard.asterisk.org/r/2204/</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 26th, 2012, 3:03 p.m., <b>Mark Michelson</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 actual process_sdp() changes are good. Ship it!
The thing that bothers me a bit is that we don't always check the return value of process_sdp(). The times we check are when processing INVITEs, 200 OKs, and ACKs. For any early media responses, we will not check the return value of process_sdp() (we will sometimes set a variable, but then we don't actually do anything with it). What implications might this have if someone sends an early media SDP with SAVP audio but no encryption attributes?</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;">Cases to consider: 180, 182, 183, and any other 1xx message which is handled by the generic case and has SDP
This change will not affect the processing of these messages during initial invites (just catches it a little earlier).
This will affect these messages in reinvites where SAVP had previously been negotiated. The result in these cases is no audio (SRTP unprotect failures) when the key has changed but was not included or working audio when the key has not changed and was not included, assuming that the media endpoint address was not modified by the malformed SDP in question.</pre>
<br />
<p>- opticron</p>
<br />
<p>On November 21st, 2012, 2:28 p.m., opticron 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.</div>
<div>By opticron.</div>
<p style="color: grey;"><i>Updated Nov. 21, 2012, 2:28 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;">Prior to this patch, Asterisk would accept encrypted media streams (RTP/SAVP audio and video) without ensuring cryptographic keys were present on reinvites. This patch ensures that the incoming SDP is consistent with RFC4568 as far as having a crypto attribute present for any SAVP streams.</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;">Tested with a broken (snom 320 with 7.3.30 firmware) and non-broken (CSIPSimple on Android) client to ensure reinvites were rejected when malformed.</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/AST-1040">AST-1040</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.8/channels/chan_sip.c <span style="color: grey">(376388)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2204/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>