ASTERISK-13526: [patch] If a SIP URI is resolved with SRV records, the port must no be in the Request-URI

[Home]

Summary: ASTERISK-13526: [patch] If a SIP URI is resolved with SRV records, the port must no be in the Request-URI

Reporter: klaus3000 (klaus3000) Labels:

Date Opened: 2009-02-06 04:04:24.000-0600 Date Closed: 2009-09-30 18:11:18

Priority: Minor Regression? No

Status: Closed/Complete Components: Channels/chan_sip/Interoperability

Versions: Frequency of
Occurrence

Related
Issues:

Environment: Attachments: ( 0) sip-ruri-fix1.diff
( 1) sip-ruri-fix2.diff
( 2) sip-ruri-fix3.diff
( 3) sip-ruri-fix4.diff
( 4) sip-ruri-fix5.diff
( 5) sip-ruri-fix6.diff

Description: Hi!

If Asterisk resolves a SIP domain by SRV, it should use the resolved IP:port for sending the transport only, not changing the RURI.

E.g. if Asterisk sends a request to sip:user@mydomain.com and resolves using SRV, then the requests URI must not contain the port.

-- Now forwarding SIP/u+437206200730153-b7058588 to 'SIP/2@app.innofon.at' (thanks to SIP/u+437206200730151-08ef44e8)
-- ast_get_srv: SRV lookup for '_sip._udp.app.innofon.at' mapped to host app.innofon.at, port 5160
Audio is at 81.16.153.184 port 12012
Adding codec 0x4 (ulaw) to SDP
Adding codec 0x8 (alaw) to SDP
Adding codec 0x400 (ilbc) to SDP
Adding codec 0x2 (gsm) to SDP
Adding non-codec 0x1 (telephone-event) to SDP
Reliably Transmitting (no NAT) to 81.16.153.184:5160:
INVITE sip:2@app.innofon.at:5160 SIP/2.0
Via: SIP/2.0/UDP 81.16.153.184:5160;branch=z9hG4bK462e8ee7;rport
From: "3 (SNOM 200)" <sip:3@81.16.153.184:5160>;tag=as054e6763
To: <sip:2@app.innofon.at:5160>
Contact: <sip:3@81.16.153.184:5160>
Call-ID: 455000696c1b0a0829b83b384670155e@81.16.153.184
CSeq: 102 INVITE
User-Agent: InnoSIP-app
Max-Forwards: 70
Date: Fri, 06 Feb 2009 09:55:15 GMT
Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, NOTIFY
Supported: replaces
Content-Type: application/sdp
Content-Length: 334

****** ADDITIONAL INFORMATION ******

-- Now forwarding SIP/u+437201230730153-b7058588 to 'SIP/2@app.mydomain.at' (thanks to SIP/u+437201230730151-08ef44e8)
-- ast_get_srv: SRV lookup for '_sip._udp.app.example.at' mapped to host app.example.at, port 5160
Audio is at 11.11.111.184 port 12012
Adding codec 0x4 (ulaw) to SDP
Adding codec 0x8 (alaw) to SDP
Adding codec 0x400 (ilbc) to SDP
Adding codec 0x2 (gsm) to SDP
Adding non-codec 0x1 (telephone-event) to SDP
Reliably Transmitting (no NAT) to 11.11.111.184:5160:
INVITE sip:2@app.example.at:5160 SIP/2.0
^^^^^^
bug, the port should be omitted as it is not present
in the requested URI (2@app.mydomain.at)

Via: SIP/2.0/UDP 11.11.111.184:5160;branch=z9hG4bK462e8ee7;rport
From: "3 (SNOM 200)" <sip:3@11.11.111.184:5160>;tag=as054e6763
To: <sip:2@app.example.at:5160>
Contact: <sip:3@11.11.111.184:5160>
Call-ID: 455000696c1b0a0829b83b384670155e@11.11.111.184
CSeq: 102 INVITE
User-Agent: XXX-SIP-app
Max-Forwards: 70
Date: Fri, 06 Feb 2009 09:55:15 GMT
Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, NOTIFY
Supported: replaces
Content-Type: application/sdp
Content-Length: 334

Comments: By: klaus3000 (klaus3000) 2009-02-06 04:16:05.000-0600

There is one more problem: If the port is explicitly specified as port 5060, Asterisk removes it from the RURI:

-- Now forwarding SIP/u+437206200730153-08ae3b40 to 'SIP/2@orf.at:5060' (thanks to SIP/u+437206200730151-08a86328)
Audio is at 81.16.153.184 port 19454
Adding codec 0x4 (ulaw) to SDP
Adding codec 0x8 (alaw) to SDP
Adding codec 0x400 (ilbc) to SDP
Adding codec 0x2 (gsm) to SDP
Adding non-codec 0x1 (telephone-event) to SDP
Reliably Transmitting (no NAT) to 194.232.104.30:5060:
INVITE sip:2@orf.at SIP/2.0
^^^^^
bug: here should be 2@orf.at:5060

Even if 5060 is the default port, these are 2 differen URIs. On of the reasons is that as soon a port is present in the URI, SRV lookups must not be performed.

See also http://bugs.digium.com/view.php?id=14419
By: Iñaki Baz Castillo (ibc) 2009-03-12 08:58:03

I fully agree on this issue.
The following two URI's are not the same:

1) sip:1234@mydomain.org
2) sip:1234@mydomain.org:5060

In case 1 SRV resolution should take place and the result could be any address and any port.
In case 2 SRV resolution should *not* take place since port is specified, so just a DNS A query must be performed for domain "mydomain.org".
By: Matthew Nicholson (mnicholson) 2009-09-02 18:21:27

Please test with the patch I uploaded and me know if it fixes your uri issues.
By: Olle Johansson (oej) 2009-09-03 03:23:02

I agree that a URI with and without port is different. But where do we get the port number from, Klaus? You say it's specified - but where?
By: Olle Johansson (oej) 2009-09-03 03:24:00

I think we will have to check dialstrings as well. If a port is given in a dialstring, we need to keep it even if it's 5060 and stop DNS lookups.
By: Olle Johansson (oej) 2009-09-03 03:24:25

and to follow up - if a port is specified in a peer, we need to keep it and make sure it's added.
By: klaus3000 (klaus3000) 2009-09-03 04:22:53

@Olle: I am not the chan_sip expert, but I see following options where the target URI comes from:
- specified in dialplan: Dial(SIP/user@hostport)
- provided by SIP client during registration
- provided in sip.conf peer definition

If the target URI contains a port, SRV lookups must not be performed and the same URI (including the port) must be used as request URI.

If the target URI does not contains a port, SRV lookups must be performed (unless an IP address is specified) and the same URI (without port) must be used as request URI.

When the URI is specified in the dialplan it is rather clear.

When the target URI is constructed from sip.conf peer definition there might be changes needed because I think the default "port" is 5060 (if not specified). Probably the default port (in the [peer] section) should be 0. This means that the default port should be used, but the port is not part of the URI. Only if the port is explicitly specified with port=5060, then the port should be added to the URI.
By: Olle Johansson (oej) 2009-09-03 06:08:10

We already do not perform an DNS lookup if there's a port number given. What I'm afraid we missed is relaying that information.
By: Matthew Nicholson (mnicholson) 2009-09-03 09:15:44

@klaus3000

There is also one additional case we need to handle:
- port specified in dialplan and specified in sip.conf or during registration: Dial(SIP/123@peer:1234)
By: Matthew Nicholson (mnicholson) 2009-09-03 09:18:22

The patch I posted should handle the cases for when the port is specified in the dialplan. I still need to modify it to work properly for the other cases (port in sip.conf). I believe the code should already work properly when the uri is gathered from a registration request.
By: klaus3000 (klaus3000) 2009-09-03 09:22:50

I wait with testing till the patch is complete
By: Matthew Nicholson (mnicholson) 2009-09-03 14:01:43

I have updated the patch to support peers.
By: Matthew Nicholson (mnicholson) 2009-09-15 13:00:23

Has anyone been able to test the latest patch?
By: Matthew Nicholson (mnicholson) 2009-09-16 15:38:05

I tested my patch and it fixes most of the problems reported here. Here is a summary of my findings for each of the scenarios where URIs are generated.

- port specified in dialplan: Dial(SIP/user@host:port)
With the patch this works properly. I tested with port = 5060 and with other port numbers.

- port provided by SIP client during registration
I don't think this works properly with fix2 but I am preparing fix3 that should fix this.

- port provided in sip.conf peer definition
This works properly with the patch.

- port specified in dialplan and specified in sip.conf or during registration: Dial(SIP/123@peer:1234)
This mostly works properly although there are problems with how peers are processed in the config file. SRV lookups are done when sip.conf is loaded regardless if a port is specified in the peer definition or not. This can cause problems if you later specify a port to the Dial command as the SRV record may have specified a different host than the one in the dialplan. This case is somewhat complicated.

I also thought of one additional case:
- asterisk generated Register requests
I don't think these are handled properly, but I am going to consider this a separate issue.
By: Matthew Nicholson (mnicholson) 2009-09-17 09:26:21

Uploaded fix4 which includes a minor fix for handling URIs received in a REGISTER request.
By: Matthew Nicholson (mnicholson) 2009-09-17 10:47:39

Review request posted here https://reviewboard.asterisk.org/r/369/
By: klaus3000 (klaus3000) 2009-09-21 10:51:59

I tested fix4 with:

- direct dialing Dial(SIP/user@hostport): works fine

- via sip-peers Dial(SIP/user@peername): fails in this case:
[peername]
type=peer
host=nxdomain.at
port=6060

Asterisk performs SRV lookup on startup (port ignored). For example if an SRV lookup is found (e.g. pointing to digium.com port 7070), Asterisk sends the request to digium.com:6060 instead of nxdomain.at:6060.
By: Matthew Nicholson (mnicholson) 2009-09-21 11:00:53

I noticed that behavior as well, that is somewhat of a separate issue, but I will see if I can work up a quick fix for that.
By: Matthew Nicholson (mnicholson) 2009-09-22 08:33:42

I have uploaded fix6 which should fix srv lookup behavior for peers with ports specified in the peer config.
By: klaus3000 (klaus3000) 2009-09-22 09:37:46

I retested srv for peers and it works now (at least outgoing). I have not tested if incoming peer identification has changed.

IMO you should also add a statement to sip.conf.sample about the new (correct) behavior, e.g:

srvlookup=yes ; Enable DNS SRV lookups on outbound calls
; Note: Asterisk only uses the first host
; in SRV records
; Disabling DNS SRV lookups disables the
; ability to place SIP calls based on domain
; names to some other SIP users on the Internet
+ ; Note: Specifying a "port" in the SIP peer
+ ; section supresses SRV lookups for this peer.
By: Matthew Nicholson (mnicholson) 2009-09-22 10:04:33

Thanks for testing. That config file note is a good idea.
By: Digium Subversion (svnbot) 2009-09-30 14:38:53

Repository: asterisk
Revision: 221360

U branches/1.4/channels/chan_sip.c
U branches/1.4/configs/sip.conf.sample

------------------------------------------------------------------------
r221360 | mnicholson | 2009-09-30 14:38:53 -0500 (Wed, 30 Sep 2009) | 10 lines

Fix SRV lookup and Request-URI generation in chan_sip.

This patch adds a new field "portinuri" to the sip dialog struct and the sip peer struct. That field is used during RURI generation to determine if the port should be included in the RURI. It is also used in some places to determine if an SRV lookup should occur.

(closes issue ASTERISK-13526)
Reported by: klaus3000
Tested by: klaus3000, mnicholson

Review: https://reviewboard.asterisk.org/r/369/

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=221360
By: Digium Subversion (svnbot) 2009-09-30 15:43:08

Repository: asterisk
Revision: 221432

_U trunk/
U trunk/channels/chan_sip.c
U trunk/configs/sip.conf.sample

------------------------------------------------------------------------
r221432 | mnicholson | 2009-09-30 15:43:07 -0500 (Wed, 30 Sep 2009) | 17 lines

Merged revisions 221360 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r221360 | mnicholson | 2009-09-30 14:36:06 -0500 (Wed, 30 Sep 2009) | 10 lines

Fix SRV lookup and Request-URI generation in chan_sip.

This patch adds a new field "portinuri" to the sip dialog struct and the sip peer struct. That field is used during RURI generation to determine if the port should be included in the RURI. It is also used in some places to determine if an SRV lookup should occur.

(closes issue ASTERISK-13526)
Reported by: klaus3000
Tested by: klaus3000, mnicholson

Review: https://reviewboard.asterisk.org/r/369/
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=221432
By: Digium Subversion (svnbot) 2009-09-30 17:28:23

Repository: asterisk
Revision: 221477

_U branches/1.6.2/
U branches/1.6.2/channels/chan_sip.c
U branches/1.6.2/configs/sip.conf.sample

------------------------------------------------------------------------
r221477 | mnicholson | 2009-09-30 17:28:23 -0500 (Wed, 30 Sep 2009) | 24 lines

Merged revisions 221432 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r221432 | mnicholson | 2009-09-30 15:40:20 -0500 (Wed, 30 Sep 2009) | 17 lines

Merged revisions 221360 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r221360 | mnicholson | 2009-09-30 14:36:06 -0500 (Wed, 30 Sep 2009) | 10 lines

Fix SRV lookup and Request-URI generation in chan_sip.

This patch adds a new field "portinuri" to the sip dialog struct and the sip peer struct. That field is used during RURI generation to determine if the port should be included in the RURI. It is also used in some places to determine if an SRV lookup should occur.

(closes issue ASTERISK-13526)
Reported by: klaus3000
Tested by: klaus3000, mnicholson

Review: https://reviewboard.asterisk.org/r/369/
........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=221477
By: Digium Subversion (svnbot) 2009-09-30 17:39:41

Repository: asterisk
Revision: 221478

_U branches/1.6.1/
U branches/1.6.1/channels/chan_sip.c
U branches/1.6.1/configs/sip.conf.sample

------------------------------------------------------------------------
r221478 | mnicholson | 2009-09-30 17:39:41 -0500 (Wed, 30 Sep 2009) | 24 lines

Merged revisions 221432 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r221432 | mnicholson | 2009-09-30 15:40:20 -0500 (Wed, 30 Sep 2009) | 17 lines

Merged revisions 221360 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r221360 | mnicholson | 2009-09-30 14:36:06 -0500 (Wed, 30 Sep 2009) | 10 lines

Fix SRV lookup and Request-URI generation in chan_sip.

This patch adds a new field "portinuri" to the sip dialog struct and the sip peer struct. That field is used during RURI generation to determine if the port should be included in the RURI. It is also used in some places to determine if an SRV lookup should occur.

(closes issue ASTERISK-13526)
Reported by: klaus3000
Tested by: klaus3000, mnicholson

Review: https://reviewboard.asterisk.org/r/369/
........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=221478
By: Digium Subversion (svnbot) 2009-09-30 18:11:16

Repository: asterisk
Revision: 221486

_U branches/1.6.0/
U branches/1.6.0/channels/chan_sip.c
U branches/1.6.0/configs/sip.conf.sample

------------------------------------------------------------------------
r221486 | mnicholson | 2009-09-30 18:11:16 -0500 (Wed, 30 Sep 2009) | 24 lines

Merged revisions 221432 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r221432 | mnicholson | 2009-09-30 15:40:20 -0500 (Wed, 30 Sep 2009) | 17 lines

Merged revisions 221360 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r221360 | mnicholson | 2009-09-30 14:36:06 -0500 (Wed, 30 Sep 2009) | 10 lines

Fix SRV lookup and Request-URI generation in chan_sip.

This patch adds a new field "portinuri" to the sip dialog struct and the sip peer struct. That field is used during RURI generation to determine if the port should be included in the RURI. It is also used in some places to determine if an SRV lookup should occur.

(closes issue ASTERISK-13526)
Reported by: klaus3000
Tested by: klaus3000, mnicholson

Review: https://reviewboard.asterisk.org/r/369/
........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=221486