<p>Richard Mudgett has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6410">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk.c: Make strict RTP learning more flexible.<br><br>Direct media can cause strict RTP to attempt to learn a remote address<br>again before it has had a chance to learn the remote address the first<br>time.  Because of the rapid relearn requests, strict RTP could latch onto<br>the first remote address and fail to latch onto the direct media remote<br>address.  As a result, you have one way audio until the call is placed on<br>and off hold.<br><br>The new algorythm learns remote addresses for a set time (1.5 seconds)<br>before locking the remote address.  In addition, we must see a configured<br>number of remote packets from the same address in a row before switching.<br><br>* Fixed strict RTP learning from always accepting the first new address<br>packet as the new stream.<br><br>* Fixed strict RTP to initialize the expected sequence number with the<br>last received sequence number instead of the last transmitted sequence<br>number.<br><br>* Fixed the predicted next sequence number calculation in<br>rtp_learning_rtp_seq_update() to handle overflow.<br><br>ASTERISK-27252<br><br>Change-Id: Ia2d3aa6e0f22906c25971e74f10027d96525f31c<br>---<br>M res/res_rtp_asterisk.c<br>1 file changed, 91 insertions(+), 41 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/10/6410/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c<br>index 77027aa..b1f6b56 100644<br>--- a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -117,7 +117,9 @@<br>     STRICT_RTP_CLOSED,   /*! Drop all RTP packets not coming from source that was learned */<br> };<br> <br>-#define DEFAULT_STRICT_RTP STRICT_RTP_CLOSED<br>+#define STRICT_RTP_LEARN_TIMEOUT      1500    /*!< milliseconds */<br>+<br>+#define DEFAULT_STRICT_RTP -1      /*!< Enabled */<br> #define DEFAULT_ICESUPPORT 1<br> <br> extern struct ast_srtp_res *res_srtp;<br>@@ -218,9 +220,11 @@<br> <br> /*! \brief RTP learning mode tracking information */<br> struct rtp_learning_info {<br>-    int max_seq;     /*!< The highest sequence number received */<br>-     int packets;     /*!< The number of remaining packets before the source is accepted */<br>+    struct ast_sockaddr proposed_address;   /*!< Proposed remote address for strict RTP */<br>+    struct timeval start;   /*!< The time learning mode was started */<br>         struct timeval received; /*!< The time of the last received packet */<br>+     int max_seq;    /*!< The highest sequence number received */<br>+      int packets;    /*!< The number of remaining packets before the source is accepted */<br> };<br> <br> #ifdef HAVE_OPENSSL_SRTP<br>@@ -1984,7 +1988,7 @@<br> #endif<br> <br> #ifdef HAVE_PJPROJECT<br>-static void rtp_learning_seq_init(struct rtp_learning_info *info, uint16_t seq);<br>+static void rtp_learning_start(struct ast_rtp *rtp);<br> <br> /* PJPROJECT ICE callback */<br> static void ast_rtp_on_ice_complete(pj_ice_sess *ice, pj_status_t status)<br>@@ -2023,8 +2027,8 @@<br>                 return;<br>       }<br> <br>- rtp->strict_rtp_state = STRICT_RTP_LEARN;<br>- rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t)rtp->seqno);<br>+       ast_verb(4, "%p -- Strict RTP learning after ICE completion\n", rtp);<br>+      rtp_learning_start(rtp);<br>      ao2_unlock(instance);<br> }<br> <br>@@ -2753,7 +2757,7 @@<br>  */<br> static void rtp_learning_seq_init(struct rtp_learning_info *info, uint16_t seq)<br> {<br>-      info->max_seq = seq - 1;<br>+  info->max_seq = seq;<br>       info->packets = learning_min_sequential;<br>   memset(&info->received, 0, sizeof(info->received));<br> }<br>@@ -2770,14 +2774,17 @@<br>  */<br> static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t seq)<br> {<br>+  /*<br>+    * During the learning mode the minimum amount of media we'll accept is<br>+   * 10ms so give a reasonable 5ms buffer just in case we get it sporadically.<br>+  */<br>   if (!ast_tvzero(info->received) && ast_tvdiff_ms(ast_tvnow(), info->received) < 5) {<br>-                /* During the probation period the minimum amount of media we'll accept is<br>-                * 10ms so give a reasonable 5ms buffer just in case we get it sporadically.<br>+         /*<br>+            * Reject a flood of packets as acceptable for learning.<br>+              * Reset the needed packets.<br>           */<br>-          return 1;<br>-    }<br>-<br>- if (seq == info->max_seq + 1) {<br>+           info->packets = learning_min_sequential - 1;<br>+      } else if (seq == (uint16_t) (info->max_seq + 1)) {<br>                /* packet is in sequence */<br>           info->packets--;<br>   } else {<br>@@ -2787,7 +2794,24 @@<br>      info->max_seq = seq;<br>       info->received = ast_tvnow();<br> <br>-  return (info->packets == 0);<br>+      return info->packets;<br>+}<br>+<br>+/*!<br>+ * \brief Start the strictrtp learning mode.<br>+ * \since 13.18.0<br>+ *<br>+ * \param rtp RTP session description<br>+ *<br>+ * \return Nothing<br>+ */<br>+static void rtp_learning_start(struct ast_rtp *rtp)<br>+{<br>+      rtp->strict_rtp_state = STRICT_RTP_LEARN;<br>+ memset(&rtp->rtp_source_learn.proposed_address, 0,<br>+            sizeof(rtp->rtp_source_learn.proposed_address));<br>+  rtp->rtp_source_learn.start = ast_tvnow();<br>+        rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t) rtp->lastrxseqno);<br> }<br> <br> #ifdef HAVE_PJPROJECT<br>@@ -3061,9 +3085,6 @@<br>   rtp->ssrc = ast_random();<br>  rtp->seqno = ast_random() & 0x7fff;<br>    rtp->strict_rtp_state = (strictrtp ? STRICT_RTP_CLOSED : STRICT_RTP_OPEN);<br>-        if (strictrtp) {<br>-             rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t)rtp->seqno);<br>-       }<br> <br>  /* Create a new socket for us to listen on and use */<br>         if ((rtp->s =<br>@@ -5076,31 +5097,59 @@<br>     }<br> <br>  /* If strict RTP protection is enabled see if we need to learn the remote address or if we need to drop the packet */<br>-        if (rtp->strict_rtp_state == STRICT_RTP_LEARN) {<br>-          if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>-                 /* We are learning a new address but have received traffic from the existing address,<br>-                         * accept it but reset the current learning for the new source so it only takes over<br>-                  * once sufficient traffic has been received. */<br>-                     rtp_learning_seq_init(&rtp->rtp_source_learn, seqno);<br>-         } else {<br>-                     /* Start trying to learn from the new address. If we pass a probationary period with<br>-                  * it, that means we've stopped getting RTP from the original source and we should<br>-                        * switch to it.<br>-                      */<br>-                  if (rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) {<br>-                             ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection. Will switch to it in %d packets\n",<br>-                                        rtp, ast_sockaddr_stringify(&addr), rtp->rtp_source_learn.packets);<br>-                           return &ast_null_frame;<br>-                  }<br>-                    ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);<br>-<br>-                    ast_verb(4, "%p -- Probation passed - setting RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));<br>+        switch (rtp->strict_rtp_state) {<br>+  case STRICT_RTP_LEARN:<br>+               if (!ast_sockaddr_isnull(&rtp->strict_rtp_address)<br>+                    && STRICT_RTP_LEARN_TIMEOUT < ast_tvdiff_ms(ast_tvnow(), rtp->rtp_source_learn.start)) {<br>+                       ast_verb(4, "%p -- Strict RTP learning complete - Locking on source address %s\n",<br>+                         rtp, ast_sockaddr_stringify(&rtp->strict_rtp_address));<br>                        rtp->strict_rtp_state = STRICT_RTP_CLOSED;<br>+                } else {<br>+                     if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>+                         /*<br>+                            * We are learning a new address but have received traffic from the existing address,<br>+                                 * accept it but reset the current learning for the new source so it only takes over<br>+                          * once sufficient traffic has been received.<br>+                                 */<br>+                          rtp_learning_seq_init(&rtp->rtp_source_learn, seqno);<br>+                         break;<br>+                       }<br>+                    /*<br>+                    * Trying to learn a new address.  If we pass a probationary period with<br>+                      * it, that means we've stopped getting RTP from the original source and<br>+                  * we should switch to it.<br>+                    */<br>+                  if (!ast_sockaddr_cmp(&rtp->rtp_source_learn.proposed_address, &addr)) {<br>+                          if (!rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) {<br>+                                    /* Accept the new RTP stream */<br>+                                      ast_verb(4, "%p -- Strict RTP switching source address to %s\n",<br>+                                           rtp, ast_sockaddr_stringify(&addr));<br>+                                     ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);<br>+                                       break;<br>+                               }<br>+                            /* Not ready to accept the RTP stream candidate */<br>+                   } else {<br>+                             /*<br>+                            * This is either an attacking stream or<br>+                              * the start of the expected new stream.<br>+                              */<br>+                          ast_sockaddr_copy(&rtp->rtp_source_learn.proposed_address, &addr);<br>+                                rtp_learning_seq_init(&rtp->rtp_source_learn, seqno);<br>+                 }<br>+                    ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection. Will switch to it in %d packets\n",<br>+                                rtp, ast_sockaddr_stringify(&addr), rtp->rtp_source_learn.packets);<br>+                   return &ast_null_frame;<br>           }<br>-    } else if (rtp->strict_rtp_state == STRICT_RTP_CLOSED && ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>+          /* Fall through */<br>+   case STRICT_RTP_CLOSED:<br>+              if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {<br>+                 break;<br>+               }<br>             ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection.\n",<br>                         rtp, ast_sockaddr_stringify(&addr));<br>              return &ast_null_frame;<br>+  case STRICT_RTP_OPEN:<br>+                break;<br>        }<br> <br>  /* If symmetric RTP is enabled see if the remote side is not what we expected and change where we are sending audio */<br>@@ -5618,13 +5667,14 @@<br> <br>    rtp->rxseqno = 0;<br> <br>-      if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN && !ast_sockaddr_isnull(addr) &&<br>-                ast_sockaddr_cmp(addr, &rtp->strict_rtp_address)) {<br>+   if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN<br>+         && !ast_sockaddr_isnull(addr) && ast_sockaddr_cmp(addr, &rtp->strict_rtp_address)) {<br>           /* We only need to learn a new strict source address if we've been told the source is<br>              * changing to something different.<br>            */<br>-          rtp->strict_rtp_state = STRICT_RTP_LEARN;<br>-         rtp_learning_seq_init(&rtp->rtp_source_learn, rtp->seqno);<br>+         ast_verb(4, "%p -- Strict RTP learning after remote address set to: %s\n",<br>+                 rtp, ast_sockaddr_stringify(addr));<br>+          rtp_learning_start(rtp);<br>      }<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6410">change 6410</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6410"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ia2d3aa6e0f22906c25971e74f10027d96525f31c </div>
<div style="display:none"> Gerrit-Change-Number: 6410 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>