[Home]

Summary:ASTERISK-13347: SIP INVITE packets are incorrectly truncated with 1.6.1 svn after approx 1020 characters
Reporter:Richard Hamnett (riksta)Labels:
Date Opened:2009-01-12 12:02:13.000-0600Date Closed:2009-01-20 14:15:35.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 14220_workaround.patch
( 1) FAILED_CALL
( 2) failed.cap
( 3) SUCCESSFUL_CALL
( 4) successful.cap
Description:Since updating from 1.6.1b4 to SVN-branch-1.6.1-r167727 i can no longer make calls to my SIP trunk provider if my call has a long CALLERID(name) string.

The sip trunk provider's techie says that my SIP INVITE packets are being truncated... eg if i set the callerid name as a short string and the INVITE packet was 1017 characters, the call goes through perfectly.

If i set the callerid name as a longer string and the INVITE packet was 1020 chars the call fails to be sent.

In #asterisk-dev Corydon76 explains that it is probably to do with the fact that SIP INVITES are no longer are built in a static buffer (That was part of the ast_str work)


I will attach a sip debug log file of a successful and unsuccessful call
Comments:By: Richard Hamnett (riksta) 2009-01-14 11:11:51.000-0600

Still failing on r168500-168600
By: Mark Michelson (mmichelson) 2009-01-14 15:52:23.000-0600

I've been looking at the two files you attached, specifically looking at the outgoing INVITEs from Asterisk. The INVITEs do not appear to be truncated. Now this could just be that Asterisk is not actually sending the same thing that is being printed when executing a sip debug on the CLI.

So here are some things that may be helpful:

1) Compare a capture using something like wireshark and what Asterisk shows when executing a sip debug. If they are different, then upload a .pcap file showing the actual INVITE that Asterisk is sending.

2) Is openSER running locally or remotely? I keep seeing that openSER is what is responding to our INVITEs, and I just was wondering if the problem might lie there instead of in Asterisk. The fact that the problem started occurring when you made an upgrade to Asterisk makes me doubt this is the case, but it doesn't hurt to ask.

I'll also take a look in the code, but your help would be very much appreciated here.
By: Olle Johansson (oej) 2009-01-15 07:40:44.000-0600

putnopvut: Check with kpfleming. He worked with size of packets during SIPit.
By: Mark Michelson (mmichelson) 2009-01-15 10:29:51.000-0600

oej: Thanks for the tip!
By: Mark Michelson (mmichelson) 2009-01-15 10:54:21.000-0600

oej, after talking with Kevin, he said that the adjustment he made was to allow for incoming INVITEs to have more than 64 lines in the SDP. This issue seems to be dealing with outgoing INVITEs being truncated at 1024 characters. My immediate thought on this is that the ast_str object which contains the request contents is not expanding as it is supposed to.

I see that riksta has attached pcap files here, so I will examine those in comparison to what the SIP debug from Asterisk says.
By: Mark Michelson (mmichelson) 2009-01-15 11:00:30.000-0600

A ha! The pcap tells all!

Asterisk's SIP debug is showing the entire packet contents, but the pcap clearly shows the failed call being truncated exactly as riksta's provider claims. So the ast_str object is expanding properly, but the sip_request's "len" field does not have the correct value, so our call to sendto() in __sip_xmit only sends the first 1024 bytes.

I'll have a close look at the add_header and add_line functions in chan_sip to see if I spot a logic flaw, esp. in the case where the ast_str expands to beyond 1024 bytes.
By: Mark Michelson (mmichelson) 2009-01-15 12:27:08.000-0600

I've got some more news here. It appears that the circumstances necessary for getting truncated outgoing INVITEs are narrower than I originally had thought. I set up a test where I sent out an INVITE with 1097 bytes in it, and the whole packet made it out properly.
By: Digium Subversion (svnbot) 2009-01-15 13:00:07.000-0600

Repository: asterisk
Revision: 168725

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r168725 | mmichelson | 2009-01-15 13:00:06 -0600 (Thu, 15 Jan 2009) | 17 lines

Remove an unneeded condition for line addition to a SIP request/response

In Asterisk 1.4 and 1.6.0, the sip_request structure had a statically
allocated buffer to hold the text of the request. There was a check in the
add_line function to not attempt to write the line into the buffer if we
did not have room for it.

In trunk and Asterisk versions starting with 1.6.1, an expandable ast_str
structure is used to hold the text. Since it may grow to fit an arbitrarily
sized string, this check in add_line is no longer valid.

I found this oddity while attempting to fix issue ASTERISK-13347; however, I do not
believe that this is the fix for that issue since the output supplied by the
reporter did not contain the warning message that would be printed had this
condition been satisfied.


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

http://svn.digium.com/view/asterisk?view=rev&revision=168725
By: Digium Subversion (svnbot) 2009-01-15 13:00:53.000-0600

Repository: asterisk
Revision: 168726

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

------------------------------------------------------------------------
r168726 | mmichelson | 2009-01-15 13:00:53 -0600 (Thu, 15 Jan 2009) | 25 lines

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

........
r168725 | mmichelson | 2009-01-15 13:00:06 -0600 (Thu, 15 Jan 2009) | 17 lines

Remove an unneeded condition for line addition to a SIP request/response

In Asterisk 1.4 and 1.6.0, the sip_request structure had a statically
allocated buffer to hold the text of the request. There was a check in the
add_line function to not attempt to write the line into the buffer if we
did not have room for it.

In trunk and Asterisk versions starting with 1.6.1, an expandable ast_str
structure is used to hold the text. Since it may grow to fit an arbitrarily
sized string, this check in add_line is no longer valid.

I found this oddity while attempting to fix issue ASTERISK-13347; however, I do not
believe that this is the fix for that issue since the output supplied by the
reporter did not contain the warning message that would be printed had this
condition been satisfied.


........

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

http://svn.digium.com/view/asterisk?view=rev&revision=168726
By: Mark Michelson (mmichelson) 2009-01-15 13:28:15.000-0600

riksta: I made a couple of changes to the SIP channel driver in 1.6.1. They seem like they could be related to the problem you are experiencing. Would you mind updating your working copy of 1.6.1 and testing again? Thanks.
By: Richard Hamnett (riksta) 2009-01-16 03:13:04.000-0600

putnopvut, I tried the latest change, it made no difference to the problem, if i add some extra codecs and a long callerid name the request fails again. If you require i can post another cap... but not sure it will help
By: Mark Michelson (mmichelson) 2009-01-16 09:05:37.000-0600

No, another capture won't be necessary. Thanks for following up. If something in the code doesn't jump out at me today while I'm looking, I may be forced to place a patch up here which prints out a bunch of debug information as SIP packets are being formed so I can track exactly what's happening. Hopefully, I won't have to do that though.
By: Mark Michelson (mmichelson) 2009-01-16 15:20:38.000-0600

All right. At this point, the problem is fully identified, and is a bit more complex to solve than I had originally thought. What I am going to do is post a workaround patch here for you to use for now. Later, when I have a full fix for the issue, I'll post it here. Thanks for your patience on this one.
By: James Golovich (jamesgolovich) 2009-01-16 15:30:27.000-0600

I was poking around the code to see if anything jumped out at me and it looks like theres another piece of unneeded code in add_sdp

       if (m_audio->len - m_audio->used < 2 || m_video->len - m_video->used < 2 ||
                       m_text->len - m_text->used < 2 || a_text->len - a_text->used < 2 ||
                       a_audio->len - a_audio->used < 2 || a_video->len - a_video->used < 2)
               ast_log(LOG_WARNING, "SIP SDP may be truncated due to undersized buffer!!\n");
By: Mark Michelson (mmichelson) 2009-01-17 17:02:40.000-0600

I have written up a fix for this issue and have placed my work on reviewboard. The URL of the review is here:

http://reviewboard.digium.com/r/126/

Please note that the diff that you find on reviewboard is for *trunk's* chan_sip.c, and not the 1.6.1 branch. Once the trunk patch has been approved, it will also be merged into 1.6.1 as well.
By: Digium Subversion (svnbot) 2009-01-20 14:10:16.000-0600

Repository: asterisk
Revision: 169557

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r169557 | mmichelson | 2009-01-20 14:10:15 -0600 (Tue, 20 Jan 2009) | 19 lines

Convert the character pointers in a sip_request to be pointer offsets

When an ast_str expands to hold more data, any pointers that were pointing
to the data prior to the expansion will be pointing at invalid memory. This
change makes such pointers used in chan_sip.c instead be offsets from the
beginning of the string so that the same math may be applied no matter where
in memory the string resides.

To help ease this transition, a macro called REQ_OFFSET_TO_STR has been added
to chan_sip.c so that given a sip_request and an offset, the string at that
offset is returned.

(closes issue ASTERISK-13347)
Reported by: riksta
Tested by: putnopvut

Review http://reviewboard.digium.com/r/126/


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

http://svn.digium.com/view/asterisk?view=rev&revision=169557
By: Digium Subversion (svnbot) 2009-01-20 14:15:35.000-0600

Repository: asterisk
Revision: 169559

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

------------------------------------------------------------------------
r169559 | mmichelson | 2009-01-20 14:15:34 -0600 (Tue, 20 Jan 2009) | 27 lines

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

........
r169557 | mmichelson | 2009-01-20 14:10:31 -0600 (Tue, 20 Jan 2009) | 19 lines

Convert the character pointers in a sip_request to be pointer offsets

When an ast_str expands to hold more data, any pointers that were pointing
to the data prior to the expansion will be pointing at invalid memory. This
change makes such pointers used in chan_sip.c instead be offsets from the
beginning of the string so that the same math may be applied no matter where
in memory the string resides.

To help ease this transition, a macro called REQ_OFFSET_TO_STR has been added
to chan_sip.c so that given a sip_request and an offset, the string at that
offset is returned.

(closes issue ASTERISK-13347)
Reported by: riksta
Tested by: putnopvut

Review http://reviewboard.digium.com/r/126/


........

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

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