stir_shaken: Fix memory leak, typo in config, tn canonicalization

* Fixed possible memory leak in tn_config:tn_get_etn() where we
weren't releasing etn if tn or eprofile were null.
* We now canonicalize TNs before using them for lookups or adding
them to Identity headers.
* Fixed a typo in stir_shaken.conf.sample.

Resolves: #716

configs/samples/stir_shaken.conf.sample

@@ -413,7 +413,7 @@ Must be set to "profile"
 
 Default: none
 
--- endpoint_behhavior--------------------------------------------------
+-- endpoint_behavior--------------------------------------------------
 Actions to be performed for endpoints referencing this profile.
 Must be one of:
 - off -

res/res_pjsip_stir_shaken.c

@@ -360,23 +360,7 @@ static char *get_dest_tn(pjsip_tx_data *tdata, const char *tag)
 			"%s: Failed to allocate memory for dest_tn\n", tag);
 	}
 
-	/* Remove everything except 0-9, *, and # in telephone number according to RFC 8224
-	 * (required by RFC 8225 as part of canonicalization) */
-	{
-		int i;
-		const char *s = uri->user.ptr;
-		char *new_tn = dest_tn;
-		/* We're only removing characters, if anything, so the buffer is guaranteed to be large enough */
-		for (i = 0; i < uri->user.slen; i++) {
-			if (isdigit(*s) || *s == '#' || *s == '*') { /* Only characters allowed */
-				*new_tn++ = *s;
-			}
-			s++;
-		}
-		*new_tn = '\0';
-		ast_trace(2, "Canonicalized telephone number " PJSTR_PRINTF_SPEC " -> %s\n",
-			PJSTR_PRINTF_VAR(uri->user), dest_tn);
-	}
+	ast_copy_pj_str(dest_tn, &uri->user, uri->user.slen + 1);
 
 	SCOPE_EXIT_RTN_VALUE(dest_tn, "%s: Done\n", tag);
 }

res/res_stir_shaken/attestation.c

@@ -72,14 +72,17 @@ enum ast_stir_shaken_as_response_code
 	RAII_VAR(struct profile_cfg *, eprofile, NULL, ao2_cleanup);
 	RAII_VAR(struct attestation_cfg *, as_cfg, NULL, ao2_cleanup);
 	RAII_VAR(struct tn_cfg *, etn, NULL, ao2_cleanup);
+	RAII_VAR(char *, canon_dest_tn , canonicalize_tn_alloc(dest_tn), ast_free);
+	RAII_VAR(char *, canon_orig_tn , canonicalize_tn_alloc(orig_tn), ast_free);
+
 	SCOPE_ENTER(3, "%s: Enter\n", tag);
 
-	if (ast_strlen_zero(orig_tn)) {
+	if (!canon_orig_tn) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_AS_INVALID_ARGUMENTS,
 			LOG_ERROR, "%s: Must provide caller_id/orig_tn\n", tag);
 	}
 
-	if (ast_strlen_zero(dest_tn)) {
+	if (!canon_dest_tn) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_AS_INVALID_ARGUMENTS,
 			LOG_ERROR, "%s: Must provide dest_tn\n", tag);
 	}
@@ -117,10 +120,10 @@ enum ast_stir_shaken_as_response_code
 			"%s: Disabled by profile\n", tag);
 	}
 
-	etn = tn_get_etn(orig_tn, eprofile);
+	etn = tn_get_etn(canon_orig_tn, eprofile);
 	if (!etn) {
 		SCOPE_EXIT_RTN_VALUE(AST_STIR_SHAKEN_AS_DISABLED,
-			"%s: No tn for orig_tn '%s'\n", tag, orig_tn);
+			"%s: No tn for orig_tn '%s'\n", tag, canon_orig_tn);
 	}
 
 	/* We don't need eprofile or as_cfg anymore so let's clean em up */
@@ -140,13 +143,13 @@ enum ast_stir_shaken_as_response_code
 	if (ast_strlen_zero(etn->acfg_common.public_cert_url)) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_AS_NO_PUBLIC_CERT_URL_AVAIL,
 			LOG_ERROR, "%s: No public cert url in tn %s, profile or attestation objects\n",
-			tag, orig_tn);
+			tag, canon_orig_tn);
 	}
 
 	if (etn->acfg_common.raw_key_length == 0) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_AS_NO_PRIVATE_KEY_AVAIL,
 			LOG_ERROR, "%s: No private key in tn %s, profile or attestation objects\n",
-			orig_tn, tag);
+			canon_orig_tn, tag);
 	}
 
 	ctx = ao2_alloc_options(sizeof(*ctx), ctx_destructor,
@@ -166,12 +169,12 @@ enum ast_stir_shaken_as_response_code
 			LOG_ERROR, "%s: Unable to allocate memory for ctx\n", tag);
 	}
 
-	if (ast_string_field_set(ctx, orig_tn, orig_tn) != 0) {
+	if (ast_string_field_set(ctx, orig_tn, canon_orig_tn) != 0) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_AS_INTERNAL_ERROR,
 			LOG_ERROR, "%s: Unable to allocate memory for ctx\n", tag);
 	}
 
-	if (ast_string_field_set(ctx, dest_tn, dest_tn)) {
+	if (ast_string_field_set(ctx, dest_tn, canon_dest_tn)) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_AS_INTERNAL_ERROR,
 			LOG_ERROR, "%s: Unable to allocate memory for ctx\n", tag);
 	}

res/res_stir_shaken/common_config.c

@@ -351,3 +351,41 @@ int common_config_load(void)
 	SCOPE_EXIT_RTN_VALUE(AST_MODULE_LOAD_SUCCESS, "Stir Shaken Load Done\n");
 }
 
+
+/* Remove everything except 0-9, *, and # in telephone number according to RFC 8224
+ * (required by RFC 8225 as part of canonicalization) */
+char *canonicalize_tn(const char *tn, char *dest_tn)
+{
+	int i;
+	const char *s = tn;
+	size_t len = tn ? strlen(tn) : 0;
+	char *new_tn = dest_tn;
+	SCOPE_ENTER(3, "tn: %s\n", S_OR(tn, "(null)"));
+
+	if (ast_strlen_zero(tn)) {
+		*dest_tn = '\0';
+		SCOPE_EXIT_RTN_VALUE(NULL, "Empty TN\n");
+	}
+
+	if (!dest_tn) {
+		SCOPE_EXIT_RTN_VALUE(NULL, "No destination buffer\n");
+	}
+
+	for (i = 0; i < len; i++) {
+		if (isdigit(*s) || *s == '#' || *s == '*') { /* Only characters allowed */
+			*new_tn++ = *s;
+		}
+		s++;
+	}
+	*new_tn = '\0';
+	SCOPE_EXIT_RTN_VALUE(dest_tn, "Canonicalized '%s' -> '%s'\n", tn, dest_tn);
+}
+
+char *canonicalize_tn_alloc(const char *tn)
+{
+	char *canon_tn = ast_strlen_zero(tn) ? NULL : ast_malloc(strlen(tn) + 1);
+	if (!canon_tn) {
+		return NULL;
+	}
+	return canonicalize_tn(tn, canon_tn);
+}

res/res_stir_shaken/common_config.h

@@ -564,5 +564,23 @@ int config_object_cli_show(void *obj, void *arg, void *data, int flags);
  */
 char *config_object_tab_complete_name(const char *word, struct ao2_container *container);
 
+/*!
+ * \brief Canonicalize a TN
+ *
+ * \param tn TN to canonicalize
+ * \param dest_tn Pointer to destination buffer to receive the new TN
+ *
+ * \retval dest_tn or NULL on failure
+ */
+char *canonicalize_tn(const char *tn, char *dest_tn);
+
+/*!
+ * \brief Canonicalize a TN into nre buffer
+ *
+ * \param tn TN to canonicalize
+ *
+ * \retval dest_tn (which must be freed with ast_free) or NULL on failure
+ */
+char *canonicalize_tn_alloc(const char *tn);
 
 #endif /* COMMON_CONFIG_H_ */

res/res_stir_shaken/tn_config.c

@@ -122,6 +122,7 @@ struct tn_cfg *tn_get_etn(const char *id, struct profile_cfg *eprofile)
 	int rc = 0;
 
 	if (!tn || !eprofile || !etn) {
+		ao2_cleanup(etn);
 		return NULL;
 	}
 

res/res_stir_shaken/verification.c

@@ -655,6 +655,8 @@ enum ast_stir_shaken_vs_response_code
 	RAII_VAR(struct ast_stir_shaken_vs_ctx *, ctx, NULL, ao2_cleanup);
 	RAII_VAR(struct profile_cfg *, profile, NULL, ao2_cleanup);
 	RAII_VAR(struct verification_cfg *, vs, NULL, ao2_cleanup);
+	RAII_VAR(char *, canon_caller_id , canonicalize_tn_alloc(caller_id), ast_free);
+
 	const char *t = S_OR(tag, S_COR(chan, ast_channel_name(chan), ""));
 	SCOPE_ENTER(3, "%s: Enter\n", t);
 
@@ -663,7 +665,7 @@ enum ast_stir_shaken_vs_response_code
 			LOG_ERROR, "%s: Must provide tag\n", t);
 	}
 
-	if (ast_strlen_zero(caller_id)) {
+	if (ast_strlen_zero(canon_caller_id)) {
 		SCOPE_EXIT_LOG_RTN_VALUE(AST_STIR_SHAKEN_VS_INVALID_ARGUMENTS,
 		LOG_ERROR, "%s: Must provide caller_id\n", t);
 	}
@@ -705,7 +707,7 @@ enum ast_stir_shaken_vs_response_code
 	}
 
 	ctx->chan = chan;
-	if (ast_string_field_set(ctx, caller_id, caller_id) != 0) {
+	if (ast_string_field_set(ctx, caller_id, canon_caller_id) != 0) {
 		SCOPE_EXIT_RTN_VALUE(AST_STIR_SHAKEN_VS_INTERNAL_ERROR);
 	}