From 74711fd7119cd76abe6b8b1dc7b8f5dd744d9c2f Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Fri, 27 Sep 2024 14:34:29 +0200 Subject: [PATCH] Simplified computation of filter paths + fix for discarded filters --- src_features/signMessageEIP712/commands_712.c | 13 +++---- src_features/signMessageEIP712/filtering.c | 34 ++++++++++++------- src_features/signMessageEIP712/filtering.h | 10 +++--- src_features/signMessageEIP712/path.c | 29 ---------------- src_features/signMessageEIP712/path.h | 1 - src_features/signMessageEIP712/ui_logic.c | 5 ++- src_features/signMessageEIP712/ui_logic.h | 2 +- 7 files changed, 37 insertions(+), 57 deletions(-) diff --git a/src_features/signMessageEIP712/commands_712.c b/src_features/signMessageEIP712/commands_712.c index ad7f6df341..dd3485a7e5 100644 --- a/src_features/signMessageEIP712/commands_712.c +++ b/src_features/signMessageEIP712/commands_712.c @@ -184,6 +184,7 @@ uint16_t handle_eip712_filtering(uint8_t p1, uint32_t *flags) { bool ret = true; bool reply_apdu = true; + uint32_t path_crc = 0; if (eip712_context == NULL) { apdu_reply(false); @@ -211,20 +212,20 @@ uint16_t handle_eip712_filtering(uint8_t p1, break; #ifdef HAVE_TRUSTED_NAME case P2_FILT_CONTRACT_NAME: - ret = filtering_trusted_name(cdata, length, p1 == 1); + ret = filtering_trusted_name(cdata, length, p1 == 1, &path_crc); break; #endif case P2_FILT_DATE_TIME: - ret = filtering_date_time(cdata, length, p1 == 1); + ret = filtering_date_time(cdata, length, p1 == 1, &path_crc); break; case P2_FILT_AMOUNT_JOIN_TOKEN: - ret = filtering_amount_join_token(cdata, length, p1 == 1); + ret = filtering_amount_join_token(cdata, length, p1 == 1, &path_crc); break; case P2_FILT_AMOUNT_JOIN_VALUE: - ret = filtering_amount_join_value(cdata, length, p1 == 1); + ret = filtering_amount_join_value(cdata, length, p1 == 1, &path_crc); break; case P2_FILT_RAW_FIELD: - ret = filtering_raw_field(cdata, length, p1 == 1); + ret = filtering_raw_field(cdata, length, p1 == 1, &path_crc); break; default: PRINTF("Unknown P2 0x%x\n", p2); @@ -232,7 +233,7 @@ uint16_t handle_eip712_filtering(uint8_t p1, ret = false; } if ((p2 > P2_FILT_MESSAGE_INFO) && ret) { - if (ui_712_push_new_filter_path()) { + if (ui_712_push_new_filter_path(path_crc)) { if (!ui_712_filters_counter_incr()) { ret = false; apdu_response_code = APDU_RESPONSE_INVALID_DATA; diff --git a/src_features/signMessageEIP712/filtering.c b/src_features/signMessageEIP712/filtering.c index c0274252a8..61d18b9261 100644 --- a/src_features/signMessageEIP712/filtering.c +++ b/src_features/signMessageEIP712/filtering.c @@ -27,12 +27,13 @@ #define TOKEN_IDX_ADDR_IN_DOMAIN 0xff /** - * Reconstruct the field path and hash it + * Reconstruct the field path and hash it for the signature and the CRC * * @param[in] hash_ctx the hashing context * @param[in] discarded if the filter targets a field that does not exist (within an empty array) + * @param[out] path_crc pointer to the CRC of the filter path */ -static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded) { +static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded, uint32_t *path_crc) { const void *field_ptr; const char *key; uint8_t key_len; @@ -40,15 +41,18 @@ static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded) { if (discarded) { key = ui_712_get_discarded_path(&key_len); hash_nbytes((uint8_t *) key, key_len, hash_ctx); + *path_crc = cx_crc32_update(*path_crc, key, key_len); } else { for (uint8_t i = 0; i < path_get_depth_count(); ++i) { if (i > 0) { hash_byte('.', hash_ctx); + *path_crc = cx_crc32_update(*path_crc, ".", 1); } if ((field_ptr = path_get_nth_field(i + 1)) != NULL) { if ((key = get_struct_field_keyname(field_ptr, &key_len)) != NULL) { // field name hash_nbytes((uint8_t *) key, key_len, hash_ctx); + *path_crc = cx_crc32_update(*path_crc, key, key_len); // array levels if (struct_field_is_array(field_ptr)) { @@ -57,6 +61,7 @@ static void hash_filtering_path(cx_hash_t *hash_ctx, bool discarded) { get_struct_field_array_lvls_array(field_ptr, &lvl_count); for (int j = 0; j < lvl_count; ++j) { hash_nbytes((uint8_t *) ".[]", 3, hash_ctx); + *path_crc = cx_crc32_update(*path_crc, ".[]", 3); } } } @@ -328,9 +333,10 @@ bool filtering_discarded_path(const uint8_t *payload, uint8_t length) { * @param[in] payload the payload to parse * @param[in] length the payload length * @param[in] discarded if the filter targets a field that is does not exist (within an empty array) + * @param[out] path_crc pointer to the CRC of the filter path * @return whether it was successful or not */ -bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discarded) { +bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc) { uint8_t name_len; const char *name; uint8_t types_count; @@ -413,7 +419,7 @@ bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discard if (!sig_verif_start(&hash_ctx, FILT_MAGIC_TRUSTED_NAME)) { return false; } - hash_filtering_path((cx_hash_t *) &hash_ctx, discarded); + hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc); hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx); hash_nbytes(types, types_count, (cx_hash_t *) &hash_ctx); hash_nbytes(sources, sources_count, (cx_hash_t *) &hash_ctx); @@ -440,9 +446,10 @@ bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discard * @param[in] payload the payload to parse * @param[in] length the payload length * @param[in] discarded if the filter targets a field that is does not exist (within an empty array) + * @param[out] path_crc pointer to the CRC of the filter path * @return whether it was successful or not */ -bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded) { +bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc) { uint8_t name_len; const char *name; uint8_t sig_len; @@ -478,7 +485,7 @@ bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded) if (!sig_verif_start(&hash_ctx, FILT_MAGIC_DATETIME)) { return false; } - hash_filtering_path((cx_hash_t *) &hash_ctx, discarded); + hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc); hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx); if (!sig_verif_end(&hash_ctx, sig, sig_len)) { return false; @@ -501,9 +508,10 @@ bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded) * @param[in] payload the payload to parse * @param[in] length the payload length * @param[in] discarded if the filter targets a field that is does not exist (within an empty array) + * @param[out] path_crc pointer to the CRC of the filter path * @return whether it was successful or not */ -bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool discarded) { +bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc) { uint8_t token_idx; uint8_t sig_len; const uint8_t *sig; @@ -533,7 +541,7 @@ bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool di if (!sig_verif_start(&hash_ctx, FILT_MAGIC_AMOUNT_JOIN_TOKEN)) { return false; } - hash_filtering_path((cx_hash_t *) &hash_ctx, discarded); + hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc); hash_byte(token_idx, (cx_hash_t *) &hash_ctx); if (!sig_verif_end(&hash_ctx, sig, sig_len)) { return false; @@ -554,9 +562,10 @@ bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool di * @param[in] payload the payload to parse * @param[in] length the payload length * @param[in] discarded if the filter targets a field that is does not exist (within an empty array) + * @param[out] path_crc pointer to the CRC of the filter path * @return whether it was successful or not */ -bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool discarded) { +bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc) { uint8_t name_len; const char *name; uint8_t token_idx; @@ -600,7 +609,7 @@ bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool di if (!sig_verif_start(&hash_ctx, FILT_MAGIC_AMOUNT_JOIN_VALUE)) { return false; } - hash_filtering_path((cx_hash_t *) &hash_ctx, discarded); + hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc); hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx); hash_byte(token_idx, (cx_hash_t *) &hash_ctx); if (!sig_verif_end(&hash_ctx, sig, sig_len)) { @@ -635,9 +644,10 @@ bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool di * @param[in] payload the payload to parse * @param[in] length the payload length * @param[in] discarded if the filter targets a field that is does not exist (within an empty array) + * @param[out] path_crc pointer to the CRC of the filter path * @return whether it was successful or not */ -bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded) { +bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc) { uint8_t name_len; const char *name; uint8_t sig_len; @@ -673,7 +683,7 @@ bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded) if (!sig_verif_start(&hash_ctx, FILT_MAGIC_RAW_FIELD)) { return false; } - hash_filtering_path((cx_hash_t *) &hash_ctx, discarded); + hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc); hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx); if (!sig_verif_end(&hash_ctx, sig, sig_len)) { return false; diff --git a/src_features/signMessageEIP712/filtering.h b/src_features/signMessageEIP712/filtering.h index 9d41df16a7..df721b22db 100644 --- a/src_features/signMessageEIP712/filtering.h +++ b/src_features/signMessageEIP712/filtering.h @@ -9,11 +9,11 @@ #define MAX_FILTERS 50 bool filtering_message_info(const uint8_t *payload, uint8_t length); -bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discarded); -bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded); -bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool discarded); -bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool discarded); -bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded); +bool filtering_trusted_name(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc); +bool filtering_date_time(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc); +bool filtering_amount_join_token(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc); +bool filtering_amount_join_value(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc); +bool filtering_raw_field(const uint8_t *payload, uint8_t length, bool discarded, uint32_t *path_crc); bool filtering_discarded_path(const uint8_t *payload, uint8_t length); #endif // HAVE_EIP712_FULL_SUPPORT diff --git a/src_features/signMessageEIP712/path.c b/src_features/signMessageEIP712/path.c index 61faa20b0b..c163a4bb41 100644 --- a/src_features/signMessageEIP712/path.c +++ b/src_features/signMessageEIP712/path.c @@ -753,35 +753,6 @@ bool path_exists_in_backup(const char *path, size_t length) { return true; } -/** - * Generate a unique checksum out of the current path - * - * Goes over the fields of the \ref path_struct with a few exceptions : we skip the root_type since - * we already go over root_struct, and in array_depths we only go over path_index since it would - * otherwise generate a different CRC for different fields which are targeted by the same filtering - * path. - * - * @return CRC-32 checksum - */ -uint32_t get_path_crc(void) { - uint32_t value = 0; - - value = cx_crc32_update(value, &path_struct->root_struct, sizeof(path_struct->root_struct)); - value = cx_crc32_update(value, &path_struct->depth_count, sizeof(path_struct->depth_count)); - value = cx_crc32_update(value, - path_struct->depths, - sizeof(path_struct->depths[0]) * path_struct->depth_count); - value = cx_crc32_update(value, - &path_struct->array_depth_count, - sizeof(path_struct->array_depth_count)); - for (int i = 0; i < path_struct->array_depth_count; ++i) { - value = cx_crc32_update(value, - &path_struct->array_depths[i].path_index, - sizeof(path_struct->array_depths[i].path_index)); - } - return value; -} - /** * Initialize the path context with its indexes in memory and sets it with a depth of 0. * diff --git a/src_features/signMessageEIP712/path.h b/src_features/signMessageEIP712/path.h index 8285d68f42..deb43ad398 100644 --- a/src_features/signMessageEIP712/path.h +++ b/src_features/signMessageEIP712/path.h @@ -40,7 +40,6 @@ bool path_exists_in_backup(const char *path, size_t length); const void *path_get_nth_field_to_last(uint8_t n); uint8_t path_get_depth_count(void); uint8_t path_backup_get_depth_count(void); -uint32_t get_path_crc(void); #endif // HAVE_EIP712_FULL_SUPPORT diff --git a/src_features/signMessageEIP712/ui_logic.c b/src_features/signMessageEIP712/ui_logic.c index e66ae90e38..da0c2c4c48 100644 --- a/src_features/signMessageEIP712/ui_logic.c +++ b/src_features/signMessageEIP712/ui_logic.c @@ -882,11 +882,10 @@ bool ui_712_show_raw_key(const void *field_ptr) { /** * Push a new filter path * + * @param[in] path_crc CRC of the filter path * @return if the path was pushed or not (in case it was already present) */ -bool ui_712_push_new_filter_path(void) { - uint32_t path_crc = get_path_crc(); - +bool ui_712_push_new_filter_path(uint32_t path_crc) { // check if already present for (int i = 0; i < ui_ctx->filters_received; ++i) { if (ui_ctx->filters_crc[i] == path_crc) { diff --git a/src_features/signMessageEIP712/ui_logic.h b/src_features/signMessageEIP712/ui_logic.h index 1cc2bc6d2c..d64e47f32e 100644 --- a/src_features/signMessageEIP712/ui_logic.h +++ b/src_features/signMessageEIP712/ui_logic.h @@ -48,7 +48,7 @@ void ui_712_token_join_prepare_addr_check(uint8_t index); void ui_712_token_join_prepare_amount(uint8_t index, const char *name, uint8_t name_length); void amount_join_set_token_received(void); bool ui_712_show_raw_key(const void *field_ptr); -bool ui_712_push_new_filter_path(void); +bool ui_712_push_new_filter_path(uint32_t path_crc); void ui_712_set_discarded_path(const char *path, uint8_t length); const char *ui_712_get_discarded_path(uint8_t *length); #ifdef HAVE_TRUSTED_NAME