res_pjsip: Remove temp transport state when a transport fails to load.

If a pjsip transport (A) fails to load, its temporary state gets left behind
causing the next transport to load (B) to pick up some of its parameters,
including its name. This can cause B to have the correct name (B) in its
transport object but the wrong name (A) in its internal state object. When a
transport state is searched for later on, transport state B is returned but a
retrieval of the actual transport object will fail because B's transport
state id is actually "A" and transport "A" doesn't exist because it failed
to load.

remove_temporary_state() is now being called in all error paths in
config_transport.c functions that call find_or_create_temporary_state().

A bit of extra debugging was also added to res_pjsip_nat.c.

Resolves: #1814

diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c
index 8be07187d0d7c364931c976e37e71acf1f3a66a9..850dd21b5ef2c2f92f58159cf7e917ce3170a2d2 100644 (file)
--- a/res/res_pjsip/config_transport.c
+++ b/res/res_pjsip/config_transport.c
@@ -1045,6 +1045,7 @@ static int transport_tls_file_handler(const struct aco_option *opt, struct ast_v
        if (!ast_file_is_readable(var->value)) {
                ast_log(LOG_ERROR, "Transport: %s: %s %s is either missing or not readable\n",
                        ast_sorcery_object_get_id(obj), var->name, var->value);
+               remove_temporary_state();
                return -1;
        }
 
@@ -1066,6 +1067,7 @@ static int transport_tls_file_handler(const struct aco_option *opt, struct ast_v
                if (stat(var->value, &state->cert_file_stat)) {
                        ast_log(LOG_ERROR, "Failed to stat certificate file '%s' for transport '%s' due to '%s'\n",
                                var->value, ast_sorcery_object_get_id(obj), strerror(errno));
+                       remove_temporary_state();
                        return -1;
                }
                ast_sorcery_object_set_has_dynamic_contents(transport);
@@ -1077,6 +1079,7 @@ static int transport_tls_file_handler(const struct aco_option *opt, struct ast_v
                if (stat(var->value, &state->privkey_file_stat)) {
                        ast_log(LOG_ERROR, "Failed to stat private key file '%s' for transport '%s' due to '%s'\n",
                                var->value, ast_sorcery_object_get_id(obj), strerror(errno));
+                       remove_temporary_state();
                        return -1;
                }
                ast_sorcery_object_set_has_dynamic_contents(transport);
@@ -1146,6 +1149,7 @@ static int transport_protocol_handler(const struct aco_option *opt, struct ast_v
                } else if (!strcasecmp(var->value, "wss")) {
                        transport->type = AST_TRANSPORT_WSS;
                } else {
+                       remove_temporary_state();
                        return -1;
                }
                transport->flow = 0;
@@ -1190,7 +1194,9 @@ static int transport_bind_handler(const struct aco_option *opt, struct ast_varia
        }
 
        rc = pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&buf, var->value), &state->host);
-
+       if (rc != PJ_SUCCESS) {
+               remove_temporary_state();
+       }
        return rc != PJ_SUCCESS ? -1 : 0;
 }
 
@@ -1232,6 +1238,7 @@ static int transport_tls_bool_handler(const struct aco_option *opt, struct ast_v
        } else if (!strcasecmp(var->name, "allow_wildcard_certs")) {
                state->allow_wildcard_certs = ast_true(var->value);
        } else {
+               remove_temporary_state();
                return -1;
        }
 
@@ -1329,6 +1336,7 @@ static int transport_tls_method_handler(const struct aco_option *opt, struct ast
        } else if (!strcasecmp(var->value, "sslv23")) {
                state->tls.method = PJSIP_SSLV23_METHOD;
        } else {
+               remove_temporary_state();
                return -1;
        }
 
@@ -1460,6 +1468,10 @@ static int transport_tls_cipher_handler(const struct aco_option *opt, struct ast
                }
                res |= transport_cipher_add(state, name);
        }
+
+       if (res) {
+               remove_temporary_state();
+       }
        return res ? -1 : 0;
 }
 #endif
@@ -1556,6 +1568,7 @@ static int transport_localnet_handler(const struct aco_option *opt, struct ast_v
        /* We use only the ast_apply_ha() which defaults to ALLOW
         * ("permit"), so we add DENY rules. */
        if (!(state->localnet = ast_append_ha("deny", var->value, state->localnet, &error))) {
+               remove_temporary_state();
                return -1;
        }
 
@@ -1619,6 +1632,7 @@ static int transport_tos_handler(const struct aco_option *opt, struct ast_variab
                ast_log(LOG_ERROR, "Error configuring transport '%s' - Could not "
                        "interpret 'tos' value '%s'\n",
                        ast_sorcery_object_get_id(transport), var->value);
+               remove_temporary_state();
                return -1;
        }
 

diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index 699e5fb08be764092b1cdb437ec9d0c13ec31a77..f4e63a61b4bd4fc32f23eb965db6e0ac27a5d656 100644 (file)
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -324,11 +324,21 @@ static pj_status_t process_nat(pjsip_tx_data *tdata)
        struct ast_sockaddr addr = { { 0, } };
        pjsip_sip_uri *uri = NULL;
        RAII_VAR(struct ao2_container *, hooks, NULL, ao2_cleanup);
+       const char *transport_type_name = "unknown";
 
        if (ast_sip_set_request_transport_details(&details, tdata, 0)) {
+               ast_debug(4, "Unable to process message for transport type '%s'\n", transport_type_name);
                return PJ_SUCCESS;
        }
 
+       if (details.transport) {
+               transport_type_name = details.transport->type_name;
+       } else if (details.factory) {
+               transport_type_name = details.factory->type_name;
+       }
+
+       ast_debug(4, "Processing outgoing message for transport type '%s'\n", transport_type_name);
+
        uri = ast_sip_get_contact_sip_uri(tdata);
        via = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_VIA, NULL);
 
@@ -336,17 +346,26 @@ static pj_status_t process_nat(pjsip_tx_data *tdata)
                return PJ_SUCCESS;
        }
 
+       ast_debug(4, "Found transport state '%s' for type '%s'\n", transport_state->id,
+               transport_type_name);
+
        if (!(transport = ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "transport", transport_state->id))) {
+               ast_debug(4, "Unable to find transport for transport state '%s' type '%s'\n", transport_state->id,
+                       transport_type_name);
                return PJ_SUCCESS;
        }
 
+       ast_debug(4, "Found transport '%s' for transport state '%s' type '%s'\n",
+               ast_sorcery_object_get_id(transport),
+               transport_state->id, transport_type_name);
+
        if (transport_state->localnet) {
                ast_sockaddr_parse(&addr, tdata->tp_info.dst_name, PARSE_PORT_FORBID);
                ast_sockaddr_set_port(&addr, tdata->tp_info.dst_port);
 
                /* See if where we are sending this request is local or not, and if not that we can get a Contact URI to modify */
                if (ast_sip_transport_is_local(transport_state, &addr)) {
-                       ast_debug(5, "Request is being sent to local address, skipping NAT manipulation\n");
+                       ast_debug(4, "Request is being sent to local address, skipping NAT manipulation\n");
                        return PJ_SUCCESS;
                }
        }