| Summary: | ASTERISK-11705: [patch] Asterisk sends SIP-over-TCP INVITE to wrong port number | ||
| Reporter: | Raj Jain (rjain) | Labels: | |
| Date Opened: | 2008-03-23 08:28:08 | Date Closed: | 2009-05-22 17:51:12 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Channels/chan_sip/TCP-TLS |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) 12282.patch ( 1) astlog_siptcpissue.txt ( 2) reg_patch_1.diff ( 3) sip_tcp_incorrect_dst_port.pcap | |
| Description: | The scenario is that a SIP UA registers with Asterisk using TCP transport. The UA puts its IP address, port number and transport=tcp parameter in the REGISTER's Conact: header. Asterisk stores these contact parameters correctly in its registration table. However, when it comes to sending an INVITE to this UA, Asterisk puts a wrong port number in the destination port number field of the TCP header. Due to this, the UA never receives the INVITE. The TCP port number is actually correct in the SIP payload including the Request-URI and the To: header in the INVITE, but it's not correct in the TCP header. Asterisk log file and wireshark trace are attached. | ||
| Comments: | By: Mark Michelson (mmichelson) 2008-03-25 18:06:12 I had a look at this, and I believe I have found the problem area. Apparently,there is a function that is called with every transmission called sip_prepare_socket. In this, one of the first things that is checked is if the file descriptor associated with this call is valid. If it is, then we immediately return. While this works fine for UDP sockets, it does not bode well for TCP connections. The problem is that the TCP connection still operates with the stored IP address and port number when we established the connection. In your case, this means that we're still operating with the same IP address and port number from which we received the REGISTER request. I strongly suspect that had your REGISTER also contained a different IP address in the Contact header than what we had received the request from, we would have sent the INVITE to the wrong address too. I have written a patch which alters the logic used here. I'm pretty certain that this will send the INVITEs to the proper address and port. Furthermore, I have attempted to do some necessary cleanup. What I have done is changed the logic so that if the file descriptor is valid AND the transport type is TCP AND the IP address and port number we have stored for the peer are different than what we previously had associated with the socket, then we will kill the thread associated with the current TCP connection (which should shut down the TCP connection) and re-initialize the socket in question. Unfortunately, I have no way of testing this, so I'd appreciate if you could test and give feedback. While I have done plenty of Asterisk coding in the past, I'll be up front and say that I have not done a lot of chan_sip work, and so if this patch doesn't fix things, please bear with me as I attempt to get this working properly. Thanks! By: Raj Jain (rjain) 2008-03-26 05:48:15 Thanks for the quick turnaround on this. The patch actually breaks registration itself. Asterisk generates 200 OK to the REGISTER but the 200 OK never gets to the machine running the client. A few seconds later Asterisk crashes. gdb backtrace and sip debug outputs are below. I'll be more than happy to test your patches as we work through this, but if you need a way to test this yourself, let me know. I'm using Windows Messenger as my SIP/TCP client. This can also be tested using SIPp. I can write up some SIPp scripts and send them your way if you like. (gdb) where #0 0x00602a60 in fgets () from /lib/tls/libc.so.6 #1 0xf66e898e in _sip_tcp_helper_thread (pvt=0x0, ser=0x9ad9fc0) at chan_sip.c:2212 #2 0x0811ab66 in ast_make_file_from_fd (data=0x9ad9fc0) at tcptls.c:445 #3 0x081247f5 in dummy_start (data=0x9ad9fc0) at utils.c:870 #4 0x007e41d5 in start_thread () from /lib/tls/libpthread.so.0 ASTERISK-1 0x006752da in clone () from /lib/tls/libc.so.6 (gdb) Connected to Asterisk SVN-trunk-r110578M currently running on localhost (pid = 9604) localhost*CLI> sip set debug on SIP Debugging enabled localhost*CLI> <--- SIP read from TCP://192.168.15.100:3630 ---> REGISTER sip:192.168.15.101 SIP/2.0 Via: SIP/2.0/TCP 192.168.15.100:9327 Max-Forwards: 70 From: <sip:9001@192.168.15.101>;tag=da00a00e905446b0bb1b6f9bf12a1696;epid=1652e880b3 To: <sip:9001@192.168.15.101> Call-ID: 92282ef187c842419ffbb5bf8bbff907 CSeq: 1 REGISTER Contact: <sip:192.168.15.100:9327;transport=tcp>;methods="INVITE, MESSAGE, INFO, SUBSCRIBE, OPTIONS, BYE, CANCEL, NOTIFY, ACK, REFER, BENOTIFY";proxy=replace User-Agent: RTC/1.3.5470 (Messenger 5.1.0701) Supported: com.microsoft.msrtc.presence, adhoclist ms-keep-alive: UAC;hop-hop=yes Event: registration Allow-Events: presence Content-Length: 0 <-------------> --- (14 headers 0 lines) --- Sending to 192.168.15.100 : 9327 (no NAT) <--- Transmitting (no NAT) to 192.168.15.100:9327 ---> SIP/2.0 200 OK Via: SIP/2.0/TCP 192.168.15.100:9327;received=192.168.15.100 From: <sip:9001@192.168.15.101>;tag=da00a00e905446b0bb1b6f9bf12a1696;epid=1652e880b3 To: <sip:9001@192.168.15.101>;tag=as55ca9fb2 Call-ID: 92282ef187c842419ffbb5bf8bbff907 CSeq: 1 REGISTER User-Agent: Asterisk PBX SVN-trunk-r110578M Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, NOTIFY Supported: replaces, timer Expires: 120 Contact: <sip:192.168.15.100:9327;transport=tcp>;expires=120 Date: Tue, 25 Mar 2008 23:35:15 GMT Content-Length: 0 <------------> Scheduling destruction of SIP dialog '92282ef187c842419ffbb5bf8bbff907' in 32000 ms (Method: REGISTER) Really destroying SIP dialog '0d43654a061f59756be514254da3d9d3@127.0.0.1' Method: REGISTER localhost*CLI> By: Mark Michelson (mmichelson) 2008-03-26 08:34:51 I'll set up sipp to try to test this out and see what's going wrong with the patch. Thanks for testing it out. By: Mark Michelson (mmichelson) 2008-03-26 09:13:50 rjain, could you send a pcap file of the REGISTER and the unreceived 200 OK? I have access to a tool which can convert a pcap file into sipp scenarios. Thanks. By: Raj Jain (rjain) 2008-03-27 17:26:44 The pcap file attached to this bug report (sip_tcp_incorrect_dst_port.pcap) contains the REGISTER message that my client is generating. By: Raj Jain (rjain) 2008-05-08 11:50:18 putnopvut, on a slightly different note - you mentioned that you have a tool that can convert pcap into SIPp scenarios. Sounds like a very useful tool. Would you mind telling what tool is this? By the way, if you need any help with this bug report, pls. let me know. The status of this bug report should be "Assigned" and not "Ready for Testing". By: Mark Michelson (mmichelson) 2008-05-08 21:20:25 Sorry for my inactivity on this issue. Focus lately has shifted towards knocking out the older bugs on the tracker, and so this one has flown under my radar for a while. Anyway, the tool that can convert from pcap to a sipp scenario is a tool written by Digium's Terry Wilson (otherwiseguy on the bug tracker). It's available via the repotools svn repository (http://svn.digium.com/repotools) and is called sniff2sipp. It's a Perl script that takes a pcap file as input and generates a sipp scenario. It has proven especially helpful before when attempting to reproduce SIP bugs that require certain amounts of load to reproduce. I have gained some more experience lately using sipp and so I could probably reproduce this problem without the need for sniff2sipp. Thanks for the prod! By: Mark Michelson (mmichelson) 2008-06-20 15:50:47 I've been taking a further look at this today. Before I dive back into coding, I've been looking through RFC 3261 to try to determine under what conditions a TCP connection should be re-used and under what other conditions a new TCP connection should be established. The information is all there, and I think I have most of it down. What I obviously broke in the first patch was what is specified in section 18.2.2. <pre> o If the "sent-protocol" is a reliable transport protocol such as TCP or SCTP, or TLS over those, the response MUST be sent using the existing connection to the source of the original request that created the transaction, if that connection is still open. This requires the server transport to maintain an association between server transactions and transport connections. If that connection is no longer open, the server SHOULD open a connection to the IP address in the "received" parameter, if present, using the port in the "sent-by" value, or the default port for that transport, if no port is specified. If that connection attempt fails, the server SHOULD use the procedures in [4] for servers in order to determine the IP address and port to open the connection and send the response to. </pre> By: Olle Johansson (oej) 2008-06-26 11:17:27 putnopvut: All of that is changing with the sip-outbound drafts as well, so we need to be careful here in what we're doing. There's a lot of documents out there that adds to, changes or just explains session handling and reuse. There's also different behaviours implemented for NAT and no-NAT scenarious in devices, while waiting for the standards. By: Leif Madsen (lmadsen) 2009-01-08 16:10:06.000-0600 /ping! Just wanted to ping this issue to see if there is anything blocking to move this forward. By: David Vossel (dvossel) 2009-05-14 10:34:19 take a look at reg_patch_1.diff. It changes the way the Contact header is parsed during a Registration. Let me know if it fixes your problem. By: Digium Subversion (svnbot) 2009-05-22 16:09:46 Repository: asterisk Revision: 196416 U trunk/channels/chan_sip.c U trunk/configs/sip.conf.sample ------------------------------------------------------------------------ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196416 By: Digium Subversion (svnbot) 2009-05-22 16:59:32 Repository: asterisk Revision: 196452 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c U branches/1.6.2/configs/sip.conf.sample ------------------------------------------------------------------------ r196452 | dvossel | 2009-05-22 16:59:32 -0500 (Fri, 22 May 2009) | 25 lines Merged revisions 196416 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196452 By: Digium Subversion (svnbot) 2009-05-22 17:35:47 Repository: asterisk Revision: 196453 _U branches/1.6.1/ U branches/1.6.1/channels/chan_sip.c U branches/1.6.1/configs/sip.conf.sample ------------------------------------------------------------------------ r196453 | dvossel | 2009-05-22 17:35:47 -0500 (Fri, 22 May 2009) | 25 lines Merged revisions 196416 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196453 By: Digium Subversion (svnbot) 2009-05-22 17:51:10 Repository: asterisk Revision: 196454 _U branches/1.6.0/ U branches/1.6.0/channels/chan_sip.c U branches/1.6.0/configs/sip.conf.sample ------------------------------------------------------------------------ r196454 | dvossel | 2009-05-22 17:51:10 -0500 (Fri, 22 May 2009) | 25 lines Merged revisions 196416 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196454 | ||