res_pjsip_outbound_registration: Don't fail on delayed processing.

Odd behaviors have been observed during outbound registrations. The most
common problem witnessed has been one where a request with
authentication credentials cannot be created after receiving a 401
response. Other behaviors include apparently processing an incorrect SIP
response.

Inspecting the code led to an apparent issue with regards to how we
handle transactions in outbound registration code. When a response to a
REGISTER arrives, we save a pointer to the transaction and then push a
task onto the registration serializer. Between the time that we save the
pointer and push the task, it's possible for the transaction to be
destroyed due to a timeout. It's also possible for the address to be
reused by the transaction layer for a new transaction.

To allow for authentication of a REGISTER request to be authenticated
after the transaction has timed out, we now hold a reference to the
original REGISTER request instead of the transaction. The function for
creating a request with authentication has been altered to take the
original request instead of the transaction where the original request
was sent.

ASTERISK-25020
Reported by Mark Michelson

Change-Id: I756c19ab05ada5d0503175db9676acf87c686d0a

diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 12fc400d2dbe33b192f49a455ea2e47b8fb44649..67c9c4b16e3dfa35f9a6aabdaa3b21f8b9053944 100644 (file)
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -697,13 +697,13 @@ struct ast_sip_outbound_authenticator {
         *
         * \param auths A vector of IDs of auth sorcery objects
         * \param challenge The SIP response with authentication challenge(s)
-        * \param tsx The transaction in which the challenge was received
+        * \param old_request The request that received the auth challenge(s)
         * \param new_request The new SIP request with challenge response(s)
         * \retval 0 Successfully created new request
         * \retval -1 Failed to create a new request
         */
        int (*create_request_with_auth)(const struct ast_sip_auth_vector *auths, struct pjsip_rx_data *challenge,
-                       struct pjsip_transaction *tsx, struct pjsip_tx_data **new_request);
+                       struct pjsip_tx_data *old_request, struct pjsip_tx_data **new_request);
 };
 
 /*!
@@ -1396,7 +1396,7 @@ enum ast_sip_check_auth_result ast_sip_check_authentication(struct ast_sip_endpo
  * the parameters and return values.
  */
 int ast_sip_create_request_with_auth(const struct ast_sip_auth_vector *auths, pjsip_rx_data *challenge,
-               pjsip_transaction *tsx, pjsip_tx_data **new_request);
+               pjsip_tx_data *tdata, pjsip_tx_data **new_request);
 
 /*!
  * \brief Determine the endpoint that has sent a SIP message

diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index a613bccf6921c778d395cca9dbe12abea938e936..8be019f8785cf5030c50358e01f715259c59693b 100644 (file)
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -2009,13 +2009,13 @@ void ast_sip_unregister_outbound_authenticator(struct ast_sip_outbound_authentic
 }
 
 int ast_sip_create_request_with_auth(const struct ast_sip_auth_vector *auths, pjsip_rx_data *challenge,
-               pjsip_transaction *tsx, pjsip_tx_data **new_request)
+               pjsip_tx_data *old_request, pjsip_tx_data **new_request)
 {
        if (!registered_outbound_authenticator) {
                ast_log(LOG_WARNING, "No SIP outbound authenticator registered. Cannot respond to authentication challenge\n");
                return -1;
        }
-       return registered_outbound_authenticator->create_request_with_auth(auths, challenge, tsx, new_request);
+       return registered_outbound_authenticator->create_request_with_auth(auths, challenge, old_request, new_request);
 }
 
 struct endpoint_identifier_list {
@@ -3107,7 +3107,7 @@ static void send_request_cb(void *token, pjsip_event *e)
                        && endpoint
                        && ++req_data->challenge_count < MAX_RX_CHALLENGES /* Not in a challenge loop */
                        && !ast_sip_create_request_with_auth(&endpoint->outbound_auths,
-                               challenge, tsx, &tdata)
+                               challenge, tsx->last_tx, &tdata)
                        && endpt_send_request(endpoint, tdata, -1, req_data, send_request_cb)
                                == PJ_SUCCESS;
                ao2_cleanup(endpoint);

diff --git a/res/res_pjsip/pjsip_outbound_auth.c b/res/res_pjsip/pjsip_outbound_auth.c
index 1f754227a476d667f23382e55d49a7378b73599e..8b39b000de19322d7783f77f1ae9123a9295e3c5 100644 (file)
--- a/res/res_pjsip/pjsip_outbound_auth.c
+++ b/res/res_pjsip/pjsip_outbound_auth.c
@@ -63,7 +63,7 @@ static pj_bool_t outbound_auth(pjsip_rx_data *rdata)
                return PJ_FALSE;
        }
 
-       if (ast_sip_create_request_with_auth(&endpoint->outbound_auths, rdata, tsx, &tdata)) {
+       if (ast_sip_create_request_with_auth(&endpoint->outbound_auths, rdata, tsx->last_tx, &tdata)) {
                return PJ_FALSE;
        }
 

diff --git a/res/res_pjsip_outbound_authenticator_digest.c b/res/res_pjsip_outbound_authenticator_digest.c
index 35e59f21ad00e1b5c307dab5b53dffa7460f00da..de77616fd5ca2809c179bbf0f16e80cdb100b5a9 100644 (file)
--- a/res/res_pjsip_outbound_authenticator_digest.c
+++ b/res/res_pjsip_outbound_authenticator_digest.c
@@ -102,13 +102,13 @@ cleanup:
 }
 
 static int digest_create_request_with_auth(const struct ast_sip_auth_vector *auths, pjsip_rx_data *challenge,
-               pjsip_transaction *tsx, pjsip_tx_data **new_request)
+               pjsip_tx_data *old_request, pjsip_tx_data **new_request)
 {
        pjsip_auth_clt_sess auth_sess;
        pjsip_cseq_hdr *cseq;
 
        if (pjsip_auth_clt_init(&auth_sess, ast_sip_get_pjsip_endpoint(),
-                               tsx->pool, 0) != PJ_SUCCESS) {
+                               old_request->pool, 0) != PJ_SUCCESS) {
                ast_log(LOG_WARNING, "Failed to initialize client authentication session\n");
                return -1;
        }
@@ -119,7 +119,7 @@ static int digest_create_request_with_auth(const struct ast_sip_auth_vector *aut
        }
 
        switch (pjsip_auth_clt_reinit_req(&auth_sess, challenge,
-                               tsx->last_tx, new_request)) {
+                               old_request, new_request)) {
        case PJ_SUCCESS:
                /* PJSIP creates a new transaction for new_request (meaning it creates a new
                 * branch). However, it recycles the Call-ID, from-tag, and CSeq from the

diff --git a/res/res_pjsip_outbound_publish.c b/res/res_pjsip_outbound_publish.c
index 8b6f6e41e5205de9c37a753f3443950ce0302b75..f7669932e2ca674f25ec70b3591f213e9f559b6a 100644 (file)
--- a/res/res_pjsip_outbound_publish.c
+++ b/res/res_pjsip_outbound_publish.c
@@ -867,8 +867,10 @@ static void sip_outbound_publish_callback(struct pjsip_publishc_cbparam *param)
        }
 
        if (param->code == 401 || param->code == 407) {
+               pjsip_transaction *tsx = pjsip_rdata_get_tsx(param->rdata);
+
                if (!ast_sip_create_request_with_auth(&publish->outbound_auths,
-                               param->rdata, pjsip_rdata_get_tsx(param->rdata), &tdata)) {
+                               param->rdata, tsx->last_tx, &tdata)) {
                        pjsip_publishc_send(client->client, tdata);
                }
                client->auth_attempts++;

diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 69e0124482926fcf2641b609cd96b9299f80ea73..c2eb62b32094e4ac8f92c288c416472f993d1e9e 100644 (file)
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -568,8 +568,8 @@ struct registration_response {
        struct sip_outbound_registration_client_state *client_state;
        /*! \brief The response message */
        pjsip_rx_data *rdata;
-       /*! \brief The response transaction */
-       pjsip_transaction *tsx;
+       /*! \brief Request for which the response was received */
+       pjsip_tx_data *old_request;
 };
 
 /*! \brief Registration response structure destructor */
@@ -581,6 +581,10 @@ static void registration_response_destroy(void *obj)
                pjsip_rx_data_free_cloned(response->rdata);
        }
 
+       if (response->old_request) {
+               pjsip_tx_data_dec_ref(response->old_request);
+       }
+
        ao2_cleanup(response->client_state);
 }
 
@@ -633,19 +637,19 @@ static int handle_registration_response(void *data)
        ast_copy_pj_str(client_uri, &info.client_uri, sizeof(client_uri));
 
        if (response->client_state->status == SIP_REGISTRATION_STOPPED) {
-               ast_debug(1, "Not handling registration response from '%s' (transaction %s). Registration already stopped\n",
-                               server_uri, response->tsx ? response->tsx->obj_name : "<none>");
+               ast_debug(1, "Not handling registration response from server '%s' for client '%s'. Registration already stopped\n",
+                               server_uri, client_uri);
                return 0;
        }
 
-       ast_debug(1, "Processing REGISTER response %d from '%s' (transaction %s)\n",
-                       response->code, server_uri, response->tsx ? response->tsx->obj_name : "<none>");
+       ast_debug(1, "Processing REGISTER response %d from server '%s' for client '%s'\n",
+                       response->code, server_uri, client_uri);
 
        if (!response->client_state->auth_attempted &&
                        (response->code == 401 || response->code == 407)) {
                pjsip_tx_data *tdata;
                if (!ast_sip_create_request_with_auth(&response->client_state->outbound_auths,
-                               response->rdata, response->tsx, &tdata)) {
+                               response->rdata, response->old_request, &tdata)) {
                        ao2_ref(response->client_state, +1);
                        response->client_state->auth_attempted = 1;
                        ast_debug(1, "Sending authenticated REGISTER to server '%s' from client '%s'\n",
@@ -748,9 +752,12 @@ static void sip_outbound_registration_response_cb(struct pjsip_regc_cbparam *par
 
        if (param->rdata) {
                struct pjsip_retry_after_hdr *retry_after = pjsip_msg_find_hdr(param->rdata->msg_info.msg, PJSIP_H_RETRY_AFTER, NULL);
+               pjsip_transaction *tsx;
 
                response->retry_after = retry_after ? retry_after->ivalue : 0;
-               response->tsx = pjsip_rdata_get_tsx(param->rdata);
+               tsx = pjsip_rdata_get_tsx(param->rdata);
+               response->old_request = tsx->last_tx;
+               pjsip_tx_data_add_ref(response->old_request);
                pjsip_rx_data_clone(param->rdata, 0, &response->rdata);
        }