| Summary: | ASTERISK-28513: Should To: be rewritten when forwarding to a phone | ||||||
| Reporter: | Brian J. Murrell (brian_j_murrell) | Labels: | pjsip | ||||
| Date Opened: | 2019-08-23 21:46:27 | Date Closed: | |||||
| Priority: | Minor | Regression? | |||||
| Status: | Open/New | Components: | Resources/res_pjsip_messaging | ||||
| Versions: | 13.28.0 | Frequency of Occurrence | Constant | ||||
| Related Issues: |
| ||||||
| Environment: | Attachments: | ||||||
| Description: | I have run into a problem with {{MESSAGE}} s and the [Linphone Android client|https://github.com/BelledonneCommunications/linphone-android].
The [issue|https://github.com/BelledonneCommunications/linphone-android/issues/605] as described in their tracker is that when {{MESSAGE}} s come to the linphone client from the same sender, they can be "filed" into many different threads, rather than all in one chat/thread. This is because linphone separates chats based on both the From: and To: headers. As I am sure you know, the {{To:}} header of a client can vary wildly based on the IP address it's connecting from. This means that every time the IP address of the SIP client changes, a new chat for the same sender is created. But the problem is that Asterisk is setting the {{To:}} header of the {{MESSAGE}} to the {{user@ip_address}} of the remote SIP client and so this means that every time the IP address of the remote SIP client changes, a new To: header is created, and so is a new chat in the SIP client. Linphone defends this behaviour by insisting that the {{To:}} header value is a logical value of the recipient for a given domain and should always remain it's logical value no matter whether it's being forwarded on to a SIP client or not. So for example, if my Asterisk server is at pbx.example.com and somebody (my VOIP provider for example) send a {{MESSAGE}} to 555-555-1212@ip-address-of-my-asterisk, when my Asterisk server receives that message and then wants to forward it on to a SIP client, the To: should be {{To: _recipient_]@pbx.example.com}}, not {{To: _recipient_@ip-address-of-SIP-client}}. They quote [RFC 3261 section 8.1.1.2|https://tools.ietf.org/html/rfc3261#section-8.1.1.2] further in defending this behaviour. My reading of it doesn't leave me with much to argue against their defence. I don't see any way to make Asterisk (with PJSIP) to follow this behaviour. | ||||||
| Comments: | By: Asterisk Team (asteriskteam) 2019-08-23 21:46:27.887-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. 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. By: Joshua C. Colp (jcolp) 2019-08-24 06:34:05.401-0500 I think it depends upon the environment and situation under which it occurs. In a SIP proxy perspective when you are just forwarding the message, then I can certainly understand it. In the case of Asterisk you aren't forwarding - Asterisk is the initial target, which then creates a brand new message. This is why it does what it does. If you'd like to change it and add the ability to make it work as you want then that could be reviewed and included. Is this something you'd like to do? By: Brian J. Murrell (brian_j_murrell) 2019-08-24 08:15:48.536-0500 In the case of Asterisk simply being somewhere somebody sends a SIP message to have it delivered to somebody else, I'm not sure I'm seeing the distinction between a SIP proxy and Asterisk. Surely, every SIP client (as opposed to another SIP PBX) is utilising Asterisk as a SIP proxy in that regard. To me that's where the distinction is. And indeed, even when Asterisk is forwarding a call from a voip provider to a subscriber, that subscriber is really an entity on the Asterisk server, not his end SIP client. Asterisk is again, just proxying that SIP message to the remote SIP client, but logically, the recipient is still really the user's presence on the Asterisk server. So ultimately, I think the behaviour depends on Asterisk knowing what it's sending the SIP message to. In the case of another SIP server, like another Asterisk, yes, I agree that the the recipient should be rewritten to be at the remote domain. But in the case of a simple SIP client just being the point of presence for the user's account on that Asterisk server, no rewriting should occur. As for coding up this change, I'm afraid I'm not at all familiar with the Asterisk code base. Have never cracked it open even. Not sure I'd know the first place to look to make this kind of change. By: Joshua C. Colp (jcolp) 2019-08-24 12:03:39.668-0500 PJSIP messaging support is contained completely within a module[1]. If you don't work on it, there is no time frame on when this would get looked into. Messaging is a seldom used thing and does not see much activity. [1] https://github.com/asterisk/asterisk/blob/master/res/res_pjsip_messaging.c By: Joshua C. Colp (jcolp) 2019-08-24 12:06:08.363-0500 From looking at the code, though, it does appear there is some logic for updating the To based on the value within the message data itself (update_to) so it would likely not be difficult to expand it further. By: Joshua C. Colp (jcolp) 2019-08-27 05:01:43.608-0500 Based on my last comment do you still not want to work on this improvement? By: Asterisk Team (asteriskteam) 2019-09-10 12:00:04.066-0500 Suspended due to lack of activity. This issue will be automatically re-opened if the reporter posts a comment. If you are not the reporter and would like this re-opened please create a new issue instead. If the new issue is related to this one a link will be created during the triage process. Further information on issue tracker usage can be found in the Asterisk Issue Guidlines [1]. [1] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines By: Brian J. Murrell (brian_j_murrell) 2019-09-26 11:23:40.915-0500 {quote} Messaging is a seldom used thing and does not see much activity. {quote} Well, I think the Linphone developers are maintaining that the general format of the {{To:}} header in all SIP packets (i.e. INVITE, etc.) coming from Asterisk are wrong and should not be getting re-written to the IP address of the client. {quote} From looking at the code, though, it does appear there is some logic for updating the To based on the value within the message data itself (update_to) so it would likely not be difficult to expand it further. {quote} To be clear, it looks like that {{update_to()}} is only updating the _display_ portion of the {{To:}} header, correct? Are you suggesting that that function could also be used to maintain the "logical" (real portion, not display portion) address of the user? Is the "logical" address of the user known at that point or has the {{To:}} header already been formed into the remote _user_@_remote-ip-address_? By: Asterisk Team (asteriskteam) 2019-09-26 11:23:41.517-0500 This issue has been reopened as a result of your commenting on it as the reporter. It will be triaged once again as applicable. By: Joshua C. Colp (jcolp) 2019-09-26 11:47:32.112-0500 The header at that point is already formed, but can be updated based on the contents of the text message (such as the To from it). By: Brian J. Murrell (brian_j_murrell) 2019-09-28 17:01:47.840-0500 I have a PoC patch that implements this. Currently it just has the name of the Asterisk server hard coded in it: {noformat} pj_str_t my_host = pj_str("example.com"); {noformat} Of course I could use something like {{gethostname()}} to get the name of the machine Asterisk is running on, but is that the most correct way to get it's name in this context or is there some internal value I should be referencing? By: Brian J. Murrell (brian_j_murrell) 2019-09-28 17:07:04.102-0500 Hrm. Actually, now that I think about it, {{gethostname()}} would not return the FQDN of the Asterisk service (i.e. the domain name that people use to connect to it, which is a CNAME), but rather the machine itself. Any thoughts? By: George Joseph (gjoseph) 2019-10-01 05:17:59.217-0500 I think the best way to handle this would be an option on either the endpoint (if you think you'll need to modify the behavior on an endpoint-by-endpoint basis) or on the global objects. Maybe {{outgoing_message_to_domain}} or something along those lines. Do you think this behavior should also apply to SIP message types other than MESSAGE? By: Brian J. Murrell (brian_j_murrell) 2019-10-01 09:43:42.944-0500 {quote} I think the best way to handle this would be an option on either the endpoint (if you think you'll need to modify the behavior on an endpoint-by-endpoint basis) {quote} I don't see this as being endpoint specific. In all honesty and straightforwardness, this doesn't feel like it should \[have to\] be a configurable item. The (at least, default) behaviour should be the behaviour I have in my PoC, which is to maintain the domain name in the outgoing message's {{To:}} header as it was in the incoming message. To illustrate with an example, if an Asterisk server is named (i.e. in DNS) _pbx.example.com_, SIP clients wanting to send a message to alice, registered on that Asterisk server, would send their message {{To: alice@pbx.example.com}}. Those messages, when sent from the Asterisk server to the SIP client alice registered from, should still be {{To: alice@pbx.example.com}}. That's what I have working in my PoC. It seems that an Asterisk server, ought to know (as a global attribute) what (canonical) domain name people are using to refer to it and that that information should be available in {{res/res_pjsip_messaging.c}} (as well as everywhere else in SIP message handling code). It seems odd to me that this is not already the case. Although I suspect it is the case. I don't recall anywhere in the myriad of configuration files where it can be told that. Of course, an Asterisk server could take steps to try to figure that out. At a pjsip level for example, if a transport is bound to an IP address, with any hope of a proper DNS configuration for that IP address: {noformat} # host 10.75.22.8 8.22.75.10.in-addr.arpa domain name pointer pbx.example.com. {noformat} {quote} Do you think this behavior should also apply to SIP message types other than MESSAGE? {quote} I think that is what the Linphone developers are suggesting. But frankly, for say, {{INVITE}} messages this doesn't seem to be as critical as it is for {{MESSAGE}} so I'm inclined to not worry about it as much. By: George Joseph (gjoseph) 2019-10-01 10:37:14.735-0500 There could be multiple reasons for making it configurable... multi-tenant setups, complex multi-homed setups, etc. I'd also be concerned about changing the current behavior out from underneath existing users. We do have "systemname" in asterisk.conf but that doesn't help if you want the domain to be different for internal and external calls. I was thinking about an option on transport to set the to_domain but then realized that we already have from_domain on endpoint so it would seem that the logical place for to_domain would be there rather than transport or global. Thoughts? By: Brian J. Murrell (brian_j_murrell) 2019-10-01 11:54:01.926-0500 Yes, I thought I made it pretty clear that I was not precluding it from being a definable option for the cases where the reasonable default was not sufficient, but just that the default behaviour of not setting it should be reasonable which IMHO is that the "name" of the Asterisk server/service is used. {quote} I'd also be concerned about changing the current behavior out from underneath existing users {quote} Unless you accept the argument from the Linphone developers that according to RFC 3261 section 8.1.1.2 it is an outright bug to be altering the recipient address on handing the message over. Surely we cannot favour maintaining buggy behaviour simply in an effort to not change (buggy) behaviour. {quote} We do have "systemname" in asterisk.conf {quote} Hrm. That might be a useful option of last resort. But the more I think about it, using a {{bind=}} address of the transport on which the endpoint will be reached to do the previously demonstrated reverse address lookup might be the preferred method, if a {{bind=}} is specified. {quote} but that doesn't help if you want the domain to be different for internal and external calls. {quote} I suppose. Is/would that be a common use-case? I'm not even sure what that configuration looks like in terms of contacts, endpoints, aors, transports, etc. to even try to wrap my head around it. {quote} I was thinking about an option on transport to set the to_domain but then realized that we already have from_domain on endpoint so it would seem that the logical place for to_domain would be there rather than transport or global. {quote} I don't think there is any particular reason to try to think of {{from_domain}} and (a proposed) {{to_domain}} at the same level of granularity in configuration. Setting a _from_ type attribute naturally factors down to "individuals" (subjects of the configuration of an Asterisk) but I think of "to" attributes (subjects of other systems) as being less granular. By: Brian J. Murrell (brian_j_murrell) 2019-10-11 06:52:16.887-0500 I tried this patch (instead of my hard-coded system name): {noformat} diff --git a/res/res_pjsip_messaging.c b/res/res_pjsip_messaging.c --- a/res/res_pjsip_messaging.c +++ b/res/res_pjsip_messaging.c @@ -43,6 +43,7 @@ #include "asterisk/res_pjsip.h" #include "asterisk/res_pjsip_session.h" #include "asterisk/taskprocessor.h" +#include "asterisk/paths.h" const pjsip_method pjsip_message_method = {PJSIP_OTHER_METHOD, {"MESSAGE", 7} }; @@ -230,10 +230,14 @@ parsed_name_addr = (pjsip_name_addr *) pjsip_parse_uri(tdata->pool, to, strlen(to), PJSIP_PARSE_URI_AS_NAMEADDR); if (parsed_name_addr) { - if (pj_strlen(&parsed_name_addr->display)) { - pjsip_name_addr *name_addr = - (pjsip_name_addr *) PJSIP_MSG_TO_HDR(tdata->msg)->uri; + pjsip_sip_uri *uri; + pjsip_name_addr *name_addr = + (pjsip_name_addr *) PJSIP_MSG_TO_HDR(tdata->msg)->uri; + uri = pjsip_uri_get_uri(name_addr); + pj_str_t my_host = pj_str(ast_config_AST_SYSTEM_NAME); + pj_strdup(tdata->pool, &uri->host, &my_host); + if (pj_strlen(&parsed_name_addr->display)) { pj_strdup(tdata->pool, &name_addr->display, &parsed_name_addr->display); } } {noformat} but that resulted in a {{To:}} with the domainname part empty. Can I not use {{ast_config_AST_SYSTEM_NAME}} here like this? By: Brian J. Murrell (brian_j_murrell) 2019-10-11 07:25:52.230-0500 Hrm. Strangely enough this does now seem to be working. By: Brian J. Murrell (brian_j_murrell) 2019-10-11 11:05:06.031-0500 Whether {{ast_config_AST_SYSTEM_NAME}} has a value seems to be intermittent. By: Brian J. Murrell (brian_j_murrell) 2019-10-14 07:46:20.673-0500 Putting the intermittency of {{ast_config_AST_SYSTEM_NAME}} having a value, how does the patch above look? Assuming it looks good, what's the process of submitting it to be included in the next release of Asterisk? By: Brian J. Murrell (brian_j_murrell) 2019-10-16 07:58:56.967-0500 Anyone? By: Joshua C. Colp (jcolp) 2019-10-16 08:17:50.143-0500 System name is not guaranteed to be non-empty, it's up to users to actually configure it or to enable the autosystemname functionality. As for the patch I think it seems fine, and as for submitting a patch it is documented on the wiki[1]. [1] https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process By: Brian J. Murrell (brian_j_murrell) 2019-10-16 08:32:46.146-0500 > System name is not guaranteed to be non-empty Ahhh. I did account for that in my latest patch but didn't seem to attach it here. Here it is: {noformat} diff --git a/res/res_pjsip_messaging.c b/res/res_pjsip_messaging.c --- a/res/res_pjsip_messaging.c +++ b/res/res_pjsip_messaging.c @@ -43,6 +43,7 @@ #include "asterisk/res_pjsip.h" #include "asterisk/res_pjsip_session.h" #include "asterisk/taskprocessor.h" +#include "asterisk/paths.h" const pjsip_method pjsip_message_method = {PJSIP_OTHER_METHOD, {"MESSAGE", 7} }; @@ -230,10 +230,17 @@ parsed_name_addr = (pjsip_name_addr *) pjsip_parse_uri(tdata->pool, to, strlen(to), PJSIP_PARSE_URI_AS_NAMEADDR); if (parsed_name_addr) { - if (pj_strlen(&parsed_name_addr->display)) { - pjsip_name_addr *name_addr = - (pjsip_name_addr *) PJSIP_MSG_TO_HDR(tdata->msg)->uri; + pjsip_sip_uri *uri; + pjsip_name_addr *name_addr = + (pjsip_name_addr *) PJSIP_MSG_TO_HDR(tdata->msg)->uri; + uri = pjsip_uri_get_uri(name_addr); + if (!ast_strlen_zero(ast_config_AST_SYSTEM_NAME)) { + pj_str_t my_host = pj_str(ast_config_AST_SYSTEM_NAME); + pj_strdup(tdata->pool, &uri->host, &my_host); + } else + ast_log(LOG_WARNING, "Unable to rewrite MESSAGE to systemname because systemname (%s) is empty\n", ast_config_AST_SYSTEM_NAME); + if (pj_strlen(&parsed_name_addr->display)) { pj_strdup(tdata->pool, &name_addr->display, &parsed_name_addr->display); } } {noformat} I do get the following warnings when building with this patch applied: {noformat} [CC] res_pjsip_messaging.c -> res_pjsip_messaging.o res_pjsip_messaging.c: In function 'update_to': res_pjsip_messaging.c:239:4: warning: passing argument 1 of 'pj_str' discards 'const' qualifier from pointer target type [enabled by default] pj_str_t my_host = pj_str(ast_config_AST_SYSTEM_NAME); ^ In file included from /home/brian/rpm/BUILD/asterisk-13.29.0/third-party/pjproject/source/pjsip/include/pjsip/sip_transport_tls.h:31:0, from /home/brian/rpm/BUILD/asterisk-13.29.0/third-party/pjproject/source/pjsip/include/pjsip.h:45, from /home/brian/rpm/BUILD/asterisk-13.29.0/third-party/pjproject/source/pjsip/include/pjsua-lib/pjsua.h:30, from res_pjsip_messaging.c:38: /home/brian/rpm/BUILD/asterisk-13.29.0/third-party/pjproject/source/pjlib/include/pj/string.h:79:20: note: expected 'char *' but argument is of type 'const char *' PJ_IDECL(pj_str_t) pj_str(char *str); ^ {noformat} Casting away the {{const}} doesn't seem like the correct solution though. Is the correct solution to take a non-const copy of {{ast_config_AST_SYSTEM_NAME}} to pass into {{pj_str()}} or is there a different way to handle this situation with {{pj_str_t}}? By: Joshua C. Colp (jcolp) 2019-10-16 08:36:21.551-0500 You would want to use the pj_strdup2 function[1] instead and not deal with pj_str. [1] https://www.pjsip.org/pjlib/docs/html/group__PJ__PSTR.htm#ga96cdcaa39650b7f36fd850f53f4ebebf By: Brian J. Murrell (brian_j_murrell) 2019-10-16 09:05:44.874-0500 Ah, yes. That's the ticket! Thanks much for your guidance! By: y (y) 2020-10-07 22:27:27.401-0500 Hi, how this issue going? can it be solved in the master branch? I proposed another solution which does NOT affect existing configurations: https://issues.asterisk.org/jira/browse/ASTERISK-28928 and the patch is (based on asterisk 16): <patch removed due to being inline and unlicensed> and in extension.conf: same => n,MessageSend(${CUT(MESSAGE(to),@,1)}@mydomain.com,${MESSAGE(from)}) By: Brian J. Murrell (brian_j_murrell) 2021-01-23 14:16:45.273-0600 [~y] I like your approach. I wonder though, in your patch above, what is the purpose of {{pj_to}}? By: y (y) 2021-03-16 03:15:35.896-0500 Hi, all I can't log in to openid.asterisk.org with the same account here, so I paste my review here, could someone help to copy it to https://gerrit.asterisk.org/c/asterisk/+/15597 ? Thanks. @Joshua Colp For your first comment, just to keep in line with FROM, as the format of $MESSAGE_FROM and $MESSAGE_TO generally looks like an email address, they should have the same format handler, and I believe we have tested a lot on update_from, although it is a little bit code redundant currently. @ George Joseph This change does NOT impact chan_pjsip.c:sendtext() and main/message.c, as they are handling message from, body, filtering content-type, and lots of C structure converting happens there, from standard SIP format to internal ast msg format. BTW, out of this issue topic, to allow content-type text/* and application/* instead of text/plain only, in another word, to replace "code = check_content_type(rdata);" with "code = check_content_type_in_dialog(rdata);", I personally developed some patches, making asterisk available forwarding some rich text like XML. However, I don't know how some simple SIP client handle text/* and application/* when received such this SIP message, I haven't create an enhancement issue yet. @Brian J. Murrell Could you please add my contact info into Author filed in the patch? Yang Chen <yang.chen@linuxe.org> Thank you. By: Joshua C. Colp (jcolp) 2021-03-16 04:09:59.072-0500 Have you signed a license agreement? If not, then you will not have access to Gerrit. I've also removed your patch as it is unlicensed code. By: y (y) 2021-03-18 23:03:51.897-0500 The status of my License sign page says: You already have a license that is approved or pending review. I am considering the following situation: Can we forward the message to multiple registered devices like Dial(${PJSIP_DIAL_CONTACTS(${EXTEN})}) does? and we set "To" in update_to according to MESSAGE(to) rather than the first parameter of MessageSend. The question is MessageSend only accepting one destination, and the implementation inside asterisk ignores MESSAGE(to), the enhancement workloads might be too much. By: y (y) 2021-04-01 06:36:38.953-0500 Yet another proposal is appending x-todomain=sip.mydoamin.org to the first parameter of MessageSend, but the usage is a little bit tricky since we need a full URI retrieved from PJSIP_DIAL_CONTACTS(${EXTEN}) and the change of dialplan looks like: -same => n,MessageSend(${CUT(MESSAGE(to),@,1)}@mydomain.com,${MESSAGE(from)) +same = n,Set(ToContacts=${PJSIP_DIAL_CONTACTS(${EXTEN})}) +same = n,While($["${SET(ToContact=${SHIFT(ToContacts,&):6})}" != ""]) +same = n,Set(TOURI=pjsip:${ToContact}\;x-to_domain=mydomain.com) +same => n,MessageSend(${TOURI},${MESSAGE(from)}) +same = n,EndWhile By: y (y) 2021-04-11 21:33:05.176-0500 This new implementation code located here: https://gerrit.asterisk.org/c/asterisk/+/15727 By: Brian J. Murrell (brian_j_murrell) 2021-04-12 08:21:09.164-0500 What benefits does the https://gerrit.asterisk.org/c/asterisk/+/15727 implementation have vs. the https://gerrit.asterisk.org/c/asterisk/+/15597 implementation? I suppose they are not mutually exclusive. By: y (y) 2021-04-12 11:48:32.956-0500 The primary change from new one, 15727, enable us recieving messages on every registered devices with same extension(e.g. you login your sip account on both PC and mobile), if we set max_contacts greater than one in the AOR section. Also avoid a possible bug which adressed from george in the last post from 15597. | ||||||