Skip to content

Commit 0119f3a

Browse files
InterLinked1Friendly Automation
authored and
Friendly Automation
committed
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
1 parent ecf49ff commit 0119f3a

File tree

5 files changed

+51
-12
lines changed

5 files changed

+51
-12
lines changed

include/asterisk/json.h

+13
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,8 @@ enum ast_json_encoding_format
777777
AST_JSON_COMPACT,
778778
/*! Formatted for human readability */
779779
AST_JSON_PRETTY,
780+
/*! Keys sorted alphabetically */
781+
AST_JSON_SORTED,
780782
};
781783

782784
/*!
@@ -804,6 +806,17 @@ enum ast_json_encoding_format
804806
*/
805807
char *ast_json_dump_string_format(struct ast_json *root, enum ast_json_encoding_format format);
806808

809+
/*!
810+
* \brief Encode a JSON value to a string, with its keys sorted.
811+
*
812+
* Returned string must be freed by calling ast_json_free().
813+
*
814+
* \param root JSON value.
815+
* \return String encoding of \a root.
816+
* \retval NULL on error.
817+
*/
818+
#define ast_json_dump_string_sorted(root) ast_json_dump_string_format(root, AST_JSON_SORTED)
819+
807820
#define ast_json_dump_str(root, dst) ast_json_dump_str_format(root, dst, AST_JSON_COMPACT)
808821

809822
/*!

main/json.c

+13-2
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,19 @@ int ast_json_object_iter_set(struct ast_json *object, struct ast_json_iter *iter
456456
*/
457457
static size_t dump_flags(enum ast_json_encoding_format format)
458458
{
459-
return format == AST_JSON_PRETTY ?
460-
JSON_INDENT(2) | JSON_PRESERVE_ORDER : JSON_COMPACT;
459+
size_t jansson_dump_flags;
460+
461+
if (format & AST_JSON_PRETTY) {
462+
jansson_dump_flags = JSON_INDENT(2);
463+
} else {
464+
jansson_dump_flags = JSON_COMPACT;
465+
}
466+
467+
if (format & AST_JSON_SORTED) {
468+
jansson_dump_flags |= JSON_SORT_KEYS;
469+
}
470+
471+
return jansson_dump_flags;
461472
}
462473

463474
char *ast_json_dump_string_format(struct ast_json *root, enum ast_json_encoding_format format)

res/ari/cli.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,10 @@ static char *ari_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
6060
ast_cli(a->fd, "ARI Status:\n");
6161
ast_cli(a->fd, "Enabled: %s\n", AST_CLI_YESNO(conf->general->enabled));
6262
ast_cli(a->fd, "Output format: ");
63-
switch (conf->general->format) {
64-
case AST_JSON_COMPACT:
65-
ast_cli(a->fd, "compact");
66-
break;
67-
case AST_JSON_PRETTY:
63+
if (conf->general->format & AST_JSON_PRETTY) {
6864
ast_cli(a->fd, "pretty");
69-
break;
65+
} else {
66+
ast_cli(a->fd, "compact");
7067
}
7168
ast_cli(a->fd, "\n");
7269
ast_cli(a->fd, "Auth realm: %s\n", conf->general->auth_realm);

res/res_pjsip_stir_shaken.c

+19-2
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,22 @@ static int add_identity_header(const struct ast_sip_session *session, pjsip_tx_d
399399
return -1;
400400
}
401401

402-
ast_copy_pj_str(dest_tn, &uri->user, uri->user.slen + 1);
402+
/* Remove everything except 0-9, *, and # in telephone number according to RFC 8224
403+
* (required by RFC 8225 as part of canonicalization) */
404+
{
405+
int i;
406+
const char *s = uri->user.ptr;
407+
char *new_tn = dest_tn;
408+
/* We're only removing characters, if anything, so the buffer is guaranteed to be large enough */
409+
for (i = 0; i < uri->user.slen; i++) {
410+
if (isdigit(*s) || *s == '#' || *s == '*') { /* Only characters allowed */
411+
*new_tn++ = *s;
412+
}
413+
s++;
414+
}
415+
*new_tn = '\0';
416+
ast_debug(4, "Canonicalized telephone number %.*s -> %s\n", (int) uri->user.slen, uri->user.ptr, dest_tn);
417+
}
403418

404419
/* x5u (public key URL), attestation, and origid will be added by ast_stir_shaken_sign */
405420
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
427442
}
428443

429444
payload = ast_json_object_get(json, "payload");
430-
dumped_string = ast_json_dump_string(payload);
445+
/* Fields must appear in lexiographic order: https://www.rfc-editor.org/rfc/rfc8588.html#section-6
446+
* https://www.rfc-editor.org/rfc/rfc8225.html#section-9 */
447+
dumped_string = ast_json_dump_string_sorted(payload);
431448
encoded_payload = ast_base64url_encode_string(dumped_string);
432449
ast_json_free(dumped_string);
433450
if (!encoded_payload) {

res/res_stir_shaken.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -1228,7 +1228,8 @@ struct ast_stir_shaken_payload *ast_stir_shaken_sign(struct ast_json *json)
12281228
tmp_json = ast_json_object_get(json, "header");
12291229
header = ast_json_dump_string(tmp_json);
12301230
tmp_json = ast_json_object_get(json, "payload");
1231-
payload = ast_json_dump_string(tmp_json);
1231+
1232+
payload = ast_json_dump_string_sorted(tmp_json);
12321233
msg_len = strlen(header) + strlen(payload) + 2;
12331234
msg = ast_calloc(1, msg_len);
12341235
if (!msg) {
@@ -1661,7 +1662,7 @@ AST_TEST_DEFINE(test_stir_shaken_verify)
16611662
tmp_json = ast_json_object_get(json, "header");
16621663
header = ast_json_dump_string(tmp_json);
16631664
tmp_json = ast_json_object_get(json, "payload");
1664-
payload = ast_json_dump_string(tmp_json);
1665+
payload = ast_json_dump_string_sorted(tmp_json);
16651666

16661667
/* Test empty header parameter */
16671668
returned_payload = ast_stir_shaken_verify("", payload, (const char *)signed_payload->signature,

0 commit comments

Comments
 (0)