[asterisk-dev] [Code Review] 3447: Send real CallerID information with P-Asserted-Identity (RFC-3325)
Jonathan Rose
reviewboard at asterisk.org
Fri Apr 18 10:33:01 CDT 2014
> On April 18, 2014, 2:06 a.m., wdoekes wrote:
> > Looks mostly good. The documentation could use some tweaking.
> > And I'm unsure what you did with fromdomain there.
> >
> >
> > Also, unrelated, your latest chart said this:
> >
> > > <pps:public> == party=calling;privacy=off;screen=no
> > > <pps:private> == party=calling;privacy=full;screen=yes
> >
> > While your comment said this:
> >
> > > Well, I had no plans of making any changes to how
> > > party/privacy/screen are set, so he can rest easy on that one.
> >
> > That did still leave the possibility for extra confusion in this regard.
> > 'screen=whatever' or 'screen=no' in both places would have been more
> > appropriate. But yes, Pavel can rest easy.
Yes, sorry, that was more for my reference when writing tests than anything else. None of those values are in any way touched by the patch.
> On April 18, 2014, 2:06 a.m., wdoekes wrote:
> > /branches/1.8/channels/chan_sip.c, lines 11428-11432
> > <https://reviewboard.asterisk.org/r/3447/diff/3/?file=57619#file57619line11428>
> >
> > Aren't you altering legacy here?
> >
> > Previously we would always use host_remote if p->fromdomain was empty. Now we only use it when trust_id_outbound=yes.
No, that condition starts in the line above, so it might be the case that you just missed the first part of the condition.
if (!fromdomain ||
((ast_test_flag(&p->flags[1], SIP_PAGE2_TRUST_ID_OUTBOUND) == SIP_PAGE2_TRUST_ID_OUTBOUND_YES) &&
!strcmp("anonymous.invalid", fromdomain))) {
/* If the fromdomain is NULL or if it was set to anonymous.invalid due to privacy settings and we trust the peer,
* use the host IP address */
fromdomain = ast_sockaddr_stringify_host_remote(&p->ourip);
}
(!fromdomain || (...)) is true in the case you are describing (fromdomain = p->fromdomain which is empty), so we use host_remote just as before.
> On April 18, 2014, 2:06 a.m., wdoekes wrote:
> > /branches/1.8/CHANGES, lines 15-17
> > <https://reviewboard.asterisk.org/r/3447/diff/3/?file=57618#file57618line15>
> >
> > Should this be "chan_sip" changes? Since there is that other driver in asterisk 12.
Fair point. There were previous logs for chan_sip called just 'SIP Changes', but you are right, precedent goes out the window in this case.
> On April 18, 2014, 2:06 a.m., wdoekes wrote:
> > /branches/1.8/CHANGES, lines 15-25
> > <https://reviewboard.asterisk.org/r/3447/diff/3/?file=57618#file57618line15>
> >
> > > By default, legacy is used.
> > > trust_id_outbound=legacy: behavior
> > > remains the same as 1.8.26.1 - When
> > > dealing with prohibited callingpres,
> > > RPID/PAI headers are created for both
> > > sendrpid=pai and sendrpid=rpid are
> > > appended, but the data is anonymized.
> >
> > sendrpid=rpid + legacy is *not* anonymous, while sendrpid=pai + legacy *is*.
> >
> > Clarity in here isn't as important to me as clarity in sip.conf though.
sendrpid=rpid + legacy is slightly anonymized... just not fully anonymized. When the peer's callingpres is set to one of the private types like 'prohib', then the fromdomain is set to FROMDOMAIN_INVALID (see initreqprep) which is anonymous.invalid. I'm not sure if there are any conditions where fromdomain will be changed again... in which case legacy wouldn't anonymize it at that point, but the presence of anonymous.invalid is why there was a need for the stuff I described in the finding below.
> On April 18, 2014, 2:06 a.m., wdoekes wrote:
> > /branches/1.8/configs/sip.conf.sample, lines 345-347
> > <https://reviewboard.asterisk.org/r/3447/diff/3/?file=57621#file57621line345>
> >
> > in case of PAI, private data will be anonymized (following historic behaviour violating RFC-3325)
> >
> > in case of RPID, this behaves as trust_id_outbound=yes
In the case of RPID, not entirely. If trust_id_outbound is legacy and the fromdomain is anonymous.invalid coming into the function that appends the RPID header, the domain chosen will be anonymous.invalid. If trust_id_outbound=yes, then because fromdomain is anonymous.invalid, the host address will be used instead.
I found it all a little confusing myself, but it was the best I could make of a bad situation.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3447/#review11681
-----------------------------------------------------------
On April 17, 2014, 3:25 p.m., Jonathan Rose wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3447/
> -----------------------------------------------------------
>
> (Updated April 17, 2014, 3:25 p.m.)
>
>
> Review request for Asterisk Developers, Joshua Colp, Matt Jordan, Mark Michelson, and wdoekes.
>
>
> Bugs: AST-1301 and ASTERISK-19465
> https://issues.asterisk.org/jira/browse/AST-1301
> https://issues.asterisk.org/jira/browse/ASTERISK-19465
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> Walter Doekes pointed out that this might cause a less than ideal situation in which people who were expecting P-Asserted-Identity not to disclose party information will now be sending privacy information, so I pulled this patch from 1.8-trunk and we will now review it here.
>
> Without this patch, P-Asserted-Identity would always use anonymous for the caller ID information, and RFC-3325 seems to indicate that P-Asserted-Identity is something that should not be anonymized, but also only sent to trusted parties. The way this was presented to me, the intent here is that if you set callerpres to prohibited for a peer that receives P-Asserted-Identity, the P-Asserted-Identity shouldn't be anonymized, only the normal From/Contact headers would be anonymized. This apparently
>
> The obvious method for dealing with this mid-release change is to make the change into an option which defaults off in 1.8-12 while defaulting on in trunk. Also I'll need to add Upgrade notes for trunk since this might not always be a desired behavior as well as CHANGES notes throughout to indicate the new option if that's what we settle on.
>
>
> Diffs
> -----
>
> /branches/1.8/configs/sip.conf.sample 412438
> /branches/1.8/channels/sip/include/sip.h 412438
> /branches/1.8/channels/chan_sip.c 412438
> /branches/1.8/CHANGES 412438
>
> Diff: https://reviewboard.asterisk.org/r/3447/diff/
>
>
> Testing
> -------
>
> Call from SIP peer A to SIP peer B
> settings for both peers:
> sendrpid = pai
> callerpres = prohib
>
>
> Invite sent from Asterisk to the recipient of the call
> ------------------------------------------------------
> Prior to patch:
>
> Audio is at 19640
> Adding codec 0x4 (ulaw) to SDP
> Adding non-codec 0x1 (telephone-event) to SDP
> Reliably Transmitting (NAT) to 10.24.18.240:5060:
> INVITE sip:123 at 10.24.18.240:5060 SIP/2.0
> Via: SIP/2.0/UDP 10.24.18.246:5060;branch=z9hG4bK2fb42910;rport
> Max-Forwards: 70
> From: "Anonymous" <sip:anonymous at anonymous.invalid>;tag=as13075548
> To: <sip:123 at 10.24.18.240:5060>
> Contact: <sip:anonymous at 10.24.18.246:5060>
> Call-ID: 762b8a5e5848d7997f38f71a770d4dd9 at 10.24.18.246:5060
> CSeq: 102 INVITE
> User-Agent: Asterisk PBX SVN-branch-1.8-r410380
> Date: Tue, 11 Mar 2014 22:59:39 GMT
> Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, NOTIFY, INFO, PUBLISH
> Supported: replaces, timer
> P-Asserted-Identity: "Anonymous" <sip:anonymous at anonymous.invalid>
> Content-Type: application/sdp
> Content-Length: 276
>
> v=0
> o=root 473543868 473543868 IN IP4 10.24.18.246
> s=Asterisk PBX SVN-branch-1.8-r410380
> c=IN IP4 10.24.18.246
> t=0 0
> m=audio 19640 RTP/AVP 0 101
> a=rtpmap:0 PCMU/8000
> a=rtpmap:101 telephone-event/8000
> a=fmtp:101 0-16
> a=silenceSupp:off - - - -
> a=ptime:20
> a=sendrecv
>
>
> After patch:
>
> Audio is at 11822
> Adding codec 0x4 (ulaw) to SDP
> Adding non-codec 0x1 (telephone-event) to SDP
> Reliably Transmitting (NAT) to 10.24.18.240:5060:
> INVITE sip:123 at 10.24.18.240:5060 SIP/2.0
> Via: SIP/2.0/UDP 10.24.18.246:5060;branch=z9hG4bK5d4a7db8;rport
> Max-Forwards: 70
> From: "Anonymous" <sip:anonymous at anonymous.invalid>;tag=as181a14e3
> To: <sip:123 at 10.24.18.240:5060>
> Contact: <sip:anonymous at 10.24.18.246:5060>
> Call-ID: 721bef28208f7633288e929c6e88824e at 10.24.18.246:5060
> CSeq: 102 INVITE
> User-Agent: Asterisk PBX SVN-branch-1.8-r410380M
> Date: Tue, 11 Mar 2014 22:57:39 GMT
> Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, NOTIFY, INFO, PUBLISH
> Supported: replaces, timer
> P-Asserted-Identity: "Goldy Locks" <sip:6018 at 10.24.18.246>
> Privacy: id
> Content-Type: application/sdp
> Content-Length: 279
>
> v=0
> o=root 1606369071 1606369071 IN IP4 10.24.18.246
> s=Asterisk PBX SVN-branch-1.8-r410380M
> c=IN IP4 10.24.18.246
> t=0 0
> m=audio 11822 RTP/AVP 0 101
> a=rtpmap:0 PCMU/8000
> a=rtpmap:101 telephone-event/8000
> a=fmtp:101 0-16
> a=silenceSupp:off - - - -
> a=ptime:20
> a=sendrecv
>
>
> Thanks,
>
> Jonathan Rose
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140418/dac32dc7/attachment-0001.html>
More information about the asterisk-dev
mailing list