res_pjsip: mediasec: Add Security-Client headers after 401 (#49)

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

Resolves: #48

res/res_pjsip/pjsip_configuration.c

@@ -256,6 +256,13 @@ static int timers_to_str(const void *obj, const intptr_t *args, char **buf)
 	return 0;
 }
 
+static int security_mechanism_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	const struct ast_sip_endpoint *endpoint = obj;
+
+	return ast_sip_security_mechanisms_to_str(&endpoint->security_mechanisms, 0, buf);
+}
+
 static int security_mechanism_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
 {
 	struct ast_sip_endpoint *endpoint = obj;
@@ -263,6 +270,20 @@ static int security_mechanism_handler(const struct aco_option *opt, struct ast_v
 	return ast_sip_security_mechanism_vector_init(&endpoint->security_mechanisms, var->value);
 }
 
+static const char *security_negotiation_map[] = {
+	[AST_SIP_SECURITY_NEG_NONE] = "no",
+	[AST_SIP_SECURITY_NEG_MEDIASEC] = "mediasec",
+};
+
+static int security_negotiation_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	const struct ast_sip_endpoint *endpoint = obj;
+	if (ARRAY_IN_BOUNDS(endpoint->security_negotiation, security_negotiation_map)) {
+		*buf = ast_strdup(security_negotiation_map[endpoint->security_negotiation]);
+	}
+	return 0;
+}
+
 int ast_sip_set_security_negotiation(enum ast_sip_security_negotiation *security_negotiation, const char *val) {
 	if (!strcasecmp("no", val)) {
 		*security_negotiation = AST_SIP_SECURITY_NEG_NONE;
@@ -2262,8 +2283,8 @@ int ast_res_pjsip_initialize_configuration(void)
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "allow_unauthenticated_options", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, allow_unauthenticated_options));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "geoloc_incoming_call_profile", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, geoloc_incoming_call_profile));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "geoloc_outgoing_call_profile", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, geoloc_outgoing_call_profile));
-	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "security_mechanisms", "", security_mechanism_handler, NULL, NULL, 0, 0);
-	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "security_negotiation", "no", security_negotiation_handler, NULL, NULL, 0, 0);
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "security_mechanisms", "", security_mechanism_handler, security_mechanism_to_str, NULL, 0, 0);
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "security_negotiation", "no", security_negotiation_handler, security_negotiation_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "send_aoc", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, send_aoc));
 
 	if (ast_sip_initialize_sorcery_transport()) {

res/res_pjsip/security_agreements.c

@@ -137,8 +137,8 @@ static char *ast_sip_security_mechanism_type_to_str(enum ast_sip_security_mechan
 	}
 }
 
-static int ast_sip_security_mechanism_to_str(const struct ast_sip_security_mechanism *security_mechanism, int add_qvalue, char **buf) {
-	char tmp[64];
+static int security_mechanism_to_str(const struct ast_sip_security_mechanism *security_mechanism, int add_qvalue, char **buf)
+{
 	size_t size;
 	size_t buf_size = 128;
 	int i;
@@ -152,22 +152,50 @@ static int ast_sip_security_mechanism_to_str(const struct ast_sip_security_mecha
 		return EINVAL;
 	}
 
-	strncat(ret, ast_sip_security_mechanism_type_to_str(security_mechanism->type), buf_size - strlen(ret) - 1);
+    snprintf(ret, buf_size - 1, "%s", ast_sip_security_mechanism_type_to_str(security_mechanism->type));
 	if (add_qvalue) {
-		snprintf(tmp, sizeof(tmp), ";q=%f.4", security_mechanism->qvalue);
-		strncat(ret, tmp, buf_size - strlen(ret) - 1);
+		snprintf(ret + strlen(ret), buf_size - 1, ";q=%f.4", security_mechanism->qvalue);
 	}
 
 	size = AST_VECTOR_SIZE(&security_mechanism->mechanism_parameters);
 	for (i = 0; i < size; ++i) {
-		snprintf(tmp, sizeof(tmp), ";%s", AST_VECTOR_GET(&security_mechanism->mechanism_parameters, i));
-		strncat(ret, tmp, buf_size - strlen(ret) - 1);
+		snprintf(ret + strlen(ret), buf_size - 1, ";%s", AST_VECTOR_GET(&security_mechanism->mechanism_parameters, i));
 	}
 
 	*buf = ret;
 	return 0;
 }
 
+int ast_sip_security_mechanisms_to_str(const struct ast_sip_security_mechanism_vector *security_mechanisms, int add_qvalue, char **buf)
+{
+	size_t vec_size;
+	struct ast_sip_security_mechanism *mech;
+	char *tmp_buf;
+	char ret[512];
+	size_t i;
+
+	if (!security_mechanisms) {
+		return -1;
+	}
+
+	vec_size = AST_VECTOR_SIZE(security_mechanisms);
+	ret[0] = '\0';
+
+	for (i = 0; i < vec_size; ++i) {
+		mech = AST_VECTOR_GET(security_mechanisms, i);
+		if (security_mechanism_to_str(mech, add_qvalue, &tmp_buf)) {
+			continue;
+		}
+		snprintf(ret + strlen(ret), sizeof(ret) - 1, "%s%s",
+		   tmp_buf, i == vec_size - 1 ? "" : ", ");
+		ast_free(tmp_buf);
+	}
+
+	*buf = ast_strdup(ret);
+
+	return 0;
+}
+
 void ast_sip_remove_headers_by_name_and_value(pjsip_msg *msg, const pj_str_t *hdr_name, const char* value)
 {
 	struct pjsip_generic_string_hdr *hdr = pjsip_msg_find_hdr_by_name(msg, hdr_name, NULL);
@@ -186,7 +214,7 @@ void ast_sip_remove_headers_by_name_and_value(pjsip_msg *msg, const pj_str_t *hd
  * \brief Parses a string representing a q_value to a float.
  *
  * Valid q values must be in the range from 0.0 to 1.0 inclusively.
- * 
+ *
  * \param q_value
  * \retval The parsed qvalue or -1.0 on failure.
  */
@@ -234,9 +262,10 @@ int ast_sip_str_to_security_mechanism(struct ast_sip_security_mechanism **securi
 			err = EINVAL;
 			goto out;
 		}
-		if (!strncmp(param, "q=0", 4) || !strncmp(param, "q=1", 4)) {
+		if (!strncmp(param, "q=", 2)) {
 			mech->qvalue = parse_qvalue(&param[2]);
 			if (mech->qvalue < 0.0) {
+				err = EINVAL;
 				goto out;
 			}
 			continue;
@@ -279,7 +308,7 @@ int ast_sip_add_security_headers(struct ast_sip_security_mechanism_vector *secur
 	mech_cnt = AST_VECTOR_SIZE(security_mechanisms);
 	for (i = 0; i < mech_cnt; ++i) {
 		mech = AST_VECTOR_GET(security_mechanisms, i);
-		if (ast_sip_security_mechanism_to_str(mech, add_qvalue, &buf)) {
+		if (security_mechanism_to_str(mech, add_qvalue, &buf)) {
 			continue;
 		}
 		ast_sip_add_header(tdata, header_name, buf);

res/res_pjsip_outbound_registration.c

@@ -425,6 +425,8 @@ struct sip_outbound_registration_client_state {
 	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,9 @@ out:
 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;
+	int add_sec_client_header = 0;
 	struct sip_outbound_registration *reg = NULL;
 	struct ast_sip_endpoint *endpt = NULL;
 	struct ao2_container *contact_container;
@@ -674,20 +679,33 @@ static void add_security_headers(struct sip_outbound_registration_client_state *
 	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);
+		} else {
+			/* necessary if a retry occures */
+			add_sec_client_header = (pjsip_msg_find_hdr_by_name(tdata->msg, &security_client, NULL) == NULL) ? 1 : 0;
+		}
+		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");
+	}
+	if (add_sec_client_header) {
+		ast_sip_add_security_headers(&client_state->security_mechanisms, "Security-Client", 0, tdata);
+	}
 
 	/* Cleanup */
 	if (contact_status) {
@@ -1216,6 +1234,7 @@ static int handle_registration_response(void *data)
 	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);
@@ -2018,6 +2037,27 @@ static int sip_outbound_registration_apply(const struct ast_sorcery *sorcery, vo
 	return 0;
 }
 
+static int security_mechanism_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	const struct sip_outbound_registration *registration = obj;
+
+	return ast_sip_security_mechanisms_to_str(&registration->security_mechanisms, 0, buf);
+}
+
+static const char *security_negotiation_map[] = {
+	[AST_SIP_SECURITY_NEG_NONE] = "no",
+	[AST_SIP_SECURITY_NEG_MEDIASEC] = "mediasec",
+};
+
+static int security_negotiation_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	const struct sip_outbound_registration *registration = obj;
+	if (ARRAY_IN_BOUNDS(registration->security_negotiation, security_negotiation_map)) {
+		*buf = ast_strdup(security_negotiation_map[registration->security_negotiation]);
+	}
+	return 0;
+}
+
 static int security_mechanisms_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
 {
 	struct sip_outbound_registration *registration = obj;
@@ -2761,8 +2801,8 @@ static int load_module(void)
 	ast_sorcery_object_field_register_custom(ast_sip_get_sorcery(), "registration", "outbound_auth", "", outbound_auth_handler, outbound_auths_to_str, outbound_auths_to_var_list, 0, 0);
 	ast_sorcery_object_field_register(ast_sip_get_sorcery(), "registration", "support_path", "no", OPT_BOOL_T, 1, FLDSET(struct sip_outbound_registration, support_path));
 	ast_sorcery_object_field_register(ast_sip_get_sorcery(), "registration", "support_outbound", "no", OPT_YESNO_T, 1, FLDSET(struct sip_outbound_registration, support_outbound));
-	ast_sorcery_object_field_register_custom(ast_sip_get_sorcery(), "registration", "security_negotiation", "no", security_negotiation_handler, NULL, NULL, 0, 0);
-	ast_sorcery_object_field_register_custom(ast_sip_get_sorcery(), "registration", "security_mechanisms", "", security_mechanisms_handler, NULL, NULL, 0, 0);
+	ast_sorcery_object_field_register_custom(ast_sip_get_sorcery(), "registration", "security_negotiation", "no", security_negotiation_handler, security_negotiation_to_str, NULL, 0, 0);
+	ast_sorcery_object_field_register_custom(ast_sip_get_sorcery(), "registration", "security_mechanisms", "", security_mechanisms_handler, security_mechanism_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register(ast_sip_get_sorcery(), "registration", "line", "no", OPT_BOOL_T, 1, FLDSET(struct sip_outbound_registration, line));
 	ast_sorcery_object_field_register(ast_sip_get_sorcery(), "registration", "endpoint", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct sip_outbound_registration, endpoint));
 

res/res_pjsip_rfc3329.c

@@ -34,18 +34,40 @@
 #include "asterisk/causes.h"
 #include "asterisk/threadpool.h"
 
+/*! \brief Private data structure used with the modules's datastore */
+struct rfc3329_store_data {
+	int last_rx_status_code;
+};
+
+static void datastore_destroy_cb(void *data)
+{
+	struct rfc3329_store_data *d = data;
+	if (d) {
+		ast_free(d);
+	}
+}
+
+/*! \brief The channel datastore the module uses to store state */
+static const struct ast_datastore_info rfc3329_store_datastore = {
+	.type = "rfc3329_store",
+	.destroy = datastore_destroy_cb
+};
+
 static void rfc3329_incoming_response(struct ast_sip_session *session, struct pjsip_rx_data *rdata)
 {
+	RAII_VAR(struct ast_datastore *, datastore, ast_sip_session_get_datastore(session, "rfc3329_store"), ao2_cleanup);
 	static const pj_str_t str_security_server = { "Security-Server", 15 };
 	struct ast_sip_contact_status *contact_status = NULL;
 	struct ast_sip_security_mechanism *mech;
+	struct rfc3329_store_data *store_data;
 	pjsip_generic_string_hdr *header;
 	char buf[128];
 	char *hdr_val;
 	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 +76,17 @@ static void rfc3329_incoming_response(struct ast_sip_session *session, struct pj
 		goto out;
 	}
 
+	if (!datastore
+		&& (datastore = ast_sip_session_alloc_datastore(&rfc3329_store_datastore, "rfc3329_store"))
+		&& (store_data = ast_calloc(1, sizeof(struct rfc3329_store_data)))) {
+
+		store_data->last_rx_status_code = rdata->msg_info.msg->line.status.code;
+		datastore->data = store_data;
+		ast_sip_session_add_datastore(session, datastore);
+	} else {
+		ast_log(AST_LOG_WARNING, "Could not store session data. Still attempting requests, but they might be missing necessary headers.\n");
+	}
+
 	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)) {
@@ -73,12 +106,14 @@ out:
 	ao2_cleanup(contact_status);
 }
 
-static void add_outgoing_request_headers(struct ast_sip_endpoint *endpoint, struct ast_sip_contact *contact, struct pjsip_tx_data *tdata)
+static void add_outgoing_request_headers(struct ast_sip_endpoint *endpoint, struct ast_sip_contact *contact, struct pjsip_tx_data *tdata,
+	struct ast_datastore *datastore)
 {
 	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;
-
+	struct rfc3329_store_data *store_data;
+	
 	if (endpoint->security_negotiation != AST_SIP_SECURITY_NEG_MEDIASEC) {
 		return;
 	}
@@ -94,23 +129,26 @@ static void add_outgoing_request_headers(struct ast_sip_endpoint *endpoint, stru
 	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)) {
-		/* Add Security-Client headers (no q-value) */
-		ast_sip_add_security_headers(&endpoint->security_mechanisms, "Security-Client", 0, tdata);
+	}
+	if (datastore) {
+		store_data = datastore->data;
+		if (store_data->last_rx_status_code == 401) {
+			/* Add Security-Client headers (no q-value) */
+			ast_sip_add_security_headers(&endpoint->security_mechanisms, "Security-Client", 0, tdata);
+		}
 	}
 	ao2_unlock(contact_status);
-	ast_sip_add_header(tdata, "Require", "mediasec");
-	ast_sip_add_header(tdata, "Proxy-Require", "mediasec");
 
 	ao2_cleanup(contact_status);
 }
 
 static void rfc3329_outgoing_request(struct ast_sip_session *session, struct pjsip_tx_data *tdata)
 {
+	RAII_VAR(struct ast_datastore *, datastore, ast_sip_session_get_datastore(session, "rfc3329_store"), ao2_cleanup);
 	if (session->contact == NULL) {
 		return;
 	}
-	add_outgoing_request_headers(session->endpoint, session->contact, tdata);
+	add_outgoing_request_headers(session->endpoint, session->contact, tdata, datastore);
 }
 
 static struct ast_sip_session_supplement rfc3329_supplement = {
@@ -120,7 +158,7 @@ static struct ast_sip_session_supplement rfc3329_supplement = {
 
 static void rfc3329_options_request(struct ast_sip_endpoint *endpoint, struct ast_sip_contact *contact, struct pjsip_tx_data *tdata)
 {
-	add_outgoing_request_headers(endpoint, contact, tdata);
+	add_outgoing_request_headers(endpoint, contact, tdata, NULL);
 }
 
 static struct ast_sip_supplement rfc3329_options_supplement = {
@@ -138,10 +176,11 @@ static int load_module(void)
 static int unload_module(void)
 {
 	ast_sip_session_unregister_supplement(&rfc3329_supplement);
+	ast_sip_unregister_supplement(&rfc3329_options_supplement);
 	return 0;
 }
 
-AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "PJSIP RFC 3329 Support (partial)",
+AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "PJSIP RFC3329 Support (partial)",
 	.support_level = AST_MODULE_SUPPORT_CORE,
 	.load = load_module,
 	.unload = unload_module,