[Home]

Summary:ASTERISK-17771: [patch] switching From-address mid-register breaks channel variables
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2011-04-29 05:12:12Date Closed:2011-05-20 11:41:14
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) issue19202_destroy_challenged_invite_chanvars.patch
Description:Hi,

I'm using a inbound proxy before my asterisk which guesses which account
someone is using based on message headers and IP addresses. This is not
reliable until I get a Digest username. When I get the digest username I
am certain which user it is, and I replace the From-address accordingly.

In this scenario, the following can happen:

(1) INVITE from CLIENT to PROXY
(2) PROXY replaces From: with "Joe" and sends to ASTERISK
(3) ASTERISK challenges the INVITE with a 40[17]

(4) INVITE from CLIENT TO PROXY using Digest username "Lucy"
(5) PROXY replaces From: with "Lucy" and sends to ASTERISK
(6) ASTERISK handles INVITE fine...

... except for the fact that the channel variables are now duplicated
(and in the wrong order).


sip.conf:

[JoeUser]
setvar=username=Joe
accountcode=JoeAccount
...
[LucyUser]
setvar=username=Lucy
accountcode=LucyAccount
...

Now, in my dialplan:
- ${CDR(accountcode)} equals "LucyAccount"
and
- ${username} equals "Joe"

Why is that? Because the channel variables set during the first INVITE
are not destroyed. And, due to the linked-list nature of chanvars, they
are in the "wrong" order, containing first username=Lucy and then
username=Joe.

When copying the chanvars to the dialplan context using
pbx_builtin_setvar_helper Joe overwrites Lucy, hence the inconsistency.


The attached patch should fix it.


Regards,
Walter Doekes
OSSO B.V.


P.S. I'm expecting someone to reply that switching From: mid-INVITE is
not legal, SIP-wise. As a pre-emptive response to that:
(1) I couldn't find the relevant RFC-part that states so and,
(2) asterisk is inconsistent in it's behaviour because it *does* accept
   the switch, apart from this bug.
Comments:By: Bradley Watkins (marquis) 2011-05-05 07:49:57

Aside from any RFC issues, I think the desired behaviour would be that the authenticated INVITE would be the source of the channel variables.  So my opinion is that this patch should be committed.
By: Digium Subversion (svnbot) 2011-05-06 15:01:17

Repository: asterisk
Revision: 317867

U   branches/1.8/channels/chan_sip.c

------------------------------------------------------------------------
r317867 | russell | 2011-05-06 15:01:17 -0500 (Fri, 06 May 2011) | 10 lines

chan_sip: Destroy variables on a sip_pvt before copying vars from the sip_peer.

Don't duplicate variables on the sip_pvt.  Just reset the variable list each
time.

(closes issue ASTERISK-17771)
Reported by: wdoekes
Patches:
     issue19202_destroy_challenged_invite_chanvars.patch uploaded by wdoekes (license 717)

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

http://svn.digium.com/view/asterisk?view=rev&revision=317867
By: Digium Subversion (svnbot) 2011-05-06 15:02:32

Repository: asterisk
Revision: 317868

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r317868 | russell | 2011-05-06 15:02:32 -0500 (Fri, 06 May 2011) | 17 lines

Merged revisions 317867 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
 r317867 | russell | 2011-05-06 15:01:16 -0500 (Fri, 06 May 2011) | 10 lines
 
 chan_sip: Destroy variables on a sip_pvt before copying vars from the sip_peer.
 
 Don't duplicate variables on the sip_pvt.  Just reset the variable list each
 time.
 
 (closes issue ASTERISK-17771)
 Reported by: wdoekes
 Patches:
       issue19202_destroy_challenged_invite_chanvars.patch uploaded by wdoekes (license 717)
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=317868
By: Walter Doekes (wdoekes) 2011-05-12 09:15:47

I believe you have committed a no-op there.

if (!(res = check_auth(p, req, peer->name, p->peersecret, p->peermd5secret, sipmethod, uri2, reliable, req->ignore))) {
..
we only get here once.. so the variables are only set once.

This was already working properly in 1.6 and 1.8.

Only in 1.4 is this broken.
By: Digium Subversion (svnbot) 2011-05-20 11:38:30

Repository: asterisk
Revision: 320055

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r320055 | dvossel | 2011-05-20 11:38:29 -0500 (Fri, 20 May 2011) | 9 lines

chan_sip: Destroy variables on a sip_pvt before copying vars from the sip_peer.

(closes issue ASTERISK-17771)
Reported by: wdoekes
Patches:
     issue19202_destroy_challenged_invite_chanvars.patch uploaded by wdoekes (license 717)



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

http://svn.digium.com/view/asterisk?view=rev&revision=320055
By: Digium Subversion (svnbot) 2011-05-20 11:41:13

Repository: asterisk
Revision: 320056

_U  branches/1.6.2/

------------------------------------------------------------------------
r320056 | dvossel | 2011-05-20 11:41:13 -0500 (Fri, 20 May 2011) | 13 lines

Blocked revisions 320055 via svnmerge

........
 r320055 | dvossel | 2011-05-20 11:38:28 -0500 (Fri, 20 May 2011) | 9 lines
 
 chan_sip: Destroy variables on a sip_pvt before copying vars from the sip_peer.
 
 (closes issue ASTERISK-17771)
 Reported by: wdoekes
 Patches:
       issue19202_destroy_challenged_invite_chanvars.patch uploaded by wdoekes (license 717)
........

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

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