Summary:	ASTERISK-13325: [patch] Not possible to register to a registrar via a host with different port number
Reporter:	nick_lewis (nick_lewis)	Labels:
Date Opened:	2009-01-08 08:51:42.000-0600	Date Closed:	2009-01-13 15:19:02.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/Registration
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_sip.c-domainport.patch ( 1) chan_sip.c-domainport2.patch ( 2) chan_sip.c-regdomain.patch
Description:	It is currently possible to register to a registrar indirectly via a host using an extended format of the registration string [transport://]user[:secret[:authuser]]@host[:port][/contact][~expiry] In this extended format the "user" portion is expanded to "username@domain". In this case the registration packet is sent to the address of the "host" but the request line uri indicates that the registrar is at the address of the "domain". This works fine as long as default port numbers are used However if the host includes a "port" other than 5060 then the request line uri also contains this port even though the registrar may be on a different port number or on the standard port There needs to be a way in the registration string to specify the port of the registar too e.g. "user" expand to "username@domain:domainport"
Comments:	By: nick_lewis (nick_lewis) 2009-01-08 12:06:59.000-0600 Please find attached a patch that permits the domain port to be specified differently from the host port in the registration string [transport://]user[@domain[:domainport]][:secret[:authuser]]@host[:port][/contact][~expiry] Note that there is some ambiguity in the boundary between [@domain[:domainport]] and [:secret[:authuser]] due to the use of the : character for all of them. The patch implements the priority of the [:domainport] option behind the [:authuser] option so secret and authuser must be present if domainport is present By: Leif Madsen (lmadsen) 2009-01-08 13:56:37.000-0600 Confirming as we have a patch to be looked at, and assigning this to putnopvut for his thoughts. The format as is, I don't think is going to work because of the ambiguity you mention. Making something required that isn't normally required I think is going to be a bit of a no-go. However, I haven't thought of a better way of doing this yet :) By: nick_lewis (nick_lewis) 2009-01-09 05:35:27.000-0600 Yes it is not an easy one. I cannot imagine having a domainport separator other than the : character but it is not possible to change the secret separator without adversely affecting compatibility. I need this functionality due to the natproxy requirements of some of the ITSP that I need to interwork with. Therefore I am prepared to live with the requirement to specify a secret and authuser. I think others in my situation may be too. I guess that one solution may be to use something like [:domainport/]  but this looks only a little less odd than [/domainport] By: Leif Madsen (lmadsen) 2009-01-09 07:20:39.000-0600 Why don't you just make it so you have to place the extra separators, but still allow the secret and authuser to be blank? udp://lmadsen@remotedomain:5666::@somehost:5060/incoming~60 Especially since the way you have the syntax laid out implies that secret and authuser should be optional. Some documentation with examples of using the syntax's will also be necessary in order to try and confuse people less ;) By: nick_lewis (nick_lewis) 2009-01-09 07:48:02.000-0600 I accept that "domainport" is a new feature but the current behaviour with the domain parameter is a bug. In practice almost all registrars operate on port 5060 but natproxies that are used to access the registrar often operate on other ports such as 5082. If a domain is specified then the port should not be appended to request line uri. The uri should appear without a port number. This would allow most applications to operate correctly without the need to introduce a new parameter to the registration string. Perhaps I will produce a patch to do this and we can to return to the registration string domainport parameter once someone thinks of a good format. By: nick_lewis (nick_lewis) 2009-01-09 08:03:59.000-0600 Please find attached a patch that resolves the issue for most real world applications. The original bug was "Not possible to register to a registrar via a host with different port number". The patch fixes this for all cases except those in which the registrar is on a non standard port. Registering directly to a registrar is unaffected. By: nick_lewis (nick_lewis) 2009-01-09 08:11:42.000-0600 Sorry blitzrage I saw bug->feature and went off to make a domainport-free bug fix without first seeing your comment on blank secret and authuser. I will study this and do another patch to operate as you suggest By: nick_lewis (nick_lewis) 2009-01-09 09:21:09.000-0600 Please find attached a patch that adds support for blank domainport secret and authuser. This time it is a diff of HEAD so it should apply better too. I will not have time to compile and test it until Monday though By: nick_lewis (nick_lewis) 2009-01-12 08:02:20.000-0600 I have tested the patch chan_sip.c-domainport2.patch and it appears to work ok both with and without secret and authuser i.e. with registration string: username@domain:domainport:secret:authuser@host:port/contact~expiry and blank format: username@domain:domainport::@host:port/contact~expiry By: Leif Madsen (lmadsen) 2009-01-12 09:56:20.000-0600 Thanks Nick! I'll mark this one as Ready For Testing as well. Hopefully we've made a good choice in the format :) By: Mark Michelson (mmichelson) 2009-01-13 14:45:53.000-0600 I've been going over the notes here and trying to think of a way to allow the port to be present in the user portion without having to have "blank" secret and authuser sections. Aside from adding a new separator to be placed in between the port and the secret, I can't think of anything. I don't really like the idea of having to add a new separator though since I think the syntax would be unwieldy, so I suppose that Nick's solution is the best choice at this point. I think one thing that will be nice is that no one will be forced to update the formatting of their register lines when upgrading to a version which has this support (unless of course they decide to add a port to the "user" section). I'll place a nice big note in the sample configuration file that makes it clear that if you place a port in the user section and you don't specify a secret or authuser, then you will need to make sure to "blank" them so there is no trouble parsing the line. As far as the actual patch goes, the code looks correct, but it looks like there are some minor indention changes that need to be made to the code surrounding your changes, esp. in transmit_register. Since they are minor, I have no problem taking care of this myself. Thanks very much for the submission! As far as I'm concerned this is ready for merge, but I'll probably get another set of eyes to take a look just to be on the safe side. By: Mark Michelson (mmichelson) 2009-01-13 15:03:17.000-0600 I got file's approval, which on chan_sip issues, is about as good as it gets. Thanks for the patch, and I'll be merging it into trunk ASAP. By: Digium Subversion (svnbot) 2009-01-13 15:18:14.000-0600 Repository: asterisk Revision: 168575 U   trunk/CHANGES U   trunk/channels/chan_sip.c U   trunk/configs/sip.conf.sample ------------------------------------------------------------------------ r168575 | mmichelson | 2009-01-13 15:18:14 -0600 (Tue, 13 Jan 2009) | 13 lines Allow specifying a port number in the user portion of a register => line in sip.conf With this commit, a register => line in sip.conf may contain a port number in the "user" section of the line. Please see CHANGES and sip.conf.sample for more details regarding this. (closes issue ASTERISK-13325) Reported by: Nick_Lewis Patches:      chan_sip.c-domainport2.patch uploaded by Nick (license 657) Tested by: Nick_Lewis ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=168575 By: Digium Subversion (svnbot) 2009-01-13 15:19:01.000-0600 Repository: asterisk Revision: 168576 _U  branches/1.6.1/ ------------------------------------------------------------------------ r168576 | mmichelson | 2009-01-13 15:19:01 -0600 (Tue, 13 Jan 2009) | 20 lines Blocked revisions 168575 via svnmerge ........ r168575 | mmichelson | 2009-01-13 15:18:13 -0600 (Tue, 13 Jan 2009) | 13 lines Allow specifying a port number in the user portion of a register => line in sip.conf With this commit, a register => line in sip.conf may contain a port number in the "user" section of the line. Please see CHANGES and sip.conf.sample for more details regarding this. (closes issue ASTERISK-13325) Reported by: Nick_Lewis Patches:      chan_sip.c-domainport2.patch uploaded by Nick (license 657) Tested by: Nick_Lewis ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=168576