[Home]

Summary:ASTERISK-14840: [patch] TCP/TLS invites(and possibly others) broken from r218504 and onward
Reporter:Elazar Broad (ebroad)Labels:
Date Opened:2009-09-15 20:40:09Date Closed:2009-09-16 14:31:20
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/TCP-TLS
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) freadfix.patch
Description:Summary says it all.

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

http://lists.digium.com/pipermail/asterisk-commits/2009-September/037136.html tacked a null on to the end of the character array read by fread(), while this is correct, I believe(please correct me if I am wrong) that ast_append_str() which ultimately calls an internal string helper, appends a null character as well, resulting in two null characters. Commenting out the line resolves the issue.
Comments:By: Kevin P. Fleming (kpfleming) 2009-09-16 09:37:11

Your anaylsis is in fact flawed; ast_append_str takes a format string and arguments, and feeds the data read by fread() in as a string argument to a format string with a single "%s" specifier. This means that the string argument must be a valid C-style string, which means it must end with a null character. ast_append_str will not copy this null character, it will act like every other C library string function and treat that null character as the end of the string.

As clearly requested in the bug posting guidelines, bug reports against chan_sip's handling of protocol issues must include a log capture showing the incoming packet data and chan_sip's handling of that data, otherwise we can't do anything to analyze the problem.
By: Elazar Broad (ebroad) 2009-09-16 10:26:47

I can post a packet capture, however, nothing is logged with 'sip set debug on'. 'core set debug 4' will dump the request as traffic received on the line, but nothing beyond that. Essentially, * completely ignores the request.


By: Mark Michelson (mmichelson) 2009-09-16 10:50:43

What you're saying doesn't add up in my head. I can't understand why that one line would result in Asterisk ignoring the request, and it doesn't make sense why commenting it out would cause things to work properly. Also, keep in mind that that line was updated on a later commit, so it may be worthwhile to issue an svn update and see if you still have the problem.

Do you see any warnings or errors? I ran several interop tests at SIPit yesterday over TCP and had no troubles.
By: Elazar Broad (ebroad) 2009-09-16 11:00:35

I checked out late last night, say 12AM EDT, you are referring to bytes_read as opposed to sizeof(buf), correct? No warnings that I can see, Ill check again with everything at 10...
By: Elazar Broad (ebroad) 2009-09-16 11:32:01

With 'core set debug 10', 'core set verbose 10' and 'sip set debug on', this is all that comes up:

--
[Sep 16 12:22:36] DEBUG[3969] chan_sip.c:  Header  0 [  0]:
[Sep 16 12:22:36] DEBUG[3969] chan_sip.c:    Body  0 [  0]:
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  0 [ 45]: INVITE sip:123@pbx.thebroadfamily.com SIP/2.0
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  1 [ 87]: Via: SIP/2.0/TCP 72.xx.213.xx4:63209;branch=z9hG4bK-d8754z-a6be270c146d5057-1---d8754z-
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  2 [ 16]: Max-Forwards: 70
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  3 [ 52]: Contact: <sip:201@72.xx.213.xx4:63208;transport=TCP>
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  4 [ 36]: To: <sip:123@pbx.thebroadfamily.com>
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  5 [ 65]: From: "EBroad"<sip:201@pbx.thebroadfamily.com>;tag=ac040e68
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  6 [ 53]: Call-ID: MmU0MDdmMTZmM2Q2OWRlOWRkOTM4NzA3NjY4OTczN2Q.
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  7 [ 14]: CSeq: 1 INVITE
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  8 [ 81]: Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, NOTIFY, MESSAGE, SUBSCRIBE, INFO
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header  9 [ 29]: Content-Type: application/sdp
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header 10 [ 42]: User-Agent: Bria release 2.5.4 stamp 53956
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header 11 [ 19]: Content-Length: 182
[Sep 16 12:22:36] DEBUG[3970] chan_sip.c:  Header 12 [  0]:
--
By: Elazar Broad (ebroad) 2009-09-16 13:39:40

Per conversation with mmichelson on IRC(Thanks!), reversing arguments 2 and 3 of fread() resolves the issue. See the attached patch.
By: Digium Subversion (svnbot) 2009-09-16 14:27:18

Repository: asterisk
Revision: 218933

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r218933 | mmichelson | 2009-09-16 14:27:18 -0500 (Wed, 16 Sep 2009) | 12 lines

Reverse order of args to fread.

This way, we don't always write a null byte into
byte 1 of the buffer

(closes issue ASTERISK-14840)
Reported by: ebroad
Patches:
     freadfix.patch uploaded by ebroad (license 878)
Tested by: ebroad


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

http://svn.digium.com/view/asterisk?view=rev&revision=218933
By: Digium Subversion (svnbot) 2009-09-16 14:28:17

Repository: asterisk
Revision: 218935

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

------------------------------------------------------------------------
r218935 | mmichelson | 2009-09-16 14:28:16 -0500 (Wed, 16 Sep 2009) | 18 lines

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

........
 r218933 | mmichelson | 2009-09-16 14:25:36 -0500 (Wed, 16 Sep 2009) | 12 lines
 
 Reverse order of args to fread.
 
 This way, we don't always write a null byte into
 byte 1 of the buffer
 
 (closes issue ASTERISK-14840)
 Reported by: ebroad
 Patches:
       freadfix.patch uploaded by ebroad (license 878)
 Tested by: ebroad
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=218935
By: Digium Subversion (svnbot) 2009-09-16 14:29:15

Repository: asterisk
Revision: 218936

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

------------------------------------------------------------------------
r218936 | mmichelson | 2009-09-16 14:29:15 -0500 (Wed, 16 Sep 2009) | 18 lines

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

........
 r218933 | mmichelson | 2009-09-16 14:25:36 -0500 (Wed, 16 Sep 2009) | 12 lines
 
 Reverse order of args to fread.
 
 This way, we don't always write a null byte into
 byte 1 of the buffer
 
 (closes issue ASTERISK-14840)
 Reported by: ebroad
 Patches:
       freadfix.patch uploaded by ebroad (license 878)
 Tested by: ebroad
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=218936
By: Digium Subversion (svnbot) 2009-09-16 14:31:19

Repository: asterisk
Revision: 218937

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

------------------------------------------------------------------------
r218937 | mmichelson | 2009-09-16 14:31:19 -0500 (Wed, 16 Sep 2009) | 18 lines

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

........
 r218933 | mmichelson | 2009-09-16 14:25:36 -0500 (Wed, 16 Sep 2009) | 12 lines
 
 Reverse order of args to fread.
 
 This way, we don't always write a null byte into
 byte 1 of the buffer
 
 (closes issue ASTERISK-14840)
 Reported by: ebroad
 Patches:
       freadfix.patch uploaded by ebroad (license 878)
 Tested by: ebroad
........

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

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