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

[wdoekes]

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

(Updated Nov. 13, 2011, 3:49 p.m.)


Review request for Asterisk Developers and Terry Wilson.


Changes
-------

Ok.. this new version does what Terry suggests:

if no username is supplied, we use the domain as the username: <sip:host:port>
effectively becomes <sip:host at host:port>

It is now against 1.8 because it's now a bugfix to fix that "sip:username" works
again.

(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                 OK          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().

(F) I removed some unneeded 'dummy' variables and added some braces.


Regards,
Walter


Summary (updated)
-------

See the bug report: there were some XXX'es in the code about code that should be removed.

The patch does this:

(1) register_verify won't accept a To: without user-part anymore (illegal according to rfc3261, 10.2)

(2) check_user_full still doesn't require a user-part, but it won't match usernames by domain anymore. (i.e. it doesn't treat sip:domain as sip:domain at domain anymore)

(3) there was some freaky logic going on in get_msg_text, I had to rewrite it to make it make sense.

(4) in the reqresp parser there were lots of if (params) inside a big if (params) block. I scrapped the useless if's.


This addresses bug ASTERISK-18389.
    https://issues.asterisk.org/jira/browse/ASTERISK-18389


Diffs (updated)
-----

  /branches/1.8/channels/chan_sip.c 345022 

Diff: https://reviewboard.asterisk.org/r/1533/diff


Testing
-------

It compiles.


Thanks,

wdoekes

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