[Asterisk-code-review] res rtp asterisk.c: Add "seqno" strictrtp option (asterisk[13])

Benjamin Keith Ford asteriskteam at digium.com
Mon Sep 24 11:40:22 CDT 2018


Benjamin Keith Ford has uploaded this change for review. ( https://gerrit.asterisk.org/10246


Change subject: res_rtp_asterisk.c: Add "seqno" strictrtp option
......................................................................

res_rtp_asterisk.c: Add "seqno" strictrtp option

When networks experience disruptions, there can be large gaps of time
between receiving packets. When strictrtp is enabled, this created
issues where a flood of packets could come in and be seen as an attack.
Another option - seqno - has been added to the strictrtp option that
ignores the time interval and goes strictly by sequence number for
validity.

Change-Id: I8a42b8d193673899c8fc22fe7f98ea87df89be71
---
M CHANGES
M configs/samples/rtp.conf.sample
M res/res_rtp_asterisk.c
3 files changed, 63 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/46/10246/1

diff --git a/CHANGES b/CHANGES
index 0398781..45c964e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,17 @@
 ==============================================================================
 
 ------------------------------------------------------------------------------
+--- Functionality changes from Asterisk 13.23.0 to Asterisk 13.24.0 ----------
+------------------------------------------------------------------------------
+
+res_rtp_asterisk
+------------------
+ * The existing strictrtp option in rtp.conf has a new choice availabe, called
+   'seqno', which behaves the same way as setting strictrtp to 'yes', but will
+   ignore the time interval during learning so that bursts of packets can still
+   trigger learning our source.
+
+------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 13.22.0 to Asterisk 13.23.0 ----------
 ------------------------------------------------------------------------------
 
diff --git a/configs/samples/rtp.conf.sample b/configs/samples/rtp.conf.sample
index de9d590..26a70d2 100644
--- a/configs/samples/rtp.conf.sample
+++ b/configs/samples/rtp.conf.sample
@@ -31,6 +31,10 @@
 ; seconds after starting learning mode.  Once learning mode completes the
 ; current stream is locked in and cannot change until the next
 ; renegotiation.
+; Valid options are "no" to disable strictrtp, "yes" to enable strictrtp,
+; and "seqno", which does the same thing as strictrtp=yes, but only checks
+; to make sure the sequence number is correct rather than checking the time
+; interval as well.
 ; This option is enabled by default.
 ; strictrtp=yes
 ;
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index e562627..9641df7 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -149,6 +149,12 @@
 	STRICT_RTP_CLOSED,   /*! Drop all RTP packets not coming from source that was learned */
 };
 
+enum strict_rtp_mode {
+	STRICT_RTP_NO = 0,	/*! Don't adhere to any strict RTP rules */
+	STRICT_RTP_YES,		/*! Strict RTP that restricts packets based on time and sequence number */
+	STRICT_RTP_SEQNO,	/*! Strict RTP that restricts packets based on sequence number */
+};
+
 /*!
  * \brief Strict RTP learning timeout time in milliseconds
  *
@@ -158,7 +164,7 @@
  */
 #define STRICT_RTP_LEARN_TIMEOUT	5000
 
-#define DEFAULT_STRICT_RTP -1	/*!< Enabled */
+#define DEFAULT_STRICT_RTP STRICT_RTP_YES	/*!< Enabled by default */
 #define DEFAULT_ICESUPPORT 1
 
 extern struct ast_srtp_res *res_srtp;
@@ -182,6 +188,11 @@
 static int strictrtp = DEFAULT_STRICT_RTP; /*!< Only accept RTP frames from a defined source. If we receive an indication of a changing source, enter learning mode. */
 static int learning_min_sequential = DEFAULT_LEARNING_MIN_SEQUENTIAL; /*!< Number of sequential RTP frames needed from a single source during learning mode to accept new source. */
 static int learning_min_duration = DEFAULT_LEARNING_MIN_DURATION; /*!< Lowest acceptable timeout between the first and the last sequential RTP frame. */
+#ifdef TEST_FRAMEWORK
+static int strict_rtp_test_event = 1;
+#else
+static int strict_rtp_test_event = 0;
+#endif
 #ifdef HAVE_PJPROJECT
 static int icesupport = DEFAULT_ICESUPPORT;
 static struct sockaddr_in stunaddr;
@@ -2834,27 +2845,30 @@
 		info->received = ast_tvnow();
 	}
 
-	switch (info->stream_type) {
-	case AST_MEDIA_TYPE_UNKNOWN:
-	case AST_MEDIA_TYPE_AUDIO:
-		/*
-		 * Protect against packet floods by checking that we
-		 * received the packet sequence in at least the minimum
-		 * allowed time.
-		 */
-		if (ast_tvzero(info->received)) {
-			info->received = ast_tvnow();
-		} else if (!info->packets
-			&& ast_tvdiff_ms(ast_tvnow(), info->received) < learning_min_duration) {
-			/* Packet flood; reset */
-			info->packets = learning_min_sequential - 1;
-			info->received = ast_tvnow();
+	/* Only check time if strictrtp is set to yes. Otherwise, we only needed to check seqno */
+	if (strictrtp == STRICT_RTP_YES) {
+		switch (info->stream_type) {
+		case AST_MEDIA_TYPE_UNKNOWN:
+		case AST_MEDIA_TYPE_AUDIO:
+			/*
+			 * Protect against packet floods by checking that we
+			 * received the packet sequence in at least the minimum
+			 * allowed time.
+			 */
+			if (ast_tvzero(info->received)) {
+				info->received = ast_tvnow();
+			} else if (!info->packets
+				&& ast_tvdiff_ms(ast_tvnow(), info->received) < learning_min_duration) {
+				/* Packet flood; reset */
+				info->packets = learning_min_sequential - 1;
+				info->received = ast_tvnow();
+			}
+			break;
+		case AST_MEDIA_TYPE_VIDEO:
+		case AST_MEDIA_TYPE_IMAGE:
+		case AST_MEDIA_TYPE_TEXT:
+			break;
 		}
-		break;
-	case AST_MEDIA_TYPE_VIDEO:
-	case AST_MEDIA_TYPE_IMAGE:
-	case AST_MEDIA_TYPE_TEXT:
-		break;
 	}
 
 	info->max_seq = seq;
@@ -5431,6 +5445,8 @@
 			&& STRICT_RTP_LEARN_TIMEOUT < ast_tvdiff_ms(ast_tvnow(), rtp->rtp_source_learn.start)) {
 			ast_verb(4, "%p -- Strict RTP learning complete - Locking on source address %s\n",
 				rtp, ast_sockaddr_stringify(&rtp->strict_rtp_address));
+			ast_test_suite_event_notify("STRICT_RTP_LEARN", "Source: %s",
+				ast_sockaddr_stringify(&rtp->strict_rtp_address));
 			rtp->strict_rtp_state = STRICT_RTP_CLOSED;
 		} else {
 			struct ast_sockaddr target_address;
@@ -5517,6 +5533,11 @@
 		}
 		ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection.\n",
 			rtp, ast_sockaddr_stringify(&addr));
+		if (strict_rtp_test_event) {
+			ast_test_suite_event_notify("STRICT_RTP_CLOSED", "Source: %s",
+				ast_sockaddr_stringify(&addr));
+			strict_rtp_test_event = 0; /* Only run this event once to prevent possible spam */
+		}
 		return &ast_null_frame;
 	case STRICT_RTP_OPEN:
 		break;
@@ -6652,6 +6673,12 @@
 	}
 	if ((s = ast_variable_retrieve(cfg, "general", "strictrtp"))) {
 		strictrtp = ast_true(s);
+		if (!strcasecmp(s, "seqno")) {
+			strictrtp = STRICT_RTP_SEQNO;
+		} else if (strictrtp == -1) {
+			/* Stick to enum values for consistency */
+			strictrtp = STRICT_RTP_YES;
+		}
 	}
 	if ((s = ast_variable_retrieve(cfg, "general", "probation"))) {
 		if ((sscanf(s, "%d", &learning_min_sequential) != 1) || learning_min_sequential <= 1) {

-- 
To view, visit https://gerrit.asterisk.org/10246
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8a42b8d193673899c8fc22fe7f98ea87df89be71
Gerrit-Change-Number: 10246
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180924/80011983/attachment-0001.html>


More information about the asterisk-code-review mailing list