From: Matthew Jordan Date: Sun, 9 Sep 2012 01:19:21 +0000 (+0000) Subject: Only re-create an SRTP session when needed; respond with correct crypto policy X-Git-Tag: 1.8.17.0-rc1~12 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=95f4884fe8f21e2bc496265f041b043db9e068ca;p=thirdparty%2Fasterisk.git Only re-create an SRTP session when needed; respond with correct crypto policy In r356604, SRTP handling was fixed to accomodate multiple crypto keys in an SDP offer and the ability to re-create an SRTP session when the crypto keys changed. In certain circumstances - most notably when a phone is put on hold after having been bridged for a significant amount of time - the act of re-creating the SRTP session causes problems for certain models of phones. The patch committed in r356604 always re-created the SRTP session regardless of whether or not the cryptographic keys changed. Since this is technically not necessary, this patch modifies the behavior to only re-create the SRTP session if Asterisk detects that the remote key has changed. This allows models of phones that do not handle the SRTP session changing to continue to work, while also providing the behavior needed for those phones that do re-negotiate cryptographic keys. In addition, in Asterisk 1.8 only, it was found that phones that offer AES_CM_128_HMAC_SHA1_32 will end up with no audio if the phone is the initiator of the call. The phone will send an INVITE request specifying that AES_CM_128_HMAC_SHA1_32 be used for the cryptographic policy; Asterisk will set its policy to that value. Unfortunately, when the call is Answered and a 200 OK is sent back to the UA, the policy sent in the response's SDP will be the hard coded value AES_CM_128_HMAC_SHA1_80. This potentially results in Asterisk using the INVITE request's policy of AES_CM_128_HMAC_SHA1_32, while the phone uses Asterisk's response of AES_CM_128_HMAC_SHA1_80. Hilarity ensues as both endpoints think the other is crazy. This patch fixes that by caching the policy from the request and responding with it. Note that this is not a problem in Asterisk 10 and later, as the ability to configure the policy was added in that version. (issue ASTERISK-20194) Reported by: Nicolo Mazzon Tested by: Nicolo Mazzon Review: https://reviewboard.asterisk.org/r/2099 git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@372709 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/channels/sip/sdp_crypto.c b/channels/sip/sdp_crypto.c index 3a95ce9a6c..ea8ea64162 100644 --- a/channels/sip/sdp_crypto.c +++ b/channels/sip/sdp_crypto.c @@ -49,6 +49,8 @@ struct sdp_crypto { char *a_crypto; unsigned char local_key[SRTP_MASTER_LEN]; char local_key64[SRTP_MASTER_LEN64]; + unsigned char remote_key[SRTP_MASTER_LEN]; + char suite[64]; }; static int set_crypto_policy(struct ast_srtp_policy *policy, int suite_val, const unsigned char *master_key, unsigned long ssrc, int inbound); @@ -257,12 +259,20 @@ int sdp_crypto_process(struct sdp_crypto *p, const char *attr, struct ast_rtp_in return -1; } - if ((key_len = ast_base64decode(remote_key, key_salt, sizeof(remote_key))) != SRTP_MASTER_LEN) { - ast_log(LOG_WARNING, "SRTP sdescriptions key %d != %d\n", key_len, SRTP_MASTER_LEN); + ast_log(LOG_WARNING, "SRTP descriptions key %d != %d\n", key_len, SRTP_MASTER_LEN); return -1; } + if (!memcmp(p->remote_key, remote_key, sizeof(p->remote_key))) { + ast_debug(1, "SRTP remote key unchanged; maintaining current policy\n"); + return 0; + } + + /* Set the accepted policy and remote key */ + ast_copy_string(p->suite, suite, sizeof(p->suite)); + memcpy(p->remote_key, remote_key, sizeof(p->remote_key)); + if (sdp_crypto_activate(p, suite_val, remote_key, rtp) < 0) { return -1; } @@ -280,13 +290,17 @@ int sdp_crypto_process(struct sdp_crypto *p, const char *attr, struct ast_rtp_in int sdp_crypto_offer(struct sdp_crypto *p) { char crypto_buf[128]; - const char *crypto_suite = "AES_CM_128_HMAC_SHA1_80"; /* Crypto offer */ + + if (ast_strlen_zero(p->suite)) { + /* Default crypto offer */ + strcpy(p->suite, "AES_CM_128_HMAC_SHA1_80"); + } if (p->a_crypto) { ast_free(p->a_crypto); } - if (snprintf(crypto_buf, sizeof(crypto_buf), "a=crypto:1 %s inline:%s\r\n", crypto_suite, p->local_key64) < 1) { + if (snprintf(crypto_buf, sizeof(crypto_buf), "a=crypto:1 %s inline:%s\r\n", p->suite, p->local_key64) < 1) { return -1; }