res_pjsip / res_pjsip_multihomed: Use the correct transport and addressing informatio...
authorJoshua Colp <jcolp@digium.com>
Mon, 19 Jan 2015 13:19:11 +0000 (13:19 +0000)
committerJoshua Colp <jcolp@digium.com>
Mon, 19 Jan 2015 13:19:11 +0000 (13:19 +0000)
The first thing this patch fixes is UAS dialogs. Previously if a transport was
configured on an endpoint and an inbound session was created there was no guarantee
that requests sent on the dialog would use the correct transport and address
information. This has now been fixed so an explicitly configured transport
is taken into account.

The second thing this patch fixes is res_pjsip_multihomed. The res_pjsip_multihomed
module attempts to determine what transport a message should go out on and what
addressing information should go into the message itself. In a scenario where
multiple transports exist bound to the same IP address but a different port the
code would incorrectly alter the transport and change the message to the wrong
transport. This change makes the res_pjsip_multihomed module smarter so it will
only change the transport and address information in the message when it is
possible and makes sense.

ASTERISK-24615 #close
Reported by: David Justl

Review: https://reviewboard.asterisk.org/r/4331/
........

Merged revisions 430755 from http://svn.asterisk.org/svn/asterisk/branches/13

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@430756 65c4cc65-6c06-0410-ace0-fbb531ad65f3

res/res_pjsip.c
res/res_pjsip_multihomed.c

index b57f91e..5a80909 100644 (file)
@@ -2276,17 +2276,29 @@ pjsip_dialog *ast_sip_create_dialog_uas(const struct ast_sip_endpoint *endpoint,
        pjsip_dialog *dlg;
        pj_str_t contact;
        pjsip_transport_type_e type = rdata->tp_info.transport->key.type;
+       pjsip_tpselector selector = { .type = PJSIP_TPSELECTOR_NONE, };
+       pjsip_transport *transport;
 
        ast_assert(status != NULL);
 
+       if (sip_get_tpselector_from_endpoint(endpoint, &selector)) {
+               return NULL;
+       }
+
+       transport = rdata->tp_info.transport;
+       if (selector.type == PJSIP_TPSELECTOR_TRANSPORT) {
+               transport = selector.u.transport;
+       }
+       type = transport->key.type;
+
        contact.ptr = pj_pool_alloc(rdata->tp_info.pool, PJSIP_MAX_URL_SIZE);
        contact.slen = pj_ansi_snprintf(contact.ptr, PJSIP_MAX_URL_SIZE,
                        "<sip:%s%.*s%s:%d%s%s>",
                        (type & PJSIP_TRANSPORT_IPV6) ? "[" : "",
-                       (int)rdata->tp_info.transport->local_name.host.slen,
-                       rdata->tp_info.transport->local_name.host.ptr,
+                       (int)transport->local_name.host.slen,
+                       transport->local_name.host.ptr,
                        (type & PJSIP_TRANSPORT_IPV6) ? "]" : "",
-                       rdata->tp_info.transport->local_name.port,
+                       transport->local_name.port,
                        (type != PJSIP_TRANSPORT_UDP && type != PJSIP_TRANSPORT_UDP6) ? ";transport=" : "",
                        (type != PJSIP_TRANSPORT_UDP && type != PJSIP_TRANSPORT_UDP6) ? pjsip_transport_get_type_name(type) : "");
 
@@ -2300,6 +2312,10 @@ pjsip_dialog *ast_sip_create_dialog_uas(const struct ast_sip_endpoint *endpoint,
                return NULL;
        }
 
+       dlg->sess_count++;
+       pjsip_dlg_set_transport(dlg, &selector);
+       dlg->sess_count--;
+
        return dlg;
 }
 
index e0ee53e..ca56b7c 100644 (file)
@@ -83,12 +83,11 @@ static int multihomed_rewrite_sdp(struct pjmedia_sdp_session *sdp)
        return 0;
 }
 
-/*! \brief Helper function which determines if the existing address has priority over new one */
-static int multihomed_rewrite_header(pj_str_t *source, pjsip_transport *transport)
+/*! \brief Helper function which determines if a transport is bound to any */
+static int multihomed_bound_any(pjsip_transport *transport)
 {
        pj_uint32_t loop6[4] = {0, 0, 0, 0};
 
-       /* If the transport is bound to any it should always rewrite */
        if ((transport->local_addr.addr.sa_family == pj_AF_INET() &&
                transport->local_addr.ipv4.sin_addr.s_addr == PJ_INADDR_ANY) ||
                (transport->local_addr.addr.sa_family == pj_AF_INET6() &&
@@ -96,18 +95,12 @@ static int multihomed_rewrite_header(pj_str_t *source, pjsip_transport *transpor
                return 1;
        }
 
-       /* If the transport is explicitly bound but the determined source differs favor the transport */
-       if (!pj_strcmp(source, &transport->local_name.host)) {
-               return 1;
-       }
-
        return 0;
 }
 
 static pj_status_t multihomed_on_tx_message(pjsip_tx_data *tdata)
 {
        pjsip_tpmgr_fla2_param prm;
-       pjsip_transport *transport = NULL;
        pjsip_cseq_hdr *cseq;
        pjsip_via_hdr *via;
 
@@ -122,24 +115,32 @@ static pj_status_t multihomed_on_tx_message(pjsip_tx_data *tdata)
                return PJ_SUCCESS;
        }
 
-       /* If the transport it is going out on is different reflect it in the message */
-       if (tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP ||
-               tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP6) {
-               transport = multihomed_get_udp_transport(&prm.ret_addr, prm.ret_port);
-       }
+       /* The port in the message should always be that of the original transport */
+       prm.ret_port = tdata->tp_info.transport->local_name.port;
 
-       /* If no new transport use the one provided by the message */
-       if (!transport) {
-               transport = tdata->tp_info.transport;
-       }
+       /* If the IP source differs from the existing transport see if we need to update it */
+       if (pj_strcmp(&prm.ret_addr, &tdata->tp_info.transport->local_name.host)) {
 
-       /* If the message should not be rewritten then abort early */
-       if (!multihomed_rewrite_header(&prm.ret_addr, transport)) {
-               return PJ_SUCCESS;
-       }
+               /* If the transport it is going out on is different reflect it in the message */
+               if (tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP ||
+                       tdata->tp_info.transport->key.type == PJSIP_TRANSPORT_UDP6) {
+                       pjsip_transport *transport;
 
-       /* Update the transport in case it has changed - we do this now in case we don't want to touch the message above */
-       tdata->tp_info.transport = transport;
+                       transport = multihomed_get_udp_transport(&prm.ret_addr, prm.ret_port);
+
+                       if (transport) {
+                               tdata->tp_info.transport = transport;
+                       }
+               }
+
+               /* If the chosen transport is not bound to any we can't use the source address as it won't get back to us */
+               if (!multihomed_bound_any(tdata->tp_info.transport)) {
+                       pj_strassign(&prm.ret_addr, &tdata->tp_info.transport->local_name.host);
+               }
+       } else {
+               /* The transport chosen will deliver this but ensure it is updated with the right information */
+               pj_strassign(&prm.ret_addr, &tdata->tp_info.transport->local_name.host);
+       }
 
        /* If the message needs to be updated with new address do so */
        if (tdata->msg->type == PJSIP_REQUEST_MSG || !(cseq = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_CSEQ, NULL)) ||
@@ -148,7 +149,7 @@ static pj_status_t multihomed_on_tx_message(pjsip_tx_data *tdata)
                if (contact && (PJSIP_URI_SCHEME_IS_SIP(contact->uri) || PJSIP_URI_SCHEME_IS_SIPS(contact->uri))) {
                        pjsip_sip_uri *uri = pjsip_uri_get_uri(contact->uri);
 
-                       /* prm.ret_addr is allocated from the tdata pool so it is perfectly fine to just do an assignment like this */
+                       /* prm.ret_addr is allocated from the tdata pool OR the transport so it is perfectly fine to just do an assignment like this */
                        pj_strassign(&uri->host, &prm.ret_addr);
                        uri->port = prm.ret_port;