[Home]

Summary:ASTERISK-07015: [patch] When SIP contains MIME header with Content-Type: multipart/mixed;boundary=..., call gets connected with no audio
Reporter:jnguyen (jnguyen)Labels:
Date Opened:2006-07-17 09:44:26Date Closed:2007-06-30 09:19:59
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Interoperability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) ast_boundary_fix_1_2_12-1.patch
( 1) Asterisk_SVN-trunk-r38158M.diff.txt
( 2) real_sip.trunk.patch
( 3) sip_debug_log.txt
( 4) SIP_Invite_MIME_boundary.txt
Description:In function find_sdp (file chan_sip.c) req->sdp_end is assigned to the beginning of the boundary instead of the end. So, chan_sip could not parse correctly the audio info.
I guess the coder missed the line to advance the x value by 2
Comments:By: Serge Vecher (serge-v) 2006-07-17 09:54:12

As per bug guidelines, you need to include a complete SIP trace with debug on show all transactions.
1) Prepare test environment (reduce the ammount of unrelated traffic on the server);
2) Make sure your logger.conf has the following line:
  console => notice,warning,error,debug
3) restart Asterik.
4) Enable SIP transaction logging with the following CLI commands:
set debug 4
set verbose 4
sip debug
5) Save complete console log to file and _attach_ said file to the bug.
By: jnguyen (jnguyen) 2006-07-17 10:11:58

I understand your concern about submitting the sip trace log. The log I have is the real production log, and I could not release it because of confidencial issue. Besides, I could not have the test environment to generate SIP MIME header. However, the bug is like a typo bug and so obvious that if someone reviews the short function find_sdp in chan_sip.c, he will figure it out immediately. Thanks
By: Serge Vecher (serge-v) 2006-07-17 12:25:52

jnguyen: it sounds simple, but the complexity of chan_sip requires that we follow a certain debugging protocol. We don't need to see private information, including IP addresses, authorization hashes, usernames, etc, just the flow of messages and respective debugging information produced by chan_sip in debug mode.

A common practice for somebody in your situation is to purge the log file of any sensitive information and posting the sanitized version.
By: Vadim Sherbakov (vinsik) 2006-07-23 17:07:40

I think i have the same problem, and after alot of poking in the chan_sip.c code i found the problem.

In some cases other side sends content type boundary id-string with quotes.
Example:
Content-Type: multipart/mixed ;boundary="boundary_test40"
After this asterisk adds two lines in the beginning of the idstring and tries to match the code '--"boundary_test40"' to '--boundary_test40'.

The log:
SIP/2.0 183 Session Progress
From: "hasse"<sip:987654321@83.x.x.245>;tag=as2dd4ba56
To: <sip:123456789@82.x.x.4>;tag=4cb7652-13c4-44c3b622-f82bda8a-6a0
Call-ID: 4564df6e6335dc144b00495e2b2459ea@83.x.x.245
CSeq: 102 INVITE
Via: SIP/2.0/UDP 83.x.x.245:5060;rport=5060;branch=z9hG4bK7e2e5cd2
Contact: <sip:123456789@82.x.x.4>
Content-Type: multipart/mixed ;boundary="Boundary44c3b623_f82bdc40_6e2a_a62589c"
Content-Length: 440

--Boundary44c3b623_f82bdc40_6e2a_a62589c
Content-Type: application/SDP

v=0
o=82.x.x.14 1153676832 1153676832 IN IP4 82.x.x.14
s=Session SDP
c=IN IP4 82.x.x.14
t=0 0
m=audio 15040 RTP/AVP 0
a=rtpmap:0 PCMU/8000
a=fmtp:96 0-15
a=rtpmap:96 telephone-event/8000
a=ptime:20

--Boundary44c3b623_f82bdc40_6e2a_a62589c
Content-Type: application/ISUP ;version=itu-t

Hope you can fix this issue as soon as possible, because i made a patch of my own and im pretty sure it's not 100% good code.


By: Serge Vecher (serge-v) 2006-07-24 09:51:53

vinsik: we will still need to see the DEBUG output with complete transaction. Please review my note 0049248 for instructions.

Thanks.
By: Vadim Sherbakov (vinsik) 2006-07-24 11:16:29

vechers: I hope this will help... :) I fixed this issue by removing quotes from the boundary string before the for loop in chan_sip.c -> find_sdp function.

jnguyen: Please post you log as well. We will get this issue resolved faster when there is more evidence! Just remove all sensitive info from the log.


By: Serge Vecher (serge-v) 2006-07-24 11:27:09

vinsik: could you please get a disclaimer on file with Digium (see bottom of http://bugs.digium.com/main_page.php) and upload your changes as a patchfile (http://www.asterisk.org/developers/Patch_Howto)? Thanks ...
By: Vadim Sherbakov (vinsik) 2006-07-24 11:42:29

vechers: I dont really think thats such a good idea. As much as i like to help others on this mather, i dont think that changes i made are a good and stable enough solution, as i'm just a beginner coder and i dont have enough experience to write proper code. Ofcourse i'll post it, but i dont recommend anybody to use it.

I bet anyone of asterisk developers can fix this issue in 5min with more proper code than this.
By: jnguyen (jnguyen) 2006-07-25 22:35:42

vinsik: I think the proper fix is to add only one line to increase the x value by 2:
for (x = 0; x < (req->lines - 2); x++) {
 if (!strncasecmp(req->line[x], boundary, strlen(boundary)) &&
     !strcasecmp(req->line[x + 1], "Content-Type: application/sdp")) {
req->sdp_start = x + 2;
/* search for the end of the body part */
/* fix the new asterisk bug (v.1.2.9.1) */
x += 2; /* THIS IS THE FIX */
for ( ; x < req->lines; x++) {
  if (!strncasecmp(req->line[x], boundary, strlen(boundary)))
break;
}
req->sdp_end = x;
return 1;
  }
}
By: Vadim Sherbakov (vinsik) 2006-07-26 03:05:27

jnguyen: What good would that do? If the previous 'if' statement wont let it go that far? Like i said before, post your log and we will see if this is even the same case...

My case is when another peer sends multipart INVITE, it sometimes close boundary id-string in quotes, and currently asterisk cannot match it to the multipart message body id-string. This is why it did not work for me.

So for the love of god. Debug/Log and Post! Remove all sensitive info if you feel like it.

Cheerz


By: jnguyen (jnguyen) 2006-07-26 09:59:03

vinsik: We have 2 scenerios here
1. "multipart/mixed" MIME has one or more than one parts. Your case has 2 parts in MIME and your "Content-Type: application/sdp" part appears first. That makes it has a closed boundary string id. So, your code works without "x += 2" (my fix). My case has only one part in MIME, and my "Content-Type: application/sdp" does not have a closed boundary string id, that's why only my fix will solve my problem.
2. The boundary string id has double quote as a part of boundary. If double quote is not allowed in the boundary string id, it's a bug from your voip service provider. Your code is just a workaround for that bug.

cheers!
By: Serge Vecher (serge-v) 2006-07-26 10:18:42

jnguyen: your logfile does not contain DEBUG output -- see vinsik's attachment to see the difference. My instructions in note 0049248 should produce such a log. Also, 1.2.5 is quite old ...
By: jnguyen (jnguyen) 2006-07-26 10:56:30

hi vechers,

Sorry, i was trying to get the exact log as you required, but my voip service provider has stopped sending us "multipart/mixed" MIME. So, I could not get the right log for the issue any more.
By: Vadim Sherbakov (vinsik) 2006-07-26 17:18:40

jnguyen: after carefull reading of RFC documents, i found that Content-type boundary id-strings can be used with double quotes. More info on this in RFC 2046 (http://www.tech-invite.com/mtypes/RFC2046%20-%20MIME%20Part%202%20-%20Media%20Types.pdf). Please ask your provider to send multipart MIME sip messages just for testing. I would like to be sure that there will be no more mishaps on this issue. :)

vechers: did some one had the time to make a better patch for the double quote issue?

Cheerz.


By: Vadim Sherbakov (vinsik) 2006-07-26 17:28:36

In my opinion, this is not a standart sdp message:
Content-Type: application/sdp

v=0
o=- 1152900684 1152900685 IN IP4 209.247.5.45
s=-
c=IN IP4 209.247.5.45
t=0 0
m=audio 60026 RTP/AVP 0 101
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-16
a=ptime:20

Missing some parameters.. i still dont see jnguyen's point in advancing x by 2. But then again im not a coder, so maybe i dont see the big picture here. :)


By: jnguyen (jnguyen) 2006-07-27 13:11:53

vinsik: i am trying to explain my point or my fix here
- inside the loop:
for (x = 0; x < (req->lines - 2); x++) {
- when it finds the boundary stringid and the header "application/sdp", we are inside the if: (assuming at line 10)
if (!strncasecmp(req->line[x], boundary, strlen(boundary)) &&
   !strcasecmp(req->line[x + 1], "Content-Type: application/sdp")) {
- Then assing req->sdp_start = x + 2; pointing to the line after "Content-Type: application/sdp" (e.g at line 12)
- Next we start looking for the end boundary stringid by using loop:
for ( ; x < req->lines; x++) {
- so, x starts at the line opening boundary stringid (line 10), we should look for the closed boundary stringid at x + 2 instead (line 12).
- We will break the lopp at the first check at statement:
if (!strncasecmp(req->line[x], boundary, strlen(boundary))
- so, req->sdp_end = x; (line 10). So, sdp_end is lesser than sdp_start by 2
- after function find_sdp, we look for info for setting up audio based on sdp_start and sdp_end. sdp_end has the incorrect value.

I hope it could help
By: Serge Vecher (serge-v) 2006-09-01 14:43:26

alright, so where are we with this?
By: Vadim Sherbakov (vinsik) 2006-09-04 01:33:15

serge-v: well it seems that me and jnguyen have a different issue.
i dont really understand the bug in jnguyen's case.

The one i found is pretty simple. Just correct the code that it would understand quotes in the boundary id. Like i said my patch is a very primitive noob code. :)
This bug also presists in 1.2.11 and in latest SVN.
By: Vadim Sherbakov (vinsik) 2006-10-08 17:03:06

serge-v: i have been studying C programming and wrote a better patch. Will work on current branch as well as on trunk.

This patch will fix boundary quotes problem.
(Not a big deal, but just in case some one has the a PSTN GW Vendor that uses quotes in boundary id string.)

will work for asterisk 1.2.8 -> current.
By: Vadim Sherbakov (vinsik) 2006-10-09 07:26:50

serge-v: hehe.. my patch is still missing jnguyen's bugfix. As soon as i fixed my code. I ran into the same issue as jnguyen. I have made a new patch with all the corrections. Please delete the patch that has been uploaded 10-08-06 16:58.

I think we can close this one.

Thank you.


By: Serge Vecher (serge-v) 2006-10-09 11:25:42

jnguyen: can you please give ast_boundary_fix_1_2_12-1.patch a shot and report the results?
By: Vadim Sherbakov (vinsik) 2006-10-12 05:17:21

serge-v: i would prefer that you delete my debug_log.txt from 07-24-06 11:14 .. there is still some vital info and it is shown on google.

I can make a new one if requested.

thanx
By: jnguyen (jnguyen) 2006-10-12 12:50:23

ast_boundary_fix_1_2_12-1.path is good, it fixes my problem.
By: jmls (jmls) 2006-11-12 02:46:41.000-0600

if this patch has fixed the problem, can we get it reviewed and comitted ?
By: Olle Johansson (oej) 2006-11-12 08:45:17.000-0600

Please retry with latest svn 1.2. I fixed this a few days ago.
By: Olle Johansson (oej) 2006-11-16 06:21:06.000-0600

I do need feedback. Thanks.
By: bcoppens (bcoppens) 2006-11-21 02:55:29.000-0600

Have been testing with the latest 1.2 SVN Asterisk SVN-branch-1.2-r47842 and have the same problem. Asterisk is unable to parse the SDP contents from the invite.
See log SIP_Invite_MIME_boundary.txt enclosed
By: Vadim Sherbakov (vinsik) 2006-11-22 01:39:02.000-0600

oej: i still dont see the fix for boundary quotes in the newest SVN.

I fixed mine with this:
if (strcasestr(boundary, "\"")) {
strncpy(boundary, boundary+1, strlen(boundary));
boundary[strlen(boundary)-1] = '\0';
}
By: bcoppens (bcoppens) 2006-11-22 04:14:29.000-0600

As you can see in the INVITE, there is a "content length" before the Content-Type: application/sdp.
For my setup it works after changing the following lines:

if (!strncasecmp(req->line[x], boundary, strlen(boundary)) &&
                   !strcasecmp(req->line[x + 2], "Content-Type: application/sdp")) {
                       x += 3;
                       req->sdp_start = x;

I am not a programmer so I would like to leave it up to someone else.
By: Brandon Kruse (bkruse) 2006-12-28 13:59:48.000-0600

ok.

I have applied this to trunk, after the customer already said it was working...

one of the changes ( x += 2; ), was already in trunk in the right place.

anyways, i have applied it to trunk and uploaded it.....
By: Olle Johansson (oej) 2007-02-15 11:46:59.000-0600

So do we have this working in 1.2, 1.4 and trunk now?
By: Vadim Sherbakov (vinsik) 2007-02-16 01:05:50.000-0600

oej: not me. I have to patch my asterisk everytime i upgrade to a newer version.
Mine operator sends the boundary code with quotes, and asterisk does not find application/SDP when they are used.
By: Olle Johansson (oej) 2007-02-16 06:00:59.000-0600

Ok, fixing the quotes, but in a different way. Wanted to remove them before we duplicated them.
By: Olle Johansson (oej) 2007-02-16 06:08:17.000-0600

Committed to 1.4, rev 54787. THanks for all the hard work in fixing this!

Hopefully, everything is finally in order for multipart messages.