[Asterisk-code-review] chan sip: Allow Preferred sRTP. (asterisk[master])

Anonymous Coward asteriskteam at digium.com
Wed Sep 7 08:19:38 CDT 2016


Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: chan_sip: Allow Preferred sRTP.
......................................................................


chan_sip: Allow Preferred sRTP.

Following the Encrypt-all-the-things paradigm:

The user enters his SIP-URI and password. Thanks to DNS-NAPTR, the phone
determines SIP-over-TLS as preferred transport. In SIP/SDP, the phone starts
the call with a crypto attribute, but not as RTP/sAVP but the RTP/AVP profile
(sRTP is preferred aka optional; not mandatory). If the VoIP server does not
support sRTP and TLS, the phone shows an open padlock icon.

This paradigm is supported by several VoIP/SIP clients on default. Some
implementations even cannot be changed to RTP/sAVP. Therefore here, this
change allows Preferred sRTP for ingress. For egress, please, create a dial
plan which starts with RTP/SAVP, and when rejected tries again with RTP/AVP.

ASTERISK-20234 #close
Reported by: tootai
Tested by: tootai, Alexander Traud
patches:
 srtp_patches.diff submitted by Matt Jordan

Change-Id: I42cb779df3a9c7b3dd03a629fb3a296aa4ceb0fd
---
M CHANGES
M channels/chan_sip.c
2 files changed, 20 insertions(+), 22 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved
  Walter Doekes: Looks good to me, but someone else must approve



diff --git a/CHANGES b/CHANGES
index 5857165..8a6c673 100644
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,13 @@
 --- Functionality changes from Asterisk 14 to Asterisk 15 --------------------
 ------------------------------------------------------------------------------
 
+chan_sip
+------------------
+ * If an offer is received with optional SRTP (a media stream with RTP/AVP but
+   which contains a crypto line) chan_sip will now accept it and enable SRTP.
+   If you would like to do optional SRTP on outbound you will need to create
+   a dialplan that dials with it enabled initially and if it fails fall back to
+   without.
 
 ------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 14.0.0 to Asterisk 14.1.0 ----------
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 6a18aa3..1d70e89 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -1480,7 +1480,7 @@
 
 /*------ SRTP Support -------- */
 static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struct ast_sdp_srtp **srtp,
-		const char *a, int secure_transport);
+		const char *a);
 
 /*------ T38 Support --------- */
 static int transmit_response_with_t38_sdp(struct sip_pvt *p, char *msg, struct sip_request *req, int retrans);
@@ -10610,9 +10610,13 @@
 						}
 					} else if (process_sdp_a_sendonly(value, &sendonly)) {
 						processed = TRUE;
-					} else if (!processed_crypto && process_crypto(p, p->rtp, &p->srtp, value, secure_audio)) {
+					} else if (!processed_crypto && process_crypto(p, p->rtp, &p->srtp, value)) {
 						processed_crypto = TRUE;
 						processed = TRUE;
+						if (secure_audio == FALSE) {
+							ast_log(AST_LOG_NOTICE, "Processed audio crypto attribute without SAVP specified; accepting anyway\n");
+							secure_audio = TRUE;
+						}
 					} else if (process_sdp_a_audio(value, p, &newaudiortp, &last_rtpmap_codec)) {
 						processed = TRUE;
 					}
@@ -10627,9 +10631,13 @@
 						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, secure_video)) {
+					} else if (!processed_crypto && process_crypto(p, p->vrtp, &p->vsrtp, value)) {
 						processed_crypto = TRUE;
 						processed = TRUE;
+						if (secure_video == FALSE) {
+							ast_log(AST_LOG_NOTICE, "Processed video crypto attribute without SAVP specified; accepting anyway\n");
+							secure_video = TRUE;
+						}
 					} else if (process_sdp_a_video(value, p, &newvideortp, &last_rtpmap_codec)) {
 						processed = TRUE;
 					}
@@ -10640,7 +10648,7 @@
 						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, 1)) {
+					} else if (!processed_crypto && process_crypto(p, p->trtp, &p->tsrtp, value)) {
 						processed_crypto = TRUE;
 						processed = TRUE;
 					}
@@ -33752,7 +33760,7 @@
 }
 
 static int process_crypto(struct sip_pvt *p, struct ast_rtp_instance *rtp, struct ast_sdp_srtp **srtp,
-		const char *a, int secure_transport)
+		const char *a)
 {
 	struct ast_rtp_engine_dtls *dtls;
 
@@ -33767,23 +33775,6 @@
 	}
 	/* 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)) {

-- 
To view, visit https://gerrit.asterisk.org/3234
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I42cb779df3a9c7b3dd03a629fb3a296aa4ceb0fd
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>



More information about the asterisk-code-review mailing list