[asterisk-dev] [Code Review] 4420: testsuite: Add nominal and off-nominal SRTP negotiation tests for key lifetime/MKI

[Mark Michelson]

> On Feb. 18, 2015, 8:58 p.m., Mark Michelson wrote:
> > /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-double-crypto.xml, line 49
> > <https://reviewboard.asterisk.org/r/4420/diff/1/?file=71426#file71426line49>
> >
> >     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) .*"

I omitted a colon in my corrected regex:

"a=crypto:[12] AES_CM_128_HMAC_SHA1_(32|80) .*"
         ^


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4420/#review14489
-----------------------------------------------------------


On Feb. 14, 2015, 3:26 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4420/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2015, 3:26 a.m.)
> 
> 
> Review request for Asterisk Developers and Olle E Johansson.
> 
> 
> Bugs: ASTERISK-17721, ASTERISK-17899 and ASTERISK-22748
>     https://issues.asterisk.org/jira/browse/ASTERISK-17721
>     https://issues.asterisk.org/jira/browse/ASTERISK-17899
>     https://issues.asterisk.org/jira/browse/ASTERISK-22748
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/test-config.yaml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_with_lifetime.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_unknown_suite.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_no_tag.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_no_suite.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_no_key.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline.csv PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/accept_nominal.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/accept_multiple_attrib_first_bad.xml 6415 
>   /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/accept.csv PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/tests.yaml 6415 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/test-config.yaml 6415 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-single-crypto.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-off-nominal-crypto.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/srtp-invite-double-crypto.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/inject_attrib_single_off_nominal.csv PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/inject_attrib_single_nominal.csv PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/sipp/inject_attrib_double_nominal.csv PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/srtp_sdp_offer_answer/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/run-test 6415 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast2/sip.conf 6415 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast2/extensions.conf 6415 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast1/sip.conf 6415 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/configs/ast1/extensions.conf 6415 
> 
> Diff: https://reviewboard.asterisk.org/r/4420/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150218/8f35a82c/attachment.html>