[Home]

Summary:ASTERISK-13550: [patch] chan_sip does not support the maddr attribute in Via headers
Reporter:frawd (frawd)Labels:
Date Opened:2009-02-10 04:12:22.000-0600Date Closed:2009-11-13 16:07:54.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Interoperability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) via_maddr.patch
Description:I recently connected Asterisk with a badly configured Nortel CS2K, which sends me the following Via header:

Via: SIP/2.0/UDP CS2KSSTEMAG:5060;maddr=213.60.204.48;branch=z9hG4bK-142abd7-ec6f41f2-4aeebdaf

CS2KSSTEMAG not being a valid DNS entry, the reply is never sent (Asterisk blocks trying to resolve the name). Even tho it is a configuration error, maddr should be used as reply address.

The attached patch fixes the issue in 1.4.23.1

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

Section 18.2.2 of RFC 3261 states:
"Otherwise, if the Via header field value contains a "maddr"
parameter, the response MUST be forwarded to the address listed
there, using the port indicated in "sent-by", or port 5060 if
none is present."
Comments:By: Olle Johansson (oej) 2009-02-10 11:23:25.000-0600

This is related to a few other bug reports on how to send replies. We should propably just send the reply to whatever address we got it from. Via headers is not really for UAS, more for proxy routing. But that is under discussion.

But following the way we currently send replies, I guess this is something to look into.
By: Olle Johansson (oej) 2009-02-10 11:25:21.000-0600

And there can be multiple via-values in one via header, so we need to search for the rightmost one...


By: Leif Madsen (lmadsen) 2009-02-10 11:36:40.000-0600

Assigning this issue to oej so he can relate it to whatever bug he was talking about on IRC that is related to this issue.
By: frawd (frawd) 2009-02-10 13:03:13.000-0600

This patch is far from being perfect (you'd also need to take into account the ttl parameters, ...), and it appears that chan_sip is also quite far from being compliant with RFC 3261.

But it just works and would probably resolve a few basic issues while chan_sip is replaced with chan_sofia. :-)

BTW, even with "nat" configured in sip.conf, it still tries to look-up the host address from the Via instead of sending the reply to whatever address we got it from. I noticed this issue as my server does not have access to DNS and those synchronous DNS requests just block everything.
By: Iñaki Baz Castillo (ibc) 2009-03-12 17:31:08

The patch is correct but with it the behaviour of Asterisk is wrong. In fact Asterisk should reply to the origin source address even without your patch:

RFC 3261 - 18.2.1
------------------
  When the server transport receives a request over any transport, it
  MUST examine the value of the "sent-by" parameter in the top Via
  header field value.  If the host portion of the "sent-by" parameter
  contains a domain name, or if it contains an IP address that differs
  from the packet source address, the server MUST add a "received"
  parameter to that Via header field value.  This parameter MUST
  contain the source address from which the packet was received.  This
  is to assist the server transport layer in sending the response,
  since it must be sent to the source IP address from which the request
  came.
------------------

In your case, since topmost Via contains a sent-by with a hostname (CS2KSSTEMAG) Asterisk should add ";received=SOURCE_IP" parameter to Via and route replies to that SOURCE_IP and port in sent-by (not present so 5060) (well, in case of existing "rport" Asterisk should send replies to origin SOURCE_PORT).
By: frawd (frawd) 2009-04-30 09:54:50

Agreed, but isn't the behaviour of Asterisk also wrong without it?

I would love to see the issue resolved, but simply do not have the skills or time to do more than just pointing out the issue and sending a dirty fix that works for me. Sorry!

Mr. oej, is there anything planned for this issue, I can help with testing if needed.
By: frawd (frawd) 2009-05-09 09:08:54

ibc, I looked over those RFC 3261 sections (18.2.1 and 18.2.2), and I guess we are both right and wrong.

In my case, Asterisk should indeed add ";received=SOURCE_IP" to Via (18.2.1 item 2), but were you are wrong is that having an "maddr" header, it should route the reply to the IP indicated there (18.2.2 item 3).

If the "maddr" header was not present, it should route replies to the SOURCE_IP as you say.

As for oej's note about having to parse the rightmost Via-value in a multiple Via line, and if I understood well the RFC section 7.3, I don't agree because it appears that:
Via: A
Via: B
is equivalent to:
Via: A, B

So looking at the topmost Via (what we have to do) is equivalent as looking at the LEFTMOST Via.
By: frawd (frawd) 2009-05-09 13:25:48

Except if I'm totally wrong about that Via thing (eoj, you mentioned those headers were not meant for UAS?), I think the patch is valid.

Another issue would be what ibc mentioned, that if the "sent-by" contains a hostname, or an IP that differs from the packet's SOURCE_IP, * should reply to the SOURCE_IP, placing a ";received=SOURCE_IP" to the Via (I think * already places a "received=" whatever happens).

This last issue is not what that bug was initially about, but I'm willing to make a patch if someone opens a new bug. My priority being to get this first patch included if it is really valid.
By: frawd (frawd) 2009-05-20 13:44:35

ping
By: Joshua C. Colp (jcolp) 2009-05-20 13:47:12

There hasn't been any progress with this, when there is you'll see it.
By: frawd (frawd) 2009-06-18 16:57:45

Still looking.. :-)

Can I be of any help?
By: Olle Johansson (oej) 2009-09-08 14:02:36

We will try to handle this when we have an Asterisk SIP meeting next week at SIPit.
By: Leif Madsen (lmadsen) 2009-09-08 14:05:45

I'm assigning this to oej because he wanted to be reminded about this issue next week at SIPit.
By: frawd (frawd) 2009-09-24 09:30:27

What's up oej?

I humbly offer my help with my almost null but positive mantis' Karma. This simple patch makes it work for me since I posted it (like 7 months ago).
By: Olle Johansson (oej) 2009-11-11 02:47:33.000-0600

There's another bug report about the reply handling. Asterisk should not reply to any address in the via, it should always reply to the source IP/port that sent us a message (as IBC says).
By: Leif Madsen (lmadsen) 2009-11-11 09:07:31.000-0600

What is the bug report number so they can be marked as related, or one marked as the child of the parent if one of them is dependent on the other?
By: Digium Subversion (svnbot) 2009-11-13 16:06:28.000-0600

Repository: asterisk
Revision: 230144

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r230144 | file | 2009-11-13 16:06:28 -0600 (Fri, 13 Nov 2009) | 8 lines

Respect the maddr parameter in the Via header.

(closes issue ASTERISK-13550)
Reported by: frawd
Patches:
     via_maddr.patch uploaded by frawd (license 610)
Tested by: frawd

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

http://svn.digium.com/view/asterisk?view=rev&revision=230144
By: Digium Subversion (svnbot) 2009-11-13 16:06:53.000-0600

Repository: asterisk
Revision: 230145

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r230145 | file | 2009-11-13 16:06:53 -0600 (Fri, 13 Nov 2009) | 15 lines

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

........
 r230144 | file | 2009-11-13 16:00:19 -0600 (Fri, 13 Nov 2009) | 8 lines
 
 Respect the maddr parameter in the Via header.
 
 (closes issue ASTERISK-13550)
 Reported by: frawd
 Patches:
       via_maddr.patch uploaded by frawd (license 610)
 Tested by: frawd
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=230145
By: Digium Subversion (svnbot) 2009-11-13 16:07:15.000-0600

Repository: asterisk
Revision: 230146

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_sip.c

------------------------------------------------------------------------
r230146 | file | 2009-11-13 16:07:15 -0600 (Fri, 13 Nov 2009) | 22 lines

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

................
 r230145 | file | 2009-11-13 16:00:44 -0600 (Fri, 13 Nov 2009) | 15 lines
 
 Merged revisions 230144 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r230144 | file | 2009-11-13 16:00:19 -0600 (Fri, 13 Nov 2009) | 8 lines
   
   Respect the maddr parameter in the Via header.
   
   (closes issue ASTERISK-13550)
   Reported by: frawd
   Patches:
         via_maddr.patch uploaded by frawd (license 610)
   Tested by: frawd
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=230146
By: Digium Subversion (svnbot) 2009-11-13 16:07:33.000-0600

Repository: asterisk
Revision: 230147

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_sip.c

------------------------------------------------------------------------
r230147 | file | 2009-11-13 16:07:33 -0600 (Fri, 13 Nov 2009) | 22 lines

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

................
 r230145 | file | 2009-11-13 16:00:44 -0600 (Fri, 13 Nov 2009) | 15 lines
 
 Merged revisions 230144 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r230144 | file | 2009-11-13 16:00:19 -0600 (Fri, 13 Nov 2009) | 8 lines
   
   Respect the maddr parameter in the Via header.
   
   (closes issue ASTERISK-13550)
   Reported by: frawd
   Patches:
         via_maddr.patch uploaded by frawd (license 610)
   Tested by: frawd
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=230147
By: Digium Subversion (svnbot) 2009-11-13 16:07:53.000-0600

Repository: asterisk
Revision: 230148

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_sip.c

------------------------------------------------------------------------
r230148 | file | 2009-11-13 16:07:52 -0600 (Fri, 13 Nov 2009) | 22 lines

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

................
 r230145 | file | 2009-11-13 16:00:44 -0600 (Fri, 13 Nov 2009) | 15 lines
 
 Merged revisions 230144 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r230144 | file | 2009-11-13 16:00:19 -0600 (Fri, 13 Nov 2009) | 8 lines
   
   Respect the maddr parameter in the Via header.
   
   (closes issue ASTERISK-13550)
   Reported by: frawd
   Patches:
         via_maddr.patch uploaded by frawd (license 610)
   Tested by: frawd
 ........
................

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

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