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

[Jenkins2]

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

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
(cherry picked from commit 9db41320b8eeb53a991bf6b781cc239acd322876)
---
M res/res_rtp_asterisk.c
1 file changed, 35 insertions(+), 34 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit
  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 e2a5809..8d4e50d 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;
 	}
 
@@ -4855,39 +4889,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/5710
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic5409f9d1a267f1d4785fc5aed867daaecca6241
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14.5
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>