[Home]

Summary:ASTERISK-12115: [patch] chan_sip: build_contact() does not put alternate port setting in Contact header
Reporter:Michael Smith (asbestoshead)Labels:
Date Opened:2008-05-30 03:04:05Date Closed:2009-03-02 17:15:56.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) contact-port.txt
( 1) patch-chan-sip-contact-port.txt
( 2) patch-chan-sip-contact-port-1605.txt
( 3) SipBind5065.pcap
Description:build_contact() looks at the *other* end's port to decide whether there's a non-standard port in use. If my end has bindport=5062 but the peer has port=5060, when I send an invite to the peer, my Contact header doesn't have ":5062".

initreqprep() has the same problem when setting the From: header.

chan_sip in Asterisk trunk and 1.6.0-beta8 (but not 1.4.19) have this problem.

I have a trivial patch for this against Moy's mfcr2 branch off trunk, but I can't find the link to sign the contributor's agreement. The patch also applies to 1.6.0beta8, although I haven't tested it there...

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

I've got two Asterisks running on the same server: 1.6.0beta8 on port 5060 for core PBX functions, and the mfcr2 branch on port 5062 as a PSTN gateway.

If a call comes in from the PSTN and is routed to the core Asterisk, and the core Asterisk hangs up the call, it sends the BYE to itself instead of to the PSTN gateway. This is because at some point the PSTN gateway sent a Contact: header with no port.

I'll upload a sip debug trace file to demonstrate the problem. It shows a call coming in from 192.168.0.10:5062 (PSTN gateway) to the core PBX with a Contact header @192.168.0.10. The incoming caller dials an extension that triggers a hangup from the core PBX, which sends a BYE to itself because of the incorrect Contact.
Comments:By: Michael Smith (asbestoshead) 2008-05-30 03:09:43

The patch is really trivial. I can't find the license agreement so I'll just describe it. In build_contact(), instead of using ntohs(p->socket.port), use ntohs(p->ourip.sin_port).

Same in initreqprep().

sip_standard_port() needs to be passed our local port as a parameter. p->socket.port is supposed to be the port the other end uses, in fact I'm pretty sure it never even gets set.
By: Raj Jain (rjain) 2008-05-30 10:57:24

Interesting. I just tried this test on 1.6.0-beta8 and I'm not seeing this issue. My Asterisk is bound to port 5065 in this test. I've uploaded my Wireshark trace to this bug report.
By: Michael Smith (asbestoshead) 2008-05-30 13:27:01

Hi rjain, I think there are some differences with our setups:
- both your local and remote SIP endpoints are on non-standard ports;
- your clients look like they're behind NAT;
- your clients are registering to your Asterisk, which may trigger a code path where p->socket.port gets filled in correctly with Asterisk's bind port.

Mike
By: Raj Jain (rjain) 2008-05-30 14:25:40

I've tried to mimic your setup. I now have SIPp running on the same machine bound to port 5060 and that acts as a SIP peer to my Asterisk. There is no NAT in the picture. I call from a zaptel device towards the SIP peer. Below is the outbound INVITE from Asterisk. The Contact: contains 5065 as you can see. Not sure what's different now between our setups.

INVITE sip:2005@159.63.73.11 SIP/2.0
Via: SIP/2.0/UDP 159.63.73.11:5065;branch=z9hG4bK6d7ec5fb;rport
Max-Forwards: 70
From: "asterisk" <sip:asterisk@159.63.73.11:5065>;tag=as75c11cc6
To: <sip:2005@159.63.73.11>
Contact: <sip:asterisk@159.63.73.11:5065>
Call-ID: 4bc81e942378afb56d10202f7c6d16c1@159.63.73.11
CSeq: 102 INVITE
User-Agent: Asterisk PBX 1.6.0-beta8
Date: Fri, 30 May 2008 19:20:16 GMT
Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, NOTIFY
Supported: replaces, timer
Content-Type: application/sdp
Content-Length: 314

v=0
o=root 2125222828 2125222828 IN IP4 159.63.73.11
s=Asterisk PBX 1.6.0-beta8
c=IN IP4 159.63.73.11
t=0 0
m=audio 18442 RTP/AVP 0 3 8 101
a=rtpmap:0 PCMU/8000
a=rtpmap:3 GSM/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-16
a=silenceSupp:off - - - -
a=ptime:20
a=sendrecv
By: Michael Smith (asbestoshead) 2008-05-30 14:39:56

The only remaining difference is the Asterisk where I had the problem is running trunk, not 1.6.0.

Oh, wait, is your sipp in Asterisk's sip.conf as a peer? That's how mine is set up. If Asterisk knows about a peer, it doesn't set p->socket.port properly. (It sets it to the peer's port, I think.)

But if it doesn't find a peer and you just dial by IP, it properly sets p->socket.port to ntohs(ourip.port).
By: Raj Jain (rjain) 2008-05-30 15:38:06

Yes, that was it. I see the problem now after I added SIPp to sip.conf as a peer (instead of directly dialing to it by IP). I'm not sure how this broke from 1.4 to 1.6. The way you're proposing fixing it, looks good, but we'll have to think if that can impact any other scenarios. Ideally, it would be nice if we can backtrace this to when the bug was introduced. Are you still planning on uploading your patch?
By: Michael Smith (asbestoshead) 2008-05-30 15:50:32

I'd love to add the patch, but even after a few hours sleep I still can't find the link to the license :) Where should I be looking? All signs say it should appear at the top of http://bugs.digium.com/ after logging in, but I just don't see it there.
By: Raj Jain (rjain) 2008-05-30 16:29:00

I know it is not where the signs say it is. I don't exactly remember where I found it. Try looking under your "My account" info.
By: Joshua C. Colp (jcolp) 2008-06-02 07:55:15

It is at the top panel.

It goes:

Report Issue | Sign License | Docs

Just click on Sign License and it will take you to the page. If it does not show up then you have already signed it.
By: Michael Smith (asbestoshead) 2008-06-02 08:14:58

That's hilarious... I must have signed the license at some point.
By: Leif Madsen (lmadsen) 2008-10-21 09:03:10

Pinging this issue! It is quite old, and has a patch associated with it which is simple, and may resolve the issue. I have assigned to putnopvut for now.
By: Mark Michelson (mmichelson) 2008-10-21 18:34:47

The patch seems fine by me, since it is inconsistent if the socket's port is the near or far end's port. The patch is almost certainly out of date considering the age of this issue, but I'll take it upon myself to get it up to date.

Thanks for the patch and sorry it took so long to get around to looking at this. I think this qualifies as one of those bugs that just sort of got lost in the pile.
By: Digium Subversion (svnbot) 2008-10-21 18:43:55

Repository: asterisk
Revision: 151464

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r151464 | mmichelson | 2008-10-21 18:43:54 -0500 (Tue, 21 Oct 2008) | 11 lines

Make the sip_standard_port function more granular by allowing separate
type and port arguments. This is necessary because when building our From
and Contact headers, we need to be absolutely sure that we are placing our
source port there and not the peer's source port.

(closes issue ASTERISK-12115)
Reported by: asbestoshead
Patches:
     patch-chan-sip-contact-port.txt uploaded by asbestoshead (license 455)


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

http://svn.digium.com/view/asterisk?view=rev&revision=151464
By: Michael Smith (asbestoshead) 2009-02-17 10:19:12.000-0600

Any chance to get this committed to 1.6.0.x? I keep forgetting to apply it to new installs and then wondering why things break :)

I'll upload a patch against 1.6.0.5.
By: John Covert (jcovert) 2009-02-18 18:46:44.000-0600

Thank you asbestoshead.  I've just applied your patch to 1.6.0.3 to solve a problem I had been avoiding, but could avoid no longer.  Works great.

/john
By: Mark Michelson (mmichelson) 2009-03-02 17:13:12.000-0600

Hmm, I'm not sure why I never merged this change into 1.6.0 and 1.6.1. Thanks for re-opening this so I can get it there.
By: Digium Subversion (svnbot) 2009-03-02 17:15:52.000-0600

Repository: asterisk
Revision: 179473

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

------------------------------------------------------------------------
r179473 | mmichelson | 2009-03-02 17:15:52 -0600 (Mon, 02 Mar 2009) | 17 lines

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

........
 r151464 | mmichelson | 2008-10-21 18:54:41 -0500 (Tue, 21 Oct 2008) | 11 lines
 
 Make the sip_standard_port function more granular by allowing separate
 type and port arguments. This is necessary because when building our From
 and Contact headers, we need to be absolutely sure that we are placing our
 source port there and not the peer's source port.
 
 (closes issue ASTERISK-12115)
 Reported by: asbestoshead
 Patches:
       patch-chan-sip-contact-port.txt uploaded by asbestoshead (license 455)
........

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

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