Merge pull request #241 from LedgerHQ/review2

Review2

Makefile

@@ -25,7 +25,7 @@ APP_LOAD_PARAMS= --curve secp256k1 $(COMMON_LOAD_PARAMS)
 
 APPVERSION_M=2
 APPVERSION_N=1
-APPVERSION_P=4
+APPVERSION_P=5
 APPVERSION=$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)
 APP_LOAD_FLAGS=--appFlags 0xa50
 
@@ -254,10 +254,6 @@ else
     endif
 endif
 
-ifeq ($(TARGET_NAME),TARGET_NANOS)
-DEFINES       += HAVE_WALLET_ID_SDK
-endif
-
 # Enabling debug PRINTF
 DEBUG ?= 0
 ifneq ($(DEBUG),0)
@@ -305,7 +301,7 @@ include $(BOLOS_SDK)/Makefile.glyphs
 APP_SOURCE_PATH  += src 
 
 APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/format.c
-INCLUDES_PATH += ${BOLOS_SDK}
+APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/crypto_helpers.c
 
 SDK_SOURCE_PATH  += lib_stusb lib_stusb_impl lib_u2f 
 ifneq ($(TARGET_NAME),TARGET_STAX)

include/btchip_ecc.h

@@ -21,8 +21,6 @@
 
 #include "os.h"
 
-#define BTCHIP_CURVE CX_CURVE_256K1
-
 void btchip_compress_public_key_value(unsigned char *value);
 
 #endif

include/btchip_helpers.h

@@ -65,12 +65,6 @@ unsigned short btchip_decode_base58_address(unsigned char *in,
                                             unsigned char *out,
                                             unsigned short outlen);
 
-void btchip_private_derive_keypair(unsigned char *bip32Path,
-                                   unsigned char derivePublic,
-                                   unsigned char *out_chainCode,
-                                   cx_ecfp_private_key_t * private_key,
-                                   cx_ecfp_public_key_t* public_key);
-
 unsigned char bip44_derivation_guard(unsigned char *bip32Path, bool is_change_path);
 unsigned char enforce_bip44_coin_type(unsigned char *bip32Path, bool for_pubkey);
 unsigned char bip32_print_path(unsigned char *bip32Path, char* out, unsigned char max_out_len);
@@ -81,9 +75,9 @@ unsigned char bip32_print_path(unsigned char *bip32Path, char* out, unsigned cha
 void btchip_swap_bytes(unsigned char *target, unsigned char *source,
                        unsigned char size);
 
-void btchip_sign_finalhash(void *keyContext,
+int btchip_sign_finalhash(unsigned char *path, size_t path_len,
                            unsigned char *in, unsigned short inlen,
-                           unsigned char *out, unsigned short outlen,
+                           unsigned char *out, size_t* outlen,
                            unsigned char rfc6979);
 
 void btchip_transaction_add_output(unsigned char *hash160Address,
@@ -94,5 +88,6 @@ unsigned char btchip_secure_memcmp(const void *buf1, const void *buf2,
 unsigned char btchip_decrease_2fa(void);
 void btchip_reset_2fa(void);
 void btchip_reset_token(void);
+int btchip_get_public_key(unsigned char* keyPath, size_t keyPath_len, uint8_t raw_pubkey[static 65], unsigned char* chainCode);
 
 #endif

src/btchip.c

@@ -27,33 +27,6 @@
 #define BTCHIP_TECHNICAL_NOT_IMPLEMENTED 0x99
 
 #define COMMON_CLA               0xB0
-#define COMMON_INS_GET_WALLET_ID 0x04
-
-#ifndef HAVE_WALLET_ID_SDK
-
-unsigned int const U_os_perso_seed_cookie[] = {
-  0xda7aba5e,
-  0xc1a551c5,
-};
-
-void handleGetWalletId(volatile unsigned short *tx) {
-  unsigned char t[64];
-  cx_ecfp_256_private_key_t priv;
-  cx_ecfp_256_public_key_t pub;
-  // seed => priv key
-  os_perso_derive_node_bip32(CX_CURVE_256K1, U_os_perso_seed_cookie, 2, t, NULL);
-  // priv key => pubkey
-  cx_ecdsa_init_private_key(CX_CURVE_256K1, t, 32, &priv);
-  cx_ecfp_generate_pair(CX_CURVE_256K1, &pub, &priv, 1);
-  // pubkey -> sha512
-  cx_hash_sha512(pub.W, sizeof(pub.W), t, sizeof(t));
-  // ! cookie !
-  memmove(G_io_apdu_buffer, t, 64);
-  btchip_context_D.sw = 0x9000;
-  *tx = 64;
-}
-
-#endif // HAVE_WALLET_ID_SDK
 
 void app_dispatch(void) {
     unsigned char cla;
@@ -64,84 +37,58 @@ void app_dispatch(void) {
     btchip_context_D.outLength = 0;
     btchip_context_D.io_flags = 0;
 
-    BEGIN_TRY {
-        TRY {
-
-#ifndef HAVE_WALLET_ID_SDK
+    // If halted, then notify
+    SB_CHECK(btchip_context_D.halted);
+    if (SB_GET(btchip_context_D.halted)) {
+        btchip_context_D.sw = BTCHIP_SW_TECHNICAL_PROBLEM;
+        goto sendSW;
+    }
 
-      if ((G_io_apdu_buffer[ISO_OFFSET_CLA] == COMMON_CLA) && (G_io_apdu_buffer[ISO_OFFSET_INS] == COMMON_INS_GET_WALLET_ID)) {
-        handleGetWalletId(&btchip_context_D.outLength);
+    cla = G_io_apdu_buffer[ISO_OFFSET_CLA];
+    ins = G_io_apdu_buffer[ISO_OFFSET_INS];
+    for (dispatched = 0; dispatched < DISPATCHER_APDUS; dispatched++) {
+        if ((cla == DISPATCHER_CLA[dispatched]) &&
+                (ins == DISPATCHER_INS[dispatched])) {
+            break;
+        }
+    }
+    if (dispatched == DISPATCHER_APDUS) {
+        btchip_context_D.sw = BTCHIP_SW_INS_NOT_SUPPORTED;
         goto sendSW;
-      }
-
-#endif
-
-            // If halted, then notify
-            SB_CHECK(btchip_context_D.halted);
-            if (SB_GET(btchip_context_D.halted)) {
-                btchip_context_D.sw = BTCHIP_SW_HALTED;
-                goto sendSW;
-            }
-
-            cla = G_io_apdu_buffer[ISO_OFFSET_CLA];
-            ins = G_io_apdu_buffer[ISO_OFFSET_INS];
-            for (dispatched = 0; dispatched < DISPATCHER_APDUS; dispatched++) {
-                if ((cla == DISPATCHER_CLA[dispatched]) &&
-                    (ins == DISPATCHER_INS[dispatched])) {
-                    break;
-                }
-            }
-            if (dispatched == DISPATCHER_APDUS) {
-                btchip_context_D.sw = BTCHIP_SW_INS_NOT_SUPPORTED;
-                goto sendSW;
-            }
-            if (DISPATCHER_DATA_IN[dispatched]) {
-                if (G_io_apdu_buffer[ISO_OFFSET_LC] == 0x00 ||
-                    btchip_context_D.inLength - 5 == 0) {
-                    btchip_context_D.sw = BTCHIP_SW_INCORRECT_LENGTH;
-                    goto sendSW;
-                }
-                // notify we need to receive data
-                // io_exchange(CHANNEL_APDU | IO_RECEIVE_DATA, 0);
-            }
-            // call the apdu handler
-            btchip_context_D.sw = ((apduProcessingFunction)PIC(
+    }
+    if (DISPATCHER_DATA_IN[dispatched]) {
+        if (G_io_apdu_buffer[ISO_OFFSET_LC] == 0x00 ||
+                btchip_context_D.inLength - 5 == 0) {
+            btchip_context_D.sw = BTCHIP_SW_INCORRECT_LENGTH;
+            goto sendSW;
+        }
+        // notify we need to receive data
+        // io_exchange(CHANNEL_APDU | IO_RECEIVE_DATA, 0);
+    }
+    // call the apdu handler
+    btchip_context_D.sw = ((apduProcessingFunction)PIC(
                 DISPATCHER_FUNCTIONS[dispatched]))();
 
-// an APDU has been replied. request for power off time extension from the
-// common ux
+    // an APDU has been replied. request for power off time extension from the
+    // common ux
 #ifdef IO_APP_ACTIVITY
-            IO_APP_ACTIVITY();
+    IO_APP_ACTIVITY();
 #endif // IO_APP_ACTIVITY
 
-        sendSW:
-            if (btchip_context_D.called_from_swap) {
-                btchip_context_D.io_flags &= ~IO_ASYNCH_REPLY;
-                if(btchip_context_D.sw != BTCHIP_SW_OK) {
-                    vars.swap_data.should_exit = 1;
-                }
-            }
-            // prepare SW after replied data
-            G_io_apdu_buffer[btchip_context_D.outLength] =
-                (btchip_context_D.sw >> 8);
-            G_io_apdu_buffer[btchip_context_D.outLength + 1] =
-                (btchip_context_D.sw & 0xff);
-            btchip_context_D.outLength += 2;
-        }
-        CATCH(EXCEPTION_IO_RESET) {
-            THROW(EXCEPTION_IO_RESET);
-        }
-        CATCH_OTHER(e) {
-            // uncaught exception detected
-            G_io_apdu_buffer[0] = 0x6F;
-            btchip_context_D.outLength = 2;
-            G_io_apdu_buffer[1] = e;
-            // we caught something suspicious
-            SB_SET(btchip_context_D.halted, 1);
+sendSW:
+    if (btchip_context_D.called_from_swap) {
+        btchip_context_D.io_flags &= ~IO_ASYNCH_REPLY;
+        if(btchip_context_D.sw != BTCHIP_SW_OK) {
+            vars.swap_data.should_exit = 1;
         }
-        FINALLY;
     }
-    END_TRY;
+    // prepare SW after replied data
+    G_io_apdu_buffer[btchip_context_D.outLength] =
+        (btchip_context_D.sw >> 8);
+    G_io_apdu_buffer[btchip_context_D.outLength + 1] =
+        (btchip_context_D.sw & 0xff);
+    btchip_context_D.outLength += 2;
+    return;
 }
 
 void app_main(void) {

src/btchip_apdu_get_trusted_input.c

@@ -73,8 +73,10 @@ unsigned short btchip_apdu_get_trusted_input() {
             return BTCHIP_SW_INCORRECT_DATA;
         }
 
-        cx_hash(&btchip_context_D.transactionHashFull.sha256.header, CX_LAST,
-                (unsigned char *)NULL, 0, G_io_apdu_buffer + TRUSTED_INPUT_SIZE, 32);
+        if (cx_hash_no_throw(&btchip_context_D.transactionHashFull.sha256.header, CX_LAST,
+                NULL, 0, G_io_apdu_buffer + TRUSTED_INPUT_SIZE, 32)) {
+            return BTCHIP_SW_TECHNICAL_PROBLEM;
+        }
 
         // Otherwise prepare
         cx_rng(G_io_apdu_buffer, 8);

src/btchip_apdu_get_wallet_public_key.c

@@ -24,21 +24,23 @@
 #include "cashaddr.h"
 #include "btchip_apdu_get_wallet_public_key.h"
 
-int get_public_key_chain_code(unsigned char* keyPath, bool uncompressedPublicKeys, unsigned char* publicKey, unsigned char* chainCode) {
-    cx_ecfp_private_key_t private_key;
-    cx_ecfp_public_key_t public_key;
+int get_public_key_chain_code(unsigned char* keyPath, size_t keyPath_len, bool uncompressedPublicKeys, unsigned char* publicKey, unsigned char* chainCode) {
+    uint8_t public_key[65];
     int keyLength = 0;
-    btchip_private_derive_keypair(keyPath, 1, chainCode, &private_key, &public_key);
+
+    if (btchip_get_public_key(keyPath, keyPath_len, public_key, chainCode)) {
+        return keyLength;
+    }
     // Then encode it
     if (uncompressedPublicKeys) {
         keyLength = 65;
     } else {
-        btchip_compress_public_key_value(public_key.W);
+        btchip_compress_public_key_value(public_key);
         keyLength = 33;
     }
 
-    memmove(publicKey, public_key.W,
-               sizeof(public_key.W));
+    memmove(publicKey, public_key,
+               sizeof(public_key));
     return keyLength;
 }
 
@@ -117,7 +119,11 @@ unsigned short btchip_apdu_get_wallet_public_key() {
     unsigned char bip44_enforced = enforce_bip44_coin_type(G_io_apdu_buffer + ISO_OFFSET_CDATA, true);
 
     G_io_apdu_buffer[0] = 65;
-    keyLength = get_public_key_chain_code(G_io_apdu_buffer + ISO_OFFSET_CDATA, uncompressedPublicKeys, G_io_apdu_buffer + 1, chainCode);
+    keyLength = get_public_key_chain_code(G_io_apdu_buffer + ISO_OFFSET_CDATA, MAX_BIP32_PATH_LENGTH, uncompressedPublicKeys, G_io_apdu_buffer + 1, chainCode);
+
+    if (keyLength == 0) {
+        return BTCHIP_SW_TECHNICAL_PROBLEM;
+    }
 
     if (cashAddr) {
         uint8_t tmp[20];