From 04828eb6efdd53b5f46761c597aa849d7cee087f Mon Sep 17 00:00:00 2001 From: keiff3r Date: Wed, 27 Nov 2024 14:10:18 +0100 Subject: [PATCH 1/3] feat(error-handling): improve error messages granularity - Add specific status words for different error scenarios in key derivation and address verification - Split generic SW_VERIFY_ADDRESS_FAIL into more specific error codes: - SW_BLS_KEY_GEN_FAIL - SW_KEY_INIT_FAIL - SW_CREDENTIAL_ID_GEN_FAIL - SW_ADDRESS_ENCODING_FAIL - SW_DERIVATION_PATH_FAIL - Enhance error handling in get_bls_private_key and get_private_key_from_path - Update function documentation with detailed error codes - Remove redundant clang-format option This change improves debugging capabilities by providing more granular error feedback. --- .clang-format | 1 - src/handler/verify_address.c | 25 +++++++++++++++++-------- src/helper/util.c | 27 ++++++++++++++++++--------- src/helper/util.h | 17 +++++++++++++---- src/sw.h | 6 ++++++ 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/.clang-format b/.clang-format index b4abfa06..c76e9fc2 100644 --- a/.clang-format +++ b/.clang-format @@ -12,7 +12,6 @@ SortIncludes: false SpaceAfterCStyleCast: true AllowShortCaseLabelsOnASingleLine: false AllowAllArgumentsOnNextLine: false -AllowAllParametersOfDeclarationOnNextLine: false AllowShortBlocksOnASingleLine: Never AllowShortFunctionsOnASingleLine: None BinPackArguments: false diff --git a/src/handler/verify_address.c b/src/handler/verify_address.c index efd20e1b..9857f830 100644 --- a/src/handler/verify_address.c +++ b/src/handler/verify_address.c @@ -134,11 +134,20 @@ int handler_verify_address(buffer_t *cdata, bool is_new_address) { LEGACY_PRF_KEY | HARDENED_OFFSET}; } + // Initialize credential ID and PRF key uint8_t credential_id[CREDENTIAL_ID_LEN]; uint8_t prf_key[32]; - if (get_bls_private_key(prf_key_path, prf_key_path_len, prf_key, sizeof(prf_key)) == -1) { - return io_send_sw(SW_VERIFY_ADDRESS_FAIL); + int rtn = get_bls_private_key(prf_key_path, prf_key_path_len, prf_key, sizeof(prf_key)); + switch (rtn) { + case -1: // derivation path error + return io_send_sw(SW_DERIVATION_PATH_FAIL); + case -2: // key initialization error + return io_send_sw(SW_KEY_INIT_FAIL); + case -3: // BLS key generation error + return io_send_sw(SW_BLS_KEY_GEN_FAIL); + default: + break; } if (get_credential_id(prf_key, @@ -146,7 +155,7 @@ int handler_verify_address(buffer_t *cdata, bool is_new_address) { G_context.verify_address_info.credential_counter, credential_id, sizeof(credential_id)) != CX_OK) { - return io_send_sw(SW_VERIFY_ADDRESS_FAIL); + return io_send_sw(SW_CREDENTIAL_ID_GEN_FAIL); } // Empty prf_key @@ -158,12 +167,12 @@ int handler_verify_address(buffer_t *cdata, bool is_new_address) { size_t address_len = sizeof(G_context.verify_address_info.address); // This function will return the number of bytes encoded, or -1 on error. - int rtn = address_to_base58(account_address, - sizeof(account_address), - G_context.verify_address_info.address, - address_len); + rtn = address_to_base58(account_address, + sizeof(account_address), + G_context.verify_address_info.address, + address_len); if (rtn < 0) { - return io_send_sw(SW_VERIFY_ADDRESS_FAIL); + return io_send_sw(SW_ADDRESS_ENCODING_FAIL); } return ui_display_verify_address(); diff --git a/src/helper/util.c b/src/helper/util.c index 3c066272..b8ea7181 100644 --- a/src/helper/util.c +++ b/src/helper/util.c @@ -55,7 +55,7 @@ int get_private_key_from_path(uint32_t *path, size_t path_len, cx_ecfp_private_k if (rtn == 0 && cx_ecfp_init_private_key_no_throw(CX_CURVE_Ed25519, private_key_data, 32, private_key) != CX_OK) { - rtn = -1; + rtn = -2; } explicit_bzero(private_key_data, sizeof(private_key_data)); return rtn; @@ -113,15 +113,24 @@ int get_bls_private_key(uint32_t *path, size_t private_key_len) { cx_ecfp_private_key_t private_key_seed; - if (get_private_key_from_path(path, path_len, &private_key_seed) == -1) { - return -1; + int rtn = get_private_key_from_path(path, path_len, &private_key_seed); + switch (rtn) { + case -1: // derivation path error + return -1; + case -2: // key initialization error + return -2; + default: + break; } - - if (bls_key_gen_from_seed(private_key_seed.d, - sizeof(private_key_seed.d), - private_key, - private_key_len) == -1) { - return -1; + rtn = bls_key_gen_from_seed(private_key_seed.d, + sizeof(private_key_seed.d), + private_key, + private_key_len); + switch (rtn) { + case -1: // BLS key generation error + return -3; + default: + break; } explicit_bzero(&private_key_seed, sizeof(private_key_seed)); diff --git a/src/helper/util.h b/src/helper/util.h index fda06f5a..b9718f29 100644 --- a/src/helper/util.h +++ b/src/helper/util.h @@ -19,7 +19,16 @@ static const uint8_t r[32] = {0x73, 0xed, 0xa7, 0x53, 0x29, 0x9d, 0x7d, 0x48, 0x int get_identity_index_account_display(); /** - * Get the BLS private key from the path. + * Derive a BLS private key from a BIP32 path by first deriving an Ed25519 seed + * and then converting it to a BLS key. + * + * @param[in] path Pointer to the BIP32 path + * @param[in] path_len Length of the BIP32 path + * @param[out] private_key Buffer to store the derived BLS private key + * @param[in] private_key_len Length of the private key buffer + * + * @return 0 on success, -1 on derivation path error, -2 on key initialization error, + * -3 on BLS key generation error */ int get_bls_private_key(uint32_t *path, size_t path_len, @@ -35,13 +44,13 @@ int address_to_base58(const uint8_t *address, size_t encoded_address_len); /** - * Derive a private key from a BIP32 path. + * Derive an Ed25519 private key from a BIP32 path using SLIP-0010 derivation. * * @param[in] path Pointer to the BIP32 path * @param[in] path_len Length of the BIP32 path - * @param[out] private_key Pointer to the private key + * @param[out] private_key Pointer to store the derived private key * - * @return 0 on success, -1 on error + * @return 0 on success, -1 on derivation error, -2 on key initialization error */ int get_private_key_from_path(uint32_t *path, size_t path_len, cx_ecfp_private_key_t *private_key); diff --git a/src/sw.h b/src/sw.h index 45509f0d..d727919f 100644 --- a/src/sw.h +++ b/src/sw.h @@ -64,3 +64,9 @@ * Status word for error in verify address. */ #define SW_VERIFY_ADDRESS_FAIL 0xB009 + +#define SW_BLS_KEY_GEN_FAIL 0xB00A +#define SW_KEY_INIT_FAIL 0xB00B +#define SW_CREDENTIAL_ID_GEN_FAIL 0xB00C +#define SW_ADDRESS_ENCODING_FAIL 0xB00D +#define SW_DERIVATION_PATH_FAIL 0xB00E \ No newline at end of file From 58d48c79a109877b1a5313852d5782ffe6aa6607 Mon Sep 17 00:00:00 2001 From: keiff3r Date: Wed, 27 Nov 2024 14:18:27 +0100 Subject: [PATCH 2/3] style: format the codebase --- src/handler/verify_address.c | 6 +++--- src/helper/util.c | 20 ++++++++++---------- src/sw.h | 8 ++++---- src/ui/action/validate.c | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/handler/verify_address.c b/src/handler/verify_address.c index 9857f830..36c20323 100644 --- a/src/handler/verify_address.c +++ b/src/handler/verify_address.c @@ -140,11 +140,11 @@ int handler_verify_address(buffer_t *cdata, bool is_new_address) { int rtn = get_bls_private_key(prf_key_path, prf_key_path_len, prf_key, sizeof(prf_key)); switch (rtn) { - case -1: // derivation path error + case -1: // derivation path error return io_send_sw(SW_DERIVATION_PATH_FAIL); - case -2: // key initialization error + case -2: // key initialization error return io_send_sw(SW_KEY_INIT_FAIL); - case -3: // BLS key generation error + case -3: // BLS key generation error return io_send_sw(SW_BLS_KEY_GEN_FAIL); default: break; diff --git a/src/helper/util.c b/src/helper/util.c index b8ea7181..b6a9ee94 100644 --- a/src/helper/util.c +++ b/src/helper/util.c @@ -115,22 +115,22 @@ int get_bls_private_key(uint32_t *path, int rtn = get_private_key_from_path(path, path_len, &private_key_seed); switch (rtn) { - case -1: // derivation path error - return -1; - case -2: // key initialization error - return -2; - default: - break; + case -1: // derivation path error + return -1; + case -2: // key initialization error + return -2; + default: + break; } rtn = bls_key_gen_from_seed(private_key_seed.d, sizeof(private_key_seed.d), private_key, private_key_len); switch (rtn) { - case -1: // BLS key generation error - return -3; - default: - break; + case -1: // BLS key generation error + return -3; + default: + break; } explicit_bzero(&private_key_seed, sizeof(private_key_seed)); diff --git a/src/sw.h b/src/sw.h index d727919f..ee019535 100644 --- a/src/sw.h +++ b/src/sw.h @@ -65,8 +65,8 @@ */ #define SW_VERIFY_ADDRESS_FAIL 0xB009 -#define SW_BLS_KEY_GEN_FAIL 0xB00A -#define SW_KEY_INIT_FAIL 0xB00B +#define SW_BLS_KEY_GEN_FAIL 0xB00A +#define SW_KEY_INIT_FAIL 0xB00B #define SW_CREDENTIAL_ID_GEN_FAIL 0xB00C -#define SW_ADDRESS_ENCODING_FAIL 0xB00D -#define SW_DERIVATION_PATH_FAIL 0xB00E \ No newline at end of file +#define SW_ADDRESS_ENCODING_FAIL 0xB00D +#define SW_DERIVATION_PATH_FAIL 0xB00E \ No newline at end of file diff --git a/src/ui/action/validate.c b/src/ui/action/validate.c index 8c95d6a8..f3782886 100644 --- a/src/ui/action/validate.c +++ b/src/ui/action/validate.c @@ -61,7 +61,7 @@ static int crypto_sign_message(void) { PRINTF("Signature: %.*H\n", sig_len, G_context.tx_info.signature); G_context.tx_info.signature_len = sig_len; - G_context.tx_info.v = (uint8_t)(info & CX_ECCINFO_PARITY_ODD); + G_context.tx_info.v = (uint8_t) (info & CX_ECCINFO_PARITY_ODD); return 0; } From c975263f566015287232c20dd8498218349cd5fb Mon Sep 17 00:00:00 2001 From: keiff3r Date: Wed, 27 Nov 2024 14:29:51 +0100 Subject: [PATCH 3/3] feat(crypto): switch from secp256k1 to ed25519 curve Changes the CURVE_APP_LOAD_PARAMS in the Makefile to use ed25519 instead of secp256k1 for cryptographic operations. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e518feda..db2916c3 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ ICON_FLEX = icons/app_concordium_40px.gif # Possibles curves are: secp256k1, secp256r1, ed25519 and bls12381g1 # If your app needs it, you can specify multiple curves by using: # `CURVE_APP_LOAD_PARAMS = ` -CURVE_APP_LOAD_PARAMS = secp256k1 +CURVE_APP_LOAD_PARAMS = ed25519 # Application allowed derivation paths. # You should request a specific path for your app.