Summary:ASTERISK-20849: SDP crypto attribute is not well formed in the SDP ANSWER
Reporter:José Luis Millán (jmillan)Labels:
Date Opened:2012-12-29 05:41:19.000-0600Date Closed:2013-01-29 11:13:58.000-0600
Versions:SVN 10.11.1 11.1.0 Frequency of
Environment:Attachments:( 0) ASTERISK-20849-1.8.diff
( 1) ASTERISK-20849-11.diff
( 2) fix_sdp_crypto_tags.diff
( 3) issue_20849_full_log
Description:The crypto tag in the SDP ANSWER is not being generated according to the crypto tag in the SDP OFFER for the chosen crypto attribute. This makes the offerer reject the SDP ANSWER as it is malformed.


For the following crypto lines in the SDP OFFER:

a=crypto:0 AES_CM_128_HMAC_SHA1_32 inline:cpascljg+FDoOgsFyVirWHQjGXGp5WTEiVU2SuYC
a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:i5JOAu02aPN5MaXlbwJofff1opYOd2mDJ21pTejP

Asterisk replies with:
a=crypto:1 AES_CM_128_HMAC_SHA1_32 inline:vdrJnisX2hBpcQvcZbpMmR9IG+Dc0EpEP1iC1EbQ

While the correct reply is:
a=crypto:0 AES_CM_128_HMAC_SHA1_32 inline:vdrJnisX2hBpcQvcZbpMmR9IG+Dc0EpEP1iC1EbQ

As per RFC4568 section 5.1:

When an offered crypto attribute is accepted, the crypto attribute in
  the answer MUST contain the following:

  *  The tag and crypto-suite from the accepted crypto attribute in the
     offer (the same crypto-suite MUST be used in the send and receive

Manually rewriting the ANSWER crypto tag accordingly before sdp is processed in the offerer does the trick.
Comments:By: Iñaki Baz Castillo (ibc) 2012-12-29 08:25:36.348-0600

Good point. Hope this bug is fixed soon.

By: Michael L. Young (elguero) 2012-12-29 22:51:48.465-0600

We require a complete debug log to help triage the issue. This document will provide instructions on how to collect debugging logs from an Asterisk machine for the purpose of helping bug marshals troubleshoot an issue: https://wiki.asterisk.org/wiki/display/AST/Collecting+Debug+Information

By: Iñaki Baz Castillo (ibc) 2012-12-30 16:12:06.718-0600

Michael, the debug you ask for provides no useful information about the reported issue. The issue description properly references the RFC 4568 section 5.1 in which it's stated how the crypto answer must be generated, and Asterisk does not implement it correctly.

The issue description is perfect, provides what is buggy and how it should be, and no debug or log of Asterisk will provide more information. Please address the issue. Thanks a lot.

By: Pedro Kiefer (pedrokiefer) 2012-12-30 19:24:57.793-0600

Not sure if the problem is related to line 297 of sip/sdp_crypto.c, which reads:
if (snprintf(crypto_buf, sizeof(crypto_buf), "a=crypto:1 AES_CM_128_HMAC_SHA1_%i inline:%s\r\n"
               taglen, p->local_key64) < 1) {

Having a hardcoded crypto tag would explain this bug. On the other hand, sdp_crypto_process seems to get it right:
snprintf(p->a_crypto, attr_len + 10, "a=crypto:%s %s inline:%s\r\n", tag, suite, p->local_key64);

But the string saved to p->a_crypto on sdp_crypto_process is ignored by sdp_crypto_offer, which I think shouldn't happen. If we already have a valid a_crypto line, use it, other wise create a new one, taking in account the tag number which was received (or 0 if we are making the offer).

By: Michael L. Young (elguero) 2012-12-30 21:50:18.222-0600

Pedro, I took a look at the code and saw the same thing you pointed out before requesting more information.  But, I wanted to try and see _why_ we are "offering" something when we should be processing the sdp and responding based on that sdp instead of behaving like we are sending an initial offer.  Hence, the reason why I asked for debug info to help see the sequence of events that leads to this issue.

Iñaki, the description was very clear in explaining _what_ is happening.  I am just trying to figure out _why_ it is happening.  I volunteer my time to help out and having debug information is very helpful in quickly pinpointing _where_ one should be looking.  We are trying to "address the issue"... just need some help to do so.


By: José Luis Millán (jmillan) 2012-12-31 03:16:21.517-0600

Attached the log (issue_20849_full_log) of an OFFER which has an only crypto line:

a=crypto:0 AES_CM_128_HMAC_SHA1_32 inline:HqxgWh4M0amLPmSIuqwWeDmL1K0Ad93YBszrGv8b

And is responded with:

a=crypto:1 AES_CM_128_HMAC_SHA1_32 inline:5556vYJkh9HdCAEiGcaucMb0DPUA4FjES1PHFDnM

By: Iñaki Baz Castillo (ibc) 2012-12-31 04:17:15.594-0600

I've added some log functions in {{sdp_crypto_process}} and confirm that the crypto line generated by that function is correct:

The received INVITE contains (in this case) two crypto lines:
a=crypto:0 AES_CM_128_HMAC_SHA1_32 inline:gXjZHXnv+BwSPN4Xqt0cqcs3toa6cPt5XFzzHyqP
a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:KraD5RQFjbhPriAxhhcaUqXDapwqq97qszuN6tYB

The {{sdp_crypto_process}} generates this crypto line (custom log added by me):
sip/sdp_crypto.c:288 sdp_crypto_process: my crypto line:  a=crypto:0 AES_CM_128_HMAC_SHA1_32 inline:BSAGNyrSxNfuJxGkd2ZzKX0qb1MYLiPq4mZrul1m

but later the crypto line added by Asterisk into the SIP response 183/200 SDP is wrong (note that the tag is "1" instead of "0" as previously parsed !!):
a=crypto:1 AES_CM_128_HMAC_SHA1_32 inline:BSAGNyrSxNfuJxGkd2ZzKX0qb1MYLiPq4mZrul1m

By: Iñaki Baz Castillo (ibc) 2012-12-31 08:03:35.253-0600

So the problem is that {{sdp_crypto_offer()}} is being called (confirmed with a custom log).

By: Iñaki Baz Castillo (ibc) 2012-12-31 08:14:34.527-0600

Ok, something found:

{{sdp_crypto_offer()}} is called by {{get_crypto_attrib()}} (in chan_sip.c) and {{get_crypto_attrib()}} is called by {{add_sdp()}} (in chan_sip.c around line 13093) in case {{add_audio}} variable is true (which obviously occurs in this case). So finally {{sdp_crypto_offer()}} is always called and invalidates the previous action of {{sdp_crypto_process()}}.

By: Pedro Kiefer (pedrokiefer) 2013-01-02 06:30:01.143-0600

Fix SDP crypto replies with wrong tag value.

Tested between Chrome running JsSIP and a Sipura SPA-2000. Works both ways, doesn't matter who initiates the call.

By: Iñaki Baz Castillo (ibc) 2013-01-02 06:39:12.635-0600

Thanks a lot Pedro. Just a question: why the second modification?:

- if (snprintf(crypto_buf, sizeof(crypto_buf), "a=crypto:1 AES_CM_128_HMAC_SHA1_%i inline:%s\r\n",
+ if (snprintf(crypto_buf, sizeof(crypto_buf), "a=crypto:0 AES_CM_128_HMAC_SHA1_%i inline:%s\r\n",

Is that needed? It's perfectly valid that the crypto line tag is "0" (when Asterisk generates the offer).

By: Pedro Kiefer (pedrokiefer) 2013-01-02 06:53:09.157-0600

Doubango fixed this by keeping the received tag on {{struct sdp_crypto}}. This is another way to fix it, not sure which one is better.


By: Pedro Kiefer (pedrokiefer) 2013-01-02 06:57:38.650-0600


The second line is not needed, asterisk can offer starting on "1". I'll remote that and repost the patch.

By: Pedro Kiefer (pedrokiefer) 2013-01-02 06:59:46.012-0600

Updated patch.

By: Iñaki Baz Castillo (ibc) 2013-01-02 10:51:24.064-0600

Pedro, your patch does work and IMHO it's correct (if the crypto data is already generated then don't do it again nor replace it).

By: David M. Lee (dlee) 2013-01-24 13:54:38.091-0600

There may be a corner case with SNOM phones I haven't considered.

By: David M. Lee (dlee) 2013-01-28 09:58:50.347-0600

I'm attaching my fixes for this bug, hopefully in a way that accounts for re-invites, etc. See the [code review|https://reviewboard.asterisk.org/r/2295/] to comment.