[Asterisk-code-review] STIR/SHAKEN: Add Date header, dest->tn, and URL checking. (asterisk[16])

Joshua Colp asteriskteam at digium.com
Wed May 26 12:46:13 CDT 2021


Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15920 )

Change subject: STIR/SHAKEN: Add Date header, dest->tn, and URL checking.
......................................................................

STIR/SHAKEN: Add Date header, dest->tn, and URL checking.

STIR/SHAKEN requires a Date header alongside the Identity header, so
that has been added. Still on the outgoing side, we were missing the
dest->tn section of the JSON payload, so that has been added as well.
Moving to the incoming side, URL checking has been added to the public
cert URL to ensure that it starts with http.

https://wiki.asterisk.org/wiki/display/AST/OpenSIPit+2021

Change-Id: Idee5b1b5e45bc3b483b3070e46ce322dca5b3f1c
---
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip_registrar.c
M res/res_pjsip_stir_shaken.c
4 files changed, 89 insertions(+), 26 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  George Joseph: Looks good to me, approved



diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index afec9d2..3d5b504 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -948,6 +948,17 @@
 };
 
 /*!
+ * \brief Adds a Date header to the tdata, formatted like:
+ * Date: Wed, 01 Jan 2021 14:53:01 GMT
+ * \since 16.19.0
+ *
+ * \note There is no checking done to see if the header already exists
+ * before adding it. It's up to the caller of this function to determine
+ * if that needs to be done or not.
+ */
+void ast_sip_add_date_header(pjsip_tx_data *tdata);
+
+/*!
  * \brief Register a SIP service in Asterisk.
  *
  * This is more-or-less a wrapper around pjsip_endpt_register_module().
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 0998fb3..24eb884 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -2967,6 +2967,18 @@
 /*! Local host address for IPv6 (string form) */
 static char host_ip_ipv6_string[PJ_INET6_ADDRSTRLEN];
 
+void ast_sip_add_date_header(pjsip_tx_data *tdata)
+{
+	char date[256];
+	struct tm tm;
+	time_t t = time(NULL);
+
+	gmtime_r(&t, &tm);
+	strftime(date, sizeof(date), "%a, %d %b %Y %T GMT", &tm);
+
+	ast_sip_add_header(tdata, "Date", date);
+}
+
 static int register_service(void *data)
 {
 	pjsip_module **module = data;
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index c4c091f..6fe4058 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -247,19 +247,6 @@
 	return 0;
 }
 
-/*! \brief Helper function which adds a Date header to a response */
-static void registrar_add_date_header(pjsip_tx_data *tdata)
-{
-	char date[256];
-	struct tm tm;
-	time_t t = time(NULL);
-
-	gmtime_r(&t, &tm);
-	strftime(date, sizeof(date), "%a, %d %b %Y %T GMT", &tm);
-
-	ast_sip_add_header(tdata, "Date", date);
-}
-
 static const pj_str_t path_hdr_name = { "Path", 4 };
 
 static int build_path_data(pjsip_rx_data *rdata, struct ast_str **path_str)
@@ -898,7 +885,7 @@
 	ao2_cleanup(response_contact);
 
 	/* Add the date header to the response, some UAs use this to set their date and time */
-	registrar_add_date_header(tdata);
+	ast_sip_add_date_header(tdata);
 
 	ao2_callback(contacts, 0, registrar_add_contact, tdata);
 
diff --git a/res/res_pjsip_stir_shaken.c b/res/res_pjsip_stir_shaken.c
index 5a38073..ea9f35a 100644
--- a/res/res_pjsip_stir_shaken.c
+++ b/res/res_pjsip_stir_shaken.c
@@ -169,7 +169,7 @@
 		return 0;
 	}
 
-	/* Trim "info=<" to get public key URL */
+	/* 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)) {
@@ -177,6 +177,12 @@
 		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;
+	}
+
 	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);
@@ -205,17 +211,20 @@
 	return 0;
 }
 
-static void add_identity_header(const struct ast_sip_session *session, pjsip_tx_data *tdata)
+static int add_identity_header(const struct ast_sip_session *session, pjsip_tx_data *tdata)
 {
 	static const pj_str_t identity_str = { "Identity", 8 };
 	pjsip_generic_string_hdr *identity_hdr;
 	pj_str_t identity_val;
 	pjsip_fromto_hdr *old_identity;
+	pjsip_fromto_hdr *to;
+	pjsip_sip_uri *uri;
 	char *signature;
 	char *public_cert_url;
 	struct ast_json *header;
 	struct ast_json *payload;
 	char *dumped_string;
+	RAII_VAR(char *, dest_tn, NULL, ast_free);
 	RAII_VAR(struct ast_json *, json, NULL, ast_json_free);
 	RAII_VAR(struct ast_stir_shaken_payload *, ss_payload, NULL, ast_stir_shaken_payload_free);
 	RAII_VAR(char *, encoded_header, NULL, ast_free);
@@ -225,21 +234,43 @@
 
 	old_identity = pjsip_msg_find_hdr_by_name(tdata->msg, &identity_str, NULL);
 	if (old_identity) {
-		return;
+		return 0;
 	}
 
+	to = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_TO, NULL);
+	if (!to) {
+		ast_log(LOG_ERROR, "Failed to find To header while adding STIR/SHAKEN Identity header\n");
+		return -1;
+	}
+
+	uri = pjsip_uri_get_uri(to->uri);
+	if (!uri) {
+		ast_log(LOG_ERROR, "Failed to retrieve URI from To header while adding STIR/SHAKEN Identity header\n");
+		return -1;
+	}
+
+	dest_tn = ast_malloc(uri->user.slen + 1);
+	if (!dest_tn) {
+		ast_log(LOG_ERROR, "Failed to allocate memory for STIR/SHAKEN dest->tn\n");
+		return -1;
+	}
+
+	ast_copy_pj_str(dest_tn, &uri->user, uri->user.slen + 1);
+
 	/* x5u (public key URL), attestation, and origid will be added by ast_stir_shaken_sign */
-	json = ast_json_pack("{s: {s: s, s: s, s: s}, s: {s: {s: s}}}", "header", "alg", "ES256", "ppt", "shaken", "typ", "passport",
-		"payload", "orig", "tn", session->id.number.str);
+	json = ast_json_pack("{s: {s: s, s: s, s: s}, s: {s: {s: s}, s: {s: s}}}",
+		"header", "alg", "ES256", "ppt", "shaken", "typ", "passport",
+		"payload", "dest", "tn", dest_tn, "orig", "tn",
+		session->id.number.str);
 	if (!json) {
 		ast_log(LOG_ERROR, "Failed to allocate memory for STIR/SHAKEN JSON\n");
-		return;
+		return -1;
 	}
 
 	ss_payload = ast_stir_shaken_sign(json);
 	if (!ss_payload) {
 		ast_log(LOG_ERROR, "Failed to allocate memory for STIR/SHAKEN payload\n");
-		return;
+		return -1;
 	}
 
 	header = ast_json_object_get(json, "header");
@@ -248,7 +279,7 @@
 	ast_json_free(dumped_string);
 	if (!encoded_header) {
 		ast_log(LOG_ERROR, "Failed to encode STIR/SHAKEN header\n");
-		return;
+		return -1;
 	}
 
 	payload = ast_json_object_get(json, "payload");
@@ -257,7 +288,7 @@
 	ast_json_free(dumped_string);
 	if (!encoded_payload) {
 		ast_log(LOG_ERROR, "Failed to encode STIR/SHAKEN payload\n");
-		return;
+		return -1;
 	}
 
 	signature = (char *)ast_stir_shaken_payload_get_signature(ss_payload);
@@ -272,7 +303,7 @@
 	combined_str = ast_calloc(1, combined_size);
 	if (!combined_str) {
 		ast_log(LOG_ERROR, "Failed to allocate memory for STIR/SHAKEN identity string\n");
-		return;
+		return -1;
 	}
 	snprintf(combined_str, combined_size, "%s.%s.%s;info=<%s>alg=%s;ppt=%s", encoded_header,
 		encoded_payload, signature, public_cert_url, STIR_SHAKEN_ENCRYPTION_ALGORITHM, STIR_SHAKEN_PPT);
@@ -281,10 +312,26 @@
 	identity_hdr = pjsip_generic_string_hdr_create(tdata->pool, &identity_str, &identity_val);
 	if (!identity_hdr) {
 		ast_log(LOG_ERROR, "Failed to create STIR/SHAKEN Identity header\n");
-		return;
+		return -1;
 	}
 
 	pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr *)identity_hdr);
+
+	return 0;
+}
+
+static void add_date_header(const struct ast_sip_session *session, pjsip_tx_data *tdata)
+{
+	static const pj_str_t date_str = { "Date", 4 };
+	pjsip_fromto_hdr *old_date;
+
+	old_date = pjsip_msg_find_hdr_by_name(tdata->msg, &date_str, NULL);
+	if (old_date) {
+		ast_debug(3, "Found old STIR/SHAKEN date header, no need to add one\n");
+		return;
+	}
+
+	ast_sip_add_date_header(tdata);
 }
 
 static void stir_shaken_outgoing_request(struct ast_sip_session *session, pjsip_tx_data *tdata)
@@ -297,7 +344,13 @@
 		return;
 	}
 
-	add_identity_header(session, tdata);
+	/* If adding the Identity header fails for some reason, there's no point
+	 * adding the Date header.
+	 */
+	if ((add_identity_header(session, tdata)) != 0) {
+		return;
+	}
+	add_date_header(session, tdata);
 }
 
 static struct ast_sip_session_supplement stir_shaken_supplement = {

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Idee5b1b5e45bc3b483b3070e46ce322dca5b3f1c
Gerrit-Change-Number: 15920
Gerrit-PatchSet: 4
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-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210526/f1bafe23/attachment-0001.html>


More information about the asterisk-code-review mailing list