separate error codes in crypto.* functions (#41)

* separate error codes in crypto.* functions
* update dependencies
* update hash used in RFC6979
* bumping version number

app/Makefile

@@ -48,7 +48,7 @@ endif
 
 APPVERSION_M=0
 APPVERSION_N=7
-APPVERSION_P=2
+APPVERSION_P=3
 
 $(info COIN  = [$(COIN)])
 ifeq ($(COIN),FLOW)

app/src/common/actions.c

@@ -17,4 +17,4 @@
 
 #include "actions.h"
 
-uint8_t action_addr_len;
+uint16_t action_addr_len;

app/src/common/actions.h

@@ -22,19 +22,21 @@
 #include <os_io_seproxyhal.h>
 #include "coin.h"
 
-extern uint8_t action_addr_len;
+extern uint16_t action_addr_len;
 
 __Z_INLINE void app_sign() {
     const uint8_t *message = tx_get_buffer();
     const uint16_t messageLength = tx_get_buffer_length();
-    const uint8_t replyLen = crypto_sign(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, message, messageLength);
 
-    if (replyLen > 0) {
-        set_code(G_io_apdu_buffer, replyLen, APDU_CODE_OK);
-        io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, replyLen + 2);
-    } else {
+    uint16_t replyLen = 0;
+    zxerr_t err = crypto_sign(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 3, message, messageLength, &replyLen);
+
+    if (err != zxerr_ok || replyLen == 0) {
         set_code(G_io_apdu_buffer, 0, APDU_CODE_SIGN_VERIFY_ERROR);
         io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, 2);
+    } else {
+        set_code(G_io_apdu_buffer, replyLen, APDU_CODE_OK);
+        io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, replyLen + 2);
     }
 }
 
@@ -46,14 +48,11 @@ __Z_INLINE void app_reject() {
 __Z_INLINE uint8_t app_fill_address() {
     // Put data directly in the apdu buffer
     MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
-    action_addr_len = crypto_fillAddress(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 2);
 
-    char buffer[100];
-    snprintf(buffer, 100, "???? %d", action_addr_len);
-    zemu_log_stack(buffer);
+    action_addr_len = 0;
+    zxerr_t err = crypto_fillAddress(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE - 2, &action_addr_len);
 
-    if (action_addr_len == 0) {
-        zemu_log_stack("crypto_fillAddress");
+    if (err != zxerr_ok || action_addr_len == 0) {
         THROW(APDU_CODE_EXECUTION_ERROR);
     }
 

app/src/crypto.c

@@ -72,17 +72,21 @@ __Z_INLINE enum cx_md_e get_cx_hash_kind() {
     }
 }
 
-uint8_t crypto_extractPublicKey(const uint32_t path[HDPATH_LEN_DEFAULT], uint8_t *pubKey, uint16_t pubKeyLen) {
+zxerr_t crypto_extractPublicKey(const uint32_t path[HDPATH_LEN_DEFAULT], uint8_t *pubKey, uint16_t pubKeyLen) {
+    zemu_log_stack("crypto_extractPublicKey");
     MEMZERO(pubKey, pubKeyLen);
+
     cx_curve_t curve = get_cx_curve();
-    if (curve==CX_CURVE_NONE) {
-        return 0;
+    if (curve!=CX_CURVE_SECP256K1 && curve!=CX_CURVE_SECP256R1 ) {
+        zemu_log_stack("extractPublicKey: invalid_crypto_settings");
+        return zxerr_invalid_crypto_settings;
     }
 
     const uint32_t domainSize = 32;
     const uint32_t pkSize = 1 + 2 * domainSize;
     if (pubKeyLen < pkSize) {
-        return 0;
+        zemu_log_stack("extractPublicKey: zxerr_buffer_too_small");
+        return zxerr_buffer_too_small;
     }
 
     cx_ecfp_public_key_t cx_publicKey;
@@ -92,13 +96,17 @@ uint8_t crypto_extractPublicKey(const uint32_t path[HDPATH_LEN_DEFAULT], uint8_t
     BEGIN_TRY
     {
         TRY {
+            zemu_log_stack("extractPublicKey: derive_node_bip32");
             os_perso_derive_node_bip32(curve,
                                        path,
                                        HDPATH_LEN_DEFAULT,
                                        privateKeyData, NULL);
 
+            zemu_log_stack("extractPublicKey: cx_ecfp_init_private_key");
             cx_ecfp_init_private_key(curve, privateKeyData, 32, &cx_privateKey);
             cx_ecfp_init_public_key(curve, NULL, 0, &cx_publicKey);
+
+            zemu_log_stack("extractPublicKey: cx_ecfp_generate_pair");
             cx_ecfp_generate_pair(curve, &cx_publicKey, &cx_privateKey, 1);
         }
         FINALLY {
@@ -109,7 +117,7 @@ uint8_t crypto_extractPublicKey(const uint32_t path[HDPATH_LEN_DEFAULT], uint8_t
     END_TRY;
 
     memcpy(pubKey, cx_publicKey.W, pkSize);
-    return pkSize;
+    return zxerr_ok;
 }
 
 typedef struct {
@@ -131,14 +139,15 @@ uint16_t digest_message(uint8_t *digest, uint16_t digestMax, const uint8_t *mess
         case sha2_256: {
             zemu_log_stack("sha2_256");
             if (digestMax < CX_SHA256_SIZE) {
-                return 0;
+                zemu_log_stack("digest_message: zxerr_buffer_too_small");
+                return zxerr_buffer_too_small;
             }
             sha256(message, messageLen, digest);
             return CX_SHA256_SIZE;
         }
         case sha3_256: {
             if (digestMax < 32) {
-                return 0;
+                return zxerr_buffer_too_small;
             }
             zemu_log_stack("sha3_256");
             cx_sha3_t sha3;
@@ -147,24 +156,35 @@ uint16_t digest_message(uint8_t *digest, uint16_t digestMax, const uint8_t *mess
             zemu_log_stack("sha3_256 ready");
             return 32;
         }
-        default:
-            return 0;
+        default: {
+            zemu_log_stack("digest_message: zxerr_invalid_crypto_settings");
+            return zxerr_invalid_crypto_settings;
+        }
     }
 }
 
-uint16_t crypto_sign(uint8_t *buffer, uint16_t signatureMaxlen, const uint8_t *message, uint16_t messageLen) {
+zxerr_t crypto_sign(uint8_t *buffer, uint16_t signatureMaxlen, const uint8_t *message, uint16_t messageLen, uint16_t *sigSize) {
+    zemu_log_stack("crypto_sign");
+
     cx_curve_t curve = get_cx_curve();
-    if (curve==CX_CURVE_NONE) {
-        return 0;
+    if (curve!=CX_CURVE_SECP256K1 && curve!=CX_CURVE_SECP256R1 ) {
+        zemu_log_stack("crypto_sign: invalid_crypto_settings");
+        return zxerr_invalid_crypto_settings;
     }
 
     const uint32_t domainSize = 32;
-    uint8_t messageDigest[128];
+    uint8_t messageDigest[32];
 
     const enum cx_md_e cxhash_kind = get_cx_hash_kind();
     const uint16_t messageDigestSize = digest_message(messageDigest, sizeof(messageDigest), message, messageLen );
-    if (cxhash_kind == CX_NONE || messageDigestSize == 0) {
-        return 0;
+    if (messageDigestSize != 32) {
+        zemu_log_stack("crypto_sign: zxerr_out_of_bounds");
+        return zxerr_out_of_bounds;
+    }
+
+    if (cxhash_kind != CX_SHA256 && cxhash_kind != CX_SHA3) {
+        zemu_log_stack("crypto_sign: zxerr_invalid_crypto_settings");
+        return zxerr_invalid_crypto_settings;
     }
 
     cx_ecfp_private_key_t cx_privateKey;
@@ -191,7 +211,7 @@ uint16_t crypto_sign(uint8_t *buffer, uint16_t signatureMaxlen, const uint8_t *m
             zemu_log_stack("cx_ecdsa_sign");
             signatureLength = cx_ecdsa_sign(&cx_privateKey,
                                             CX_RND_RFC6979 | CX_LAST,
-                                            cxhash_kind,
+                                            CX_SHA256,
                                             messageDigest,
                                             messageDigestSize,
                                             signature->der_signature,
@@ -209,11 +229,12 @@ uint16_t crypto_sign(uint8_t *buffer, uint16_t signatureMaxlen, const uint8_t *m
     err_convert_e err = convertDERtoRSV(signature->der_signature, info,  signature->r, signature->s, &signature->v);
     if (err != no_error) {
         // Error while converting so return length 0
-        return 0;
+        return zxerr_invalid_crypto_settings;
     }
 
     // return actual size using value from signatureLength
-    return sizeof_field(signature_t, r) + sizeof_field(signature_t, s) + sizeof_field(signature_t, v) + signatureLength;
+    *sigSize = sizeof_field(signature_t, r) + sizeof_field(signature_t, s) + sizeof_field(signature_t, v) + signatureLength;
+    return zxerr_ok;
 }
 
 typedef struct {
@@ -222,22 +243,25 @@ typedef struct {
     uint8_t padding[4];
 } __attribute__((packed)) answer_t;
 
-uint16_t crypto_fillAddress(uint8_t *buffer, uint16_t buffer_len) {
+zxerr_t crypto_fillAddress(uint8_t *buffer, uint16_t buffer_len, uint16_t *addrLen) {
     MEMZERO(buffer, buffer_len);
 
     if (buffer_len < sizeof(answer_t)) {
-        return 0;
+        zemu_log_stack("crypto_fillAddress: zxerr_buffer_too_small");
+        return zxerr_buffer_too_small;
     }
 
     answer_t *const answer = (answer_t *) buffer;
 
-    if (crypto_extractPublicKey(hdPath, answer->publicKey, sizeof_field(answer_t, publicKey)) == 0 ) {
-        return 0;
+    zxerr_t err = crypto_extractPublicKey(hdPath, answer->publicKey, sizeof_field(answer_t, publicKey));
+    if ( err != zxerr_ok ) {
+        return err;
     }
 
     array_to_hexstr(answer->addrStr, sizeof_field(answer_t, addrStr) + 2, answer->publicKey, sizeof_field(answer_t, publicKey) );
 
-    return sizeof(answer_t) - sizeof_field(answer_t, padding);
+    *addrLen = sizeof(answer_t) - sizeof_field(answer_t, padding);
+    return zxerr_ok;
 }
 
 #endif

app/src/crypto.h

@@ -24,6 +24,7 @@ extern "C" {
 #include "coin.h"
 #include <stdbool.h>
 #include <sigutils.h>
+#include <zxerror.h>
 
 typedef enum {
     hash_unknown,
@@ -48,14 +49,15 @@ extern uint32_t hdPath[HDPATH_LEN_DEFAULT];
 
 bool isTestnet();
 
-uint8_t crypto_extractPublicKey(const uint32_t path[HDPATH_LEN_DEFAULT], uint8_t *pubKey, uint16_t pubKeyLen);
+zxerr_t crypto_extractPublicKey(const uint32_t path[HDPATH_LEN_DEFAULT], uint8_t *pubKey, uint16_t pubKeyLen);
 
-uint16_t crypto_fillAddress(uint8_t *buffer, uint16_t bufferLen);
+zxerr_t crypto_fillAddress(uint8_t *buffer, uint16_t bufferLen, uint16_t *addrLen);
 
-uint16_t crypto_sign(uint8_t *signature,
+zxerr_t crypto_sign(uint8_t *signature,
                      uint16_t signatureMaxlen,
                      const uint8_t *message,
-                     uint16_t messageLen);
+                     uint16_t messageLen,
+                     uint16_t *sigSize);
 
 #ifdef __cplusplus
 }

app/src/rlp.c

@@ -124,7 +124,7 @@ parser_error_t rlp_readUInt64(const parser_context_t *ctx,
 
     // handle case when string is a single byte
     if (ctx->bufferLen == 1) {
-        uint8_t tmp;
+        uint8_t tmp = 0;
         CHECK_PARSER_ERR(rlp_readByte(ctx, kind, &tmp))
         *value = tmp;
         return parser_ok;

deps/ledger-zxlib/include/zxerror.h

@@ -25,14 +25,48 @@ extern "C" {
     if (err!=zxerr_ok) return err;}
 
 typedef enum {
-    zxerr_ok,
-    zxerr_no_data,
-    zxerr_buffer_too_small,
-    zxerr_out_of_bounds,
-    zxerr_encoding_failed,
-    zxerr_unknown
+    zxerr_unknown = 0b00000000,
+    zxerr_ok = 0b00000011,
+    zxerr_no_data = 0b00000101,
+    zxerr_buffer_too_small = 0b00000110,
+    zxerr_out_of_bounds = 0b00001001,
+    zxerr_encoding_failed = 0b00001010,
+    zxerr_invalid_crypto_settings = 0b00001100,
 } zxerr_t;
 
+//0b00000000
+//0b00000011
+//0b00000101
+//0b00000110
+//0b00001001
+//0b00001010
+//0b00001100
+//0b00001111
+//0b00010001
+//0b00010010
+//0b00010100
+//0b00010111
+//0b00011000
+//0b00011011
+//0b00011101
+//0b00011110
+//0b00100001
+//0b00100010
+//0b00100100
+//0b00100111
+//0b00101000
+//0b00101011
+//0b00101101
+//0b00101110
+//0b00110000
+//0b00110011
+//0b00110101
+//0b00110110
+//0b00111001
+//0b00111010
+//0b00111100
+//0b00111111
+
 #ifdef __cplusplus
 }
 #endif

deps/ledger-zxlib/include/zxversion.h

@@ -16,5 +16,5 @@
 #pragma once
 
 #define ZXLIB_MAJOR     5
-#define ZXLIB_MINOR     0
-#define ZXLIB_PATCH     1
+#define ZXLIB_MINOR     1
+#define ZXLIB_PATCH     0

tests_zemu/package.json

@@ -17,7 +17,7 @@
   ],
   "dependencies": {
     "@zondax/ledger-flow": "^0.0.1",
-    "@zondax/zemu": "^0.6.0"
+    "@zondax/zemu": "^0.7.0"
   },
   "devDependencies": {
     "@babel/cli": "^7.11.6",
@@ -32,7 +32,7 @@
     "eslint-config-airbnb-base": "^14.2.0",
     "eslint-config-prettier": "^6.11.0",
     "eslint-plugin-import": "^2.22.0",
-    "eslint-plugin-jest": "^23.20.0",
+    "eslint-plugin-jest": "^24.0.0",
     "eslint-plugin-prettier": "^3.1.4",
     "jest": "26.4.2",
     "jest-serial-runner": "^1.1.0",

tests_zemu/tests/test.js

@@ -495,7 +495,7 @@ describe('Basic checks', function () {
             // Wait until we are not in the main menu
             await sim.waitUntilScreenIsNot(sim.getMainMenuSnapshot());
 
-            await sim.compareSnapshotsAndAccept(".", "sign_secp256k1_basic_verify_addNewKey_sha3_256", 14);
+            await sim.compareSnapshotsAndAccept(".", "sign_secp256k1_basic_verify_addNewKey_sha2_256", 14);
 
             let resp = await signatureRequest;
             console.log(resp);