Merge pull request #60 from onflow/getpubkey_changes

Changed getPublicKey call. The call now gets slot index as an argument and reads address and derivation path from it. It returns address and pubkey. Added address valiadation into setSlot calls. Several refactors
onflow · Sep 27, 2021 · c54d735 · c54d735
2 parents 33b52c5 + ba0bce6
commit c54d735
Show file tree

Hide file tree

Showing 70 changed files with 3,267 additions and 3,738 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -64,22 +64,22 @@ jobs:
           command: |
             make
       - run:
-          name: Install node + yarn
+          name: Install node + yarn 
           command: |
-            nvm install 14.4.0
-            nvm use 14.4.0
+            nvm install 12.16.2
+            nvm use 12.16.2
             npm install -g yarn
       - run:
           name: Build/Install build js deps
           command: |
-            nvm use 14.4.0
+            nvm use 12.16.2
             make zemu_install
       - run:
           name: Run zemu tests
           command: |
-            nvm use 14.4.0
+            nvm use 12.16.2
             make zemu_test
-          no_output_timeout: 20m
+          no_output_timeout: 30m
 
   build_package:
     docker:

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -89,6 +89,7 @@ file(GLOB_RECURSE LIB_SRC
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/uint256.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_impl.c
+        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/coin.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/json/json_parser.c
         app/src/base32.c
         app/src/crypto.c

diff --git a/README.md b/README.md
@@ -143,7 +143,7 @@ To run a single specific test:
 
 > At the moment, the recommendation is to run from the IDE. Remember to run `make` if you change the app.
 
-## Using a real device
+## Using a real device for integration tests
 
 ### How to prepare your DEVELOPMENT! device:
 
@@ -190,6 +190,12 @@ Many of our integration tests expect the device to be configured with a known te
 
 - Run `make dev_ca`. The device will receive a development certificate to avoid constant manual confirmations.
 
+## Using a real device for a single test
+
+- Use `make clean`, `make`, and `make load` to load the app to the ledger device.
+- Make adjustments in `js/test/transactions.spec.js` if needed
+- Run `yarn test` in the `js` folder. 
+
 ### Loading the app
 
 The Makefile will build the firmware in a docker container and leave the binary in the correct directory.

diff --git a/app/Makefile b/app/Makefile
@@ -48,7 +48,7 @@ endif
 
 APPVERSION_M=0
 APPVERSION_N=9
-APPVERSION_P=10
+APPVERSION_P=11
 
 $(info COIN  = [$(COIN)])
 ifeq ($(COIN),FLOW)

diff --git a/app/src/account.c b/app/src/account.c
@@ -14,6 +14,7 @@
 *  limitations under the License.
 ********************************************************************************/
 #include "account.h"
+#include "coin.h"
 #include "zxmacros.h"
 #include "stdbool.h"
 #include "apdu_codes.h"
@@ -32,6 +33,9 @@ slot_store_t NV_CONST
 N_slot_store_impl __attribute__ ((aligned(64)));
 #define N_slot_store (*(NV_VOLATILE slot_store_t *)PIC(&N_slot_store_impl))
 
+
+
+
 uint8_t slot_is_empty(const account_slot_t *tmp) {
     return tmp->path.data[0] == 0;
 }
@@ -72,7 +76,7 @@ zxerr_t slot_getItem(int8_t displayIdx,
                 }
                 case 1: {
                     snprintf(outKey, outKeyLen, "Account");
-                    array_to_hexstr(outVal, outValLen, tmp_slot.account.data, 8);
+                    array_to_hexstr_with_0x(outVal, outValLen, tmp_slot.account.data, 8);
                     return zxerr_ok;
                 }
                 case 2: {
@@ -96,7 +100,7 @@ zxerr_t slot_getItem(int8_t displayIdx,
                 }
                 case 1: {
                     snprintf(outKey, outKeyLen, "Old Account");
-                    array_to_hexstr(outVal, outValLen, oldSlot->account.data, 8);
+                    array_to_hexstr_with_0x(outVal, outValLen, oldSlot->account.data, 8);
                     return zxerr_ok;
                 }
                 case 2: {
@@ -108,7 +112,7 @@ zxerr_t slot_getItem(int8_t displayIdx,
                 }
                 case 3: {
                     snprintf(outKey, outKeyLen, "New Account");
-                    array_to_hexstr(outVal, outValLen, tmp_slot.account.data, 8);
+                    array_to_hexstr_with_0x(outVal, outValLen, tmp_slot.account.data, 8);
                     return zxerr_ok;
                 }
                 case 4: {
@@ -133,7 +137,7 @@ zxerr_t slot_getItem(int8_t displayIdx,
                 }
                 case 1: {
                     snprintf(outKey, outKeyLen, "Old Account");
-                    array_to_hexstr(outVal, outValLen, oldSlot->account.data, 8);
+                    array_to_hexstr_with_0x(outVal, outValLen, oldSlot->account.data, 8);
                     return zxerr_ok;
                 }
                 case 2: {
@@ -204,13 +208,10 @@ zxerr_t slot_parseSlot(uint8_t *buffer, uint16_t bufferLen) {
         return zxerr_out_of_bounds;
     }
 
+    //We copy account and path. Note that path is copied in app_main.c:extractHDPath (for sign transaction call).
     MEMCPY(&tmp_slot, buffer + 1, sizeof(account_slot_t));
 
-    const bool mainnet = tmp_slot.path.data[0] == HDPATH_0_DEFAULT && tmp_slot.path.data[1] == HDPATH_1_DEFAULT;
-    const bool testnet = tmp_slot.path.data[0] == HDPATH_0_TESTNET && tmp_slot.path.data[1] == HDPATH_1_TESTNET;
-    const bool empty = tmp_slot.path.data[0] == 0 && tmp_slot.path.data[1] == 0;
-
-    if (!mainnet && !testnet && !empty) {
+    if (!path_is_mainnet(tmp_slot.path) && !path_is_testnet(tmp_slot.path) && !path_is_empty(tmp_slot.path)) {
         array_to_hexstr(bufferUI, sizeof(bufferUI), tmp_slot.account.data, 8);
         zemu_log(bufferUI);
         zemu_log("\n");
@@ -223,6 +224,27 @@ zxerr_t slot_parseSlot(uint8_t *buffer, uint16_t bufferLen) {
         return zxerr_out_of_bounds;
     }
 
+    //account validation
+    if (!path_is_empty(tmp_slot.path)) { //no validation required if the new slot should be empty
+        uint64_t account = 0;
+        //LE <-> BE conversion necessary to validate account
+        for(int i=0; i<SLOT_ACCOUNT_SIZE; i++) account = 256*account + tmp_slot.account.data[i];
+
+        if (path_is_mainnet(tmp_slot.path)) {
+            if (!validateChainAddress(CODEWORD_MAINNET, account)) {
+                zemu_log_stack("invalid account");
+                return zxerr_out_of_bounds;
+            }
+        }
+        if (path_is_testnet(tmp_slot.path)) {
+            if (!validateChainAddress(CODEWORD_TESTNET, account) && 
+                    !validateChainAddress(CODEWORD_EMULATORNET, account)) {
+                zemu_log_stack("invalid account");
+                return zxerr_out_of_bounds;
+            }
+        }
+    }
+
     tmp_slotop = SLOT_OP_UPDATE;
     if (slot_is_empty(&N_slot_store.slot[tmp_slotIdx])) {
         tmp_slotop = SLOT_OP_SET;

diff --git a/app/src/account.h b/app/src/account.h
@@ -30,10 +30,6 @@ typedef struct {
     uint8_t data[SLOT_ACCOUNT_SIZE];
 } flow_account_t;
 
-typedef struct {
-    uint32_t data[HDPATH_LEN_DEFAULT];
-} flow_path_t;
-
 typedef struct {
     flow_account_t account;
     flow_path_t path;
@@ -61,6 +57,22 @@ zxerr_t slot_parseSlot(uint8_t *buffer, uint16_t bufferLen);
 
 void app_slot_setSlot();
 
+//To add to zxlib after fork
+//For now just plain display
+__Z_INLINE uint32_t array_to_hexstr_with_0x(char *dst, uint16_t dstLen, const uint8_t *src, uint8_t count) {
+/*    if (dstLen < 2) {
+        return 0;
+    }
+    dst[0]='0'; 
+    dst[1]='x';
+    uint32_t res = array_to_hexstr(dst+2, dstLen-2, src, count);
+    if (res == 0) {
+        return 0;
+    }
+    return res;*/
+    return array_to_hexstr(dst, dstLen, src, count);
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/app/src/addr.c b/app/src/addr.c
@@ -15,39 +15,66 @@
 ********************************************************************************/
 
 #include <stdio.h>
+#include <os.h>
 #include "coin.h"
 #include "zxerror.h"
 #include "zxmacros.h"
 #include "app_mode.h"
 #include "crypto.h"
+#include "account.h"
 
 zxerr_t addr_getNumItems(uint8_t *num_items) {
     zemu_log_stack("addr_getNumItems");
-    *num_items = 1;
     if (app_mode_expert()) {
-        *num_items = 2;
+        *num_items = 4;
+        return zxerr_ok;
     }
+    *num_items = 3;
     return zxerr_ok;
 }
 
+//To add to zxlib after fork
+__Z_INLINE void pageHexStringFromBuf(char *outValue, uint16_t outValueLen,
+                           uint8_t *inValue, uint16_t inValueLen,
+                           uint8_t pageIdx, uint8_t *pageCount) {
+    char buf[2*inValueLen+1];
+    uint_fast32_t len = array_to_hexstr(buf, sizeof(buf), inValue, inValueLen);
+    if (len == 0) {
+        return;
+    }
+    pageString(outValue, outValueLen, buf, pageIdx, pageCount);
+}
+
+
 zxerr_t addr_getItem(int8_t displayIdx,
                      char *outKey, uint16_t outKeyLen,
                      char *outVal, uint16_t outValLen,
                      uint8_t pageIdx, uint8_t *pageCount) {
     zemu_log_stack("addr_getItem");
     switch (displayIdx) {
-        case 0:
-            snprintf(outKey, outKeyLen, "Pub Key");
-            pageString(outVal, outValLen, (char *) (G_io_apdu_buffer + VIEW_ADDRESS_OFFSET_SECP256K1), pageIdx, pageCount);
+        case 0: {
+            snprintf(outKey, outKeyLen, "Account");
+            array_to_hexstr_with_0x(outVal, outValLen, G_io_apdu_buffer, sizeof(flow_account_t));
             return zxerr_ok;
+        }
         case 1: {
+            snprintf(outKey, outKeyLen, "Pub Key");
+            pageHexStringFromBuf(outVal, outValLen, G_io_apdu_buffer + sizeof(flow_account_t), PUBLIC_KEY_LEN, pageIdx, pageCount);
+            return zxerr_ok;
+        }
+        case 2: {
+            snprintf(outKey, outKeyLen, "Warning");
+            pageString(outVal, outValLen, "Ledger does not check if the on-chain account includes the pub key!", pageIdx, pageCount);
+            return zxerr_ok;        
+        }
+        case 3: {
             if (!app_mode_expert()) {
                 return zxerr_no_data;
             }
 
             snprintf(outKey, outKeyLen, "Your Path");
             char buffer[300];
-            bip32_to_str(buffer, sizeof(buffer), hdPath, HDPATH_LEN_DEFAULT);
+            bip32_to_str(buffer, sizeof(buffer), hdPath.data, HDPATH_LEN_DEFAULT);
             pageString(outVal, outValLen, buffer, pageIdx, pageCount);
             return zxerr_ok;
         }

diff --git a/app/src/apdu_handler.c b/app/src/apdu_handler.c
@@ -31,12 +31,42 @@
 #include "zxmacros.h"
 
 __Z_INLINE void handleGetPubkey(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
-    extractHDPath(rx, OFFSET_DATA);
+    if (rx != 6) {
+        THROW(APDU_CODE_DATA_INVALID);
+    }
+
+    uint8_t displayAndConfirm = G_io_apdu_buffer[OFFSET_P1];
+    const uint8_t slotIdx = G_io_apdu_buffer[OFFSET_DATA];
+
+    char buffer[20];
+    snprintf(buffer, sizeof(buffer), "%d", slotIdx);
+    zemu_log_stack(buffer);
+
+    account_slot_t slot;
+    zxerr_t err = slot_getSlot(slotIdx,(uint8_t *) &slot, sizeof(slot));
+    snprintf(buffer, sizeof(buffer), "err: %d", err);
+    zemu_log_stack(buffer);
+
+    if (err == zxerr_no_data) { 
+        zemu_log_stack("Empty slot");
+        THROW(APDU_CODE_DATA_INVALID);
+    }
+    if (err != zxerr_ok) {
+        THROW(APDU_CODE_EXECUTION_ERROR);
+    }
+
+    MEMCPY(&hdPath, slot.path.data, sizeof(hdPath)); 
+
+    //We fill account and pubkey
+    MEMCPY(G_io_apdu_buffer, &slot.account, sizeof(slot.account));
 
-    uint8_t requireConfirmation = G_io_apdu_buffer[OFFSET_P1];
+    *tx = sizeof(slot.account);
+    *tx += app_fill_pubkey(G_io_apdu_buffer + sizeof(slot.account), IO_APDU_BUFFER_SIZE - sizeof(slot.account)); 
 
-    if (requireConfirmation) {
-        app_fill_address();
+    //Display or return the buffer    
+    if (displayAndConfirm) {
+        //we set the returning length for app_reply_address
+        action_addr_len = *tx;
 
         view_review_init(addr_getItem, addr_getNumItems, app_reply_address);
         view_review_show();
@@ -45,7 +75,6 @@ __Z_INLINE void handleGetPubkey(volatile uint32_t *flags, volatile uint32_t *tx,
         return;
     }
 
-    *tx = app_fill_address();
     THROW(APDU_CODE_OK);
 }
 

diff --git a/app/src/coin.c b/app/src/coin.c
@@ -0,0 +1,58 @@
+/*******************************************************************************
+*   (c) 2019 Zondax GmbH
+*
+*  Licensed under the Apache License, Version 2.0 (the "License");
+*  you may not use this file except in compliance with the License.
+*  You may obtain a copy of the License at
+*
+*      http://www.apache.org/licenses/LICENSE-2.0
+*
+*  Unless required by applicable law or agreed to in writing, software
+*  distributed under the License is distributed on an "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+*  See the License for the specific language governing permissions and
+*  limitations under the License.
+********************************************************************************/
+
+#include "crypto.h"
+// based on Dapper provided code at https://github.com/onflow/flow-go-sdk/blob/96796f0cabc1847d7879a5230ab55fd3cdd41ae8/address.go#L286
+
+const uint16_t linearCodeN = 64;
+
+const uint32_t parityCheckMatrixColumns[] = {
+        0x00001, 0x00002, 0x00004, 0x00008,
+        0x00010, 0x00020, 0x00040, 0x00080,
+        0x00100, 0x00200, 0x00400, 0x00800,
+        0x01000, 0x02000, 0x04000, 0x08000,
+        0x10000, 0x20000, 0x40000, 0x7328d,
+        0x6689a, 0x6112f, 0x6084b, 0x433fd,
+        0x42aab, 0x41951, 0x233ce, 0x22a81,
+        0x21948, 0x1ef60, 0x1deca, 0x1c639,
+        0x1bdd8, 0x1a535, 0x194ac, 0x18c46,
+        0x1632b, 0x1529b, 0x14a43, 0x13184,
+        0x12942, 0x118c1, 0x0f812, 0x0e027,
+        0x0d00e, 0x0c83c, 0x0b01d, 0x0a831,
+        0x0982b, 0x07034, 0x0682a, 0x05819,
+        0x03807, 0x007d2, 0x00727, 0x0068e,
+        0x0067c, 0x0059d, 0x004eb, 0x003b4,
+        0x0036a, 0x002d9, 0x001c7, 0x0003f,
+};
+
+bool validateChainAddress(uint64_t chainCodeWord, uint64_t address) {
+    uint64_t codeWord = address ^chainCodeWord;
+
+    if (codeWord == 0) {
+        return false;
+    }
+
+    uint64_t parity = 0;
+    for (uint16_t i = 0; i < linearCodeN; i++) {
+        if ((codeWord & 1) == 1) {
+            parity ^= parityCheckMatrixColumns[i];
+        }
+        codeWord >>= 1;
+    }
+
+    return parity == 0;
+}
+