[Asterisk-code-review] res pjsip outbound registration: Don't fail on delayed proce... (asterisk[13])
Mark Michelson
asteriskteam at digium.com
Mon Apr 27 17:01:03 CDT 2015
Mark Michelson has uploaded a new change for review.
https://gerrit.asterisk.org/265
Change subject: res_pjsip_outbound_registration: Don't fail on delayed processing: 13.
......................................................................
res_pjsip_outbound_registration: Don't fail on delayed processing: 13.
This is the Asterisk 13 version of a change to master that allows for
registration responses to be processed successfully potentially after
the original transaction has timed out. The main difference between this
and the master change is that the master version has API changes that
are unacceptable for 13. For 13, this is worked around by adding a new
API call that the outbound registration code uses instead.
The following is the text from the master version of this commit:
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.
There are two fixes introduced here.
First, we prevent the transaction from being destroyed before we are
ready by adding a reference to the transaction group lock and
decrementing that reference once we are completely finished processing
the REGISTER response.
Even though we now ensure that the transaction memory cannot be freed
before we are ready, this is not enough to ensure that we can
successfully perform actions like creating a REGISTER with
authentication credentials. This is because when the transaction times
out, most data is wiped from the transaction. Even though the
transaction is not freed, it's a shell of its former self.
To allow for authentication of a REGISTER request to be authenticated
after the transaction has timed out, we now also hold a reference to the
original REGISTER request. 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: If1ee5f601be839479a219424f0358a229f358f7c
---
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip_outbound_authenticator_digest.c
M res/res_pjsip_outbound_registration.c
4 files changed, 58 insertions(+), 6 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/65/265/1
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index a15e967..cbae595 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -702,6 +702,18 @@
*/
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);
+ /*!
+ * \brief Create a new request with authentication credentials based on old request
+ *
+ * \param auths A vector of IDs of auth sorcery objects
+ * \param challenge The SIP response with authentication challenge(s)
+ * \param old_request The request that resulted in 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_from_old)(const struct ast_sip_auth_vector *auths, struct pjsip_rx_data *challenge,
+ struct pjsip_tx_data *old_request, struct pjsip_tx_data **new_request);
};
/*!
@@ -1397,6 +1409,17 @@
pjsip_transaction *tsx, pjsip_tx_data **new_request);
/*!
+ * \brief Create a response to an authentication challenge
+ *
+ * This will call into an outbound authenticator's create_request_with_auth callback
+ * to create a new request with authentication credentials. See the create_request_with_auth_from_old
+ * callback in the \ref ast_sip_outbound_authenticator structure for details about
+ * the parameters and return values.
+ */
+int ast_sip_create_request_with_auth_from_old(const struct ast_sip_auth_vector *auths, pjsip_rx_data *challenge,
+ pjsip_tx_data *old_request, pjsip_tx_data **new_request);
+
+/*!
* \brief Determine the endpoint that has sent a SIP message
*
* This will call into each of the registered endpoint identifiers'
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index bde0de1..5f68c44 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -2012,6 +2012,16 @@
return registered_outbound_authenticator->create_request_with_auth(auths, challenge, tsx, new_request);
}
+int ast_sip_create_request_with_auth_from_old(const struct ast_sip_auth_vector *auths, pjsip_rx_data *challenge,
+ 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_from_old(auths, challenge, old_request, new_request);
+}
+
struct endpoint_identifier_list {
const char *name;
unsigned int priority;
diff --git a/res/res_pjsip_outbound_authenticator_digest.c b/res/res_pjsip_outbound_authenticator_digest.c
index 35e59f2..aa35fba 100644
--- a/res/res_pjsip_outbound_authenticator_digest.c
+++ b/res/res_pjsip_outbound_authenticator_digest.c
@@ -101,14 +101,14 @@
return res;
}
-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)
+static int digest_create_request_with_auth_from_old(const struct ast_sip_auth_vector *auths, pjsip_rx_data *challenge,
+ 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 @@
}
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
@@ -150,6 +150,12 @@
return -1;
}
+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)
+{
+ return digest_create_request_with_auth_from_old(auths, challenge, tsx->last_tx, new_request);
+}
+
static struct ast_sip_outbound_authenticator digest_authenticator = {
.create_request_with_auth = digest_create_request_with_auth,
};
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index b8f7112..c024a44 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -489,6 +489,8 @@
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 */
@@ -498,6 +500,14 @@
if (response->rdata) {
pjsip_rx_data_free_cloned(response->rdata);
+ }
+
+ if (response->tsx) {
+ pj_grp_lock_dec_ref(response->tsx->grp_lock);
+ }
+
+ if (response->old_request) {
+ pjsip_tx_data_dec_ref(response->old_request);
}
ao2_cleanup(response->client_state);
@@ -558,8 +568,8 @@
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)) {
+ if (!ast_sip_create_request_with_auth_from_old(&response->client_state->outbound_auths,
+ response->rdata, response->old_request, &tdata)) {
ao2_ref(response->client_state, +1);
response->client_state->auth_attempted = 1;
if (pjsip_regc_send(response->client_state->client, tdata) != PJ_SUCCESS) {
@@ -653,6 +663,9 @@
response->retry_after = retry_after ? retry_after->ivalue : 0;
response->tsx = pjsip_rdata_get_tsx(param->rdata);
+ response->old_request = response->tsx->last_tx;
+ pjsip_tx_data_add_ref(response->old_request);
+ pj_grp_lock_add_ref(response->tsx->grp_lock);
pjsip_rx_data_clone(param->rdata, 0, &response->rdata);
}
--
To view, visit https://gerrit.asterisk.org/265
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If1ee5f601be839479a219424f0358a229f358f7c
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list