[asterisk-dev] [Code Review] unbreaking <sip:username> from-uri support for REGISTER

[Terry Wilson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1533/#review4838
-----------------------------------------------------------

Ship it!


Looks good. I'd really like to see tests written that cover this behavior. It looks like you manually tested all of the cases--would that be easy to translate into some testsuite tests? If not, let me know and I'll work on creating them.


/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1533/#comment9028>

    The way I look at it, we don't treat it as a username-only URI, we instead make the policy decision that we allow admins to create accounts named after domains and match a registration from an entity claiming to represent that domain to the user named after that domain.
    
    There really isn't any question as to whether or not it is valid, it is entirely a policy decision.


- Terry


On Nov. 14, 2011, 1:51 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1533/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2011, 1:51 p.m.)
> 
> 
> Review request for Asterisk Developers and Terry Wilson.
> 
> 
> Summary
> -------
> 
> This issue addesses the regression that "sip:username" no longer works as
> a REGISTER username.
> 
> The changeset does mainly this:
> 
> if no username is supplied, we use the domain as the username: <sip:host:port>
> effectively becomes <sip:host at host:port>
> 
> (My personal preference would be having sip:hostport be rejected for both
> register and invites. Especially since sip:username registration had been broken
> for quite some time. But there seems to be Digium consensus to keep it. It
> was not broken for invites, so here is the re-addition for registers.)
> 
> 
> The details of the changeset:
> -----------------------------
> 
> (A) When registering/inviting, we now disallow an empty domain after the '@'
> 
>    from-uri                 BEFORE      AFTER
>  - sip:username at domain       OK          OK
>  - sip:username              fail        OK
>  - sip:username@             OK          fail
>  - sip:@username             fail        OK <-- side-effect of parse_uri
>  
> (B) When registering, we now check domain ACLs when in place, even when no
>     domain is supplied:
> 
>    from-uri                     BEFORE      AFTER      
>  - sip:validuser at validdomain     OK          OK
>  - sip:validuser                 fail        fail
>  - sip:validuser@                OK          fail
>  - sip:validboth                 fail        OK
>  - sip:validboth@                OK          fail
>  - sip:@validboth                fail        OK <-- side-effect of parse_uri
> 
> (C) When refusing a register with an invalid domain, we send the fake auth
>     rejection.
> 
> (D) The domain is checked for empty in check_user_full() for consistency with
>     register_verify().
> 
> (E) I renamed 'of' to 'name' in check_user_full() for clarity and similarity
>     with register_verify(). I renamed a couple of 'dummy' variables to
>     'unused_password' and standardized the variable declarations to the most
>     common occurrence.
> 
> 
> This addresses bug ASTERISK-18389.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18389
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 345163 
> 
> Diff: https://reviewboard.asterisk.org/r/1533/diff
> 
> 
> Testing
> -------
> 
> Tested the BEFORE and AFTER results when registering and inviting.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111128/94c81732/attachment-0001.htm>