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

diff --git a/configs/samples/stir_shaken.conf.sample b/configs/samples/stir_shaken.conf.sample
index 5a15d0f4d050c4eab80e29697e7652f5b6973646..c7ee89230e8e1d0568ec10e22cac4fcf4abb2a90 100644 (file)
--- a/configs/samples/stir_shaken.conf.sample
+++ b/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 -

diff --git a/res/res_pjsip_stir_shaken.c b/res/res_pjsip_stir_shaken.c
index fa2103a8574318f47346b411c56337c2d0b9831d..f64152f3a23319672ab25703e9f5d41fa3693982 100644 (file)
--- a/res/res_pjsip_stir_shaken.c
+++ b/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);
 }

diff --git a/res/res_stir_shaken/attestation.c b/res/res_stir_shaken/attestation.c
index 0583fdbaa015f8eee871c28974d3eb9612558155..91e688d16d293daa64bacd7062388a96952ac642 100644 (file)
--- a/res/res_stir_shaken/attestation.c
+++ b/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);
        }

diff --git a/res/res_stir_shaken/common_config.c b/res/res_stir_shaken/common_config.c
index f7446b7cea446df7ecac3ec6dd08d1f9963c8166..f753b41ca60ac11cea46d86bde466e0c98f8c2f8 100644 (file)
--- a/res/res_stir_shaken/common_config.c
+++ b/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);
+}

diff --git a/res/res_stir_shaken/common_config.h b/res/res_stir_shaken/common_config.h
index c0e56b9a913e8ae8ecfa33da53959a5d4766de02..b4154757c3620280820ade52dfe52057f27e632a 100644 (file)
--- a/res/res_stir_shaken/common_config.h
+++ b/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_ */

diff --git a/res/res_stir_shaken/tn_config.c b/res/res_stir_shaken/tn_config.c
index 123b0519d84aca8f9572c65396d6022ca87556c2..89b9f47be57aec9ee07896ac510225fd4765ad5c 100644 (file)
--- a/res/res_stir_shaken/tn_config.c
+++ b/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;
        }
 

diff --git a/res/res_stir_shaken/verification.c b/res/res_stir_shaken/verification.c
index f8d21cfdf460d9f2cd7ecc0b9e2e73310d4bcfdd..4be93a097556995f730ea1d70848de85a5dc5a2f 100644 (file)
--- a/res/res_stir_shaken/verification.c
+++ b/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);
        }