Summary:	ASTERISK-29370: chan_sip does not recognize application/hook-flash
Reporter:	N A (InterLinked)	Labels:	patch
Date Opened:	2021-03-26 08:33:59	Date Closed:	2021-05-17 08:43:04
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:	18.2.0	Frequency of Occurrence	Constant
Related Issues:
Environment:	Debian 10	Attachments:	( 0) flashdiff.patch
Description:	Every time I send a hook flash event on one of my ATAs (SIP INFO), it sends an application/hook-flash to Asterisk that would always result in two WARNINGs about it being unrecognized. I noticed there was code in chan_sip to handle the 16 flash event code, so I tried debugging to see if I could fix that. Actually, it seems the problem here is that the headers for the event are not coming in correctly, and I'm not entirely sure why. I added some lines to print out some of the information it's getting, and it's definitely not right: I don't know how much this is actually related to the flashing issue, but in any case, there seems to be something rather off with the sip_get_header function or how it gets used. Here is the code I added - notice that Content-Type seems to be getting read as Content-Length, since they are the same, and match up with the info in the sip debug: const char *c2 = sip_get_header(req, "Content-Length"); .... ast_log(LOG_WARNING, "Event: %d, Content Type: %s\n", event, c); //nav // handle SIP INFO flash events from Grandstream, previously e.g. handle_request_info: Unable to parse INFO message from 85538d91-ae288b51@192.168.10.105. Content // always turns out that: buffer is empty, c is 32731, and event is 11 on Linksys ATA and 9 on Grandstream if (!strcasecmp(c, "32731" && (event == 9 || event == 11))) { /* send a FLASH event */ struct ast_frame f = { AST_FRAME_CONTROL, { AST_CONTROL_FLASH, } }; ast_queue_frame(p->owner, &f); //if (sipdebug) { ast_verbose("* DTMF-relay event received: FLASH\n"); //} transmit_response(p, "200 OK", req); return; } Ignore the weird if conditional, that was just for testing and isn't right at all. Also, notice that the "event" is the content length and that the "content type" is some number around 32,000 that keeps incrementing slightly. It increases every time Asterisk restarts. Not sure what this is, but I don't think it's the content type. This leads me to believe that Asterisk is getting all the wrong headers for this SIP info packet. Here is a SIP debug that demonstrates the issue: [Mar 26 09:20:50] <--- SIP read from TLS:PUBLIC-IP-REDACTED:37654 ---> [Mar 26 09:20:50] INFO sip:01@SERVER-IP-REDACTED:SERVER-PORT-REDACTED;transport=tls SIP/2.0 [Mar 26 09:20:50] Via: SIP/2.0/TLS 192.168.10.106:38967;branch=z9hG4bK170601491;rport [Mar 26 09:20:50] From: "ATAx4" <sip:ATAx4@SERVER-IP-REDACTED:SERVER-PORT-REDACTED>;tag=2050706533 [Mar 26 09:20:50] To: <sip:01@SERVER-IP-REDACTED:SERVER-PORT-REDACTED>;tag=as16838c32 [Mar 26 09:20:50] Call-ID: 358973344-38967-17@BJC.BGI.BA.BAG [Mar 26 09:20:50] CSeq: 132 INFO [Mar 26 09:20:50] Contact: <sip:ATAx4@192.168.10.106:38967;transport=tls> [Mar 26 09:20:50] Max-Forwards: 70 [Mar 26 09:20:50] Supported: replaces, path, timer, eventlist [Mar 26 09:20:50] User-Agent: Grandstream HT704 1.0.10.3 [Mar 26 09:20:50] Allow: INVITE, ACK, OPTIONS, CANCEL, BYE, SUBSCRIBE, NOTIFY, INFO, REFER, UPDATE [Mar 26 09:20:50] Content-Type: application/hook-flash [Mar 26 09:20:50] Content-Length: 9 [Mar 26 09:20:50] [Mar 26 09:20:50] signal=hf [Mar 26 09:20:50] <-------------> [Mar 26 09:20:50] --- (13 headers 1 lines) --- [Mar 26 09:20:50] Receiving INFO! [2021-03-26 09:20:50] WARNING[13835][C-00000003]: chan_sip.c:22804 handle_request_info: Event: 0, Content Type: 9 Content Length: 9 [2021-03-26 09:20:50] WARNING[13835][C-00000003]: chan_sip.c:22821 handle_request_info: Unable to parse INFO message from 358973344-38967-17@BJC.BGI.BA.BAG. Content [Mar 26 09:22:27] <--- SIP read from TLS:PUBLIC-IP-REDACTED:5072 ---> [Mar 26 09:22:27] INFO sip:01@SERVER-IP-REDACTED:SERVER-PORT-REDACTED;transport=tls SIP/2.0 [Mar 26 09:22:27] Via: SIP/2.0/TLS 192.168.10.105:5072;branch=z9hG4bK-17e5ecf8 [Mar 26 09:22:27] From: "ATAxLB1" <sip:ATAxLB1@SERVER-IP-REDACTED>;tag=66280aa944f70150o0 [Mar 26 09:22:27] To: <sip:01@SERVER-IP-REDACTED>;tag=as598b9104 [Mar 26 09:22:27] Call-ID: 7451d45d-33d6a32c@192.168.10.105 [Mar 26 09:22:27] CSeq: 102 INFO [Mar 26 09:22:27] Max-Forwards: 70 [Mar 26 09:22:27] User-Agent: Linksys/SPA2102-5.2.13(004) [Mar 26 09:22:27] Content-Length: 11 [Mar 26 09:22:27] Content-Type: application/hook-flash [Mar 26 09:22:27] [Mar 26 09:22:27] Signal=hf [Mar 26 09:22:27] <-------------> [Mar 26 09:22:27] --- (10 headers 1 lines) --- [Mar 26 09:22:27] Receiving INFO! [2021-03-26 09:22:27] WARNING[13774][C-00000004]: chan_sip.c:22804 handle_request_info: Event: 0, Content Type: 11 Content Length: 11 [2021-03-26 09:22:27] WARNING[13774][C-00000004]: chan_sip.c:22821 handle_request_info: Unable to parse INFO message from 7451d45d-33d6a32c@192.168.10.105. Content In theory, I could try to work with what I'm getting here and patch chan_sip so it recognizes these as a hook flash event, but the variables are all wrong so I think that would be a bit nonsensical since this is indicative of some other larger issue. I am not familiar with the rest of chan_sip, so I am reporting this, and I would like to see if I can get the hook flashes recognized once I have the proper headers to work with.
Comments:	By: Asterisk Team (asteriskteam) 2021-03-26 08:34:02.313-0500 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur. Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/]. By: Joshua C. Colp (jcolp) 2021-03-26 08:41:12.245-0500 "application/hook-flash" is not a supported Content-Type in chan_sip, or in chan_pjsip. The code does not exist to support it. The only supported flash mechanism is using the normal DTMF content type such as "application/dtmf-relay" or "application/dtmf" with a signal of '!'. There is no higher level issue besides that. Additionally these just recognize flash and pass it through to the remote side if present. If not present then they are essentially ignored. If you are adding code and needing help with that, then the asterisk-dev mailing list would be the best place as you've already posted to. Note though that chan_sip is community supported and receives no other changes, and it's really up to whether someone takes an interest in what you're doing or not as to whether they would respond. By: N A (InterLinked) 2021-03-26 08:46:10.479-0500 I realize hook-flash is not supported, and I think you told me that before. Unfortunately, all my ATAs send application/hook-flash, not application/dtmf 16 for some reason. It should be trivial to add logic to support this, and if Digium is not going to do it, then I will and have no problem with adding the feature. However, right now the problem is that the headers are not even coming in right. I can see the correct headers in a SIP debug, but when I print them out, I am getting nonsensical values (different headers). Once this works right, I could simply add an if (application/hookflash) { do the same thing as the 16 flash code }. So, are you saying that sip_get_header is not currently capable of parsing "application/hook-flash", and that would need to be updated to add this? That is, sip_get_header is not capable of getting any arbitrary header sent, it needs to be part of some "whitelist"? By: N A (InterLinked) 2021-03-26 08:52:22.310-0500 Actually, I'm not sure I follow. Here are the relevant functions: static const char *__get_header(const struct sip_request *req, const char *name, int *start) { /* * Technically you can place arbitrary whitespace both before and after the ':' in * a header, although RFC3261 clearly says you shouldn't before, and place just * one afterwards.  If you shouldn't do it, what absolute idiot decided it was * a good idea to say you can do it, and if you can do it, why in the hell would. * you say you shouldn't. */ const char *sname = find_alias(name, NULL); int x, len = strlen(name), slen = (sname ? 1 : 0); for (x = *start; x < req->headers; x++) { const char *header = REQ_OFFSET_TO_STR(req, header[x]); int smatch = 0, match = !strncasecmp(header, name, len); if (slen) { smatch = !strncasecmp(header, sname, slen); } if (match || smatch) { /* skip name */ const char *r = header + (match ? len : slen ); /* HCOLON has optional SP/HTAB; skip past those */ while (*r == ' ' || *r == '\t') { ++r; } if (*r == ':') { *start = x+1; return ast_skip_blanks(r+1); } } } /* Don't return NULL, so sip_get_header is always a valid pointer */ return ""; } /*! \brief Get header from SIP request \return Always return something, so don't check for NULL because it won't happen :-) */ const char *sip_get_header(const struct sip_request *req, const char *name) { int start = 0; return __get_header(req, name, &start); } I don't see anything here that would limit them to not correctly returning "application/hook-flash" if I were to call sip_get_header. By: Joshua C. Colp (jcolp) 2021-03-26 08:52:38.887-0500 I don't remember much if any of the chan_sip implementation so I can't answer that off the top of my head. Sangoma as a company does not spend any time on chan_sip, it is community supported and up to others as to whether they want to look/invest time in such a thing. Additionally the chan_sip module is currently scheduled for potential removal as of Asterisk 21. By: N A (InterLinked) 2021-03-26 09:39:44.254-0500 Oddly enough, it seems by the bottom of the block, some of the variables are overwritten (can't see how though), but at the top of the block, it's A-OK. So, checking for application/hook-flash at the very beginning does not impose a problem. I modified the code and it works as expected now: [Mar 26 10:29:54] * DTMF-relay event received: FLASH [2021-03-26 10:29:54] WARNING[2148][C-0000000c]: file.c:1709 waitstream_core: Unexpected control subclass '9' This is doing the same thing as it would when it gets a "16" DTMF. > Additionally these just recognize flash and pass it through to the remote side if present. If not present then they are essentially ignored. By remote side, do you mean the channel that it is connected to? Does this include Local channels? As currently implemented, is this an event that is actionable (that AMI or something could see and then act on) or just gets passively passed through to a compatible channel? Now that it is "detecting" the hook flashes right, what I would like to do is be able to act on these. Essentially, I need to trigger some dialplan execution. What would be the best way to pick up on this event be - AMI? I don't need to do anything on the channel the flash came from, just detect it and do stuff out of band to manipulate channels. I could write it out to a call file which would be processed instantly, but most people will probably ignore these so I don't want to force an action in that case, just have the event "available" so some entity could act on it if desired - for instance, to trigger the Flash() application on a DAHDI channel, as one example. In short, I already have enough to submit a patch for chan_sip.c, but if it's not already actionable, I would like to include that in my patch as well. By: Joshua C. Colp (jcolp) 2021-03-26 09:42:23.217-0500 They just get passed through. I don't know if such an event is raised in AMI regarding it. By: N A (InterLinked) 2021-03-26 10:05:03.915-0500 But "passed through" to what, exactly? If the SIP device is connected to a local channel, does it get passed through that and how far? Would the Local channel keep propagating this event? If it doesn't currently raise an event in AMI, would that be the best way to do it? I'd like the event to be generic and not implementation-dependent. I haven't used AMI before, though it seems I could raise a channel event, as opposed to a global one. I'm having difficulty finding an example of this though; no "ami" or "manager" in chan_pjsip and a few obscure ones in chan_sip. I did also notice this: case AST_CONTROL_FLASH: /* We don't currently handle AST_CONTROL_FLASH here, but it is expected, so we don't need to warn either. */ res = -1; break; This is in sip_indicate, though, and the user channel is not what needs to be notified per se so I don't think AST_CONTROL_FLASH should be handled there either. By: Joshua C. Colp (jcolp) 2021-03-26 10:13:12.010-0500 The Local channel would continue propagating it, just like DTMF, through to the remote party if bridged. If not bridged then it would be dropped. If bridged then the remote party channel driver is responsible for doing anything (or nothing) with it. This is the way frames like these flow. I don't have example code for raising such an event or know the best place to put it for flash. Flash support in Asterisk is seldom used, little touched, little understood. DTMF would be the closest thing to use as an example. That's done in manager_channels.c, based on stasis messages published to the channel. You are very much in territory not touched by many. By: N A (InterLinked) 2021-03-26 10:51:01.716-0500 Ah, so the Local channel will propagate it, but be unable to inherently do anything with it? That is, keep propagating until it exits Asterisk to some other device, if it does at all, but be unable to do anything with it itself? Something promising seemed to be something like:  astman_send_ack(s, m, "Hook Flash Received"); I can see astman_send_err being used in chan_sip.c, but I need an s and an m to pass in as arguments, and it seems these only are present in functions intended to be called from AMI (e.g. AMI to a SIP peer). So, the opposite of what I'm looking for. This from manager_channels.c appears to be similar to exactly what I need: manager_event(EVENT_FLAG_DTMF, "DTMFBegin", "%s" "Digit: %s\r\n" "Direction: %s\r\n", ast_str_buffer(channel_event_string), digit, direction); What's funny is the comment here is: <para>DTMF digit received or transmitted (0-9, A-E, # or *</para>. Not sure what "E" could be if it wasn't "Flash". Typo? Not sure what to think. However, only ast_manager_event_blob and ast_manager_event_blob_create are present in chan_sip.c, related to a static struct that's globally defined. chan_sip #includes manager.h, which does #define manager_event(), so it looks like I might be able to call it directly. I'm assuming the manager library takes care of including the channel the event was raised on for me. manager.h defines EVENT_FLAG_DTMF as (1 << 8): #define EVENT_FLAG_SYSTEM           (1 << 0) /* System events such as module load/unload */ #define EVENT_FLAG_CALL             (1 << 1) /* Call event, such as state change, etc */ #define EVENT_FLAG_LOG              (1 << 2) /* Log events */ #define EVENT_FLAG_VERBOSE          (1 << 3) /* Verbose messages */ #define EVENT_FLAG_COMMAND          (1 << 4) /* Ability to read/set commands */ #define EVENT_FLAG_AGENT            (1 << 5) /* Ability to read/set agent info */ #define EVENT_FLAG_USER             (1 << 6) /* Ability to read/set user info */ #define EVENT_FLAG_CONFIG           (1 << 7) /* Ability to modify configurations */ #define EVENT_FLAG_DTMF             (1 << 8) /* Ability to read DTMF events */ #define EVENT_FLAG_REPORTING        (1 << 9) /* Reporting events such as rtcp sent */ #define EVENT_FLAG_CDR              (1 << 10) /* CDR events */ #define EVENT_FLAG_DIALPLAN         (1 << 11) /* Dialplan events (VarSet, NewExten) */ #define EVENT_FLAG_ORIGINATE        (1 << 12) /* Originate a call to an extension */ #define EVENT_FLAG_AGI              (1 << 13) /* AGI events */ #define EVENT_FLAG_HOOKRESPONSE     (1 << 14) /* Hook Response */ #define EVENT_FLAG_CC               (1 << 15) /* Call Completion events */ #define EVENT_FLAG_AOC              (1 << 16) /* Advice Of Charge events */ #define EVENT_FLAG_TEST             (1 << 17) /* Test event used to signal the Asterisk Test Suite */ #define EVENT_FLAG_SECURITY         (1 << 18) /* Security Message as AMI Event */ /*XXX Why shifted by 30? XXX */ #define EVENT_FLAG_MESSAGE          (1 << 30) /* MESSAGE events. */ So, I'm assuming I would need something like #define EVENT_FLAG_FLASH (1 << 19) /* Hook flash as AMI event */ And then in manager.c, add: { EVENT_FLAG_FLASH, "flash" }, I think I have a good idea of how to proceed, it seems I will need to patch 6 or 7 files in total for this. Only a couple lines are actually needed for chan_sip, so I may submit that separately. Here is the odd thing: Linksys: [2021-03-26 14:37:27] WARNING[2121][C-00000182]: chan_sip.c:22639 handle_request_info: Event: 10, Content Type: application/hook-flash Content Length: 11 Agent: Linksys/SPA2102-5.2.13(004) [Mar 26 14:37:27] * DTMF-relay event received: FLASH [2021-03-26 14:37:27] WARNING[24053][C-00000182]: file.c:1709 waitstream_core: Unexpected control subclass '9' Grandstream: [2021-03-26 14:37:11] WARNING[2057][C-00000181]: chan_sip.c:22639 handle_request_info: Event: 10, Content Type: application/hook-flash Content Length: 9 Agent: Grandstream HT704 1.0.10.3 [Mar 26 14:37:11] * DTMF-relay event received: FLASH [2021-03-26 14:37:11] WARNING[24037][C-00000181]: channel.c:3322 ast_waitfordigit_full: Unexpected control subclass '9' On a Linksys ATA, the subclass 9 error is coming from file.c. On a Grandstream, it's coming from channel.c. I looked around and there are very similar things in each file. However, only channel.c includes the stasis and ast_channel_publish_blob code. Both files include the other's header file, so I don't know what to think. It seems like nothing would stop me from using functions define in channel.c from file.c. I could get the GS working by mirroring the DTMF function calls, from channel.c to stasis_channels.h, to stasis_channels.c, to manager_channels.c, to manager_channels.c and stasis.c. Incidentally, the DTMFBegin and DTMFEnd manager events are also missing from my Asterisk somehow, even though the Asterisk 18 documentation includes this. The source code has it, it just doesn't show up with "manager show events". There are some other discrepancies, but I noticed this one more immediately. I'm unable to do manager show event dtmfend/dtmfbegin as well. By: N A (InterLinked) 2021-03-26 14:42:15.083-0500 Attached is the patch that successfully resolves issue 29370, bringing application/hook-flash support to chan_sip and treating these events the same way as SIP INFO 16 DTMF events. No further handling is provided in chan_sip. By: N A (InterLinked) 2021-04-02 14:50:49.519-0500 License agreement accepted, so patch can be processed. By: Friendly Automation (friendly-automation) 2021-05-17 08:43:04.988-0500 Change 15886 merged by Friendly Automation: chan_sip: Expand hook flash recognition. [https://gerrit.asterisk.org/c/asterisk/+/15886|https://gerrit.asterisk.org/c/asterisk/+/15886] By: Friendly Automation (friendly-automation) 2021-05-17 08:55:41.079-0500 Change 15874 merged by Joshua Colp: chan_sip: Expand hook flash recognition. [https://gerrit.asterisk.org/c/asterisk/+/15874|https://gerrit.asterisk.org/c/asterisk/+/15874] By: Friendly Automation (friendly-automation) 2021-05-17 08:56:02.070-0500 Change 15875 merged by Joshua Colp: chan_sip: Expand hook flash recognition. [https://gerrit.asterisk.org/c/asterisk/+/15875|https://gerrit.asterisk.org/c/asterisk/+/15875]