Merge pull request #228 from Zondax/feat/remove_protobuf

Feat/remove protobuf

.github/workflows/guidelines_enforcer.yml

@@ -21,5 +21,3 @@ jobs:
   guidelines_enforcer:
     name: Call Ledger guidelines_enforcer
     uses: LedgerHQ/ledger-app-workflows/.github/workflows/reusable_guidelines_enforcer.yml@v1
-    with:
-      relative_app_directory: app

CMakeLists.txt

@@ -85,13 +85,6 @@ file(GLOB_RECURSE TINYCBOR_SRC
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/tinycbor/src/cborvalidation.c
         )
 
-#  static libs
-file(GLOB_RECURSE TINYPB_SRC
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/nanopb_tiny/pb_common.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/nanopb_tiny/pb_decode.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/nanopb_tiny/pb_encode.c
-        )
-
 file(GLOB_RECURSE LIB_SRC
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/hexutils.c
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/src/app_mode.c
@@ -108,22 +101,19 @@ file(GLOB_RECURSE LIB_SRC
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_impl.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/crypto.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/base32.c
-        ${CMAKE_CURRENT_SOURCE_DIR}/app/src/protobuf/*.c
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/parser_print_*.c
         )
 
 add_library(app_lib STATIC
         ${LIB_SRC}
         ${TINYCBOR_SRC}
-        ${TINYPB_SRC}
         )
 
 target_include_directories(app_lib PUBLIC
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/BLAKE2/ref
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/ledger-zxlib/include
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/tinycbor/src
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/picohash/
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/nanopb_tiny/
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/candid
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
@@ -146,7 +136,6 @@ target_include_directories(unittests PRIVATE
         ${CMAKE_CURRENT_SOURCE_DIR}/app/src/lib
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/tinycbor/src
         ${CMAKE_CURRENT_SOURCE_DIR}/deps/picohash/
-        ${CMAKE_CURRENT_SOURCE_DIR}/deps/nanopb_tiny/
         )
 
 
@@ -166,7 +155,6 @@ set_tests_properties(unittests PROPERTIES WORKING_DIRECTORY ${CMAKE_CURRENT_SOUR
 if (ENABLE_FUZZING)
     set(FUZZ_TARGETS
             parser_parse
-            parser_protobuf
             )
 
     foreach (target ${FUZZ_TARGETS})

Makefile

@@ -26,9 +26,6 @@ ifeq ($(BOLOS_SDK),)
 ZXLIB_COMPILE_STAX ?= 1
 include $(CURDIR)/deps/ledger-zxlib/dockerized_build.mk
 
-proto:
-	cd $(CURDIR)/app/src/protobuf && $(CURDIR)/deps/nanopb/generator/protoc ./base_types.proto ./types.proto ./governance.proto ./dfinity.proto --nanopb_out=.
-
 else
 default:
 	$(MAKE) -C app

app/Makefile

@@ -56,8 +56,7 @@ endif
 
 APP_LOAD_PARAMS = --curve secp256k1 $(COMMON_LOAD_PARAMS) --path $(APPPATH)
 
-NANOS_STACK_SIZE := 2050
-
+APP_STACK_MIN_SIZE := 3000
 include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.devices
 
 $(info TARGET_NAME  = [$(TARGET_NAME)])
@@ -72,9 +71,6 @@ CFLAGS += -Wvla  -Wno-implicit-fallthrough
 CFLAGS += -I$(MY_DIR)/../deps/tinycbor/src
 APP_SOURCE_PATH += $(MY_DIR)/../deps/tinycbor-ledger
 
-CFLAGS += -I$(MY_DIR)/../deps/nanopb/
-APP_SOURCE_PATH += $(MY_DIR)/../deps/nanopb_tiny/
-
 .PHONY: rust
 rust:
 	@echo "No rust code"

app/Makefile.version

@@ -1,6 +1,6 @@
 # This is the major version of this release
-APPVERSION_M=2
+APPVERSION_M=3
 # This is the minor version of this release
-APPVERSION_N=4
+APPVERSION_N=0
 # This is the patch version of this release
-APPVERSION_P=9
+APPVERSION_P=0

app/src/apdu_handler.c

@@ -73,7 +73,7 @@ __Z_INLINE bool process_chunk(volatile uint32_t *tx, uint32_t rx) {
         THROW(APDU_CODE_DATA_INVALID);
     }
 
-    bool is_stake_tx = parser_tx_obj.special_transfer_type == neuron_stake_transaction;
+    const bool is_stake_tx = parser_tx_obj.special_transfer_type == neuron_stake_transaction;
 
     uint32_t added;
     switch (payloadType) {
@@ -128,9 +128,9 @@ __Z_INLINE bool process_chunk(volatile uint32_t *tx, uint32_t rx) {
 __Z_INLINE void handleGetAddr(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
     extractHDPath(rx, OFFSET_DATA);
 
-    uint8_t requireConfirmation = G_io_apdu_buffer[OFFSET_P1];
+    const uint8_t requireConfirmation = G_io_apdu_buffer[OFFSET_P1];
 
-    zxerr_t zxerr = app_fill_address();
+    const zxerr_t zxerr = app_fill_address();
     if (zxerr != zxerr_ok) {
         *tx = 0;
         THROW(APDU_CODE_DATA_INVALID);
@@ -158,7 +158,7 @@ __Z_INLINE void handleSign(volatile uint32_t *flags, volatile uint32_t *tx, uint
     CHECK_APP_CANARY()
 
     if (error_msg != NULL) {
-        int error_msg_length = strlen(error_msg);
+        const uint32_t error_msg_length = strnlen(error_msg, sizeof(G_io_apdu_buffer));
         MEMCPY(G_io_apdu_buffer, error_msg, error_msg_length);
         *tx += (error_msg_length);
         THROW(APDU_CODE_DATA_INVALID);
@@ -181,7 +181,7 @@ __Z_INLINE void handleSignCombined(volatile uint32_t *flags, volatile uint32_t *
     CHECK_APP_CANARY()
 
     if (error_msg != NULL) {
-        int error_msg_length = strlen(error_msg);
+        const uint32_t error_msg_length = strnlen(error_msg, sizeof(G_io_apdu_buffer));
         MEMCPY(G_io_apdu_buffer, error_msg, error_msg_length);
         *tx += (error_msg_length);
         THROW(APDU_CODE_DATA_INVALID);
@@ -202,7 +202,9 @@ __Z_INLINE void handle_getversion(__Z_UNUSED volatile uint32_t *flags, volatile
     G_io_apdu_buffer[1] = LEDGER_MAJOR_VERSION;
     G_io_apdu_buffer[2] = LEDGER_MINOR_VERSION;
     G_io_apdu_buffer[3] = LEDGER_PATCH_VERSION;
-    G_io_apdu_buffer[4] = !IS_UX_ALLOWED;
+    // sdk won't pass the apdu message if device is locked
+    // keeping it for backwards compatibility
+    G_io_apdu_buffer[4] = 0;
 
     G_io_apdu_buffer[5] = (TARGET_ID >> 24) & 0xFF;
     G_io_apdu_buffer[6] = (TARGET_ID >> 16) & 0xFF;
@@ -214,7 +216,7 @@ __Z_INLINE void handle_getversion(__Z_UNUSED volatile uint32_t *flags, volatile
 }
 
 void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
-    uint16_t sw = 0;
+    volatile uint16_t sw = 0;
 
     BEGIN_TRY
     {
@@ -278,7 +280,7 @@ void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
                     break;
             }
             G_io_apdu_buffer[*tx] = sw >> 8;
-            G_io_apdu_buffer[*tx + 1] = sw;
+            G_io_apdu_buffer[*tx + 1] = sw & 0xFF;
             *tx += 2;
         }
         FINALLY

app/src/crypto.c

@@ -22,11 +22,6 @@
 
 uint32_t hdPath[HDPATH_LEN_DEFAULT];
 
-bool isTestnet() {
-    return hdPath[0] == HDPATH_0_TESTNET &&
-           hdPath[1] == HDPATH_1_TESTNET;
-}
-
 uint8_t const DER_PREFIX[] = {0x30, 0x56, 0x30, 0x10, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01,
                               0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x0a, 0x03, 0x42, 0x00};
 
@@ -53,7 +48,7 @@ zxerr_t hash_sha224(uint8_t *input, uint16_t inputLen, uint8_t *output, uint16_t
     }
     cx_sha256_t ctx;
     cx_sha224_init(&ctx);
-    cx_hash_no_throw(&ctx.header, CX_LAST, input, inputLen, output, 224);
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, CX_LAST, input, inputLen, output, 224));
     return zxerr_ok;
 }
 
@@ -69,11 +64,11 @@ zxerr_t crypto_extractPublicKey(uint8_t *pubKey, uint16_t pubKeyLen) {
     zxerr_t err = zxerr_ledger_api_error;
     CATCH_CXERROR(os_derive_bip32_no_throw(CX_CURVE_256K1, hdPath,
                                            HDPATH_LEN_DEFAULT,
-                                           privateKeyData, NULL))
+                                           privateKeyData, NULL));
 
-    CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_256K1, privateKeyData, 32, &cx_privateKey))
-    CATCH_CXERROR(cx_ecfp_init_public_key_no_throw(CX_CURVE_256K1, NULL, 0, &cx_publicKey))
-    CATCH_CXERROR(cx_ecfp_generate_pair_no_throw(CX_CURVE_256K1, &cx_publicKey, &cx_privateKey, 1))
+    CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_256K1, privateKeyData, 32, &cx_privateKey));
+    CATCH_CXERROR(cx_ecfp_init_public_key_no_throw(CX_CURVE_256K1, NULL, 0, &cx_publicKey));
+    CATCH_CXERROR(cx_ecfp_generate_pair_no_throw(CX_CURVE_256K1, &cx_publicKey, &cx_privateKey, 1));
     memcpy(pubKey, cx_publicKey.W, SECP256K1_PK_LEN);
     err = zxerr_ok;
 
@@ -118,36 +113,36 @@ typedef struct {
 #define HASH_U64(FIELDNAME, FIELDVALUE, TMPDIGEST) { \
     MEMZERO(TMPDIGEST,sizeof(TMPDIGEST));                      \
     cx_hash_sha256((uint8_t *)FIELDNAME, sizeof(FIELDNAME) - 1, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0);         \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0));         \
     uint8_t ingressbuf[10];                                             \
     uint16_t enc_size = 0;                                              \
     CHECK_ZXERR(compressLEB128(FIELDVALUE, sizeof(ingressbuf), ingressbuf, &enc_size)); \
     cx_hash_sha256((uint8_t *)ingressbuf, enc_size, tmpdigest, CX_SHA256_SIZE);         \
-    cx_hash_no_throw(&ctx.header, 0, tmpdigest, CX_SHA256_SIZE, NULL, 0);                        \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, tmpdigest, CX_SHA256_SIZE, NULL, 0));                        \
 }
 
 #define HASH_BYTES_INTERMEDIATE(FIELDNAME, FIELDVALUE, TMPDIGEST) { \
     MEMZERO(TMPDIGEST,sizeof(TMPDIGEST));                      \
     cx_hash_sha256((uint8_t *)FIELDNAME, sizeof(FIELDNAME) - 1, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0);         \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0));         \
     cx_hash_sha256((uint8_t *)(FIELDVALUE).data, (FIELDVALUE).len, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0);                               \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0));                               \
 }
 
 #define HASH_BYTES_END(FIELDNAME, FIELDVALUE, TMPDIGEST, ENDDIGEST) { \
     MEMZERO(TMPDIGEST,sizeof(TMPDIGEST));                      \
     cx_hash_sha256((uint8_t *)FIELDNAME, sizeof(FIELDNAME) - 1, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0);         \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0));         \
     cx_hash_sha256((uint8_t *)(FIELDVALUE).data, (FIELDVALUE).len, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, CX_LAST, TMPDIGEST, CX_SHA256_SIZE, ENDDIGEST, CX_SHA256_SIZE);        \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, CX_LAST, TMPDIGEST, CX_SHA256_SIZE, ENDDIGEST, CX_SHA256_SIZE));        \
 }
 
 #define HASH_BYTES_PTR_END(FIELDNAME, FIELDVALUE, TMPDIGEST, ENDDIGEST) { \
     MEMZERO(TMPDIGEST,sizeof(TMPDIGEST));                      \
     cx_hash_sha256((uint8_t *)FIELDNAME, sizeof(FIELDNAME) - 1, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0);         \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, TMPDIGEST, CX_SHA256_SIZE, NULL, 0));         \
     cx_hash_sha256((uint8_t *)(FIELDVALUE).dataPtr, (FIELDVALUE).len, TMPDIGEST, CX_SHA256_SIZE); \
-    cx_hash_no_throw(&ctx.header, CX_LAST, TMPDIGEST, CX_SHA256_SIZE, ENDDIGEST, CX_SHA256_SIZE);        \
+    CHECK_CX_OK(cx_hash_no_throw(&ctx.header, CX_LAST, TMPDIGEST, CX_SHA256_SIZE, ENDDIGEST, CX_SHA256_SIZE));        \
 }
 
 zxerr_t crypto_getDigest(uint8_t *digest, txtype_e txtype){
@@ -178,15 +173,15 @@ zxerr_t crypto_getDigest(uint8_t *digest, txtype_e txtype){
             HASH_U64("ingress_expiry",fields->ingress_expiry, tmpdigest);
 
             cx_hash_sha256((uint8_t *)"paths", 5, tmpdigest, CX_SHA256_SIZE);
-            cx_hash_no_throw(&ctx.header, 0, tmpdigest, CX_SHA256_SIZE, NULL, 0);
+            CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, tmpdigest, CX_SHA256_SIZE, NULL, 0));
 
             uint8_t arrayBuffer[PATH_MAX_ARRAY * CX_SHA256_SIZE];
             for (size_t index = 0; index < fields->paths.arrayLen ; index++){
                     cx_hash_sha256((uint8_t *)fields->paths.paths[index].data, fields->paths.paths[index].len, arrayBuffer + index * CX_SHA256_SIZE, CX_SHA256_SIZE);
             }
             cx_hash_sha256(arrayBuffer, fields->paths.arrayLen*CX_SHA256_SIZE, tmpdigest, CX_SHA256_SIZE);
             cx_hash_sha256(tmpdigest, CX_SHA256_SIZE, tmpdigest, CX_SHA256_SIZE);
-            cx_hash_no_throw(&ctx.header, 0, tmpdigest, CX_SHA256_SIZE, NULL, 0);
+            CHECK_CX_OK(cx_hash_no_throw(&ctx.header, 0, tmpdigest, CX_SHA256_SIZE, NULL, 0));
 
             HASH_BYTES_END("request_type", parser_tx_obj.request_type, tmpdigest, digest);
             return zxerr_ok;
@@ -205,8 +200,7 @@ zxerr_t crypto_sign(uint8_t *signatureBuffer,
         return zxerr_buffer_too_small;
     }
 
-    uint8_t message_digest[CX_SHA256_SIZE];
-    MEMZERO(message_digest,sizeof(message_digest));
+    uint8_t message_digest[CX_SHA256_SIZE] = {0};
 
     signatureBuffer[0] = 0x0a;
     MEMCPY(&signatureBuffer[1], (uint8_t *)"ic-request",SIGN_PREFIX_SIZE - 1);
@@ -228,9 +222,9 @@ zxerr_t crypto_sign(uint8_t *signatureBuffer,
     CATCH_CXERROR(os_derive_bip32_no_throw(CX_CURVE_SECP256K1,
                              hdPath,
                              HDPATH_LEN_DEFAULT,
-                             privateKeyData, NULL))
+                             privateKeyData, NULL));
 
-    CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeyData, 32, &cx_privateKey))
+    CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeyData, 32, &cx_privateKey));
 
     // Sign
     CATCH_CXERROR(cx_ecdsa_sign_no_throw(&cx_privateKey,
@@ -240,7 +234,7 @@ zxerr_t crypto_sign(uint8_t *signatureBuffer,
                            CX_SHA256_SIZE,
                            signature->der_signature,
                            &signatureLength,
-                           &info))
+                           &info));
 
     err_convert_e err_c = convertDERtoRSV(signature->der_signature, info,  signature->r, signature->s, &signature->v);
     if (err_c != no_error) {
@@ -309,9 +303,9 @@ zxerr_t crypto_sign_combined(uint8_t *signatureBuffer,
     CATCH_CXERROR(os_derive_bip32_no_throw(CX_CURVE_SECP256K1,
                                            hdPath,
                                            HDPATH_LEN_DEFAULT,
-                                           privateKeyData, NULL))
+                                           privateKeyData, NULL));
 
-    CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeyData, 32, &cx_privateKey))
+    CATCH_CXERROR(cx_ecfp_init_private_key_no_throw(CX_CURVE_SECP256K1, privateKeyData, 32, &cx_privateKey));
 
     // Sign request
     CATCH_CXERROR(cx_ecdsa_sign_no_throw(&cx_privateKey,
@@ -321,7 +315,7 @@ zxerr_t crypto_sign_combined(uint8_t *signatureBuffer,
                                          CX_SHA256_SIZE,
                                          sigma.der_signature,
                                          &sigLen,
-                                         &info))
+                                         &info));
 
     err_convert_e err_c = convertDERtoRSV(sigma.der_signature, info,  sigma.r, sigma.s, &sigma.v);
     if (err_c != no_error) {
@@ -343,7 +337,7 @@ zxerr_t crypto_sign_combined(uint8_t *signatureBuffer,
                                              CX_SHA256_SIZE,
                                              sigma.der_signature,
                                              &sigLen,
-                                             &info))
+                                             &info));
 
         err_c = convertDERtoRSV(sigma.der_signature, info,  sigma.r, sigma.s, &sigma.v);
         if (err_c != no_error) {
@@ -665,9 +659,7 @@ zxerr_t crypto_fillAddress(uint8_t *buffer, uint16_t buffer_len, uint16_t *addrL
     CHECK_ZXERR(crypto_computePrincipal(answer->publicKey, answer->principalBytes));
 
     //For now only defeault subaccount, maybe later grab 32 bytes from the apdu buffer.
-    uint8_t zero_subaccount[DFINITY_SUBACCOUNT_LEN];
-    MEMZERO(zero_subaccount, DFINITY_SUBACCOUNT_LEN);
-
+    uint8_t zero_subaccount[DFINITY_SUBACCOUNT_LEN] = {0};
     CHECK_ZXERR(crypto_principalToSubaccount(answer->principalBytes, sizeof_field(answer_t, principalBytes),
                                              zero_subaccount, DFINITY_SUBACCOUNT_LEN, answer->subAccountBytes,
                                              sizeof_field(answer_t, subAccountBytes)));