[Asterisk-code-review] STIR/SHAKEN: Option split and response codes. (asterisk[16])

Friendly Automation asteriskteam at digium.com
Wed Oct 27 09:56:21 CDT 2021


Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/16497 )

Change subject: STIR/SHAKEN: Option split and response codes.
......................................................................

STIR/SHAKEN: Option split and response codes.

The stir_shaken configuration option now has 4 different choices to pick
from: off, attest, verify, and on. Off and on behave the same way they
do now. Attest will only perform attestation on the endpoint, and verify
will only perform verification on the endpoint.

Certain responses are required to be sent based on certain conditions
for STIR/SHAKEN. For example, if we get a Date header that is outside of
the time range that is considered valid, a 403 Stale Date response
should be sent. This and several other responses have been added.

Change-Id: I4ac1ecf652cd0e336006b0ca638dc826b5b1ebf7
---
A doc/UPGRADE-staging/stir_shaken_option_split.txt
M include/asterisk/res_pjsip.h
M include/asterisk/res_stir_shaken.h
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip_session.c
M res/res_pjsip_stir_shaken.c
M res/res_stir_shaken.c
7 files changed, 420 insertions(+), 114 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Verified
  Friendly Automation: Approved for Submit



diff --git a/doc/UPGRADE-staging/stir_shaken_option_split.txt b/doc/UPGRADE-staging/stir_shaken_option_split.txt
new file mode 100644
index 0000000..79df214
--- /dev/null
+++ b/doc/UPGRADE-staging/stir_shaken_option_split.txt
@@ -0,0 +1,7 @@
+Subject: STIR/SHAKEN
+
+The STIR/SHAKEN configuration option has been split into
+4 different choices: off, attest, verify, and on. Off and
+on behave the same way as before. Attest will only perform
+attestation on the endpoint, and verify will only perform
+verification on the endpoint.
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index bf92ef7..289ad11 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -62,6 +62,22 @@
 #define PJSIP_EXPIRES_NOT_SPECIFIED	((pj_uint32_t)-1)
 #endif
 
+/* Response codes from RFC8224 */
+#define AST_STIR_SHAKEN_RESPONSE_CODE_STALE_DATE 403
+#define AST_STIR_SHAKEN_RESPONSE_CODE_USE_IDENTITY_HEADER 428
+#define AST_STIR_SHAKEN_RESPONSE_CODE_USE_SUPPORTED_PASSPORT_FORMAT 428
+#define AST_STIR_SHAKEN_RESPONSE_CODE_BAD_IDENTITY_INFO 436
+#define AST_STIR_SHAKEN_RESPONSE_CODE_UNSUPPORTED_CREDENTIAL 437
+#define AST_STIR_SHAKEN_RESPONSE_CODE_INVALID_IDENTITY_HEADER 438
+
+/* Response strings from RFC8224 */
+#define AST_STIR_SHAKEN_RESPONSE_STR_STALE_DATE "Stale Date"
+#define AST_STIR_SHAKEN_RESPONSE_STR_USE_IDENTITY_HEADER "Use Identity Header"
+#define AST_STIR_SHAKEN_RESPONSE_STR_USE_SUPPORTED_PASSPORT_FORMAT "Use Supported PASSporT Format"
+#define AST_STIR_SHAKEN_RESPONSE_STR_BAD_IDENTITY_INFO "Bad Identity Info"
+#define AST_STIR_SHAKEN_RESPONSE_STR_UNSUPPORTED_CREDENTIAL "Unsupported Credential"
+#define AST_STIR_SHAKEN_RESPONSE_STR_INVALID_IDENTITY_HEADER "Invalid Identity Header"
+
 /* Forward declarations of PJSIP stuff */
 struct pjsip_rx_data;
 struct pjsip_module;
@@ -499,6 +515,17 @@
 	AST_SIP_REDIRECT_URI_PJSIP,
 };
 
+enum ast_sip_stir_shaken_behavior {
+	/*! Don't do any STIR/SHAKEN operations */
+	AST_SIP_STIR_SHAKEN_OFF = 0,
+	/*! Only do STIR/SHAKEN attestation */
+	AST_SIP_STIR_SHAKEN_ATTEST = 1,
+	/*! Only do STIR/SHAKEN verification */
+	AST_SIP_STIR_SHAKEN_VERIFY = 2,
+	/*! Do STIR/SHAKEN attestation and verification */
+	AST_SIP_STIR_SHAKEN_ON = 3,
+};
+
 /*!
  * \brief Session timers options
  */
@@ -841,7 +868,7 @@
 	unsigned int send_connected_line;
 	/*! Ignore 183 if no SDP is present */
 	unsigned int ignore_183_without_sdp;
-	/*! Enable STIR/SHAKEN support on this endpoint */
+	/*! Set which STIR/SHAKEN behaviors we want on this endpoint */
 	unsigned int stir_shaken;
 	/*! Should we authenticate OPTIONS requests per RFC 3261? */
 	unsigned int allow_unauthenticated_options;
diff --git a/include/asterisk/res_stir_shaken.h b/include/asterisk/res_stir_shaken.h
index 5175907..92eb0ec 100644
--- a/include/asterisk/res_stir_shaken.h
+++ b/include/asterisk/res_stir_shaken.h
@@ -29,6 +29,13 @@
 	AST_STIR_SHAKEN_VERIFY_PASSED, /*! Signature verified and contents match signaling */
 };
 
+/*! Different from ast_stir_shaken_verification_result. Used to determine why ast_stir_shaken_verify returned NULL */
+enum ast_stir_shaken_verify_failure_reason {
+	AST_STIR_SHAKEN_VERIFY_FAILED_MEMORY_ALLOC, /*! Memory allocation failure */
+	AST_STIR_SHAKEN_VERIFY_FAILED_TO_GET_CERT, /*! Failed to get the credentials to verify */
+	AST_STIR_SHAKEN_VERIFY_FAILED_SIGNATURE_VALIDATION, /*! Failed validating the signature */
+};
+
 struct ast_stir_shaken_payload;
 
 struct ast_json;
@@ -88,6 +95,24 @@
 	const char *algorithm, const char *public_cert_url);
 
 /*!
+ * \brief Same as ast_stir_shaken_verify, but will populate a struct with additional information on failure
+ *
+ * \note failure_code will be written to in this function
+ *
+ * \param header The payload header
+ * \param payload The payload section
+ * \param signature The payload signature
+ * \param algorithm The signature algorithm
+ * \param public_cert_url The public key URL
+ * \param failure_code Additional failure information
+ *
+ * \retval ast_stir_shaken_payload on success
+ * \retval NULL on failure
+ */
+struct ast_stir_shaken_payload *ast_stir_shaken_verify2(const char *header, const char *payload, const char *signature,
+	const char *algorithm, const char *public_cert_url, int *failure_code);
+
+/*!
  * \brief Retrieve the stir/shaken sorcery context
  *
  * \retval The stir/shaken sorcery context
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index ee30ada..0a6d46a 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -717,6 +717,44 @@
 	return 0;
 }
 
+static int stir_shaken_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
+{
+	struct ast_sip_endpoint *endpoint = obj;
+
+	if (!strcasecmp("off", var->value)) {
+		endpoint->stir_shaken = AST_SIP_STIR_SHAKEN_OFF;
+	} else if (!strcasecmp("attest", var->value)) {
+		endpoint->stir_shaken = AST_SIP_STIR_SHAKEN_ATTEST;
+	} else if (!strcasecmp("verify", var->value)) {
+		endpoint->stir_shaken = AST_SIP_STIR_SHAKEN_VERIFY;
+	} else if (!strcasecmp("on", var->value)) {
+		endpoint->stir_shaken = AST_SIP_STIR_SHAKEN_ON;
+	} else {
+		ast_log(LOG_WARNING, "'%s' is not a valid value for option "
+			"'stir_shaken' for endpoint %s\n",
+			var->value, ast_sorcery_object_get_id(endpoint));
+		return -1;
+	}
+
+	return 0;
+}
+
+static const char *stir_shaken_map[] = {
+	[AST_SIP_STIR_SHAKEN_OFF] "off",
+	[AST_SIP_STIR_SHAKEN_ATTEST] = "attest",
+	[AST_SIP_STIR_SHAKEN_VERIFY] = "verify",
+	[AST_SIP_STIR_SHAKEN_ON] = "on",
+};
+
+static int stir_shaken_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	const struct ast_sip_endpoint *endpoint = obj;
+	if (ARRAY_IN_BOUNDS(endpoint->stir_shaken, stir_shaken_map)) {
+		*buf = ast_strdup(stir_shaken_map[endpoint->stir_shaken]);
+	}
+	return 0;
+}
+
 static int group_handler(const struct aco_option *opt,
 			 struct ast_variable *var, void *obj)
 {
@@ -1968,7 +2006,7 @@
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "accept_multiple_sdp_answers", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, media.rtp.accept_multiple_sdp_answers));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "suppress_q850_reason_headers", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, suppress_q850_reason_headers));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "ignore_183_without_sdp", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, ignore_183_without_sdp));
-	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "stir_shaken", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, stir_shaken));
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "stir_shaken", "off", stir_shaken_handler, stir_shaken_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "allow_unauthenticated_options", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, allow_unauthenticated_options));
 
 	if (ast_sip_initialize_sorcery_transport()) {
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index c8478d4..1b1d8de 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -4057,12 +4057,25 @@
 {
 	RAII_VAR(struct ast_sip_endpoint *, endpoint,
 			ast_pjsip_rdata_get_endpoint(rdata), ao2_cleanup);
+	static const pj_str_t identity_str = { "Identity", 8 };
+	const pj_str_t use_identity_header_str = {
+		AST_STIR_SHAKEN_RESPONSE_STR_USE_IDENTITY_HEADER,
+		strlen(AST_STIR_SHAKEN_RESPONSE_STR_USE_IDENTITY_HEADER)
+	};
 	pjsip_inv_session *inv_session = NULL;
 	struct ast_sip_session *session;
 	struct new_invite invite;
 
 	ast_assert(endpoint != NULL);
 
+	if ((endpoint->stir_shaken & AST_SIP_STIR_SHAKEN_VERIFY) &&
+		!ast_sip_rdata_get_header_value(rdata, identity_str)) {
+		pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata,
+			AST_STIR_SHAKEN_RESPONSE_CODE_USE_IDENTITY_HEADER, &use_identity_header_str, NULL, NULL);
+		ast_debug(3, "No Identity header when we require one\n");
+		return;
+	}
+
 	inv_session = pre_session_setup(rdata, endpoint);
 	if (!inv_session) {
 		/* pre_session_setup() returns a response on failure */
diff --git a/res/res_pjsip_stir_shaken.c b/res/res_pjsip_stir_shaken.c
index 1519d34..b78c154 100644
--- a/res/res_pjsip_stir_shaken.c
+++ b/res/res_pjsip_stir_shaken.c
@@ -35,6 +35,9 @@
 
 #include "asterisk/res_stir_shaken.h"
 
+/*! The Date header will not be valid after this many milliseconds (60 seconds recommended) */
+#define STIR_SHAKEN_DATE_HEADER_TIMEOUT 60000
+
 /*!
  * \brief Get the attestation from the payload
  *
@@ -112,6 +115,62 @@
 	return 0;
 }
 
+static int check_date_header(pjsip_rx_data *rdata)
+{
+	static const pj_str_t date_hdr_str = { "Date", 4 };
+	char *date_hdr_val;
+	struct ast_tm date_hdr_tm;
+	struct timeval date_hdr_timeval;
+	struct timeval current_timeval;
+	char *remainder;
+	char timezone[80] = { 0 };
+	int64_t time_diff;
+
+	date_hdr_val = ast_sip_rdata_get_header_value(rdata, date_hdr_str);
+	if (ast_strlen_zero(date_hdr_val)) {
+		ast_log(LOG_ERROR, "Failed to get Date header from incoming INVITE for STIR/SHAKEN\n");
+		return -1;
+	}
+
+	if (!(remainder = ast_strptime(date_hdr_val, "%a, %d %b %Y %T", &date_hdr_tm))) {
+		ast_log(LOG_ERROR, "Failed to parse Date header\n");
+		return -1;
+	}
+
+	sscanf(remainder, "%79s", timezone);
+
+	if (ast_strlen_zero(timezone)) {
+		ast_log(LOG_ERROR, "A timezone is required for STIR/SHAKEN Date header, but we didn't get one\n");
+		return -1;
+	}
+
+	date_hdr_timeval = ast_mktime(&date_hdr_tm, timezone);
+	current_timeval = ast_tvnow();
+
+	time_diff = ast_tvdiff_ms(current_timeval, date_hdr_timeval);
+	if (time_diff < 0) {
+		/* An INVITE from the future! */
+		ast_log(LOG_ERROR, "STIR/SHAKEN Date header has a future date\n");
+		return -1;
+	} else if (time_diff > STIR_SHAKEN_DATE_HEADER_TIMEOUT) {
+		ast_log(LOG_ERROR, "STIR/SHAKEN Date header was outside of the allowable range (60 seconds)\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+/* Send a response back and end the session */
+static void stir_shaken_inv_end_session(struct ast_sip_session *session, pjsip_rx_data *rdata, int response_code, const pj_str_t response_str)
+{
+	pjsip_tx_data *tdata;
+
+	if (pjsip_inv_end_session(session->inv_session, response_code, &response_str, &tdata) == PJ_SUCCESS) {
+		pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL);
+	}
+	ast_hangup(session->channel);
+}
+
 /*!
  * \internal
  * \brief Session supplement callback on an incoming INVITE request
@@ -125,6 +184,27 @@
 static int stir_shaken_incoming_request(struct ast_sip_session *session, pjsip_rx_data *rdata)
 {
 	static const pj_str_t identity_str = { "Identity", 8 };
+	const pj_str_t bad_identity_info_str = {
+		AST_STIR_SHAKEN_RESPONSE_STR_BAD_IDENTITY_INFO,
+		strlen(AST_STIR_SHAKEN_RESPONSE_STR_BAD_IDENTITY_INFO)
+	};
+	const pj_str_t unsupported_credential_str = {
+		AST_STIR_SHAKEN_RESPONSE_STR_UNSUPPORTED_CREDENTIAL,
+		strlen(AST_STIR_SHAKEN_RESPONSE_STR_UNSUPPORTED_CREDENTIAL)
+	};
+	const pj_str_t stale_date_str = {
+		AST_STIR_SHAKEN_RESPONSE_STR_STALE_DATE,
+		strlen(AST_STIR_SHAKEN_RESPONSE_STR_STALE_DATE)
+	};
+	const pj_str_t use_supported_passport_format_str = {
+		AST_STIR_SHAKEN_RESPONSE_STR_USE_SUPPORTED_PASSPORT_FORMAT,
+		strlen(AST_STIR_SHAKEN_RESPONSE_STR_USE_SUPPORTED_PASSPORT_FORMAT)
+	};
+	const pj_str_t invalid_identity_hdr_str = {
+		AST_STIR_SHAKEN_RESPONSE_STR_INVALID_IDENTITY_HEADER,
+		strlen(AST_STIR_SHAKEN_RESPONSE_STR_INVALID_IDENTITY_HEADER)
+	};
+	const pj_str_t server_internal_error_str = { "Server Internal Error", 21 };
 	char *identity_hdr_val;
 	char *encoded_val;
 	struct ast_channel *chan = session->channel;
@@ -135,10 +215,17 @@
 	char *algorithm;
 	char *public_cert_url;
 	char *attestation;
+	char *ppt;
 	int mismatch = 0;
 	struct ast_stir_shaken_payload *ss_payload;
+	int failure_code = 0;
 
-	if (!session->endpoint->stir_shaken) {
+	/* Check if this is a reinvite. If it is, we don't need to do anything */
+	if (rdata->msg_info.to->tag.slen) {
+		return 0;
+	}
+
+	if ((session->endpoint->stir_shaken & AST_SIP_STIR_SHAKEN_VERIFY) == 0) {
 		return 0;
 	}
 
@@ -151,50 +238,100 @@
 	encoded_val = strtok_r(identity_hdr_val, ".", &identity_hdr_val);
 	header = ast_base64url_decode_string(encoded_val);
 	if (ast_strlen_zero(header)) {
-		ast_stir_shaken_add_verification(chan, caller_id, "", AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
+		ast_debug(3, "STIR/SHAKEN INVITE for %s is missing header\n",
+			ast_sorcery_object_get_id(session->endpoint));
+		stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_BAD_IDENTITY_INFO, bad_identity_info_str);
+		return 1;
 	}
 
 	encoded_val = strtok_r(identity_hdr_val, ".", &identity_hdr_val);
 	payload = ast_base64url_decode_string(encoded_val);
 	if (ast_strlen_zero(payload)) {
-		ast_stir_shaken_add_verification(chan, caller_id, "", AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
+		ast_debug(3, "STIR/SHAKEN INVITE for %s is missing payload\n",
+			ast_sorcery_object_get_id(session->endpoint));
+		stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_BAD_IDENTITY_INFO, bad_identity_info_str);
+		return 1;
 	}
 
 	/* It's fine to leave the signature encoded */
 	signature = strtok_r(identity_hdr_val, ";", &identity_hdr_val);
 	if (ast_strlen_zero(signature)) {
-		ast_stir_shaken_add_verification(chan, caller_id, "", AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
+		ast_debug(3, "STIR/SHAKEN INVITE for %s is missing signature\n",
+			ast_sorcery_object_get_id(session->endpoint));
+		stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_BAD_IDENTITY_INFO, bad_identity_info_str);
+		return 1;
 	}
 
 	/* Trim "info=<" to get public cert URL */
 	strtok_r(identity_hdr_val, "<", &identity_hdr_val);
 	public_cert_url = strtok_r(identity_hdr_val, ">", &identity_hdr_val);
-	if (ast_strlen_zero(public_cert_url)) {
-		ast_stir_shaken_add_verification(chan, caller_id, "", AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
-	}
 
 	/* Make sure the public URL is actually a URL */
-	if (!ast_begins_with(public_cert_url, "http")) {
-		ast_stir_shaken_add_verification(chan, caller_id, "", AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
+	if (ast_strlen_zero(public_cert_url) || !ast_begins_with(public_cert_url, "http")) {
+		/* RFC8224 states that if we can't acquire the credentials needed
+		 * by the verification service, we should send a 436 */
+		ast_debug(3, "STIR/SHAKEN INVITE for %s did not  have valid URL (%s)\n",
+			ast_sorcery_object_get_id(session->endpoint), public_cert_url);
+		stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_BAD_IDENTITY_INFO, bad_identity_info_str);
+		return 1;
 	}
 
 	algorithm = strtok_r(identity_hdr_val, ";", &identity_hdr_val);
 	if (ast_strlen_zero(algorithm)) {
-		ast_stir_shaken_add_verification(chan, caller_id, "", AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
+		/* RFC8224 states that if the algorithm is not specified, use ES256 */
+		algorithm = STIR_SHAKEN_ENCRYPTION_ALGORITHM;
+	} else {
+		strtok_r(algorithm, "=", &algorithm);
+		if (strcmp(algorithm, STIR_SHAKEN_ENCRYPTION_ALGORITHM)) {
+			/* RFC8224 states that if we don't support the algorithm, send a 437 */
+			ast_debug(3, "STIR/SHAKEN INVITE for %s uses an unsupported algorithm (%s)\n",
+				ast_sorcery_object_get_id(session->endpoint), algorithm);
+			stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_UNSUPPORTED_CREDENTIAL, unsupported_credential_str);
+			return 1;
+		}
+	}
+
+	/* The only thing left should be ppt=shaken (which could have more values later),
+	 * unless using the compact PASSport form */
+	strtok_r(identity_hdr_val, "=", &identity_hdr_val);
+	ppt = ast_strip(identity_hdr_val);
+	if (!ast_strlen_zero(ppt) && strcmp(ppt, STIR_SHAKEN_PPT)) {
+		ast_log(LOG_ERROR, "STIR/SHAKEN INVITE for %s has unsupported ppt (%s)\n",
+			ast_sorcery_object_get_id(session->endpoint), ppt);
+		stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_USE_SUPPORTED_PASSPORT_FORMAT, use_supported_passport_format_str);
+		return 1;
+	}
+
+	if (check_date_header(rdata)) {
+		ast_debug(3, "STIR/SHAKEN INVITE for %s has old Date header\n",
+			ast_sorcery_object_get_id(session->endpoint));
+		stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_STALE_DATE, stale_date_str);
+		return 1;
 	}
 
 	attestation = get_attestation_from_payload(payload);
 
-	ss_payload = ast_stir_shaken_verify(header, payload, signature, algorithm, public_cert_url);
+	ss_payload = ast_stir_shaken_verify2(header, payload, signature, algorithm, public_cert_url, &failure_code);
 	if (!ss_payload) {
-		ast_stir_shaken_add_verification(chan, caller_id, attestation, AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
-		return 0;
+
+		if (failure_code == AST_STIR_SHAKEN_VERIFY_FAILED_TO_GET_CERT) {
+			/* RFC8224 states that if we can't get the credentials we need, send a 437 */
+			ast_debug(3, "STIR/SHAKEN INVITE for %s failed to acquire cert during verification process\n",
+				ast_sorcery_object_get_id(session->endpoint));
+			stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_UNSUPPORTED_CREDENTIAL, unsupported_credential_str);
+		} else if (failure_code == AST_STIR_SHAKEN_VERIFY_FAILED_MEMORY_ALLOC) {
+			ast_log(LOG_ERROR, "Failed to allocate memory during STIR/SHAKEN verification"
+				" for %s\n", ast_sorcery_object_get_id(session->endpoint));
+			stir_shaken_inv_end_session(session, rdata, 500, server_internal_error_str);
+		} else if (failure_code == AST_STIR_SHAKEN_VERIFY_FAILED_SIGNATURE_VALIDATION) {
+			/* RFC8224 states that if we can't validate the signature, send a 438 */
+			ast_debug(3, "STIR/SHAKEN INVITE for %s failed signature validation during verification process\n",
+				ast_sorcery_object_get_id(session->endpoint));
+			ast_stir_shaken_add_verification(chan, caller_id, attestation, AST_STIR_SHAKEN_VERIFY_SIGNATURE_FAILED);
+			stir_shaken_inv_end_session(session, rdata, AST_STIR_SHAKEN_RESPONSE_CODE_INVALID_IDENTITY_HEADER, invalid_identity_hdr_str);
+		}
+
+		return 1;
 	}
 	ast_stir_shaken_payload_free(ss_payload);
 
@@ -336,7 +473,7 @@
 
 static void stir_shaken_outgoing_request(struct ast_sip_session *session, pjsip_tx_data *tdata)
 {
-	if (!session->endpoint->stir_shaken) {
+	if ((session->endpoint->stir_shaken & AST_SIP_STIR_SHAKEN_ATTEST) == 0) {
 		return;
 	}
 
diff --git a/res/res_stir_shaken.c b/res/res_stir_shaken.c
index 1d8c785..373a1a1 100644
--- a/res/res_stir_shaken.c
+++ b/res/res_stir_shaken.c
@@ -617,9 +617,146 @@
 	return filename;
 }
 
+/*!
+ * \brief Verifies that the string parameters are not empty for STIR/SHAKEN verification
+ *
+ * \retval 0 on success
+ * \retval 1 on failure
+ */
+static int stir_shaken_verify_check_empty_strings(const char *header, const char *payload, const char *signature,
+	const char *algorithm, const char *public_cert_url)
+{
+	if (ast_strlen_zero(header)) {
+		ast_log(LOG_ERROR, "'header' is required for STIR/SHAKEN verification\n");
+		return 1;
+	}
+
+	if (ast_strlen_zero(payload)) {
+		ast_log(LOG_ERROR, "'payload' is required for STIR/SHAKEN verification\n");
+		return 1;
+	}
+
+	if (ast_strlen_zero(signature)) {
+		ast_log(LOG_ERROR, "'signature' is required for STIR/SHAKEN verification\n");
+		return 1;
+	}
+
+	if (ast_strlen_zero(algorithm)) {
+		ast_log(LOG_ERROR, "'algorithm' is required for STIR/SHAKEN verification\n");
+		return 1;
+	}
+
+	if (ast_strlen_zero(public_cert_url)) {
+		ast_log(LOG_ERROR, "'public_cert_url' is required for STIR/SHAKEN verification\n");
+		return 1;
+	}
+
+	return 0;
+}
+
+/*!
+ * \brief Get or set up the file path for the certificate
+ *
+ * \note This function will allocate memory for file_path and dir_path and populate them
+ *
+ * \retval 0 on success
+ * \retval 1 on failure
+ */
+static int stir_shaken_verify_setup_file_paths(const char *public_cert_url, char **file_path, char **dir_path, int *curl)
+{
+	*file_path = get_path_to_public_key(public_cert_url);
+	if (ast_asprintf(dir_path, "%s/keys/%s", ast_config_AST_DATA_DIR, STIR_SHAKEN_DIR_NAME) < 0) {
+		return 1;
+	}
+
+	/* If we don't have an entry in AstDB, CURL from the provided URL */
+	if (ast_strlen_zero(*file_path)) {
+		/* Remove this entry from the database, since we will be
+		 * downloading a new file anyways.
+		 */
+		remove_public_key_from_astdb(public_cert_url);
+
+		/* Go ahead and free file_path, in case anything was allocated above */
+		ast_free(*file_path);
+
+		/* Download to the default path */
+		*file_path = run_curl(public_cert_url, *dir_path);
+		if (!(*file_path)) {
+			return 1;
+		}
+
+		/* Signal that we have already downloaded a new file, no reason to do it again */
+		*curl = 1;
+
+		/* We should have a successful download at this point, so
+		 * add an entry to the database.
+		 */
+		add_public_key_to_astdb(public_cert_url, *file_path);
+	}
+
+	return 0;
+}
+
+/*!
+ * \brief See if the cert is expired. If it is, remove it and try downloading again if we haven't already.
+ *
+ * \retval 0 on success
+ * \retval 1 on failure
+ */
+static int stir_shaken_verify_validate_cert(const char *public_cert_url, char **file_path, char *dir_path, int *curl,
+	EVP_PKEY **public_key)
+{
+	if (public_key_is_expired(public_cert_url)) {
+
+		ast_debug(3, "Public cert '%s' is expired\n", public_cert_url);
+
+		remove_public_key_from_astdb(public_cert_url);
+
+		/* If this fails, then there's nothing we can do */
+		ast_free(*file_path);
+		*file_path = curl_and_check_expiration(public_cert_url, dir_path, curl);
+		if (!(*file_path)) {
+			return 1;
+		}
+	}
+
+	/* First attempt to read the key. If it fails, try downloading the file,
+	 * unless we already did. Check for expiration again */
+	*public_key = stir_shaken_read_key(*file_path, 0);
+	if (!(*public_key)) {
+
+		ast_debug(3, "Failed first read of public key file '%s'\n", *file_path);
+
+		remove_public_key_from_astdb(public_cert_url);
+
+		ast_free(*file_path);
+		*file_path = curl_and_check_expiration(public_cert_url, dir_path, curl);
+		if (!(*file_path)) {
+			return 1;
+		}
+
+		*public_key = stir_shaken_read_key(*file_path, 0);
+		if (!(*public_key)) {
+			ast_log(LOG_ERROR, "Failed to read public key from '%s'\n", *file_path);
+			remove_public_key_from_astdb(public_cert_url);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 struct ast_stir_shaken_payload *ast_stir_shaken_verify(const char *header, const char *payload, const char *signature,
 	const char *algorithm, const char *public_cert_url)
 {
+	int code = 0;
+
+	return ast_stir_shaken_verify2(header, payload, signature, algorithm, public_cert_url, &code);
+}
+
+struct ast_stir_shaken_payload *ast_stir_shaken_verify2(const char *header, const char *payload, const char *signature,
+	const char *algorithm, const char *public_cert_url, int *failure_code)
+{
 	struct ast_stir_shaken_payload *ret_payload;
 	EVP_PKEY *public_key;
 	int curl = 0;
@@ -628,28 +765,7 @@
 	RAII_VAR(char *, combined_str, NULL, ast_free);
 	size_t combined_size;
 
-	if (ast_strlen_zero(header)) {
-		ast_log(LOG_ERROR, "'header' is required for STIR/SHAKEN verification\n");
-		return NULL;
-	}
-
-	if (ast_strlen_zero(payload)) {
-		ast_log(LOG_ERROR, "'payload' is required for STIR/SHAKEN verification\n");
-		return NULL;
-	}
-
-	if (ast_strlen_zero(signature)) {
-		ast_log(LOG_ERROR, "'signature' is required for STIR/SHAKEN verification\n");
-		return NULL;
-	}
-
-	if (ast_strlen_zero(algorithm)) {
-		ast_log(LOG_ERROR, "'algorithm' is required for STIR/SHAKEN verification\n");
-		return NULL;
-	}
-
-	if (ast_strlen_zero(public_cert_url)) {
-		ast_log(LOG_ERROR, "'public_cert_url' is required for STIR/SHAKEN verification\n");
+	if (stir_shaken_verify_check_empty_strings(header, payload, signature, algorithm, public_cert_url)) {
 		return NULL;
 	}
 
@@ -663,72 +779,14 @@
 	 * {configurable) directories, we already have the storage mechanism in place.
 	 * The only thing that would be left to do is pull from the configuration.
 	 */
-	file_path = get_path_to_public_key(public_cert_url);
-	if (ast_asprintf(&dir_path, "%s/keys/%s", ast_config_AST_DATA_DIR, STIR_SHAKEN_DIR_NAME) < 0) {
+	if (stir_shaken_verify_setup_file_paths(public_cert_url, &file_path, &dir_path, &curl)) {
 		return NULL;
 	}
 
-	/* If we don't have an entry in AstDB, CURL from the provided URL */
-	if (ast_strlen_zero(file_path)) {
-		/* Remove this entry from the database, since we will be
-		 * downloading a new file anyways.
-		 */
-		remove_public_key_from_astdb(public_cert_url);
-
-		/* Go ahead and free file_path, in case anything was allocated above */
-		ast_free(file_path);
-
-		/* Download to the default path */
-		file_path = run_curl(public_cert_url, dir_path);
-		if (!file_path) {
-			return NULL;
-		}
-
-		/* Signal that we have already downloaded a new file, no reason to do it again */
-		curl = 1;
-
-		/* We should have a successful download at this point, so
-		 * add an entry to the database.
-		 */
-		add_public_key_to_astdb(public_cert_url, file_path);
-	}
-
 	/* Check to see if the cert we downloaded (or already had) is expired */
-	if (public_key_is_expired(public_cert_url)) {
-
-		ast_debug(3, "Public cert '%s' is expired\n", public_cert_url);
-
-		remove_public_key_from_astdb(public_cert_url);
-
-		/* If this fails, then there's nothing we can do */
-		ast_free(file_path);
-		file_path = curl_and_check_expiration(public_cert_url, dir_path, &curl);
-		if (!file_path) {
-			return NULL;
-		}
-	}
-
-	/* First attempt to read the key. If it fails, try downloading the file,
-	 * unless we already did. Check for expiration again */
-	public_key = stir_shaken_read_key(file_path, 0);
-	if (!public_key) {
-
-		ast_debug(3, "Failed first read of public key file '%s'\n", file_path);
-
-		remove_public_key_from_astdb(public_cert_url);
-
-		ast_free(file_path);
-		file_path = curl_and_check_expiration(public_cert_url, dir_path, &curl);
-		if (!file_path) {
-			return NULL;
-		}
-
-		public_key = stir_shaken_read_key(file_path, 0);
-		if (!public_key) {
-			ast_log(LOG_ERROR, "Failed to read public key from '%s'\n", file_path);
-			remove_public_key_from_astdb(public_cert_url);
-			return NULL;
-		}
+	if (stir_shaken_verify_validate_cert(public_cert_url, &file_path, dir_path, &curl, &public_key)) {
+		*failure_code = AST_STIR_SHAKEN_VERIFY_FAILED_TO_GET_CERT;
+		return NULL;
 	}
 
 	/* Combine the header and payload to get the original signed message: header.payload */
@@ -737,11 +795,13 @@
 	if (!combined_str) {
 		ast_log(LOG_ERROR, "Failed to allocate space for message to verify\n");
 		EVP_PKEY_free(public_key);
+		*failure_code = AST_STIR_SHAKEN_VERIFY_FAILED_MEMORY_ALLOC;
 		return NULL;
 	}
 	snprintf(combined_str, combined_size, "%s.%s", header, payload);
 	if (stir_shaken_verify_signature(combined_str, signature, public_key)) {
 		ast_log(LOG_ERROR, "Failed to verify signature\n");
+		*failure_code = AST_STIR_SHAKEN_VERIFY_FAILED_SIGNATURE_VALIDATION;
 		EVP_PKEY_free(public_key);
 		return NULL;
 	}
@@ -752,12 +812,14 @@
 	ret_payload = ast_calloc(1, sizeof(*ret_payload));
 	if (!ret_payload) {
 		ast_log(LOG_ERROR, "Failed to allocate STIR/SHAKEN payload\n");
+		*failure_code = AST_STIR_SHAKEN_VERIFY_FAILED_MEMORY_ALLOC;
 		return NULL;
 	}
 
 	ret_payload->header = ast_json_load_string(header, NULL);
 	if (!ret_payload->header) {
 		ast_log(LOG_ERROR, "Failed to create JSON from header\n");
+		*failure_code = AST_STIR_SHAKEN_VERIFY_FAILED_MEMORY_ALLOC;
 		ast_stir_shaken_payload_free(ret_payload);
 		return NULL;
 	}
@@ -765,6 +827,7 @@
 	ret_payload->payload = ast_json_load_string(payload, NULL);
 	if (!ret_payload->payload) {
 		ast_log(LOG_ERROR, "Failed to create JSON from payload\n");
+		*failure_code = AST_STIR_SHAKEN_VERIFY_FAILED_MEMORY_ALLOC;
 		ast_stir_shaken_payload_free(ret_payload);
 		return NULL;
 	}
@@ -834,15 +897,11 @@
 		goto cleanup;
 	}
 
-	/* Check the alg value for "ES256" */
+	/* Check to see if there is a value for alg */
 	val = ast_json_string_get(ast_json_object_get(obj, "alg"));
-	if (ast_strlen_zero(val)) {
-		ast_log(LOG_ERROR, "STIR/SHAKEN JWT did not have required field 'alg'\n");
-		goto cleanup;
-	}
-	if (strcmp(val, STIR_SHAKEN_ENCRYPTION_ALGORITHM)) {
-		ast_log(LOG_ERROR, "STIR/SHAKEN JWT field 'alg' did not have "
-			"required value '%s' (was '%s')\n", STIR_SHAKEN_ENCRYPTION_ALGORITHM, val);
+	if (!ast_strlen_zero(val) && strcmp(val, STIR_SHAKEN_ENCRYPTION_ALGORITHM)) {
+		/* If alg is not present that's fine; if it is and is not ES256, cleanup */
+		ast_log(LOG_ERROR, "STIR/SHAKEN JWT did not have supported type for field 'alg' (was %s)\n", val);
 		goto cleanup;
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I4ac1ecf652cd0e336006b0ca638dc826b5b1ebf7
Gerrit-Change-Number: 16497
Gerrit-PatchSet: 8
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211027/bf1b5662/attachment-0001.html>


More information about the asterisk-code-review mailing list