<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/6413">View Change</a></p><p>Patch set 2:</p><p>(4 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/6413/2/res/res_rtp_asterisk.c">File res/res_rtp_asterisk.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6413/2/res/res_rtp_asterisk.c@5374">Patch Set #2, Line 5374:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Ast1 may see an initial RTP stream from<br> * Ast2 and then a stream from PartyB. Since Ast1 may not have<br> * seen or learned the RTP stream from Ast2 before being told to learn<br> * the stream from PartyB, Ast1 may lock onto the Ast2 stream and<br> * never learn the PartyB stream.<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'd just replace this with "Ast1 may lock onto the Ast2 stream and never le</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6413/2/res/res_rtp_asterisk.c@5382">Patch Set #2, Line 5382:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> * To mitigate against attack, the learning state cannot switch<br> * streams while there are competing streams. The competing streams<br> * interfere with each other's qualification. Once we accept a<br> * stream and reach the timeout, an attacker cannot interfere<br> * anymore.<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So as long as there are competing streams, we'll lock onto none of them rig</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">There are a few scenarios and each one assumes that the streams are continuous:<br>1) We already have a known stream source and the known stream wants to change to a new source. An attacking stream will block learning the new stream source. After the timeout we re-lock onto the original stream source which likely went away. The result is one way audio.</p><p style="white-space: pre-wrap; word-wrap: break-word;">2) We already have a known stream source and the known stream doesn't want to change source addresses. An attacking stream will not be able to replace the known stream. After the timeout we re-lock onto the known stream. The call is not affected.</p><p style="white-space: pre-wrap; word-wrap: break-word;">3) We don't have a known stream. This presumably is the start of a call. Competing streams will result in staying in learning mode until a stream becomes the victor. The result is one way audio until there is a victor. We cannot timeout because we have no known stream to lock onto.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If we learn a stream before the timeout we will be in scenario 1) or 2) when a competing stream starts.</p><p style="white-space: pre-wrap; word-wrap: break-word;"><br>Were you wanting me to add this information to the comments?</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6413/2/res/res_rtp_asterisk.c@5400">Patch Set #2, Line 5400:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">learning for the new source so it only takes over once<br> * sufficient traffic has been received.<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">As long as we receive packets from the original source, we keep resetting t</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6413/2/res/res_rtp_asterisk.c@5407">Patch Set #2, Line 5407:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> /*<br> * We give preferential treatment to the requested target address<br> * (negotiated SDP address) where we are to send our RTP. However,<br> * the other end has no obligation to send from that address even<br> * though it is practically a requirement when NAT is involved.<br> */<br> ast_rtp_instance_get_requested_target_address(instance, &target_address);<br> if (!ast_sockaddr_cmp(&target_address, &addr)) {<br> /* Accept the negotiated target RTP stream as the source */<br> ast_verb(4, "%p -- Strict RTP switching to RTP target address %s as source\n",<br> rtp, ast_sockaddr_stringify(&addr));<br> ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);<br> rtp_learning_seq_init(&rtp->rtp_source_learn, seqno);<br> break;<br> }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">How preferential? Maybe this should be before the old/new source check?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It is preferential for becoming the new source when it is not the current source.</p><p style="white-space: pre-wrap; word-wrap: break-word;">We cannot move this check before the current source check. If it is not the current source it becomes the current source. If it is the current source then the check is redundant and we will spam the switching source message.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/6413">change 6413</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/6413"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ia2d3aa6e0f22906c25971e74f10027d96525f31c </div>
<div style="display:none"> Gerrit-Change-Number: 6413 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 15 Sep 2017 17:50:04 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>