Merge pull request #373 from LedgerHQ/mk_syscall

Switching to new Master Key Fingerprint Syscall +  Derivation Path Hardening

Author: iartemov-ledger

Makefile

@@ -19,26 +19,9 @@ ifeq ($(BOLOS_SDK),)
 $(error Environment variable BOLOS_SDK is not set)
 endif
 
-# TODO: Compile with the right path restrictions
-#
-#       The right path restriction would be something like
-#         --path "*'/0'"
-#       for mainnet, and
-#         --path "*'/1'"
-#       for testnet.
-#
-#       That is, restrict the BIP-44 coin_type, but not the purpose.
-#       However, such wildcards are not currently supported by the OS.
-#
-#       Note that the app still requires explicit user approval before exporting
-#       any xpub outside of a small set of allowed standard paths.
-
 # Application allowed derivation curves.
 CURVE_APP_LOAD_PARAMS = secp256k1
 
-# Application allowed derivation paths.
-PATH_APP_LOAD_PARAMS = ""
-
 # Allowed SLIP21 paths
 PATH_SLIP21_APP_LOAD_PARAMS = "LEDGER-Wallet policy"
 
@@ -73,12 +56,14 @@ endif
 ########################################
 #     Application custom permissions   #
 ########################################
-HAVE_APPLICATION_FLAG_DERIVE_MASTER = 1
 HAVE_APPLICATION_FLAG_GLOBAL_PIN = 1
 HAVE_APPLICATION_FLAG_BOLOS_SETTINGS = 1
 HAVE_APPLICATION_FLAG_LIBRARY = 1
 
 ifeq ($(COIN),bitcoin_testnet)
+    # Application allowed derivation paths (testnet).
+    PATH_APP_LOAD_PARAMS = "*/1'"
+
     # Bitcoin testnet, no legacy support
     DEFINES   += BIP32_PUBKEY_VERSION=0x043587CF
     DEFINES   += BIP44_COIN_TYPE=1
@@ -89,6 +74,9 @@ ifeq ($(COIN),bitcoin_testnet)
 
     APPNAME = "Bitcoin Test"
 else ifeq ($(COIN),bitcoin)
+    # Application allowed derivation paths (mainnet).
+    PATH_APP_LOAD_PARAMS = "*/0'"
+
     # the version for performance tests automatically approves all requests
     # there is no reason to ever compile the mainnet app with this flag
     ifneq ($(AUTOAPPROVE_FOR_PERF_TESTS),0)

src/crypto.c

@@ -35,6 +35,7 @@
 #include "common/read.h"
 #include "common/write.h"
 
+#include "../boilerplate/sw.h"
 #include "../debug-helpers/debug.h"
 
 #include "crypto.h"
@@ -241,26 +242,28 @@ void crypto_get_checksum(const uint8_t *in, uint16_t in_len, uint8_t out[static
     memmove(out, buffer, 4);
 }
 
-bool crypto_get_compressed_pubkey_at_path(const uint32_t bip32_path[],
-                                          uint8_t bip32_path_len,
-                                          uint8_t pubkey[static 33],
-                                          uint8_t chain_code[]) {
+cx_err_t crypto_get_compressed_pubkey_at_path(const uint32_t bip32_path[],
+                                              uint8_t bip32_path_len,
+                                              uint8_t pubkey[static 33],
+                                              uint8_t chain_code[]) {
     uint8_t raw_public_key[65];
+    cx_err_t error = CX_OK;
 
-    if (bip32_derive_get_pubkey_256(CX_CURVE_256K1,
-                                    bip32_path,
-                                    bip32_path_len,
-                                    raw_public_key,
-                                    chain_code,
-                                    CX_SHA512) != CX_OK) {
-        return false;
+    error = bip32_derive_get_pubkey_256(CX_CURVE_256K1,
+                                        bip32_path,
+                                        bip32_path_len,
+                                        raw_public_key,
+                                        chain_code,
+                                        CX_SHA512);
+    if (error != CX_OK) {
+        return error;
     }
 
     if (crypto_get_compressed_pubkey(raw_public_key, pubkey) < 0) {
-        return false;
+        return CX_INTERNAL_ERROR;
     }
 
-    return true;
+    return error;
 }
 
 uint32_t crypto_get_key_fingerprint(const uint8_t pub_key[static 33]) {
@@ -271,10 +274,14 @@ uint32_t crypto_get_key_fingerprint(const uint8_t pub_key[static 33]) {
 }
 
 uint32_t crypto_get_master_key_fingerprint() {
-    uint8_t master_pub_key[33];
-    uint32_t bip32_path[] = {};
-    crypto_get_compressed_pubkey_at_path(bip32_path, 0, master_pub_key, NULL);
-    return crypto_get_key_fingerprint(master_pub_key);
+    uint8_t master_key_identifier[CX_RIPEMD160_SIZE] = {0};
+
+    int res = os_perso_get_master_key_identifier(master_key_identifier, CX_RIPEMD160_SIZE);
+    LEDGER_ASSERT(
+        res == CX_OK,
+        "Unexpected error in os_perso_get_master_key_identifier computation. Returned: %d",
+        res);
+    return read_u32_be(master_key_identifier, 0);
 }
 
 bool crypto_derive_symmetric_key(const char *label, size_t label_len, uint8_t key[static 32]) {
@@ -311,23 +318,26 @@ bool crypto_derive_symmetric_key(const char *label, size_t label_len, uint8_t ke
     return ret == CX_OK;
 }
 
-int get_extended_pubkey_at_path(const uint32_t bip32_path[],
-                                uint8_t bip32_path_len,
-                                uint32_t bip32_pubkey_version,
-                                serialized_extended_pubkey_t *out_pubkey) {
+cx_err_t get_extended_pubkey_at_path(const uint32_t bip32_path[],
+                                     uint8_t bip32_path_len,
+                                     uint32_t bip32_pubkey_version,
+                                     serialized_extended_pubkey_t *out_pubkey) {
     // find parent key's fingerprint and child number
     uint32_t parent_fingerprint = 0;
     uint32_t child_number = 0;
+    cx_err_t error = CX_OK;
+
     if (bip32_path_len > 0) {
         // here we reuse the storage for the parent keys that we will later use
         // for the response, in order to save memory
 
         uint8_t parent_pubkey[33];
-        if (!crypto_get_compressed_pubkey_at_path(bip32_path,
-                                                  bip32_path_len - 1,
-                                                  parent_pubkey,
-                                                  NULL)) {
-            return -1;
+        error = crypto_get_compressed_pubkey_at_path(bip32_path,
+                                                     bip32_path_len - 1,
+                                                     parent_pubkey,
+                                                     NULL);
+        if (error != CX_OK) {
+            return error;
         }
 
         parent_fingerprint = crypto_get_key_fingerprint(parent_pubkey);
@@ -339,14 +349,31 @@ int get_extended_pubkey_at_path(const uint32_t bip32_path[],
     write_u32_be(out_pubkey->parent_fingerprint, 0, parent_fingerprint);
     write_u32_be(out_pubkey->child_number, 0, child_number);
 
-    if (!crypto_get_compressed_pubkey_at_path(bip32_path,
-                                              bip32_path_len,
-                                              out_pubkey->compressed_pubkey,
-                                              out_pubkey->chain_code)) {
-        return -1;
+    return crypto_get_compressed_pubkey_at_path(bip32_path,
+                                                bip32_path_len,
+                                                out_pubkey->compressed_pubkey,
+                                                out_pubkey->chain_code);
+}
+
+uint16_t cx_err_to_sw(cx_err_t error) {
+    if (error == CX_OK) {
+        return SW_OK;
     }
 
-    return 0;
+    /* The error codes are not currently defined in the SDK */
+    if (error == 0x4212) {
+        PRINTF(
+            "Attempt to derive a key at root level without "
+            "HAVE_APPLICATION_FLAG_DERIVE_MASTER permission.\n");
+        return SW_NOT_SUPPORTED;
+    }
+    if (error == 0x4215) {
+        PRINTF("Attempt to derive a key at unauthorized path.\n");
+        return SW_NOT_SUPPORTED;
+    }
+
+    PRINTF("Failed getting bip32 pubkey, error = 0x%08X\n", error);
+    return SW_BAD_STATE;
 }
 
 int base58_encode_address(const uint8_t in[20], uint32_t version, char *out, size_t out_len) {

src/crypto.h

@@ -252,10 +252,10 @@ void crypto_get_checksum(const uint8_t *in, uint16_t in_len, uint8_t out[static
  *
  * @return true on success, false in case of error.
  */
-bool crypto_get_compressed_pubkey_at_path(const uint32_t bip32_path[],
-                                          uint8_t bip32_path_len,
-                                          uint8_t pubkey[static 33],
-                                          uint8_t chain_code[]);
+cx_err_t crypto_get_compressed_pubkey_at_path(const uint32_t bip32_path[],
+                                              uint8_t bip32_path_len,
+                                              uint8_t pubkey[static 33],
+                                              uint8_t chain_code[]);
 
 /**
  * Computes the fingerprint of a compressed key as per BIP32; that is, the first 4 bytes of the
@@ -289,10 +289,10 @@ uint32_t crypto_get_master_key_fingerprint();
  *
  * @return 0 on success, or -1 on error.
  */
-int get_extended_pubkey_at_path(const uint32_t bip32_path[],
-                                uint8_t bip32_path_len,
-                                uint32_t bip32_pubkey_version,
-                                serialized_extended_pubkey_t *out_pubkey);
+cx_err_t get_extended_pubkey_at_path(const uint32_t bip32_path[],
+                                     uint8_t bip32_path_len,
+                                     uint32_t bip32_pubkey_version,
+                                     serialized_extended_pubkey_t *out_pubkey);
 
 /**
  * Derives the level-1 symmetric key at the given label using SLIP-0021.
@@ -474,3 +474,13 @@ int crypto_tr_tweak_seckey(const uint8_t seckey[static 32],
                            const uint8_t *h,
                            size_t h_len,
                            uint8_t out[static 32]);
+
+/**
+ * Converts cx_err_t to 2-bytes SW and prints out debug information.
+ *
+ * @param[in] error
+ *  Cryptographic error code
+ *
+ * @return SW (SW_OK on success, other value on error).
+ */
+uint16_t cx_err_to_sw(cx_err_t error);

src/handler/get_extended_pubkey.c

@@ -137,12 +137,13 @@ void handler_get_extended_pubkey(dispatcher_context_t *dc, uint8_t protocol_vers
     }
 
     serialized_extended_pubkey_check_t pubkey_check;
-    if (0 > get_extended_pubkey_at_path(bip32_path,
-                                        bip32_path_len,
-                                        BIP32_PUBKEY_VERSION,
-                                        &pubkey_check.serialized_extended_pubkey)) {
-        PRINTF("Failed getting bip32 pubkey\n");
-        SEND_SW(dc, SW_BAD_STATE);
+    uint16_t sw =
+        cx_err_to_sw(get_extended_pubkey_at_path(bip32_path,
+                                                 bip32_path_len,
+                                                 BIP32_PUBKEY_VERSION,
+                                                 &pubkey_check.serialized_extended_pubkey));
+    if (SW_OK != sw) {
+        SEND_SW(dc, sw);
         return;
     }
 

src/handler/get_master_fingerprint.c

@@ -17,6 +17,8 @@
 
 #include <stdint.h>
 
+#include "os_seed.h"
+
 #include "boilerplate/dispatcher.h"
 #include "boilerplate/sw.h"
 #include "../commands.h"
@@ -27,14 +29,12 @@
 void handler_get_master_fingerprint(dispatcher_context_t *dc, uint8_t protocol_version) {
     (void) protocol_version;
 
-    uint8_t master_pubkey[33];
-    if (!crypto_get_compressed_pubkey_at_path((uint32_t[]){}, 0, master_pubkey, NULL)) {
+    uint8_t master_key_identifier[CX_RIPEMD160_SIZE] = {0};
+
+    if (os_perso_get_master_key_identifier(master_key_identifier, CX_RIPEMD160_SIZE) != CX_OK) {
         SEND_SW(dc, SW_BAD_STATE);  // should never happen
         return;
     }
 
-    uint8_t master_fingerprint_be[4];
-    write_u32_be(master_fingerprint_be, 0, crypto_get_key_fingerprint(master_pubkey));
-
-    SEND_RESPONSE(dc, master_fingerprint_be, sizeof(master_fingerprint_be), SW_OK);
+    SEND_RESPONSE(dc, master_key_identifier, 4, SW_OK);
 }

src/handler/lib/policy.c

@@ -1478,10 +1478,10 @@ bool is_wallet_policy_standard(dispatcher_context_t *dispatcher_context,
 
     // generate pubkey and check if it matches
     serialized_extended_pubkey_t derived_pubkey;
-    if (0 > get_extended_pubkey_at_path(key_info.master_key_derivation,
-                                        key_info.master_key_derivation_len,
-                                        BIP32_PUBKEY_VERSION,
-                                        &derived_pubkey)) {
+    if (CX_OK != get_extended_pubkey_at_path(key_info.master_key_derivation,
+                                             key_info.master_key_derivation_len,
+                                             BIP32_PUBKEY_VERSION,
+                                             &derived_pubkey)) {
         PRINTF("Failed to derive pubkey\n");
         return false;
     }

src/handler/register_wallet.c

@@ -193,13 +193,13 @@ void handler_register_wallet(dispatcher_context_t *dc, uint8_t protocol_version)
                 read_u32_be(key_info.master_key_fingerprint, 0) == master_key_fingerprint) {
                 // we verify that we can actually generate the same pubkey
                 serialized_extended_pubkey_t pubkey_derived;
-                int serialized_pubkey_len =
-                    get_extended_pubkey_at_path(key_info.master_key_derivation,
-                                                key_info.master_key_derivation_len,
-                                                BIP32_PUBKEY_VERSION,
-                                                &pubkey_derived);
-                if (serialized_pubkey_len == -1) {
-                    SEND_SW(dc, SW_BAD_STATE);
+                uint16_t sw =
+                    cx_err_to_sw(get_extended_pubkey_at_path(key_info.master_key_derivation,
+                                                             key_info.master_key_derivation_len,
+                                                             BIP32_PUBKEY_VERSION,
+                                                             &pubkey_derived));
+                if (SW_OK != sw) {
+                    SEND_SW(dc, sw);
                     return;
                 }
 

src/handler/sign_psbt.c

@@ -399,10 +399,10 @@ static bool __attribute__((noinline)) get_and_verify_key_info(dispatcher_context
     // it could be a collision on the fingerprint; we verify that we can actually generate
     // the same pubkey
     serialized_extended_pubkey_t derived_pubkey;
-    if (0 > get_extended_pubkey_at_path(key_info.master_key_derivation,
-                                        key_info.master_key_derivation_len,
-                                        BIP32_PUBKEY_VERSION,
-                                        &derived_pubkey)) {
+    if (CX_OK != get_extended_pubkey_at_path(key_info.master_key_derivation,
+                                             key_info.master_key_derivation_len,
+                                             BIP32_PUBKEY_VERSION,
+                                             &derived_pubkey)) {
         return false;
     }
 

src/swap/handle_check_address.c

@@ -110,10 +110,8 @@ int handle_check_address(check_address_parameters_t* params) {
         return false;
     }
 
-    if (!crypto_get_compressed_pubkey_at_path(path.path,
-                                              path.length,
-                                              compressed_public_key,
-                                              NULL)) {
+    if (CX_OK !=
+        crypto_get_compressed_pubkey_at_path(path.path, path.length, compressed_public_key, NULL)) {
         return 0;
     }
     char address[MAX_ADDRESS_LENGTH_STR + 1];
@@ -133,4 +131,4 @@ int handle_check_address(check_address_parameters_t* params) {
     }
     PRINTF("Addresses match\n");
     return 1;
-}
+}