<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/2364/">https://reviewboard.asterisk.org/r/2364/</a>
     </td>
    </tr>
   </table>
   <br />



 <p>Ship it!</p>







<div>




<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://reviewboard.asterisk.org/r/2364/diff/2/?file=33723#file33723line2218" style="color: black; font-weight: bold; text-decoration: underline;">branches/1.8/res/res_rtp_asterisk.c</a>
    <span style="font-weight: normal;">

     (Diff revision 2)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtcp)</pre></td>

  </tr>
 </tbody>




 
 



 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2210</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="cm">/* Hmm, not the strict address. Perhaps we&#39;re getting audio from the alternate? */</span></pre></td>
  </tr>

 </tbody>





 
 

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2205</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">ast_sockaddr_cmp</span><span class="p">(</span><span class="o">&amp;</span><span class="n">rtp</span><span class="o">-&gt;</span><span class="n">alt_rtp_address</span><span class="p">,</span> <span class="o">&amp;</span><span class="n">addr</span><span class="p">))</span> <span class="p">{</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2211</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">ast_sockaddr_cmp</span><span class="p">(</span><span class="o">&amp;</span><span class="n">rtp</span><span class="o">-&gt;</span><span class="n">alt_rtp_address</span><span class="p">,</span> <span class="o">&amp;</span><span class="n">addr</span><span class="p">))</span> <span class="p">{</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2206</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="cm">/* ooh, we did! You&#39;re now the new expected address, son! */</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2212</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="cm">/* ooh, we did! You&#39;re now the new expected address, son! */</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2207</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="n">ast_sockaddr_copy</span><span class="p">(</span><span class="o">&amp;</span><span class="n">rtp</span><span class="o">-&gt;</span><span class="n">strict_rtp_address</span><span class="p">,</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2213</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="n">ast_sockaddr_copy</span><span class="p">(</span><span class="o">&amp;</span><span class="n">rtp</span><span class="o">-&gt;</span><span class="n">strict_rtp_address</span><span class="p">,</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2208</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                                  <span class="o">&amp;</span><span class="n">addr</span><span class="p">);</span></pre></td>
    <th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2214</font></th>
    <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                                  <span class="o">&amp;</span><span class="n">addr</span><span class="p">);</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I completely forgot about the existence of this in res_rtp_asterisk. The alt_rtp_address was added as a means of dealing with a similar strictrtp problem, except in reverse. Double-secret probation is a more generic solution that would solve both the current problem and the one that the alt_rtp_address was meant to solve. I&#39;d be all for completely removing the alt_rtp_address stuff from the code completely since it&#39;s only used in a rather hack-y situation in chan_sip.

To give some background, the alt_rtp_address was introduced because we would receive a reinvite with a new RTP address in the SDP. Unfortunately, due to the timing of the arrival of the reinvite, we had to reply with a 491. The problem is that we would start receiving media from the address in the SDp in the reinvite we received. Since we were locked onto a different address due to strictrtp, we would drop media from the new address. The solution was to let the RTP layer know that it may start receiving media from a new alternate address instead of the one that it was locked on to. If the RTP layer started to receive media from that address, it would immediately make the alt address the new strict address.

With double secret probation code in place, it means that we could actually remove the alt_rtp_address code entirely since the RTP layer would eventually re-train on the proper new remote address.</pre>
</div>
<br />



<p>- Mark</p>


<br />
<p>On March 4th, 2013, 9:28 a.m., Matt Jordan wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Asterisk Developers, Joshua Colp and Mark Michelson.</div>
<div>By Matt Jordan.</div>


<p style="color: grey;"><i>Updated March 4, 2013, 9:28 a.m.</i></p>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Consider the following scenario:

* Phone 1 connected to Asterisk 1, where the configuration forces the media to go through Asterisk 1
* Asterisk 1 and Asterisk 2 connected, where the configuration will attempt directmedia for any session established between the two
* Phone 2 connected to Asterisk 2, where the configuration will attempt directmedia for any session established between the two
* Both Asterisk 1 and Asterisk 2 have strictrtp enabled (which is the default setting)

        d    d            dm  dm            dm  dm
Phone 1 &lt;----&gt; Asterisk 1 &lt;----&gt; Asterisk 2 &lt;----&gt; Phone 2

If Phone 1 calls Phone 2, the expected media path (once the re-INVITE flurry has finished) should look like:

Phone 1 &lt;--&gt; Asterisk 1 &lt;--&gt; Phone 2

The way in which this happens typically originates with Asterisk 2. Once it receives the 200 OK from Phone 2 and passes it on to Asterisk 1, it will send re-INVITEs to both Asterisk 1 and Phone 2 to negotiate itself out of the media path. While this is happening, however, it continues to pass RTP through from Phone 2 to Asterisk 1, and from Asterisk 1 to Phone 2.

Asterisk is usually fairly quick to re-INVITE itself out of the media path. Testing has shown, however, that phones (for a fair number of models) can take a bit longer to accept the re-INVITE and issue a response. The sequence will usually look something like this:

                    SIP                                         RTP (from Asterisk 2&#39;s perspective)
Asterisk 1       Asterisk 2          Phone 2                Asterisk 1      Asterisk 2       Phone 2
                                                                &lt;----------&gt;         &lt;----------&gt;
     INVITE med to Phone 2                                      &lt;----------&gt;         &lt;----------&gt;
   &lt;-----------------  INVITE med to Asterisk 1                 &lt;----------&gt;         &lt;----------&gt;
                     ------------------&gt;                        &lt;----------&gt;         &lt;----------&gt;
     200 OK                                                     &lt;----------&gt;         &lt;----------&gt;
   ------------------&gt;                                          &lt;----------          &lt;----------
                     .                                          &lt;----------          &lt;----------
                     .                                          &lt;----------          &lt;----------
                     .    200 OK                                &lt;----------          &lt;----------
                      &lt;-----------------                              (finally out of the path)

The problem here occurs at Asterisk 1: it receives the re-INVITE notifying that it is going to have a change in the RTP source, and it re-initializes the strictrtp settings accordingly. Unfortunately, because Phone 2 is slow on the uptake, RTP keeps flowing from Phone 2 through Asterisk 2 to Asterisk 1. Asterisk 2 does its duty and forwards things along, which results in Asterisk 1 re-locking the RTP source back onto Asterisk 2. Eventually, Phone 2 wakes up and re-directs its RTP to Asterisk 1 - but by then, strictrtp is closed and Asterisk 1 expects the RTP to come from Asterisk 2, and the RTP packets are rejected.

The problem is, even though Asterisk 1 &quot;knows&quot; it&#39;s going to get a new RTP source, due to NAT, it can&#39;t know who the source is. It&#39;s valid in some scenarios for the new RTP source to look exactly like the old RTP source (if, for example, everything was going through a TURN server) - so it has to simply lock onto the entity sending it RTP once it has a source update. We also can&#39;t control how long it takes an endpoint to respond to the re-INVITE - packet loss, if nothing else, would reproduce this scenario.

The solution this patch provides is to implement a &#39;secret probation&#39; mode for RTP packets received after strictrtp has closed and locked onto an RTP source. This patch does this by doing the following when we&#39;ve closed on an RTP source:
 * If an RTP packet comes in from a new alternate source, we perform the usual probation mode checking on that RTP source. If we pass probation, we lock onto that as the new RTP source.
 * If an RTP packet comes in from the current source, we reset the counters on the alternate source&#39;s probation.

This has the effect of switching to a new, alternate source if the current source stops sending packets and we pass a probation mode check on the alternate source. We will never lock onto an alternate source if the current source keeps sending RTP, as we reset the probation mode counters on the alternate source. This prevents the one-way audio scenario as eventually, Asterisk 2 stops sending RTP and Asterisk 1 will switch over to Phone 2 as the RTP source. It avoids the vulnerability strictrtp was designed to prevent as, once locked on, the RTP source will never change so long as the source continues to send RTP.


</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Two different scenarios were tested that resulted in this issue. Both involved multiple phones hanging off of multiple Asterisk instances with different combinations of directmedia/no directmedia. This particular problem occurred about 50% of the time, due to different models of phones responding faster/slower to the re-INVITEs.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="https://issues.asterisk.org/jira/browse/AST-1124">AST-1124</a>, 

 <a href="https://issues.asterisk.org/jira/browse/AST-1125">AST-1125</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>branches/1.8/res/res_rtp_asterisk.c <span style="color: grey">(382367)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/2364/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>