diff options
author | Walter Doekes <walter+github@wjd.nu> | 2016-09-06 09:41:06 +0200 |
---|---|---|
committer | Walter Doekes <walter+asterisk@wjd.nu> | 2016-09-06 02:56:22 -0500 |
commit | d04ae7d1d81ee157bea1295b2316e278c951d877 (patch) | |
tree | 1ae80b6f55a04a2242bebb22731a6f1e8e796a35 /channels | |
parent | 9470848fba269f9f8f8fc763b05e72efab6d7b70 (diff) |
chan_sip: Don't refuse calls with "optional crypto"; fall back to RTP.
Certain SNOM phones send so-called "optional crypto" in their SDP body.
Regular SRTP setup looks like this:
m=audio 64620 RTP/SAVP 8 0 9 99 3 18 4 101
a=crypto:1 AES_CM_128_HMAC_SHA1_32 inline:...
SNOM-style "optional crypto" looks like this:
m=audio 61438 RTP/AVP 8 0 9 99 3 18 4 101
a=crypto:1 AES_CM_128_HMAC_SHA1_32 inline:...
A crypto line is supplied, but the m-line does not have SAVP.
When res_srtp.so is *not* loaded, then chan_sip.so treats the optional
crypto as regular RTP, but when res_srtp.so *is* loaded, it refuses the
incoming call with the following message:
WARNING: process_sdp: Failed to receive SDP offer/answer with
required SRTP crypto attributes for audio
For platforms that want to start providing SRTP this presents a
compatibility problem.
This changeset lets chan_sip handle the SDP as if no crypto-line was
supplied: i.e. accept the call as regular RTP, just like it did before
res_srtp was loaded.
Now you'll get this informative warning instead:
WARNING: Ignoring crypto attribute in SDP because RTP transport is
insecure
ASTERISK-23989 #close
Reported by: Olle Johansson
Change-Id: I91a15ae05a0296e398d6b65f53bb11afde1d80e2
Diffstat (limited to 'channels')
-rw-r--r-- | channels/chan_sip.c | 29 |
1 files changed, 24 insertions, 5 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index c0aae46c5..c8987a00a 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -1477,7 +1477,8 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res static void handle_response(struct sip_pvt *p, int resp, const char *rest, struct sip_request *req, uint32_t seqno); /*------ SRTP Support -------- */ -static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struct ast_sdp_srtp **srtp, const char *a); +static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struct ast_sdp_srtp **srtp, + const char *a, int secure_transport); /*------ T38 Support --------- */ static int transmit_response_with_t38_sdp(struct sip_pvt *p, char *msg, struct sip_request *req, int retrans); @@ -10618,7 +10619,7 @@ static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action } } else if (process_sdp_a_sendonly(value, &sendonly)) { processed = TRUE; - } else if (!processed_crypto && process_crypto(p, p->rtp, &p->srtp, value)) { + } else if (!processed_crypto && process_crypto(p, p->rtp, &p->srtp, value, secure_audio)) { processed_crypto = TRUE; processed = TRUE; } else if (process_sdp_a_audio(value, p, &newaudiortp, &last_rtpmap_codec)) { @@ -10635,7 +10636,7 @@ static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action if (p->vsrtp) { ast_set_flag(p->vsrtp, AST_SRTP_CRYPTO_OFFER_OK); } - } else if (!processed_crypto && process_crypto(p, p->vrtp, &p->vsrtp, value)) { + } else if (!processed_crypto && process_crypto(p, p->vrtp, &p->vsrtp, value, secure_video)) { processed_crypto = TRUE; processed = TRUE; } else if (process_sdp_a_video(value, p, &newvideortp, &last_rtpmap_codec)) { @@ -10648,7 +10649,7 @@ static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action processed = TRUE; } else if (process_sdp_a_text(value, p, &newtextrtp, red_fmtp, &red_num_gen, red_data_pt, &last_rtpmap_codec)) { processed = TRUE; - } else if (!processed_crypto && process_crypto(p, p->trtp, &p->tsrtp, value)) { + } else if (!processed_crypto && process_crypto(p, p->trtp, &p->tsrtp, value, 1)) { processed_crypto = TRUE; processed = TRUE; } @@ -33640,7 +33641,8 @@ static void sip_send_all_mwi_subscriptions(void) ao2_iterator_destroy(&iter); } -static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struct ast_sdp_srtp **srtp, const char *a) +static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struct ast_sdp_srtp **srtp, + const char *a, int secure_transport) { struct ast_rtp_engine_dtls *dtls; @@ -33656,6 +33658,23 @@ static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struc /* skip "crypto:" */ a += strlen("crypto:"); + if (!secure_transport) { + /* > The Secure Real-time Transport Protocol (SRTP) + * > [RFC3711] provides security services for RTP media + * > and is signaled by use of secure RTP transport (e.g., + * > "RTP/SAVP" or "RTP/SAVPF") in an SDP media (m=) line. + * > ... + * > The "crypto" attribute MUST only appear at the SDP + * > media level (not at the session level). + * + * Ergo, we can trust RTP/(S)AVP to be read from the m= + * line before we get here. If it was RTP/AVP, then this + * is SNOM-specific optional SRTP. Ignore it. + */ + ast_log(LOG_WARNING, "Ignoring crypto attribute in SDP because RTP transport is insecure\n"); + return FALSE; + } + if (!*srtp) { if (ast_test_flag(&p->flags[0], SIP_OUTGOING)) { ast_log(LOG_WARNING, "Ignoring unexpected crypto attribute in SDP answer\n"); |