res_pjsip_stir_shaken: Fix JSON field ordering and disallowed TN characters.

The current STIR/SHAKEN signing process is inconsistent with the
RFCs in a couple ways that can cause interoperability issues.

RFC8225 specifies that the keys must be ordered lexicographically, but
currently the fields are simply ordered according to the order
in which they were added to the JSON object, which is not
compliant with the RFC and can cause issues with some carriers.

To fix this, we now leverage libjansson's ability to dump a JSON
object sorted by key value, yielding the correct field ordering.

Additionally, telephone numbers must have any leading + prefix removed
and must not contain characters outside of 0-9, *, and # in order
to comply with the RFCs. Numbers are now properly formatted as such.

ASTERISK-30407 #close

Change-Id: Iab76d39447c4b8cf133de85657dba02fda07f9a2

diff --git a/include/asterisk/json.h b/include/asterisk/json.h
index 5edc3a9754ebc5952ff05d5ecebcca9ee523c942..5b2d61422d40d4de9698d358816d1e1bce830062 100644 (file)
--- a/include/asterisk/json.h
+++ b/include/asterisk/json.h
@@ -777,6 +777,8 @@ enum ast_json_encoding_format
        AST_JSON_COMPACT,
        /*! Formatted for human readability */
        AST_JSON_PRETTY,
+       /*! Keys sorted alphabetically */
+       AST_JSON_SORTED,
 };
 
 /*!
@@ -804,6 +806,17 @@ enum ast_json_encoding_format
  */
 char *ast_json_dump_string_format(struct ast_json *root, enum ast_json_encoding_format format);
 
+/*!
+ * \brief Encode a JSON value to a string, with its keys sorted.
+ *
+ * Returned string must be freed by calling ast_json_free().
+ *
+ * \param root JSON value.
+ * \return String encoding of \a root.
+ * \retval NULL on error.
+ */
+#define ast_json_dump_string_sorted(root) ast_json_dump_string_format(root, AST_JSON_SORTED)
+
 #define ast_json_dump_str(root, dst) ast_json_dump_str_format(root, dst, AST_JSON_COMPACT)
 
 /*!

diff --git a/main/json.c b/main/json.c
index 616b12e67fd790b966ed6ae53f347b288b772d74..afb653a229a3217d618a2a0eb9971b8c18ae0dd2 100644 (file)
--- a/main/json.c
+++ b/main/json.c
@@ -456,8 +456,19 @@ int ast_json_object_iter_set(struct ast_json *object, struct ast_json_iter *iter
  */
 static size_t dump_flags(enum ast_json_encoding_format format)
 {
-       return format == AST_JSON_PRETTY ?
-               JSON_INDENT(2) | JSON_PRESERVE_ORDER : JSON_COMPACT;
+       size_t jansson_dump_flags;
+
+       if (format & AST_JSON_PRETTY) {
+               jansson_dump_flags = JSON_INDENT(2);
+       } else {
+               jansson_dump_flags = JSON_COMPACT;
+       }
+
+       if (format & AST_JSON_SORTED) {
+               jansson_dump_flags |= JSON_SORT_KEYS;
+       }
+
+       return jansson_dump_flags;
 }
 
 char *ast_json_dump_string_format(struct ast_json *root, enum ast_json_encoding_format format)

diff --git a/res/ari/cli.c b/res/ari/cli.c
index 9d0eb3099bc64f6251dadca20e32a17831bacd48..f9d9cecfb78d216c6d16e3bfc482847df3134ce6 100644 (file)
--- a/res/ari/cli.c
+++ b/res/ari/cli.c
@@ -60,13 +60,10 @@ static char *ari_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
        ast_cli(a->fd, "ARI Status:\n");
        ast_cli(a->fd, "Enabled: %s\n", AST_CLI_YESNO(conf->general->enabled));
        ast_cli(a->fd, "Output format: ");
-       switch (conf->general->format) {
-       case AST_JSON_COMPACT:
-               ast_cli(a->fd, "compact");
-               break;
-       case AST_JSON_PRETTY:
+       if (conf->general->format & AST_JSON_PRETTY) {
                ast_cli(a->fd, "pretty");
-               break;
+       } else {
+               ast_cli(a->fd, "compact");
        }
        ast_cli(a->fd, "\n");
        ast_cli(a->fd, "Auth realm: %s\n", conf->general->auth_realm);

diff --git a/res/res_pjsip_stir_shaken.c b/res/res_pjsip_stir_shaken.c
index 82c8df0f8b95753d7b63ebce945c0af76475ae45..1089e60a7c476b00301974b519f48f3e5df2d488 100644 (file)
--- a/res/res_pjsip_stir_shaken.c
+++ b/res/res_pjsip_stir_shaken.c
@@ -399,7 +399,22 @@ static int add_identity_header(const struct ast_sip_session *session, pjsip_tx_d
                return -1;
        }
 
-       ast_copy_pj_str(dest_tn, &uri->user, uri->user.slen + 1);
+       /* 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_debug(4, "Canonicalized telephone number %.*s -> %s\n", (int) uri->user.slen, uri->user.ptr, dest_tn);
+       }
 
        /* 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]}, s: {s: s}}}",
@@ -427,7 +442,9 @@ static int add_identity_header(const struct ast_sip_session *session, pjsip_tx_d
        }
 
        payload = ast_json_object_get(json, "payload");
-       dumped_string = ast_json_dump_string(payload);
+       /* Fields must appear in lexiographic order: https://www.rfc-editor.org/rfc/rfc8588.html#section-6
+        * https://www.rfc-editor.org/rfc/rfc8225.html#section-9 */
+       dumped_string = ast_json_dump_string_sorted(payload);
        encoded_payload = ast_base64url_encode_string(dumped_string);
        ast_json_free(dumped_string);
        if (!encoded_payload) {

diff --git a/res/res_stir_shaken.c b/res/res_stir_shaken.c
index a4eae5bcc00e255eda422e5773408021bebee718..efb8be957df4bd1b3b4faa55aa8e9c33cd10a977 100644 (file)
--- a/res/res_stir_shaken.c
+++ b/res/res_stir_shaken.c
@@ -1228,7 +1228,8 @@ struct ast_stir_shaken_payload *ast_stir_shaken_sign(struct ast_json *json)
        tmp_json = ast_json_object_get(json, "header");
        header = ast_json_dump_string(tmp_json);
        tmp_json = ast_json_object_get(json, "payload");
-       payload = ast_json_dump_string(tmp_json);
+
+       payload = ast_json_dump_string_sorted(tmp_json);
        msg_len = strlen(header) + strlen(payload) + 2;
        msg = ast_calloc(1, msg_len);
        if (!msg) {
@@ -1661,7 +1662,7 @@ AST_TEST_DEFINE(test_stir_shaken_verify)
        tmp_json = ast_json_object_get(json, "header");
        header = ast_json_dump_string(tmp_json);
        tmp_json = ast_json_object_get(json, "payload");
-       payload = ast_json_dump_string(tmp_json);
+       payload = ast_json_dump_string_sorted(tmp_json);
 
        /* Test empty header parameter */
        returned_payload = ast_stir_shaken_verify("", payload, (const char *)signed_payload->signature,