<p>N A has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/19897">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_stir_shaken: Fix JSON field ordering and canonicalization.<br><br>The current STIR/SHAKEN signing process is inconsistent with the<br>RFCs in a couple ways that can cause interoperability issues.<br><br>RFC8225 specifies that the keys must be ordered lexicographically, but<br>currently the fields are simply ordered according to the order<br>in which they were added to the JSON object, which is not<br>compliant with the RFC and can cause issues with some carriers.<br><br>To fix this, a JSON API is added that makes use of libjansson's<br>ability to dump a JSON object sorted by key value. This results<br>in the correct ordering in the dumped string.<br><br>Additionally, the destination number must be canonicalized, i.e.<br>have any leading + prefix removed, in order to comply with the RFCs.<br>This is now done, so the payload contains a properly formatted number.<br><br>ASTERISK-30407 #close<br><br>Change-Id: Iab76d39447c4b8cf133de85657dba02fda07f9a2<br>---<br>M include/asterisk/json.h<br>M main/json.c<br>M res/res_pjsip_stir_shaken.c<br>M res/res_stir_shaken.c<br>4 files changed, 69 insertions(+), 3 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/97/19897/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/asterisk/json.h b/include/asterisk/json.h</span><br><span>index 5edc3a9..0a52312 100644</span><br><span>--- a/include/asterisk/json.h</span><br><span>+++ b/include/asterisk/json.h</span><br><span>@@ -804,6 +804,17 @@</span><br><span>  */</span><br><span> char *ast_json_dump_string_format(struct ast_json *root, enum ast_json_encoding_format format);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief Encode a JSON value to a string, with its keys sorted.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Returned string must be freed by calling ast_json_free().</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param root JSON value.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \return String encoding of \a root.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \retval NULL on error.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+char *ast_json_dump_string_sorted(struct ast_json *root);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #define ast_json_dump_str(root, dst) ast_json_dump_str_format(root, dst, AST_JSON_COMPACT)</span><br><span> </span><br><span> /*!</span><br><span>diff --git a/main/json.c b/main/json.c</span><br><span>index 616b12e..7a571bc 100644</span><br><span>--- a/main/json.c</span><br><span>+++ b/main/json.c</span><br><span>@@ -465,6 +465,11 @@</span><br><span>      return json_dumps((json_t *)root, dump_flags(format));</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+char *ast_json_dump_string_sorted(struct ast_json *root)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  return json_dumps((json_t *)root, JSON_SORT_KEYS);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int write_to_ast_str(const char *buffer, size_t size, void *data)</span><br><span> {</span><br><span>     struct ast_str **dst = data;</span><br><span>diff --git a/res/res_pjsip_stir_shaken.c b/res/res_pjsip_stir_shaken.c</span><br><span>index 82c8df0..d4a4ca3 100644</span><br><span>--- a/res/res_pjsip_stir_shaken.c</span><br><span>+++ b/res/res_pjsip_stir_shaken.c</span><br><span>@@ -375,6 +375,7 @@</span><br><span>  RAII_VAR(char *, encoded_payload, NULL, ast_free);</span><br><span>   RAII_VAR(char *, combined_str, NULL, ast_free);</span><br><span>      size_t combined_size;</span><br><span style="color: hsl(120, 100%, 40%);">+ char *canonical_num;</span><br><span> </span><br><span>     old_identity = pjsip_msg_find_hdr_by_name(tdata->msg, &identity_str, NULL);</span><br><span>   if (old_identity) {</span><br><span>@@ -401,11 +402,30 @@</span><br><span> </span><br><span>      ast_copy_pj_str(dest_tn, &uri->user, uri->user.slen + 1);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+       /* Canonicalize the telephone number according to RFC 8224 (required by RFC 8225) */</span><br><span style="color: hsl(120, 100%, 40%);">+  canonical_num = ast_malloc(strlen(dest_tn) + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+      if (!canonical_num) {</span><br><span style="color: hsl(120, 100%, 40%);">+         return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+    } else {</span><br><span style="color: hsl(120, 100%, 40%);">+              const char *s = dest_tn;</span><br><span style="color: hsl(120, 100%, 40%);">+              char *new = canonical_num;</span><br><span style="color: hsl(120, 100%, 40%);">+            /* We're only removing characters, if anything, so the buffer is guaranteed to be large enough */</span><br><span style="color: hsl(120, 100%, 40%);">+         while (*s) {</span><br><span style="color: hsl(120, 100%, 40%);">+                  if (isdigit(*s) || *s == '#' || *s == '*') { /* Only characters allowed */</span><br><span style="color: hsl(120, 100%, 40%);">+                            *new++ = *s;</span><br><span style="color: hsl(120, 100%, 40%);">+                  }</span><br><span style="color: hsl(120, 100%, 40%);">+                     s++;</span><br><span style="color: hsl(120, 100%, 40%);">+          }</span><br><span style="color: hsl(120, 100%, 40%);">+             *new = '\0';</span><br><span style="color: hsl(120, 100%, 40%);">+          ast_debug(4, "Canonicalized telephone number %s -> %s\n", dest_tn, canonical_num);</span><br><span style="color: hsl(120, 100%, 40%);">+       }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  /* x5u (public key URL), attestation, and origid will be added by ast_stir_shaken_sign */</span><br><span>    json = ast_json_pack("{s: {s: s, s: s, s: s}, s: {s: {s: [s]}, s: {s: s}}}",</span><br><span>               "header", "alg", "ES256", "ppt", "shaken", "typ", "passport",</span><br><span style="color: hsl(0, 100%, 40%);">-             "payload", "dest", "tn", dest_tn, "orig", "tn",</span><br><span style="color: hsl(120, 100%, 40%);">+             "payload", "dest", "tn", canonical_num, "orig", "tn",</span><br><span>              session->id.number.str);</span><br><span style="color: hsl(120, 100%, 40%);">+   ast_free(canonical_num);</span><br><span>     if (!json) {</span><br><span>                 ast_log(LOG_ERROR, "Failed to allocate memory for STIR/SHAKEN JSON\n");</span><br><span>            return -1;</span><br><span>@@ -427,7 +447,9 @@</span><br><span>     }</span><br><span> </span><br><span>        payload = ast_json_object_get(json, "payload");</span><br><span style="color: hsl(0, 100%, 40%);">-       dumped_string = ast_json_dump_string(payload);</span><br><span style="color: hsl(120, 100%, 40%);">+        /* Fields must appear in lexiographic order: https://www.rfc-editor.org/rfc/rfc8588.html#section-6</span><br><span style="color: hsl(120, 100%, 40%);">+     * https://www.rfc-editor.org/rfc/rfc8225.html#section-9 */</span><br><span style="color: hsl(120, 100%, 40%);">+   dumped_string = ast_json_dump_string_sorted(payload);</span><br><span>        encoded_payload = ast_base64url_encode_string(dumped_string);</span><br><span>        ast_json_free(dumped_string);</span><br><span>        if (!encoded_payload) {</span><br><span>diff --git a/res/res_stir_shaken.c b/res/res_stir_shaken.c</span><br><span>index a4eae5b..b8ae6c4 100644</span><br><span>--- a/res/res_stir_shaken.c</span><br><span>+++ b/res/res_stir_shaken.c</span><br><span>@@ -1228,7 +1228,8 @@</span><br><span>     tmp_json = ast_json_object_get(json, "header");</span><br><span>    header = ast_json_dump_string(tmp_json);</span><br><span>     tmp_json = ast_json_object_get(json, "payload");</span><br><span style="color: hsl(0, 100%, 40%);">-      payload = ast_json_dump_string(tmp_json);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   payload = ast_json_dump_string_sorted(tmp_json);</span><br><span>     msg_len = strlen(header) + strlen(payload) + 2;</span><br><span>      msg = ast_calloc(1, msg_len);</span><br><span>        if (!msg) {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19897">change 19897</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/19897"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iab76d39447c4b8cf133de85657dba02fda07f9a2 </div>
<div style="display:none"> Gerrit-Change-Number: 19897 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: N A <asterisk@phreaknet.org> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>