[Home]

Summary:ASTERISK-08148: On call transfer, need to be able to retrieve SIP Referred-by header from the incoming REFER
Reporter:John Covert (jcovert)Labels:
Date Opened:2006-11-17 08:05:57.000-0600Date Closed:2007-07-06 19:18:19
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Transfers
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:Asterisk currently makes the assumption that any transfer should be handled through the standard mechanisms in the local dialplan.  For many cases, especially for transfers initiated using pbx-transfer via "t" or "T" in the Dial command, this is true.  It's also true for sip clients within the PBX.

However, this assumption breaks down for calls to and from other SIP clients of a SIP provider, or for incoming SIP guest calls (where there is even a serious security issue, see below).  Consider the following.  Asterisk has either received or placed a call via a peer who is a SIP provider whom we will call Voicexxx for the purposes of this discussion.  The other end of the call is another Voicexxx client.  The other Voicexxx client initiates a SIP transfer, and the way Voicexxx process such a transfer is by sending Asterisk a SIP REFER command.

Currently, asterisk sends this through the dialplan, and Asterisk establishes a completely new call, routing it the way Asterisk prefers to route such calls, based on the dialplan.  However, this may not be correct, if the call really should be routed out via Voicexxx and billed to the tranferrer, not to us.  Voicexxx has provided a Referred-by header in the REFER which contains a billing token, and is perfectly willing to accept a call from Asterisk, even an off-network call, and bill it to the other customer who initiated the transfer if this Referred-by header is returned on a subsequent INVITE (and their proxy has even managed to avoid any security implications in their network by keeping track of the relationship between a REFER sent and an INVITE received).

To do this, access to the Referred-by header as received is required when Asterisk starts up the transfer in the transfer context.  It cannot be retrieved from the SIP_HEADER() function because by the time we are active in the transfer context we the REFER information is long gone.

The following suggested change has been tested in 1.2 and should work without modification in 1.4 and trunk:

Insert the following code into routine get_refer_info of chan_sip.c, after line  6906 "referred_by += 4;" of 1.2(47743), before line 8538 "Check for arguments in the refer_to header" of 1.4(47764), and before line 8715 "Check for arguments in the refer_to header" of trunk(47765):

       if (sip_pvt->owner && (peer = ast_bridged_channel(sip_pvt->owner))) {
          pbx_builtin_setvar_helper(peer, "SIPREFERRINGCONTEXT", sip_pvt->context);
          pbx_builtin_setvar_helper(peer, "SIPREFERTO", refer_to);
          pbx_builtin_setvar_helper(peer, "SIPREFERREDBYHDR", p_referred_by);
       }

This provides three local variables to the dialplan when the call is activated in the TRANSFER_CONTEXT: SIPREFERRINGCONTEXT: the context associated with the SIP peer initiating the transfer, SIPREFERREDBYHDR: the full, unmodified Referred-by header received in the REFER, and SIPREFERTO, the portion of the Refer-To header inside brackets and beyond the "sip:", in case that is also needed to handle the transfer. Below is a dialplan example of how this might be handled:

[general]
TRANSFER_CONTEXT=station_transfer

[station_transfer]
exten => _X.,1,Noop(BLINDTRANSFER ${BLINDTRANSFER})
exten => _X.,n,Noop(SIPREFERRINGCONTEXT ${SIPREFERRINGCONTEXT})
exten => _X.,n,Noop(SIPREFERREDBYHDR ${SIPREFERREDBYHDR})
exten => _X.,n,Noop(SIPREFERTO ${SIPREFERTO})
; if SIPREFERRINGCONTEXT is null this is a transfer with a "#" or "*"
; which we are handling within the PBX
exten => _X.,n,GotoIf($["${SET(refcon=${SIPREFERRINGCONTEXT})}" = ""]?pbx-transfer,${EXTEN},1)
; This transfer was initiated by a SIP REFER
; Clear SIPREFERRINGCONTEXT so that it is not seen again in case the
; next xfer is PBX xfer w/ "#"
; otherwise it would still continue to be inherited
exten => _X.,n,Set(SIPREFERRINGCONTEXT=)
; if it's a known provider needing Referred-by processing, go there
exten => _X.,n,GotoIf($["${refcon}" = "inbound-voicexxx"]?voicexxx-xfr,${EXTEN},1)
; if it's a local SIP peer (based on the context of the peer)
; allow them to transfer to exactly (and only) what they could
; have dialled
exten => _X.,n,GotoIf($["${refcon:0:11}" = "dialstation"]?${refcon},${EXTEN},1)
; someone else
exten => _X.,n,Goto(station_transfer_restricted,${EXTEN},1)

[voicexxx-xfr]
; This is a SIP provider that we know will give us a Referred-by
; header that we want to send back, and always process transfers
; initiated on their network through their network
;
exten => _X.,1,SipAddHeader(Referred-By: ${SIPREFERREDBYHDR})
exten => _X.,n,Dial(SIP/${EXTEN}@voicexxx,120)

[station_transfer_restricted]
;
; We don't know who this is.
; We might want to check ${SIPREFERTO} to see if the transfer is
; even within our own domain.
; But in any case, don't allow outside transfers
; You might allow any local station, or whatever you would have otherwise
; allowed for a default SIP call
;

[pbx-transfer]
;
; This is a "#" or "*" transfer, not necessarily involving SIP at all.
; How to handle its security is not the subject of this SIP change.


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

It should be clear from the above discussion that use of the TRANSFER_CONTEXT variable introduces new security issues wrt SIP peers and transfers, which this patch makes possible to address.

I'll be glad to write up the necessary Wiki entries with the new variables, as well as a complete page on security implications of transfers (both SIP transfer and PBX transfer with Dial "t" "T" options) once this patch is incorporated.
Comments:By: John Covert (jcovert) 2006-11-17 08:16:23.000-0600

[general] should be [globals]

By: Serge Vecher (serge-v) 2006-11-20 14:41:25.000-0600

I think a better place for this issue is asterisk-dev mailing list.

By: Olle Johansson (oej) 2006-11-21 06:42:12.000-0600

This has all changed in 1.4. Please look into that version of asterisk. You can no disable SIP transfers by the "ALLOWTRANSFER" configuration option and we do set a lot of variables for the dialplan. Please review that and come back with suggestions on changes based on that version. Thanks.

By: Olle Johansson (oej) 2006-12-01 03:42:13.000-0600

have you checked the 1.4 implementation?

By: John Covert (jcovert) 2006-12-01 06:58:10.000-0600

I read the 1.4 code, and see that you've done some great work on improving this.  But before commenting wanted to actually bring it up on a non-production system and verify that I understand what the code actually does.  Since I only own Macs, and it was reported to me that there is (at the moment) a problem (being fixed) on the Mac platform which would prevent me from actually running 1.4, you haven't heard from me.

What I believe I understand from the code is that you have provided much of the necessary information.  However, the main piece of information needed is the entire, unmodified "Referred-by:" header received in the incoming REFER.  It is required to send the entire header exactly as received back to the ITSP in the resulting INVITE.  My suggested change provided this in ${SIPREFERREDBYHDR}.  From reading the code, I think you've provided only part of the header in SIPTRANSFER_REFERER.

I haven't yet figured out whether I can get the information my suggested change provides in ${SIPREFERRINGCONTEXT}.  If the _referring_ sippeer name is available, this could be obtained from ${SIPPEER(...)}.  Although you end up in that context if TRANSFER_CONTEXT is not set, with TRANSFER_CONTEXT set, you need to have information unambiguously identifying the source of the REFER.  The information in headers on the REFER is not unambiguous.  You may need to handle a transfer _differently_ depending on who is initiating the transfer, a SIP client within the PBX, or a SIP partner at the other end of an ITSP connection, or a random SIP caller from an unknown point.

I will try to actually run 1.4 as soon as possible to try out your changes.

Regards/john

By: Olle Johansson (oej) 2006-12-01 11:29:02.000-0600

I am running 1.4 happily on my OS/X system. Good luck trying!

By: John Covert (jcovert) 2006-12-01 15:48:28.000-0600

I'm glad to hear that you've got 1.4 running on Mac OS X.  I'll try to get it going on my G4 laptop soon.  However, this week I've been teaching the Sokol-Associates Asterisk Bootcamp, and this afternoon I had time to take over one of the classroom lab systems running Linux and install 1.4 on it.

My reading of the code was shown to be correct when I performed the necessary experiments.  Therefore my original request for the addition of code to return ${SIPREFERREDBYHDR} remains.  And although I categorized this as a feature request, it really could be considered a bug fix, because without it, the existing handling of the SIP REFER method in the presence of the Referred-by mechanism is a definite violation of RFC 3892:

   A UA accepting a REFER request (a referee) to a SIP URI (using either
  the sip: or sips: scheme) MUST copy any Referred-By header field
  value and token into the referenced request without modification.

Only by returning the full header to the dialplan logic is it possible for the dialplan code to issue a SipAddHeader(Referred-By: ${SIPREFERREDBYHDR}) making it possible for the resulting INVITE to comply with the RFC.

Theoretically (well, more than theoretically, since the RFC provides for it) a mime token might be in the body of the request also needs to be passed forward unmodified, but I'll ignore that for this request since it's significantly more difficult; the mechanisms for inserting content into an INVITE do not yet exist.

To review my request: I had proposed three variables: SIPREFERREDBYHEADER, SIPREFERTO, and SIPREFERRINGCONTEXT.  I believe you are providing enough information in the inbound extension and the new variable SIPDOMAIN so that I do not need SIPREFERTO.  (I actually hadn't used it, but had included it in my trial code simply for completeness).  Although I could get SIPREFERRINGCONTEXT from ${SIPPEER(${COMPLEXTRANSFORMATIONOF(${BRIDGEPEER})},context}, the complex transformation is inconvenient.  It would be nice to just have it set so that a transformation in the dialplan isn't necessary.  SIPREFERREDBYHEADER is needed in any case to fix the noncompliance with the RFC.

The following code has been tested in 1.4, inserted into chan_sip.c in routine get_refer_info immediately after the line "char *lessthan;" which occurs a few lines after the comment "/* Get referred by header if it exists */"

               if (transferer->owner) {
                 struct ast_channel *peer = ast_bridged_channel(transferer->owner);
                 if (peer) {
                   pbx_builtin_setvar_helper(peer, "SIPREFERRINGCONTEXT", transferer->context);
                   pbx_builtin_setvar_helper(peer, "SIPREFERREDBYHDR", p_referred_by);
                 }
               }

I hope you will consider it, decide favorably, and include the above change.  And although serge changed the version for this report from 1.2.13 to 1.4.beta3, please also consider including the original suggestion (leaving out the REFERTO variable) for 1.2.13 as well.

Regards/john

By: John Covert (jcovert) 2006-12-01 17:23:30.000-0600

An alternative and probably better point to insert this code might be a couple of lines earlier, immediately after the line "p_referred_by = get_header(req, "Referred-By");" and before the test for whether there actually is a value returned.

This would result in SIPREFERRINGCONTEXT being created even if there is no "Referred-By" header, and would also remove any residual SIPREFERREDBYHDR from a previous transfer if there was not one supplied on the current transfer.

/john

By: Caio Begotti (caio1982) 2007-01-25 08:42:28.000-0600

No more updates on this issue or something?

By: Corey McFadden (coreymcf) 2007-05-07 14:14:51

Is there progress on this one?  It seems like the 'Referred-by' header would solve a long-simmering issue with one of our customers who complains that their prior system changed the caller-id field after a successful attended transfer to show the original caller rather than the transferring party...

By: Olle Johansson (oej) 2007-05-09 08:47:37

Let's go back to the original issue. Why isn't the 1.4 code that does insert a Referred-by header working? It should happen automatically, not by sip_add_header in the dialplan.

By: John Covert (jcovert) 2007-05-09 09:58:29

On the call resulting from a Dial application after an incoming REFER, the SIP channel doesn't add a Referred-by header, because it has no code to do so, nor should it, because it isn't aware of what's going on in the dialplan.

As my "summary" says, the issue here is with dialplan processing of the incoming REFER, and the resulting INVITE that happens when we decide what to do and issue the Dial application to the new destination.  The SIP channel treats that as a completely new call, as it should.  The processing of a transfer from a party at the other end of a remote connection (not a local SIP device) needs to be handled carefully.  The Referred-by information is only relevant if we are going to Dial on the same provider.  If we decide to process the call some different way on a different provider, then we may neither need nor want to send a Referred-by header.

The beauty of Asterisk is that it lets us have control.  Generating a Referred-by header willy-nilly when the next call goes out some different way may cause the new provider to choke on the processing of the call.

In any case, back to the current need for this code, as I wrote in my note 55676 on 12-01-06 at 15:48, the RFC states that the full contents of the Referred-by must be transferred without modification into the INVITE that goes back out (if we're going to send it at all).  And that's the main problem with the variable currently set in the dialplan.  As you can see from that note, I tested my code in 1.4, and provided it back to you.  I hope you will accept it and set two variables: SIPREFERREDBYHDR and SIPREFERRINGCONTEXT.  Please review my justification for needing both of these in note 55676.  However, placement should be a few lines earlier, as noted in the immediately following note 55689 at 17:23.  Thank you.



By: Joshua C. Colp (jcolp) 2007-06-27 18:13:03

Put into trunk as of revision 72354.

By: Digium Subversion (svnbot) 2007-07-04 18:38:36

Repository: asterisk
Revision: 73297

------------------------------------------------------------------------
r73297 | file | 2007-07-04 18:38:34 -0500 (Wed, 04 Jul 2007) | 817 lines

Merged revisions 72207,72232-72233,72241,72258,72261,72274,72304,72325-72326,72329-72330,72332,72337,72354,72358,72382,72384,72437,72452,72454-72457,72466,72490-72492,72494,72524,72539,72555,72557,72598,72600,72666,72670,72700-72701,72706,72727,72738,72741,72767,72807,72867-72869,72889,72920-72923,72927-72932,72935-72940,72982,72986-72987,73003,73006,73054,73127,73144,73174-73175,73191,73209,73254 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r72207 | kpfleming | 2007-06-27 16:13:54 -0300 (Wed, 27 Jun 2007) | 10 lines

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

........
r72205 | kpfleming | 2007-06-27 14:13:21 -0500 (Wed, 27 Jun 2007) | 2 lines

use the proper type for storing group number bits so that if someone specifies 'group=42' it will actually work instead of being silently ignored

........

................
r72232 | mmichelson | 2007-06-27 16:50:21 -0300 (Wed, 27 Jun 2007) | 10 lines

Adding feature to support the storage and retrieval of voicemail greetings using IMAP storage.
This feature may be turned on by adding imapgreetings=yes to the general section of voicemail.conf
voicemail.conf.sample has details on the options added.

As a result, IMAP storage now has RETRIEVE and DISPOSE macros defined.

In addition to the IMAP greeting changes, I also have added an enum for the voicemail folders
and so now the code should be easier to understand and maintain when it comes to this area.


................
r72233 | file | 2007-06-27 16:57:36 -0300 (Wed, 27 Jun 2007) | 2 lines

Fix -T option. (issue ASTERISK-9767 reported by xylome)

................
r72241 | file | 2007-06-27 17:07:46 -0300 (Wed, 27 Jun 2007) | 2 lines

Fix up properties.

................
r72258 | file | 2007-06-27 17:26:53 -0300 (Wed, 27 Jun 2007) | 18 lines

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

................
r72257 | file | 2007-06-27 16:25:24 -0400 (Wed, 27 Jun 2007) | 10 lines

Merged revisions 72256 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72256 | file | 2007-06-27 16:23:24 -0400 (Wed, 27 Jun 2007) | 2 lines

I may possibly get shot for doing this... but... defer CDR processing until after the channel has been dealt with. This should eliminate all of the issues with channels going funky (SIP/PRI) when you are posting CDRs to a database that is either slow or unavailable and do not want to enable batching.

........

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

................
r72261 | bbryant | 2007-06-27 17:47:45 -0300 (Wed, 27 Jun 2007) | 20 lines

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

................
r72260 | bbryant | 2007-06-27 15:46:12 -0500 (Wed, 27 Jun 2007) | 12 lines

Merged revisions 72259 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72259 | bbryant | 2007-06-27 15:43:53 -0500 (Wed, 27 Jun 2007) | 4 lines

Fixes 1000ad when controlling terminal disappears.

Issue ASTERISK-9371, ASTERISK-9710

........

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

................
r72274 | russell | 2007-06-27 18:09:24 -0300 (Wed, 27 Jun 2007) | 21 lines

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

................
r72272 | russell | 2007-06-27 16:08:34 -0500 (Wed, 27 Jun 2007) | 13 lines

Merged revisions 72267 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72267 | russell | 2007-06-27 16:06:45 -0500 (Wed, 27 Jun 2007) | 5 lines

Fix a minor issue with parsing the priority number.  You could have as much
whitespace as you want around a numeric priority, but you couldn't have any
whitespace around a special priority like "n" or "hint".
(issue ASTERISK-9736, reported by mitheloc, fixed by me)

........

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

................
r72304 | mattf | 2007-06-27 18:44:13 -0300 (Wed, 27 Jun 2007) | 1 line

Let's NOT create a deadlock scenario here
................
r72325 | qwell | 2007-06-27 19:17:59 -0300 (Wed, 27 Jun 2007) | 4 lines

Add support for Thai language in say.c

Issue 9417, patch by dome, with some cleanup done by me.

................
r72326 | qwell | 2007-06-27 19:27:09 -0300 (Wed, 27 Jun 2007) | 4 lines

Fix a segfault when trying to tab complete the "core show uptime" command.

Reported in #asterisk-dev on IRC by jcmoore, fixed by me.

................
r72329 | mmichelson | 2007-06-27 19:47:08 -0300 (Wed, 27 Jun 2007) | 4 lines

Added ability to customize which buttons control forward, reverse, pause, and stop during message playback.
(closes issue 9474, reported and patched by jaroth with modifications by me)


................
r72330 | file | 2007-06-27 19:48:15 -0300 (Wed, 27 Jun 2007) | 18 lines

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

................
r72328 | file | 2007-06-27 18:45:49 -0400 (Wed, 27 Jun 2007) | 10 lines

Merged revisions 72327 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72327 | file | 2007-06-27 18:43:11 -0400 (Wed, 27 Jun 2007) | 2 lines

Fix issue where queue log events might be missing. (issue ASTERISK-7561 reported by mtryfoss)

........

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

................
r72332 | file | 2007-06-27 19:58:53 -0300 (Wed, 27 Jun 2007) | 10 lines

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

........
r72331 | file | 2007-06-27 18:58:02 -0400 (Wed, 27 Jun 2007) | 2 lines

Make payload IDs for iLBC/Speex match to our list. Since these are dynamic payloads the other side shouldn't care. (issue ASTERISK-9150 reported by irroot)

........

................
r72337 | bbryant | 2007-06-27 20:04:06 -0300 (Wed, 27 Jun 2007) | 18 lines

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

................
r72335 | bbryant | 2007-06-27 18:03:01 -0500 (Wed, 27 Jun 2007) | 10 lines

Merged revisions 72333 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72333 | bbryant | 2007-06-27 17:58:53 -0500 (Wed, 27 Jun 2007) | 2 lines

Reverted changes for earlier revisions 72259 to 72261. Issue ASTERISK-9371, ASTERISK-9710

........

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

................
r72354 | file | 2007-06-27 20:13:09 -0300 (Wed, 27 Jun 2007) | 2 lines

Add SIPREFERRINGCONTEXT and SIPREFERREDBYHDR variables when a transfer takes place. (issue ASTERISK-8148 reported by jcovert)

................
r72358 | file | 2007-06-27 20:14:39 -0300 (Wed, 27 Jun 2007) | 2 lines

Silly jingle...

................
r72382 | file | 2007-06-27 20:26:46 -0300 (Wed, 27 Jun 2007) | 18 lines

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

................
r72381 | file | 2007-06-27 19:25:12 -0400 (Wed, 27 Jun 2007) | 10 lines

Merged revisions 72378 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72378 | file | 2007-06-27 19:24:01 -0400 (Wed, 27 Jun 2007) | 2 lines

Update documentation to clarify variable usage with MixMonitor. (issue ASTERISK-9218 reported by netoguy)

........

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

................
r72384 | bbryant | 2007-06-27 20:30:31 -0300 (Wed, 27 Jun 2007) | 19 lines

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

................
r72383 | bbryant | 2007-06-27 18:29:14 -0500 (Wed, 27 Jun 2007) | 11 lines

Merged revisions 72373 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r72373 | bbryant | 2007-06-27 18:22:13 -0500 (Wed, 27 Jun 2007) | 3 lines

Reinstating patch. This actually fixes the problem, however I was running a development branch without it and mistakenly thought it wasn't fixed.
Fixes issue ASTERISK-9710, and ASTERISK-9371: 100