mirror of
				https://github.com/asterisk/asterisk.git
				synced 2025-10-31 02:37:10 +00:00 
			
		
		
		
	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
(cherry picked from commit 1bbcb98558)
			
			
This commit is contained in:
		
				
					committed by
					
						 Asterisk Development Team
						Asterisk Development Team
					
				
			
			
				
	
			
			
			
						parent
						
							0e7852c427
						
					
				
				
					commit
					77384b93a1
				
			| @@ -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); | ||||
|   | ||||
| @@ -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) { | ||||
|   | ||||
| @@ -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, | ||||
|   | ||||
		Reference in New Issue
	
	Block a user