<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/4420/">https://reviewboard.asterisk.org/r/4420/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 18th, 2015, 8:58 p.m. UTC, <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/4420/diff/1/?file=71426#file71426line49" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-double-crypto.xml</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">49</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nt"><ereg</span> <span class="na">regexp=</span><span class="s">"a=crypto:[1-2] AES_CM_128_HMAC_SHA1_[32|80].*"</span></pre></td>
</tr>
</tbody>
</table>
<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 regex being used here has some issues.
First the bit towards the end for [32|80] will match values you don't want it to. Square brackets defines a character class, which matches only a single character. I *think* that this would match a single 3, a single 2 or 8, or a single 0. This means that the expected valid values of 32 or 80 would be accepted, but it also means invalid values of 3, 0, 81, 39, etc. would be accepted.
Also, .* being used afterwards means that completely invalid values like AES_CM_128_HMAC_SHA1_32HIMOM would be accepted. Asterisk should be placing a space after the crypto suite in all of its 200 OK responses, so this space should be included in the regex.
The regex should be
"a=crypto[12] AES_CM_128_HMAC_SHA1_(31|80) .*"</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I omitted a colon in my corrected regex:
"a=crypto:[12] AES_CM_128_HMAC_SHA1_(32|80) .*"
^</pre>
<br />
<p>- Mark</p>
<br />
<p>On February 14th, 2015, 3:26 a.m. UTC, Matt Jordan 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.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and Olle E Johansson.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated Feb. 14, 2015, 3:26 a.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-17721">ASTERISK-17721</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-17899">ASTERISK-17899</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-22748">ASTERISK-22748</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
testsuite
</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;">This patch adds nominal and off-nominal tests for SDP negotiation of SDES-SRTP crypto attributes, for both chan_sip and chan_pjsip.
Specifically:
* Moved the tests/channels/SIP/sip_srtp test to tests/channels/SIP/sip_srtp/srtp_call. This change was done merely to allow for additional SRTP tests for chan_sip, and is not part of this review. Note that due to a quirk of Review Board, the moved files don't show up in the review.
* Added tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer. This covers both nominal and off-nominal SDP offers with SDES-SRTP for chan_sip. Note that the scenarios use injection files to vary the crypto attributes.
* Updated tests/channels/PJSIP/srtp_negotiation.
- All but two off-nominal scenarios were moved into a single off nominal scenario, decline.xml. This uses the same injection file for off nominal parameter testing as the chan_sip variant.
- Updated accept_nominal.xml to use an injection file to test multiple nominal values.
- Updated accept_multiple_attrib_first_bad.xml to not use a crypto attribute with lifetime as a 'bad' attribute.</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>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/test-config.yaml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_with_lifetime.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_unknown_suite.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_no_tag.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_no_suite.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_no_key.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline.csv <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/accept_nominal.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/accept_multiple_attrib_first_bad.xml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/accept.csv <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/tests.yaml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/test-config.yaml <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-single-crypto.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-off-nominal-crypto.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-double-crypto.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/inject_attrib_single_off_nominal.csv <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/inject_attrib_single_nominal.csv <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/inject_attrib_double_nominal.csv <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/configs/ast1/sip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/run-test <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast2/sip.conf <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast2/extensions.conf <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast1/sip.conf <span style="color: grey">(6415)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast1/extensions.conf <span style="color: grey">(6415)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4420/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>