[Home]

Summary:ASTERISK-11502: [patch] chan_sip in pedantic mode : error in tag checking
Reporter:Frederic LE FOLL (flefoll)Labels:
Date Opened:2008-02-25 05:06:46.000-0600Date Closed:2008-02-25 17:19:20.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip.c.br14.patch_pedantic_no_totag
( 1) chan_sip.c.trunk.patch_pedantic_no_totag
Description:When chan_sip is in pedantic mode, find_call() checks tags in addition of Call-IDs.

But there is an error in "found" flag evaluation. The test below is positive if "tag" pointer is null, when it should be positive if "tag" STRING is EMPTY :
found = ([...] && (!pedanticsipchecking || !tag || ast_strlen_zero(p->theirtag) || !strcmp(p->theirtag, tag)));

Due to initial tag="" declaration and due to gettag() function behaviour, tag pointer should never be null, but pointed string may be empty ("\0").

I suggest to replace "!tag" with "ast_strlen_zero(tag)".

This will address the situation where an INVITE is CANCEL'ed and response to CANCEL contains no To Tag. See example in RFC 3665, "Session Initiation Protocol (SIP) Basic Call Flow Examples", 3.8. "Unsuccessful No Answer". Also seen in real life.
- With "!tag" condition : find_call fails, thus CANCEL is retransmitted
- With "ast_strlen_zero(tag)" condition : find_call succeeds, thus CANCEL retransmission is properly stopped.

I propose a patch for branch 1.4 and for trunk.


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

Another issue, <http://bugs.digium.com/view.php?id=11536>, is about pedantic mode and To Tag in response to CANCEL. The patch I propose here does not address this other issue that is quite different.
Comments:By: Digium Subversion (svnbot) 2008-02-25 09:13:40.000-0600

Repository: asterisk
Revision: 104082

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r104082 | file | 2008-02-25 09:13:39 -0600 (Mon, 25 Feb 2008) | 6 lines

Due to recent changes tag will no longer be NULL if not present so we have to use ast_strlen_zero to see if it's actually blank.
(closes issue ASTERISK-11502)
Reported by: flefoll
Patches:
     chan_sip.c.br14.patch_pedantic_no_totag uploaded by flefoll (license 244)

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

http://svn.digium.com/view/asterisk?view=rev&revision=104082
By: Digium Subversion (svnbot) 2008-02-25 09:16:20.000-0600

Repository: asterisk
Revision: 104083

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r104083 | file | 2008-02-25 09:16:20 -0600 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104082 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104082 | file | 2008-02-25 11:17:18 -0400 (Mon, 25 Feb 2008) | 6 lines

Due to recent changes tag will no longer be NULL if not present so we have to use ast_strlen_zero to see if it's actually blank.
(closes issue ASTERISK-11502)
Reported by: flefoll
Patches:
     chan_sip.c.br14.patch_pedantic_no_totag uploaded by flefoll (license 244)

........

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

http://svn.digium.com/view/asterisk?view=rev&revision=104083
By: Digium Subversion (svnbot) 2008-02-25 13:02:51.000-0600

Repository: asterisk
Revision: 104090

_U  team/seanbright/NoLossCDR-Redux/
U   team/seanbright/NoLossCDR-Redux/channels/chan_agent.c
U   team/seanbright/NoLossCDR-Redux/channels/chan_iax2.c
U   team/seanbright/NoLossCDR-Redux/channels/chan_sip.c
U   team/seanbright/NoLossCDR-Redux/configs/sip.conf.sample
U   team/seanbright/NoLossCDR-Redux/doc/siptls.txt
U   team/seanbright/NoLossCDR-Redux/res/res_config_pgsql.c

------------------------------------------------------------------------
r104090 | seanbright | 2008-02-25 13:02:46 -0600 (Mon, 25 Feb 2008) | 68 lines

Merged revisions 104081,104083,104085,104087-104089 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r104081 | file | 2008-02-25 10:12:48 -0500 (Mon, 25 Feb 2008) | 2 lines

Fix building of trunk. dbpass is always going to exist.

................
r104083 | file | 2008-02-25 10:19:58 -0500 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104082 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104082 | file | 2008-02-25 11:17:18 -0400 (Mon, 25 Feb 2008) | 6 lines

Due to recent changes tag will no longer be NULL if not present so we have to use ast_strlen_zero to see if it's actually blank.
(closes issue ASTERISK-11502)
Reported by: flefoll
Patches:
     chan_sip.c.br14.patch_pedantic_no_totag uploaded by flefoll (license 244)

........

................
r104085 | file | 2008-02-25 11:18:46 -0500 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104084 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104084 | file | 2008-02-25 12:16:13 -0400 (Mon, 25 Feb 2008) | 6 lines

If a resubscription comes in for a dialog we no longer know about tell the remote side that the dialog does not exist so they subscribe again using a new dialog.
(closes issue ASTERISK-10305)
Reported by: s0l4rb03
Patches:
     10727-2.diff uploaded by file (license 11)

........

................
r104087 | russell | 2008-02-25 13:38:51 -0500 (Mon, 25 Feb 2008) | 12 lines

Merged revisions 104086 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104086 | russell | 2008-02-25 12:38:10 -0600 (Mon, 25 Feb 2008) | 4 lines

Ensure that the channel doesn't disappear in agent_logoff().  If it does, it
could cause a crash.
(fixes the crash reported in BE-396)

........

................
r104088 | bbryant | 2008-02-25 14:00:16 -0500 (Mon, 25 Feb 2008) | 1 line

Adding more tls configuration details to sip.conf sample, with a list of valid ciphers provided in both files. .. First commit since July, woot
................
r104089 | file | 2008-02-25 14:02:33 -0500 (Mon, 25 Feb 2008) | 2 lines

Instead of outputting a verbose message every so often let's make it a debug message.

................

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

http://svn.digium.com/view/asterisk?view=rev&revision=104090
By: Digium Subversion (svnbot) 2008-02-25 17:19:20.000-0600

Repository: asterisk
Revision: 104105

_U  team/murf/bug11210/
U   team/murf/bug11210/apps/app_voicemail.c
U   team/murf/bug11210/channels/chan_agent.c
U   team/murf/bug11210/channels/chan_iax2.c
U   team/murf/bug11210/channels/chan_sip.c
U   team/murf/bug11210/configs/sip.conf.sample
U   team/murf/bug11210/doc/siptls.txt
U   team/murf/bug11210/funcs/func_global.c
U   team/murf/bug11210/main/config.c

------------------------------------------------------------------------
r104105 | murf | 2008-02-25 17:19:19 -0600 (Mon, 25 Feb 2008) | 133 lines

Merged revisions 104081,104083,104085,104087-104089,104093,104096-104098 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r104081 | file | 2008-02-25 08:12:48 -0700 (Mon, 25 Feb 2008) | 2 lines

Fix building of trunk. dbpass is always going to exist.

................
r104083 | file | 2008-02-25 08:19:58 -0700 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104082 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104082 | file | 2008-02-25 11:17:18 -0400 (Mon, 25 Feb 2008) | 6 lines

Due to recent changes tag will no longer be NULL if not present so we have to use ast_strlen_zero to see if it's actually blank.
(closes issue ASTERISK-11502)
Reported by: flefoll
Patches:
     chan_sip.c.br14.patch_pedantic_no_totag uploaded by flefoll (license 244)

........

................
r104085 | file | 2008-02-25 09:18:46 -0700 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104084 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104084 | file | 2008-02-25 12:16:13 -0400 (Mon, 25 Feb 2008) | 6 lines

If a resubscription comes in for a dialog we no longer know about tell the remote side that the dialog does not exist so they subscribe again using a new dialog.
(closes issue ASTERISK-10305)
Reported by: s0l4rb03
Patches:
     10727-2.diff uploaded by file (license 11)

........

................
r104087 | russell | 2008-02-25 11:38:51 -0700 (Mon, 25 Feb 2008) | 12 lines

Merged revisions 104086 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104086 | russell | 2008-02-25 12:38:10 -0600 (Mon, 25 Feb 2008) | 4 lines

Ensure that the channel doesn't disappear in agent_logoff().  If it does, it
could cause a crash.
(fixes the crash reported in BE-396)

........

................
r104088 | bbryant | 2008-02-25 12:00:16 -0700 (Mon, 25 Feb 2008) | 1 line

Adding more tls configuration details to sip.conf sample, with a list of valid ciphers provided in both files. .. First commit since July, woot
................
r104089 | file | 2008-02-25 12:02:33 -0700 (Mon, 25 Feb 2008) | 2 lines

Instead of outputting a verbose message every so often let's make it a debug message.

................
r104093 | qwell | 2008-02-25 13:50:57 -0700 (Mon, 25 Feb 2008) | 19 lines

Merged revisions 104092 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104092 | qwell | 2008-02-25 14:49:42 -0600 (Mon, 25 Feb 2008) | 11 lines

Allow the use of #include and #exec in situations where the max include depth was only 1.
Specifically, this fixes using #include and #exec in extconfig.conf.

This was basically caused because the config file itself raises the include level to 1.

I opted not to raise the include limit, because recursion here could cause very bizarre behavior.

Pointed out, and tested by jmls

(closes issue ASTERISK-11505)

........

................
r104096 | file | 2008-02-25 14:40:30 -0700 (Mon, 25 Feb 2008) | 14 lines

Merged revisions 104095 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104095 | file | 2008-02-25 17:37:20 -0400 (Mon, 25 Feb 2008) | 6 lines

Make it so a users.conf user creates both a SIP peer and a SIP user. The user will be used for inbound authentication for the device, and peer will be used for placing calls to the device.
(closes issue ASTERISK-8785)
Reported by: queuetue
Patches:
     sip-gui-friend.diff uploaded by qwell (license 4)

........

................
r104097 | tilghman | 2008-02-25 14:53:36 -0700 (Mon, 25 Feb 2008) | 13 lines

Merged revisions 104094 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r104094 | tilghman | 2008-02-25 15:31:47 -0600 (Mon, 25 Feb 2008) | 5 lines

If the destination folder is full, don't delete a message when exiting.
(closes issue ASTERISK-11506)
Reported by: selsky
Patch by: (myself)

........

................
r104098 | tilghman | 2008-02-25 14:56:19 -0700 (Mon, 25 Feb 2008) | 7 lines

Shared space for variables (instead of letting other channels muck with your own)
(closes issue ASTERISK-11392)
Reported by: ramonpeek
Patches:
      20080208__bug11943__2.diff.txt uploaded by Corydon76 (license 14)
Tested by: jmls

................

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

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