[Home]

Summary:ASTERISK-14796: [patch] with 'transport=tls' and host not dynamic, port defaults to 5060 rather than 5061.
Reporter:David Vossel (dvossel)Labels:
Date Opened:2009-09-08 11:53:59Date Closed:2009-10-01 16:10:03
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/TCP-TLS
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) peerportfix.patch
( 1) sip_port_config_trunk.diff
Description:[tls_peer]
context=default
type=peer
host=blah
transport=tls

Outgoing connections using this peer will default to port 5060 even though tls uses 5061 by default.  A work around for this is to explicitly set port=5061, but this should not be required.
Comments:By: David Vossel (dvossel) 2009-09-14 17:50:45

This happens for tls registrations as well.  If no port is given it defaults to 5060 rather than 5061
By: Elazar Broad (ebroad) 2009-09-14 17:53:00

Just out of curiosity, it this on a local network or are you using externip?


By: Elazar Broad (ebroad) 2009-09-15 11:32:13

I think I found the issue. We set the default port on the peer in build_peer() via a call to set_peer_defaults(), later, we check to see if peer->addr.sin_port is empty, and if it is set the port on a per transport basis. The problem is that peer->addr.sin_port will never be empty, so we need to set it to 0 when we parse the transport for the peer if transport=tls. Please try the attached patch.


By: Elazar Broad (ebroad) 2009-09-15 11:54:15

Nevermind, I should have checked the review board first: https://reviewboard.asterisk.org/r/357/.
By: David Vossel (dvossel) 2009-09-15 12:06:52

sorry, this is my fault.  I should have updated the issue to link the the reviewboard patch right as I made it.  It still needs to handle registrations with the right port though.
By: Elazar Broad (ebroad) 2009-09-15 12:14:39

Dumb question, are you using tls://user@domain?
By: Elazar Broad (ebroad) 2009-09-15 12:30:03

Found the register problem, chan_sip.c line 7612:
<snip>
 if (portnum < 0) {
portnum = STANDARD_TLS_PORT;
 }
</snip>

should be

if (!portnum) {
 portnum = STANDARD_TLS_PORT;
}
By: David Vossel (dvossel) 2009-09-15 12:31:42

"Dumb question, are you using tls://user@domain? [^] "

no, that was just an example.
By: David Vossel (dvossel) 2009-09-15 12:41:29

"Found the register problem, chan_sip.c line 7612:
"<snip>
 if (portnum < 0) {
   portnum = STANDARD_TLS_PORT;
 }
</snip>

should be

if (!portnum) {
 portnum = STANDARD_TLS_PORT;
} "

Perhaps it should be (portnum <= 0) since the block above this could possibly set portnum = -1.

I found this as well.

line 11866

...
if (r->transport == SIP_TRANSPORT_TLS || r->transport == SIP_TRANSPORT_TCP) {
   p->socket.port = sip_tcp_desc.local_address.sin_port;
}
...

perhaps if transport == TLS port should equal sip_tls_desc.local_address.sin_port
By: Elazar Broad (ebroad) 2009-09-15 12:54:15

That's true, though portnum is a signed int, so !portnum will always return true whether portnum = 0 or -1, either way will work.

"perhaps if transport == TLS port should equal sip_tls_desc.local_address.sin_port"
ast_sip_ouraddrfor will set the correct port(it's called a little further down, see line 3570 in ast_sip_ouraddrfor), so these 3 lines may not even be necessary.
By: David Vossel (dvossel) 2009-09-15 14:15:00

I believe !portnum will only return true if portnum is 0, I don't think it matters if portnum is negative or not if it is set to anything other than 0.  

The ast_sip_ouraddrfor sets p->sa.sin_addr.sin_port not p->socket.port as the lines above it do.  p->socket.port is used for our contact port, if that isn't set right I guess they try to contact us on the wrong port.
By: Elazar Broad (ebroad) 2009-09-15 14:26:02

"I believe !portnum will only return true if portnum is 0, I don't think it matters if portnum is negative or not if it is set to anything other than 0."

You are correct.

"The ast_sip_ouraddrfor sets p->sa.sin_addr.sin_port not p->socket.port as the lines above it do. p->socket.port is used for our contact port, if that isn't set right I guess they try to contact us on the wrong port."

Correct me if I am wrong, ast_sip_ouraddrfor() sets us, which transmit_resgister() passes p->ourip. build_contact() uses p->ourip.sin_port(see line 10683).
By: David Vossel (dvossel) 2009-09-21 15:41:48

I believe the patch I just uploaded, sip_port_config_trunk.diff, should address all of these problems.
By: Elazar Broad (ebroad) 2009-09-22 10:22:58

Unfortunately I do not have the time to test this right now...
By: Digium Subversion (svnbot) 2009-10-01 14:36:25

Repository: asterisk
Revision: 221697

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r221697 | dvossel | 2009-10-01 14:36:24 -0500 (Thu, 01 Oct 2009) | 9 lines

outbound tls connections were not defaulting to port 5061

(closes issue ASTERISK-14796)
Reported by: dvossel
Patches:
     sip_port_config_trunk.diff uploaded by dvossel (license 671)
Tested by: dvossel


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

http://svn.digium.com/view/asterisk?view=rev&revision=221697
By: Digium Subversion (svnbot) 2009-10-01 14:38:03

Repository: asterisk
Revision: 221698

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

------------------------------------------------------------------------
r221698 | dvossel | 2009-10-01 14:38:03 -0500 (Thu, 01 Oct 2009) | 15 lines

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

........
 r221697 | dvossel | 2009-10-01 14:33:33 -0500 (Thu, 01 Oct 2009) | 9 lines
 
 outbound tls connections were not defaulting to port 5061
 
 (closes issue ASTERISK-14796)
 Reported by: dvossel
 Patches:
       sip_port_config_trunk.diff uploaded by dvossel (license 671)
 Tested by: dvossel
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=221698
By: Digium Subversion (svnbot) 2009-10-01 14:55:16

Repository: asterisk
Revision: 221702

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

------------------------------------------------------------------------
r221702 | dvossel | 2009-10-01 14:55:15 -0500 (Thu, 01 Oct 2009) | 15 lines

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

........
 r221697 | dvossel | 2009-10-01 14:33:33 -0500 (Thu, 01 Oct 2009) | 9 lines
 
 outbound tls connections were not defaulting to port 5061
 
 (closes issue ASTERISK-14796)
 Reported by: dvossel
 Patches:
       sip_port_config_trunk.diff uploaded by dvossel (license 671)
 Tested by: dvossel
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=221702
By: Digium Subversion (svnbot) 2009-10-01 16:07:22

Repository: asterisk
Revision: 221745

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

------------------------------------------------------------------------
r221745 | dvossel | 2009-10-01 16:07:22 -0500 (Thu, 01 Oct 2009) | 15 lines

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

........
 r221697 | dvossel | 2009-10-01 14:33:33 -0500 (Thu, 01 Oct 2009) | 9 lines
 
 outbound tls connections were not defaulting to port 5061
 
 (closes issue ASTERISK-14796)
 Reported by: dvossel
 Patches:
       sip_port_config_trunk.diff uploaded by dvossel (license 671)
 Tested by: dvossel
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=221745
By: Digium Subversion (svnbot) 2009-10-01 16:10:02

Repository: asterisk
Revision: 221697

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r221697 | dvossel | 2009-10-01 14:33:33 -0500 (Thu, 01 Oct 2009) | 10 lines

outbound tls connections were not defaulting to port 5061

(closes issue ASTERISK-14796)
Reported by: dvossel
Patches:
     sip_port_config_trunk.diff uploaded by dvossel (license 671)
Tested by: dvossel

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

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

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