[Asterisk-code-review] res_pjsip: mediasec: Add Security-Client headers after 401 (asterisk[18])

Maximilian Fridrich asteriskteam at digium.com
Tue Dec 20 03:49:51 CST 2022


Maximilian Fridrich has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/19740 )


Change subject: res_pjsip: mediasec: Add Security-Client headers after 401
......................................................................

res_pjsip: mediasec: Add Security-Client headers after 401

When using mediasec, requests sent after a 401 must still contain the
Security-Client header according to
draft-dawes-sipcore-mediasec-parameter.

ASTERISK-30276

Change-Id: Ief7857365f221b1ef28672a27cc3fb27384c8d0f
---
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_rfc3329.c
2 files changed, 58 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/40/19740/1

diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 538f65f..9aefefe 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -425,6 +425,8 @@
 	unsigned int destroy:1;
 	/*! \brief Non-zero if we have attempted sending a REGISTER with authentication */
 	unsigned int auth_attempted:1;
+	/*! \brief Status code of last response if we have tried to register before */
+	int last_status_code;
 	/*! \brief The name of the transport to be used for the registration */
 	char *transport_name;
 	/*! \brief The name of the registration sorcery object */
@@ -642,6 +644,8 @@
 static void add_security_headers(struct sip_outbound_registration_client_state *client_state,
 	pjsip_tx_data *tdata)
 {
+	int add_require_header = 1;
+	int add_proxy_require_header = 1;
 	struct sip_outbound_registration *reg = NULL;
 	struct ast_sip_endpoint *endpt = NULL;
 	struct ao2_container *contact_container;
@@ -674,20 +678,27 @@
 	if (!contact_status && AST_VECTOR_SIZE(&client_state->server_security_mechanisms)) {
 		sec_mechs = &client_state->server_security_mechanisms;
 	}
-	if (client_state->status == SIP_REGISTRATION_REGISTERED || client_state->status == SIP_REGISTRATION_REJECTED_TEMPORARY
-			|| client_state->auth_attempted) {
+	if (client_state->status == SIP_REGISTRATION_REJECTED_TEMPORARY || client_state->auth_attempted) {
 		if (sec_mechs != NULL && pjsip_msg_find_hdr_by_name(tdata->msg, &security_verify, NULL) == NULL) {
 			ast_sip_add_security_headers(sec_mechs, "Security-Verify", 0, tdata);
 		}
-		ast_sip_remove_headers_by_name_and_value(tdata->msg, &security_client, NULL);
-		ast_sip_remove_headers_by_name_and_value(tdata->msg, &proxy_require, "mediasec");
-		ast_sip_remove_headers_by_name_and_value(tdata->msg, &require, "mediasec");
+		if (client_state->last_status_code == 494) {
+			ast_sip_remove_headers_by_name_and_value(tdata->msg, &security_client, NULL);
+		}
+		add_require_header =
+			(pjsip_msg_find_hdr_by_name(tdata->msg, &require, NULL) == NULL) ? 1 : 0;
+		add_proxy_require_header =
+			(pjsip_msg_find_hdr_by_name(tdata->msg, &proxy_require, NULL) == NULL) ? 1 : 0;
 	} else {
 		ast_sip_add_security_headers(&client_state->security_mechanisms, "Security-Client", 0, tdata);
 	}
 
-	ast_sip_add_header(tdata, "Require", "mediasec");
-	ast_sip_add_header(tdata, "Proxy-Require", "mediasec");
+	if (add_require_header) {
+		ast_sip_add_header(tdata, "Require", "mediasec");
+	}
+	if (add_proxy_require_header) {
+		ast_sip_add_header(tdata, "Proxy-Require", "mediasec");
+	}
 
 	/* Cleanup */
 	if (contact_status) {
@@ -1216,6 +1227,7 @@
 	pjsip_regc_get_info(response->client_state->client, &info);
 	ast_copy_pj_str(server_uri, &info.server_uri, sizeof(server_uri));
 	ast_copy_pj_str(client_uri, &info.client_uri, sizeof(client_uri));
+	response->client_state->last_status_code = response->code;
 
 	ast_debug(1, "Processing REGISTER response %d from server '%s' for client '%s'\n",
 			response->code, server_uri, client_uri);
diff --git a/res/res_pjsip_rfc3329.c b/res/res_pjsip_rfc3329.c
index 167dfa0..ae0cded 100644
--- a/res/res_pjsip_rfc3329.c
+++ b/res/res_pjsip_rfc3329.c
@@ -34,6 +34,11 @@
 #include "asterisk/causes.h"
 #include "asterisk/threadpool.h"
 
+static pjsip_module rfc3329_module = {
+	.name = { "RFC 3329 Module", 15 },
+	.id = -1,
+};
+
 static void rfc3329_incoming_response(struct ast_sip_session *session, struct pjsip_rx_data *rdata)
 {
 	static const pj_str_t str_security_server = { "Security-Server", 15 };
@@ -45,7 +50,8 @@
 	char *mechanism;
 
 	if (!session || !session->endpoint || !session->endpoint->security_negotiation
-		|| !session->contact || !(contact_status = ast_sip_get_contact_status(session->contact))) {
+		|| !session->contact || !(contact_status = ast_sip_get_contact_status(session->contact))
+		|| !session->inv_session->dlg) {
 		return;
 	}
 
@@ -54,6 +60,10 @@
 		goto out;
 	}
 
+	if (pjsip_dlg_has_usage(session->inv_session->dlg, &rfc3329_module) != PJ_TRUE) {
+		pjsip_dlg_add_usage(session->inv_session->dlg, &rfc3329_module, rdata);
+	}
+
 	header = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &str_security_server, NULL);
 	for (; header;
 		header = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &str_security_server, header->next)) {
@@ -78,7 +88,9 @@
 	static const pj_str_t security_verify = { "Security-Verify", 15 };
 	struct pjsip_generic_string_hdr *hdr = NULL;
 	struct ast_sip_contact_status *contact_status = NULL;
-
+	pjsip_dialog *dlg = pjsip_tdata_get_dlg(tdata);
+	pjsip_rx_data *last_rdata = NULL;
+	
 	if (endpoint->security_negotiation != AST_SIP_SECURITY_NEG_MEDIASEC) {
 		return;
 	}
@@ -90,11 +102,17 @@
 		return;
 	}
 
+	if (dlg && pjsip_dlg_has_usage(dlg, &rfc3329_module)) {
+		last_rdata = pjsip_dlg_get_mod_data(dlg, rfc3329_module.id);
+	}
+
 	ao2_lock(contact_status);
 	if (AST_VECTOR_SIZE(&contact_status->security_mechanisms) && hdr == NULL) {
 		/* Add Security-Verify headers (with q-value) */
 		ast_sip_add_security_headers(&contact_status->security_mechanisms, "Security-Verify", 0, tdata);
-	} else if (!hdr && AST_VECTOR_SIZE(&endpoint->security_mechanisms)) {
+	}
+	if ((last_rdata && last_rdata->msg_info.msg->line.status.code == 401) ||
+		(!hdr && AST_VECTOR_SIZE(&endpoint->security_mechanisms))) {
 		/* Add Security-Client headers (no q-value) */
 		ast_sip_add_security_headers(&endpoint->security_mechanisms, "Security-Client", 0, tdata);
 	}
@@ -132,12 +150,15 @@
 {
 	ast_sip_session_register_supplement(&rfc3329_supplement);
 	ast_sip_register_supplement(&rfc3329_options_supplement);
+	ast_sip_register_service(&rfc3329_module);
 	return AST_MODULE_LOAD_SUCCESS;
 }
 
 static int unload_module(void)
 {
 	ast_sip_session_unregister_supplement(&rfc3329_supplement);
+	ast_sip_unregister_supplement(&rfc3329_options_supplement);
+	ast_sip_unregister_service(&rfc3329_module);
 	return 0;
 }
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19740
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Ief7857365f221b1ef28672a27cc3fb27384c8d0f
Gerrit-Change-Number: 19740
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Fridrich <m.fridrich at commend.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221220/d1b363a7/attachment-0001.html>


More information about the asterisk-code-review mailing list