<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/1803/">https://reviewboard.asterisk.org/r/1803/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On March 7th, 2012, 1:01 a.m., <b>wdoekes</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for posting the patch here. For those who have not read the bugreport, I&#39;ll summarize my problems with this patch:

When dealing with peers, some should be allowed to see private/prohibited CLI&#39;s (CLIR) and some shouldn&#39;t -- sip2pstn links usually should and endusers usually shouldn&#39;t. I call these trusted and untrusted, but someone may have a better term form them.

Current behaviour in Asterisk is as follows:
 - sendrpid=no =&gt; the peer does *not* get CLIR
 - sendrpid=yes/rpid =&gt; the peer *does* get CLIR (with privacy=full tag)
 - sendrpid=pai =&gt; the peer does *not* get CLIR (but Anonymous@...)

Yes, that is inconsistent, but that is the current situation.


What is good about this patch:

- The Privacy header is sent properly. However, IMO, this is only useful for these so-called &#39;trusted&#39; peers.

- There is now a difference between trust levels.


My problems with this patch:

- It breaks backwards compatibility: sendrpid=pai will show CLIR to anyone.

- There is no way to disable sending of CLIR. When we don&#39;t trust the peer, the PAI should not get sent.
  Now, all it does is set From to Anonymous, but still send the CLIR.
   http://tools.ietf.org/html/rfc3325#section-10.2 
   &quot;&quot;&quot;The next proxy removes the P-Asserted-Identity
   header field and the request for Privacy before forwarding this
   request onward to the biloxi.com proxy server which it does not
   trust.&quot;&quot;&quot;

- The same goes for RPID, but there CLIR to untrusted peers should probably behave as PAI does now: set Anonymous@ in the header.


I like this fix, but it needs some tweaking IMO.</pre>
 </blockquote>







</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi,
my comments now :)
&quot;...Current behaviour in Asterisk is as follows:
 - sendrpid=no =&gt; the peer does *not* get CLIR
 - sendrpid=yes/rpid =&gt; the peer *does* get CLIR (with privacy=full tag)
 - sendrpid=pai =&gt; the peer does *not* get CLIR (but Anonymous@...)
...

- It breaks backwards compatibility: sendrpid=pai will show CLIR to anyone.&quot;

the current behaviour of PAI is buggy, there is no such possibility to send anonymous@anonymous.invalid in PAI header - according to RFC, and this is a bug in current version. My patch fixes it. Now there is no way to send in PAI: anonymous@anonymous.invalid only the true number (as in RFC). The anonymous@anonymous.invalid is put to FROM header accordingly if you not trust the peer. If you trust, then there will go the true presentation. When you are sending CLIR in PAI Privacy header is changed to &quot;id&quot;, and that&#39;s the right behaviour according to RFC.

&quot;- There is no way to disable sending of CLIR. When we don&#39;t trust the peer, the PAI should not get sent.
  Now, all it does is set From to Anonymous, but still send the CLIR.
   http://tools.ietf.org/html/rfc3325#section-10.2 
   &quot;&quot;&quot;The next proxy removes the P-Asserted-Identity
   header field and the request for Privacy before forwarding this
   request onward to the biloxi.com proxy server which it does not
   trust.&quot;&quot;&quot;

Simply set &quot;no&quot; for sendrpid for this peer. I do not understand your problem with that. If you do not like to send PAI header to peer, than sendrpid should be send to no.

&quot;- The same goes for RPID, but there CLIR to untrusted peers should probably behave as PAI does now: set Anonymous@ in the header.&quot;

It works as in RFC now. Anonymous@anonymous.invalid is send only in FROM header in &quot;rpid,untrusted&quot;, if trusted in FROM we have CPN.
In RPID header always go CPN and if CLIR privacy is set to full.

</pre>
<br />








<p>- jamicque</p>


<br />
<p>On March 6th, 2012, 5:16 p.m., jamicque 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.</div>
<div>By jamicque.</div>


<p style="color: grey;"><i>Updated March 6, 2012, 5:16 p.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;">It seams that in Asterisk privacy with PAI is not implemented correctly.

According to RFC 3325 when using privacy, FROM header should be set to anonymous@anonymous.invalid and PAI header should be set to caller num and name. The privacy is implemented by adding privacy: id header.
Now when we use pai and callpres=prohib in P-Asserted-Identity header we have something which is not correct to any rfc.
P-Asserted-Identity: &quot;Anonymous&quot; &lt;sip:anonymous@anonymous.invalid&gt;

What my patch does:
1) it adds Privacy header when PAI is used (values &quot;none&quot; or &quot;id&quot; depending on callpres)
2)
3) &quot;sendrpid&quot; configuration option have been expanded:
now it can have those values:

    no - nothing changed
    yes - rpid header is added, when call PRES=prohi, FROM header is not changed
    rpid - the same as yes
    pai - pai header is added, when call PRES=prohi, FROM header is not changed

NEW VALUES:

    rpid,trusted (NEW) - the same as yes
    rpid,untrusted (NEW) - rpid header is added, when call PRES=prohi, FROM header is chenged to anonymous@anonymous.invalid
    pai,trusted (NEW) - the same as pai
    pai,untrusted (NEW) - pai header is added, when call PRES=prohi, FROM header is chenged to anonymous@anonymous.invalid - as in RFC 3325
</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;">I&#39;ve done some basing test with outgoing calls and everything seems to wroks fine.</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/ASTERISK-19465">ASTERISK-19465</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>/trunk/channels/chan_sip.c <span style="color: grey">(358434)</span></li>

 <li>/trunk/channels/sip/include/sip.h <span style="color: grey">(358434)</span></li>

 <li>/trunk/configs/sip.conf.sample <span style="color: grey">(358434)</span></li>

</ul>

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




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








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