[Asterisk-code-review] res rtp asterisk: rtcp mux using the wrong srtp unprotecting... (asterisk[13])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5677 )

Change subject: res_rtp_asterisk: rtcp mux using the wrong srtp unprotecting algorithm
......................................................................


res_rtp_asterisk: rtcp mux using the wrong srtp unprotecting algorithm

When using rtcp mux if an rtcp payload came in it would still use the srtp
unprotect algorithm instead of the srtp unprotect rtcp method. Since rtcp
data was being passed to the rtp unprotect method this would result in an
error.

This patch ensures that the correct unprotect method is chosen by making
sure the passed in rtcp flag is appropriately set when rtcp mux is enabled
and an rtcp payload is received.

ASTERISK-26979 #close

Change-Id: Ic5409f9d1a267f1d4785fc5aed867daaecca6241
---
M res/res_rtp_asterisk.c
1 file changed, 35 insertions(+), 34 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  George Joseph: Verified
  Jenkins2: Approved for Submit
  Sean Bright: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index b7cc059..673f9c1 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -2373,6 +2373,39 @@
 }
 #endif
 
+static int rtcp_mux(struct ast_rtp *rtp, const unsigned char *packet)
+{
+	uint8_t version;
+	uint8_t pt;
+	uint8_t m;
+
+	if (!rtp->rtcp || rtp->rtcp->type != AST_RTP_INSTANCE_RTCP_MUX) {
+		return 0;
+	}
+
+	version = (packet[0] & 0XC0) >> 6;
+	if (version == 0) {
+		/* version 0 indicates this is a STUN packet and shouldn't
+		 * be interpreted as a possible RTCP packet
+		 */
+		return 0;
+	}
+
+	/* The second octet of a packet will be one of the following:
+	 * For RTP: The marker bit (1 bit) and the RTP payload type (7 bits)
+	 * For RTCP: The payload type (8)
+	 *
+	 * RTP has a forbidden range of payload types (64-95) since these
+	 * will conflict with RTCP payload numbers if the marker bit is set.
+	 */
+	m = packet[1] & 0x80;
+	pt = packet[1] & 0x7F;
+	if (m && pt >= 64 && pt <= 95) {
+		return 1;
+	}
+	return 0;
+}
+
 /*! \pre instance is locked */
 static int __rtp_recvfrom(struct ast_rtp_instance *instance, void *buf, size_t size, int flags, struct ast_sockaddr *sa, int rtcp)
 {
@@ -2495,7 +2528,8 @@
 	}
 #endif
 
-	if ((*in & 0xC0) && res_srtp && srtp && res_srtp->unprotect(srtp, buf, &len, rtcp) < 0) {
+	if ((*in & 0xC0) && res_srtp && srtp && res_srtp->unprotect(
+		    srtp, buf, &len, rtcp || rtcp_mux(rtp, buf)) < 0) {
 	   return -1;
 	}
 
@@ -4856,39 +4890,6 @@
 
 	ao2_unlock(instance1);
 	ao2_lock(instance);
-	return 0;
-}
-
-static int rtcp_mux(struct ast_rtp *rtp, const unsigned char *packet)
-{
-	uint8_t version;
-	uint8_t pt;
-	uint8_t m;
-
-	if (!rtp->rtcp || rtp->rtcp->type != AST_RTP_INSTANCE_RTCP_MUX) {
-		return 0;
-	}
-
-	version = (packet[0] & 0XC0) >> 6;
-	if (version == 0) {
-		/* version 0 indicates this is a STUN packet and shouldn't
-		 * be interpreted as a possible RTCP packet
-		 */
-		return 0;
-	}
-
-	/* The second octet of a packet will be one of the following:
-	 * For RTP: The marker bit (1 bit) and the RTP payload type (7 bits)
-	 * For RTCP: The payload type (8)
-	 *
-	 * RTP has a forbidden range of payload types (64-95) since these
-	 * will conflict with RTCP payload numbers if the marker bit is set.
-	 */
-	m = packet[1] & 0x80;
-	pt = packet[1] & 0x7F;
-	if (m && pt >= 64 && pt <= 95) {
-		return 1;
-	}
 	return 0;
 }
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic5409f9d1a267f1d4785fc5aed867daaecca6241
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>