<p>N A has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/20054">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_stir_shaken: Fix JSON field ordering and disallowed TN characters.<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, we now leverage libjansson's ability to dump a JSON<br>object sorted by key value, yielding the correct field ordering.<br><br>Additionally, telephone numbers must have any leading + prefix removed<br>and must not contain characters outside of 0-9, *, and # in order<br>to comply with the RFCs. Numbers are now properly formatted as such.<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/ari/cli.c<br>M res/res_pjsip_stir_shaken.c<br>M res/res_stir_shaken.c<br>5 files changed, 77 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/54/20054/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..5b2d614 100644</span><br><span>--- a/include/asterisk/json.h</span><br><span>+++ b/include/asterisk/json.h</span><br><span>@@ -777,6 +777,8 @@</span><br><span> AST_JSON_COMPACT,</span><br><span> /*! Formatted for human readability */</span><br><span> AST_JSON_PRETTY,</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Keys sorted alphabetically */</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_JSON_SORTED,</span><br><span> };</span><br><span> </span><br><span> /*!</span><br><span>@@ -804,6 +806,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%);">+#define ast_json_dump_string_sorted(root) ast_json_dump_string_format(root, AST_JSON_SORTED)</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..afb653a 100644</span><br><span>--- a/main/json.c</span><br><span>+++ b/main/json.c</span><br><span>@@ -456,8 +456,19 @@</span><br><span> */</span><br><span> static size_t dump_flags(enum ast_json_encoding_format format)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- return format == AST_JSON_PRETTY ?</span><br><span style="color: hsl(0, 100%, 40%);">- JSON_INDENT(2) | JSON_PRESERVE_ORDER : JSON_COMPACT;</span><br><span style="color: hsl(120, 100%, 40%);">+ size_t jansson_dump_flags;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (format & AST_JSON_PRETTY) {</span><br><span style="color: hsl(120, 100%, 40%);">+ jansson_dump_flags = JSON_INDENT(2);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ jansson_dump_flags = JSON_COMPACT;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (format & AST_JSON_SORTED) {</span><br><span style="color: hsl(120, 100%, 40%);">+ jansson_dump_flags |= JSON_SORT_KEYS;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return jansson_dump_flags;</span><br><span> }</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>diff --git a/res/ari/cli.c b/res/ari/cli.c</span><br><span>index 9d0eb30..f9d9cec 100644</span><br><span>--- a/res/ari/cli.c</span><br><span>+++ b/res/ari/cli.c</span><br><span>@@ -60,13 +60,10 @@</span><br><span> ast_cli(a->fd, "ARI Status:\n");</span><br><span> ast_cli(a->fd, "Enabled: %s\n", AST_CLI_YESNO(conf->general->enabled));</span><br><span> ast_cli(a->fd, "Output format: ");</span><br><span style="color: hsl(0, 100%, 40%);">- switch (conf->general->format) {</span><br><span style="color: hsl(0, 100%, 40%);">- case AST_JSON_COMPACT:</span><br><span style="color: hsl(0, 100%, 40%);">- ast_cli(a->fd, "compact");</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case AST_JSON_PRETTY:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (conf->general->format & AST_JSON_PRETTY) {</span><br><span> ast_cli(a->fd, "pretty");</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_cli(a->fd, "compact");</span><br><span> }</span><br><span> ast_cli(a->fd, "\n");</span><br><span> ast_cli(a->fd, "Auth realm: %s\n", conf->general->auth_realm);</span><br><span>diff --git a/res/res_pjsip_stir_shaken.c b/res/res_pjsip_stir_shaken.c</span><br><span>index 82c8df0..1089e60 100644</span><br><span>--- a/res/res_pjsip_stir_shaken.c</span><br><span>+++ b/res/res_pjsip_stir_shaken.c</span><br><span>@@ -399,7 +399,22 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_copy_pj_str(dest_tn, &uri->user, uri->user.slen + 1);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Remove everything except 0-9, *, and # in telephone number according to RFC 8224</span><br><span style="color: hsl(120, 100%, 40%);">+ * (required by RFC 8225 as part of canonicalization) */</span><br><span style="color: hsl(120, 100%, 40%);">+ {</span><br><span style="color: hsl(120, 100%, 40%);">+ int i;</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *s = uri->user.ptr;</span><br><span style="color: hsl(120, 100%, 40%);">+ char *new_tn = dest_tn;</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%);">+ for (i = 0; i < uri->user.slen; i++) {</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_tn++ = *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_tn = '\0';</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_debug(4, "Canonicalized telephone number %.*s -> %s\n", (int) uri->user.slen, uri->user.ptr, dest_tn);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </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>@@ -427,7 +442,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..efb8be9 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>@@ -1661,7 +1662,7 @@</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%);">+ payload = ast_json_dump_string_sorted(tmp_json);</span><br><span> </span><br><span> /* Test empty header parameter */</span><br><span> returned_payload = ast_stir_shaken_verify("", payload, (const char *)signed_payload->signature,</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/20054">change 20054</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/+/20054"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: Iab76d39447c4b8cf133de85657dba02fda07f9a2 </div>
<div style="display:none"> Gerrit-Change-Number: 20054 </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>