[Home]

Summary:ASTERISK-13040: [patch] SIP/TLS enabled - just one call possible - 481 Call/Transaction Does Not Exist
Reporter:Stefan Tichy (st)Labels:
Date Opened:2008-11-09 10:03:08.000-0600Date Closed:2009-06-16 12:11:56
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/TCP-TLS
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 13865.patch
( 1) 20090506_prueba_tls-1.6.2.0-beta1_patch_v5
( 2) 20090506_prueba_tls-svn190902_patch_v5
( 3) 20090508_sipdebug_tls-eyebeam
( 4) 20090508_sipdebug_tls-polycom
( 5) dont_add_port_if_tls.patch
( 6) full-5062
( 7) full-tls-std
( 8) tls_port_v2.patch
( 9) tls_port_v3.patch
(10) tls_port_v4.patch
(11) tls_port_v5.patch
(12) transport_issues.diff
Description:Settup consits of Asterisk 1.6.1-beta1 and Snom320 firmware 7.3.7
TLS is enabled using standard port 5061 for the first example and port 5062 for the second example.

Dialplan is very simple: Answer; Wait(1); Playback(...); Hangup;

The first call of the first example does work but it is not possible to call the same number again: 481 Call/Transaction Does Not Exist

The first call of the second example has no "BYE" and has to be cancelled at the phone.


(IMHO a new category chan_sip/TLS should be created)

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

two files containing CLI output / SIP trace
Comments:By: Leif Madsen (lmadsen) 2009-01-05 13:11:20.000-0600

Assigning this to Terry to see if he can move this forward at all. Thanks!
By: James Golovich (jamesgolovich) 2009-01-30 13:53:35.000-0600

Assuming I can reproduce this behavior with something other than a SNOM (I don't have a SNOM) and my daughter gives me a chance, I'll take a look at this.
By: Leif Madsen (lmadsen) 2009-01-31 11:54:04.000-0600

Added new category Channels/chan_sip/TLS as suggested by the reporter, and moved this to that category.
By: Mark Michelson (mmichelson) 2009-04-03 16:59:27

Before attempting to move this forward any, there have been several fixes in the 1.6.1 branch regarding TLS support in SIP. Since this is such a simple test, would it be possible to check with the latest 1.6.1 rc (1.6.1.0-rc4, I believe)?

Also, is TLS required for this problem to occur? Does the same behavior occur if you use TCP as your transport?
By: Stefan Tichy (st) 2009-04-13 09:23:33

Tried again using Asterisk 1.6.1.0-rc4 and Snom 7.3.14.

If TCP is used I don't see any problems, but if TLS is required the phone does not receive BYE. It does not matter if port 5061 or port 5062 is used.

If the phone sends BYE, asterisk ignores it. It does not even show up in a sip trace.
By: Stefan Tichy (st) 2009-04-13 12:21:24

Replaced the Snom 320 by Auerswald VoIP250. Don't have that problem in this case.

The Snom receives "200 OK" from port 5061 and sends ACK to port 5060. Asterisk never sends BYE without this ACK.

I will send a bug report to snom.


By: Kristijan Vrban (kristijan) 2009-04-13 18:45:31

added the dont_add_port_if_tls.patch for this issue.

as the todo line in function build_contact already describes, asterisk should not add the port number to the contact header. or if, then it should set the right one. in this case the tls port 5061, and not 5060

the snom phone behaves correct, because it do what chan_sip (wrongly) say to him:
contact me at 5060


By: Mark Michelson (mmichelson) 2009-04-15 11:26:36

Kristijan: Thanks for the patch. Sorry if this question seems stupid, but did you test with and without the patch applied? I am assuming that you did.

First of all, let me state that you are definitely correct that we should not add the port to the contact header if it is the standard port for the transport type being used.

What bugs me, though, is that p->ourip.sin_port is set to 5060 in this scenario. It should be 5061. If it were set correctly to begin with, we wouldn't need to patch build_contact to solve this issue.
By: Kristijan Vrban (vrban) 2009-04-15 12:52:24

because p->ourip.sin_port is sourced from udpbindaddr, and not by tlsbindaddr.

But on the quick i did not find the place where ourip is set.

p.s. i am kristijan, by a mistake i have two accounts.
By: Mark Michelson (mmichelson) 2009-04-15 14:23:04

ourip is typically set using the function ast_sip_ouraddrfor. Looking at the code, there is a lot of logic to figure out whether to use an internal or external IP address when determining what to place in SIP packets.

With basic setups, such as mine, the outcome is that ourip is set to the bindaddr (which is the udpbindaddr). A possible remedy for this would be to pass the transport to the sip_ouraddrfor function so that we can determine whether to use the udp, tcp, or tls bindaddr. I will put together a patch that implements this change.

In the end, I think both the change I have described here and the one you posted as a patch need to be implemented.
By: Mark Michelson (mmichelson) 2009-04-15 14:33:56

I've uploaded 13865.patch for testing. It incorporates both the changes by Kristijan/vrban and another change I made to ast_sip_ouraddrfor which will hopefully populate the p->ourip field correctly.
By: Kristijan Vrban (vrban) 2009-04-15 14:36:02

yes, sip_ouraddrfor seems to be the right place to set the port number by checking the transport.

And then in build_contact check if the given port is a standard port for this transport, then dont set the port number in contact.

If the port is not the standard port for this transport, then set the contact including the port number.
By: Mark Michelson (mmichelson) 2009-04-15 14:44:01

I made a typo in the first patch. I have fixed it now, though.
By: Kristijan Vrban (vrban) 2009-04-15 22:38:52

new patch: tls_port_v2.patch

it was a bit more complicated as expected:

1.
ast_sip_ouraddrfor is called by sip_alloc if the call come,
and p->socket.type was static set to SIP_TRANSPORT_UDP in sip_alloc,
so i forwarded also struct sip_request from tcp_helper_thread that we can
set p->socket.type dynamic. Only then is was possibly to get the transport
type in ast_sip_ouraddrfor to know which ip and port we need to set.

2.
We needed to watch in ast_sip_ouraddrfor also if the default ip for the transport is
0.0.0.0, then we set only the port for the specific transport. and for the ip we use internip

3.
In build_contact we still need four differnt ast_string_field_build
to set the ;transport= the snome phone need it to send us TCP or TLS
otherwise it always fall back to UDP

4.
Can i use this patch as candidature to get a traineeship at digium?


By: Kristijan Vrban (vrban) 2009-04-16 16:31:51

tls_port_v3.patch made against r188742, decreased a notice log to a debug log, and added a comment
By: Mark Michelson (mmichelson) 2009-04-16 16:53:53

Thanks for the patches, vrban. I am running out of time today, so I'll have to wait until tomorrow to review them.
By: Mark Michelson (mmichelson) 2009-04-20 16:00:33

Sorry I took so long to review your patch. I took a look and saw one thing that seemed wrong and another that seemed a little bit...off.

In ast_sip_ouraddrfor, you sometimes reference p->ourip. Instead, you should be using the "us" variable. This way, things are more consistent.

Also, in sip_alloc, you set p->socket.type to the value of req->socket.type. I am having difficulty finding where req->socket.type is set when receiving an incoming SIP request. Where and when is req->socket.type set?
By: Kristijan Vrban (vrban) 2009-04-23 06:42:04

>st_sip_ouraddrfor, you sometimes reference p->ourip. Instead, you should be using the "us" variable. This way, things are more consistent.

done.

>I am having difficulty finding where req->socket.type is set

req.socket.type is set in: sip_tcp_helper_thread or sipsock_read,
then going to:
handle_request_do ->
find_call ->
sip_alloc ->
ast_sip_ouraddrfor, and here we are.


By: Kristijan Vrban (vrban) 2009-04-28 13:02:42

tls_port_v5.patch:

added two break; into ast_sip_ouraddrfor. otherwise for TCP/TLS we fall through and get the udpbindaddr if udpbindaddr != 0.0.0.0 in sip.conf

the ast_debug i added in ast_sip_ouraddrfor i offset to right spot.


By: Juan Manuel Coronado Z. (jmacz) 2009-05-06 16:10:26

Tried the patch against the same revision it was made (190902) and then applied it to Asterisk-1.6.0.2-beta1 and repeated the test.

In both cases, Polycom phones (320 and 330) were used. Both have "Port: 5061" set on their web config page. In the Asterisk side, tried both specifying and not specifying a port for the peer with the same results.

In both cases if the A party finishes the call, BYE message is correctly handled by the B party; but if B party ends the call, then A still answers with a SIP 481 message.

I'm attaching files 20090506_prueba_tls-svn190902_patch_v5 and 20090506_prueba_tls-1.6.2.0-beta1_patch_v5 with SIP debug for the calls tested in both installations. Hope this will be of some help.

I'll try the current SVN head and Asterisk-1.6.1.0 and come back with the results asap.
By: Kristijan Vrban (vrban) 2009-05-07 02:20:06

hello jmacz, the BYE message asterisk send to the Polycom seems to be ok. I dont know why the Polycom answer with "481 Call Leg/Transaction Does Not Exist"

Iam testing here with a snom320 (FW-7.3.14) phone, and the snom answer with OK in this case.

But all the rest of the TLS/Port issue itself looks ok.

Can you try a newer firmware with the Polycom? Latest is 3.1.3RevB


By: Juan Manuel Coronado Z. (jmacz) 2009-05-08 16:15:15

Hi Vrban, after upgrading the Phone from firmware version 2.2.0.0047 to 3.1.3.0439 (which didn't work either) and doing some more tests with the Eyebeam softphone (v1.5.7), I found that there's a subtle difference between the INVITE and BYE messages from/to the IP phone and from/to the softphone. Calls between Eyebeam softphones always work. Calls from an Eyebeam Softphone to a Polycom behaves correctly if the Polycom hangs up. But calls between Polycoms or from a Polycom to an Eyebeam were the softphone hangs up, get stucked (not the case if the A party -the Polycom 320- , also terminates the call, case in which the BYE message seems to be well handled).

The only thing different I've noticed so far is the URI of the first example vs the second one.

The one that works (Eyebeam --> Polycom --> Polycom Answers --> Polycom Hangs UP --> Eyebeam receives a BYE and hangs up OK):

INVITE sip:<NUMBER>@<UA ADDRESS>:<UA PORT>;transport=tls SIP/2.0
Via: SIP/2.0/TLS <ASTERISK ADDRESS>:5061;branch=z9hG4bK3a21876a;rport

BYE sip:<NUMBER>@<ASTERISK ADDRESS>;transport=tls SIP/2.0
Via: SIP/2.0/TLS <UA ADDRESS>:<UA PORT>;branch=z9hG4bKb62ee8a34F14EC08

Versus the one that doesn't (Polycom --> Eyebeam --> Eyebeam Answers --> Eyebeam Hangs UP --> Polycom doesn't hangs UP and sends 481 error):

INVITE sip:<NUMBER>@<ASTERISK ADDRESS>;user=phone;transport=tls SIP/2.0
Via: SIP/2.0/TLS <UA ADDRESS>:<UA PORT>;branch=z9hG4bKa1885f262AB2844F

BYE sip:<NUMBER>@<UA ADDRESS>:<UA PORT>;transport=tls SIP/2.0
Via: SIP/2.0/TLS <ASTERISK ADDRESS>:5061;branch=z9hG4bK0cd0e4b4;rport

I've tried many configurations on the phone without luck (specifying the TLS port as 5061, setting an outbound proxy with the Asterisk's IP, etc).

I'm attaching two calls between the Polycom IP SP 320 (ext 27) with the new firmware, and the Eyebeam (ext 33):

Call without issues: 20090508_sipdebug_tls-eyebeam
Call still with the issue: 20090508_sipdebug_tls-polycom

I'll continue testing and will try to reach some help from Polycom to see If I can figure out what's going with this.
By: Digium Subversion (svnbot) 2009-06-16 11:03:31

Repository: asterisk
Revision: 200946

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r200946 | dvossel | 2009-06-16 11:03:31 -0500 (Tue, 16 Jun 2009) | 32 lines

SIP transport type issues

What this patch addresses:
1. ast_sip_ouraddrfor() by default binds to the UDP address/port
reguardless if the sip->pvt is of type UDP or not.  Now when no
remapping is required, ast_sip_ouraddrfor() checks the sip_pvt's
transport type, attempting to set the address and port to the
correct TCP/TLS bindings if necessary.
2.  It is not necessary to send the port number in the Contact
header unless the port is non-standard for the transport type.
This patch fixes this and removes the todo note.
3.  In sip_alloc(), the default dialog built always uses transport
type UDP.  Now sip_alloc() looks at the sip_request (if present)
and determines what transport type to use by default.
4.  When changing the transport type of a sip_socket, the file
descriptor must be set to -1 and in some cases the tcptls_session's
ref count must be decremented and set to NULL.  I've encountered
several issues associated with this process and have created a function,
set_socket_transport(), to handle the setting of the socket type.


(closes issue ASTERISK-13040)
Reported by: st
Patches:
     dont_add_port_if_tls.patch uploaded by Kristijan (license 753)
     13865.patch uploaded by mmichelson (license 60)
     tls_port_v5.patch uploaded by vrban (license 756)
     transport_issues.diff uploaded by dvossel (license 671)
Tested by: mmichelson, Kristijan, vrban, jmacz, dvossel

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

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

http://svn.digium.com/view/asterisk?view=rev&revision=200946
By: Digium Subversion (svnbot) 2009-06-16 11:28:50

Repository: asterisk
Revision: 200984

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

------------------------------------------------------------------------
r200984 | dvossel | 2009-06-16 11:28:50 -0500 (Tue, 16 Jun 2009) | 39 lines

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

........
 r200946 | dvossel | 2009-06-16 11:03:30 -0500 (Tue, 16 Jun 2009) | 32 lines
 
 SIP transport type issues
 
 What this patch addresses:
 1. ast_sip_ouraddrfor() by default binds to the UDP address/port
 reguardless if the sip->pvt is of type UDP or not.  Now when no
 remapping is required, ast_sip_ouraddrfor() checks the sip_pvt's
 transport type, attempting to set the address and port to the
 correct TCP/TLS bindings if necessary.
 2.  It is not necessary to send the port number in the Contact
 header unless the port is non-standard for the transport type.
 This patch fixes this and removes the todo note.
 3.  In sip_alloc(), the default dialog built always uses transport
 type UDP.  Now sip_alloc() looks at the sip_request (if present)
 and determines what transport type to use by default.
 4.  When changing the transport type of a sip_socket, the file
 descriptor must be set to -1 and in some cases the tcptls_session's
 ref count must be decremented and set to NULL.  I've encountered
 several issues associated with this process and have created a function,
 set_socket_transport(), to handle the setting of the socket type.
 
 
 (closes issue ASTERISK-13040)
 Reported by: st
 Patches:
       dont_add_port_if_tls.patch uploaded by Kristijan (license 753)
       13865.patch uploaded by mmichelson (license 60)
       tls_port_v5.patch uploaded by vrban (license 756)
       transport_issues.diff uploaded by dvossel (license 671)
 Tested by: mmichelson, Kristijan, vrban, jmacz, dvossel
 
 Review: https://reviewboard.asterisk.org/r/278/
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=200984
By: Digium Subversion (svnbot) 2009-06-16 11:34:21

Repository: asterisk
Revision: 200987

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

------------------------------------------------------------------------
r200987 | dvossel | 2009-06-16 11:34:21 -0500 (Tue, 16 Jun 2009) | 39 lines

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

........
 r200946 | dvossel | 2009-06-16 11:03:30 -0500 (Tue, 16 Jun 2009) | 32 lines
 
 SIP transport type issues
 
 What this patch addresses:
 1. ast_sip_ouraddrfor() by default binds to the UDP address/port
 reguardless if the sip->pvt is of type UDP or not.  Now when no
 remapping is required, ast_sip_ouraddrfor() checks the sip_pvt's
 transport type, attempting to set the address and port to the
 correct TCP/TLS bindings if necessary.
 2.  It is not necessary to send the port number in the Contact
 header unless the port is non-standard for the transport type.
 This patch fixes this and removes the todo note.
 3.  In sip_alloc(), the default dialog built always uses transport
 type UDP.  Now sip_alloc() looks at the sip_request (if present)
 and determines what transport type to use by default.
 4.  When changing the transport type of a sip_socket, the file
 descriptor must be set to -1 and in some cases the tcptls_session's
 ref count must be decremented and set to NULL.  I've encountered
 several issues associated with this process and have created a function,
 set_socket_transport(), to handle the setting of the socket type.
 
 
 (closes issue ASTERISK-13040)
 Reported by: st
 Patches:
       dont_add_port_if_tls.patch uploaded by Kristijan (license 753)
       13865.patch uploaded by mmichelson (license 60)
       tls_port_v5.patch uploaded by vrban (license 756)
       transport_issues.diff uploaded by dvossel (license 671)
 Tested by: mmichelson, Kristijan, vrban, jmacz, dvossel
 
 Review: https://reviewboard.asterisk.org/r/278/
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=200987
By: Digium Subversion (svnbot) 2009-06-16 12:11:52

Repository: asterisk
Revision: 200992

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

------------------------------------------------------------------------
r200992 | dvossel | 2009-06-16 12:11:52 -0500 (Tue, 16 Jun 2009) | 39 lines

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

........
 r200946 | dvossel | 2009-06-16 11:03:30 -0500 (Tue, 16 Jun 2009) | 32 lines
 
 SIP transport type issues
 
 What this patch addresses:
 1. ast_sip_ouraddrfor() by default binds to the UDP address/port
 reguardless if the sip->pvt is of type UDP or not.  Now when no
 remapping is required, ast_sip_ouraddrfor() checks the sip_pvt's
 transport type, attempting to set the address and port to the
 correct TCP/TLS bindings if necessary.
 2.  It is not necessary to send the port number in the Contact
 header unless the port is non-standard for the transport type.
 This patch fixes this and removes the todo note.
 3.  In sip_alloc(), the default dialog built always uses transport
 type UDP.  Now sip_alloc() looks at the sip_request (if present)
 and determines what transport type to use by default.
 4.  When changing the transport type of a sip_socket, the file
 descriptor must be set to -1 and in some cases the tcptls_session's
 ref count must be decremented and set to NULL.  I've encountered
 several issues associated with this process and have created a function,
 set_socket_transport(), to handle the setting of the socket type.
 
 
 (closes issue ASTERISK-13040)
 Reported by: st
 Patches:
       dont_add_port_if_tls.patch uploaded by Kristijan (license 753)
       13865.patch uploaded by mmichelson (license 60)
       tls_port_v5.patch uploaded by vrban (license 756)
       transport_issues.diff uploaded by dvossel (license 671)
 Tested by: mmichelson, Kristijan, vrban, jmacz, dvossel
 
 Review: https://reviewboard.asterisk.org/r/278/
........

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

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