From deb97ed02f267940a2cb5801289cf57fdd2c4ae6 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sun, 12 Jul 2020 21:08:55 +0200 Subject: [PATCH] Fix incorrect signatures (+additional tests) (#48) * Fix incorrect signatures * go back to initial screen --- app/Makefile | 2 +- app/src/coin.h | 2 - app/src/common/actions.h | 4 +- tests_zemu/package.json | 3 +- tests_zemu/tests/test.js | 27 ++++++++----- tests_zemu/tests/utils.js | 20 ++++++++++ tests_zemu/yarn.lock | 81 ++++++++++++++++++++++++++++++++++++--- 7 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 tests_zemu/tests/utils.js diff --git a/app/Makefile b/app/Makefile index 7a5528c9..b9b43f66 100755 --- a/app/Makefile +++ b/app/Makefile @@ -48,7 +48,7 @@ endif APPVERSION_M=0 APPVERSION_N=17 -APPVERSION_P=0 +APPVERSION_P=1 $(info COIN = [$(COIN)]) ifeq ($(COIN),FIL) diff --git a/app/src/coin.h b/app/src/coin.h index d454e9dc..bf857155 100644 --- a/app/src/coin.h +++ b/app/src/coin.h @@ -43,8 +43,6 @@ typedef enum { #define VIEW_ADDRESS_OFFSET_SECP256K1 (SECP256K1_PK_LEN + ADDRESS_PROTOCOL_SECP256K1_PAYLOAD_LEN + ADDRESS_PROTOCOL_LEN + 2) -#define CRYPTO_BLOB_SKIP_BYTES 1 - #define COIN_AMOUNT_DECIMAL_PLACES 18 #define COIN_SUPPORTED_TX_VERSION 0 diff --git a/app/src/common/actions.h b/app/src/common/actions.h index 60b19e5b..b388cc1d 100644 --- a/app/src/common/actions.h +++ b/app/src/common/actions.h @@ -27,8 +27,8 @@ extern uint8_t action_addr_len; __Z_INLINE void app_sign() { uint8_t *signature = G_io_apdu_buffer; - const uint8_t *message = tx_get_buffer() + CRYPTO_BLOB_SKIP_BYTES; - const uint16_t messageLength = tx_get_buffer_length() - CRYPTO_BLOB_SKIP_BYTES; + const uint8_t *message = tx_get_buffer(); + const uint16_t messageLength = tx_get_buffer_length(); const uint8_t replyLen = crypto_sign(signature, IO_APDU_BUFFER_SIZE - 3, message, messageLength); if (replyLen > 0) { diff --git a/tests_zemu/package.json b/tests_zemu/package.json index b00f8266..489e3b4c 100644 --- a/tests_zemu/package.json +++ b/tests_zemu/package.json @@ -36,6 +36,7 @@ "jest": "26.1.0", "jest-serial-runner": "^1.1.0", "crypto-js": "4.0.0", - "tweetnacl": "^1.0.3" + "secp256k1": "^4.0.1", + "blakejs": "^1.1.0" } } diff --git a/tests_zemu/tests/test.js b/tests_zemu/tests/test.js index a3ea682f..15a04677 100644 --- a/tests_zemu/tests/test.js +++ b/tests_zemu/tests/test.js @@ -17,6 +17,8 @@ import jest, {expect} from "jest"; import Zemu from "@zondax/zemu"; import FilecoinApp from "@zondax/ledger-filecoin"; +import {getDigest} from "./utils"; +import * as secp256k1 from "secp256k1"; const Resolve = require("path").resolve; const APP_PATH = Resolve("../app/bin/app.elf"); @@ -26,7 +28,7 @@ const sim_options = { logging: true, start_delay: 3000, custom: `-s "${APP_SEED}"` -// , X11: true + , X11: true }; jest.setTimeout(25000) @@ -127,9 +129,8 @@ describe('Basic checks', function () { // Derivation path. First 3 items are automatically hardened! const path = "m/44'/461'/5'/0/3"; const respRequest = app.showAddressAndPubKey(path); - - // We need to wait until the app responds to the APDU - await Zemu.sleep(2000); + // Wait until we are not in the main menu + await sim.waitUntilScreenIsNot(sim.getMainMenuSnapshot()); // Now navigate the address / path await sim.snapshot(`${snapshotPrefixTmp}${snapshotCount++}.png`); @@ -155,7 +156,7 @@ describe('Basic checks', function () { } }); - it('sign basic', async function () { + it('sign basic & verify', async function () { const snapshotPrefixGolden = "snapshots/sign-basic/"; const snapshotPrefixTmp = "snapshots-tmp/sign-basic/"; let snapshotCount = 0; @@ -178,8 +179,8 @@ describe('Basic checks', function () { // do not wait here.. const signatureRequest = app.sign(path, txBlob); - - await Zemu.sleep(2000); + // Wait until we are not in the main menu + await sim.waitUntilScreenIsNot(sim.getMainMenuSnapshot()); // Reference window await sim.snapshot(`${snapshotPrefixTmp}${snapshotCount++}.png`); @@ -195,6 +196,13 @@ describe('Basic checks', function () { expect(resp.return_code).toEqual(0x9000); expect(resp.error_message).toEqual("No errors"); + + // Verify signature + const pk = Uint8Array.from(pkResponse.compressed_pk) + const digest = getDigest( txBlob ); + const signature = secp256k1.signatureImport(Uint8Array.from(resp.signature_der)); + const signatureOk = secp256k1.ecdsaVerify(signature, digest, pk); + expect(signatureOk).toEqual(true); } finally { await sim.close(); } @@ -242,6 +250,7 @@ describe('Basic checks', function () { // Put the app in expert mode await sim.clickRight(); await sim.clickBoth(); + await sim.clickLeft(); const path = "m/44'/461'/0'/0/1"; const txBlob = Buffer.from( @@ -256,8 +265,8 @@ describe('Basic checks', function () { // do not wait here.. const signatureRequest = app.sign(path, txBlob); - - await Zemu.sleep(2000); + // Wait until we are not in the main menu + await sim.waitUntilScreenIsNot(sim.getMainMenuSnapshot()); // Reference window await sim.snapshot(`${snapshotPrefixTmp}${snapshotCount++}.png`); diff --git a/tests_zemu/tests/utils.js b/tests_zemu/tests/utils.js new file mode 100644 index 00000000..290ffdf5 --- /dev/null +++ b/tests_zemu/tests/utils.js @@ -0,0 +1,20 @@ +import blake from "blakejs"; + +const CID_PREFIX = Buffer.from([0x01, 0x71, 0xa0, 0xe4, 0x02, 0x20]); + +export function getCID(message) { + const blakeCtx = blake.blake2bInit(32); + blake.blake2bUpdate(blakeCtx, message); + const hash = blake.blake2bFinal(blakeCtx); + return Buffer.concat([CID_PREFIX, hash]); +} + +export function getDigest(message) { + // digest = blake2-256( prefix + blake2b-256(tx) ) + + const blakeCtx = blake.blake2bInit(32); + blake.blake2bUpdate(blakeCtx, getCID(message)); + return blake.blake2bFinal(blakeCtx); +} + +module.exports = { getCID, getDigest }; diff --git a/tests_zemu/yarn.lock b/tests_zemu/yarn.lock index 39b886c4..d05f79df 100644 --- a/tests_zemu/yarn.lock +++ b/tests_zemu/yarn.lock @@ -2164,6 +2164,16 @@ bl@^4.0.1: inherits "^2.0.4" readable-stream "^3.4.0" +blakejs@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/blakejs/-/blakejs-1.1.0.tgz#69df92ef953aa88ca51a32df6ab1c54a155fc7a5" + integrity sha1-ad+S75U6qIylGjLfarHFShVfx6U= + +bn.js@^4.4.0: + version "4.11.9" + resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.11.9.tgz#26d556829458f9d1e81fc48952493d0ba3507828" + integrity sha512-E6QoYqCKZfgatHTdHzs1RRKP7ip4vvm+EyRUeE2RF0NblwVvb0p6jSVeNTOFxPn26QXN2o6SMfNxKp6kU8zQaw== + brace-expansion@^1.1.7: version "1.1.11" resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-1.1.11.tgz#3c7fcbf529d87226f3d2f52b966ff5271eb441dd" @@ -2195,6 +2205,11 @@ braces@^3.0.1: dependencies: fill-range "^7.0.1" +brorand@^1.0.1: + version "1.1.0" + resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f" + integrity sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8= + browser-process-hrtime@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/browser-process-hrtime/-/browser-process-hrtime-1.0.0.tgz#3c9b4b7d782c8121e56f10106d84c0d0ffc94626" @@ -2807,6 +2822,19 @@ electron-to-chromium@^1.3.488: resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.496.tgz#3f43d32930481d82ad3663d79658e7c59a58af0b" integrity sha512-TXY4mwoyowwi4Lsrq9vcTUYBThyc1b2hXaTZI13p8/FRhY2CTaq5lK+DVjhYkKiTLsKt569Xes+0J5JsVXFurQ== +elliptic@^6.5.2: + version "6.5.3" + resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.3.tgz#cb59eb2efdaf73a0bd78ccd7015a62ad6e0f93d6" + integrity sha512-IMqzv5wNQf+E6aHeIqATs0tOLeOTwj1QKbRcS3jBbYkl5oLAserA8yJTT7/VyHUYG91PRmPyeQDObKLPpeS4dw== + dependencies: + bn.js "^4.4.0" + brorand "^1.0.1" + hash.js "^1.0.0" + hmac-drbg "^1.0.0" + inherits "^2.0.1" + minimalistic-assert "^1.0.0" + minimalistic-crypto-utils "^1.0.0" + emoji-regex@^7.0.1: version "7.0.3" resolved "https://registry.yarnpkg.com/emoji-regex/-/emoji-regex-7.0.3.tgz#933a04052860c85e83c122479c4748a8e4c72156" @@ -3595,6 +3623,23 @@ has@^1.0.3: dependencies: function-bind "^1.1.1" +hash.js@^1.0.0, hash.js@^1.0.3: + version "1.1.7" + resolved "https://registry.yarnpkg.com/hash.js/-/hash.js-1.1.7.tgz#0babca538e8d4ee4a0f8988d68866537a003cf42" + integrity sha512-taOaskGt4z4SOANNseOviYDvjEJinIkRgmp7LbKP2YTTmVxWBl87s/uzK9r+44BclBSp2X7K1hqeNfz9JbBeXA== + dependencies: + inherits "^2.0.3" + minimalistic-assert "^1.0.1" + +hmac-drbg@^1.0.0: + version "1.0.1" + resolved "https://registry.yarnpkg.com/hmac-drbg/-/hmac-drbg-1.0.1.tgz#d2745701025a6c775a6c545793ed502fc0c649a1" + integrity sha1-0nRXAQJabHdabFRXk+1QL8DGSaE= + dependencies: + hash.js "^1.0.3" + minimalistic-assert "^1.0.0" + minimalistic-crypto-utils "^1.0.1" + homedir-polyfill@^1.0.1: version "1.0.3" resolved "https://registry.yarnpkg.com/homedir-polyfill/-/homedir-polyfill-1.0.3.tgz#743298cef4e5af3e194161fbadcc2151d3a058e8" @@ -3698,7 +3743,7 @@ inflight@^1.0.4: once "^1.3.0" wrappy "1" -inherits@2, inherits@^2.0.3, inherits@^2.0.4, inherits@~2.0.3: +inherits@2, inherits@^2.0.1, inherits@^2.0.3, inherits@^2.0.4, inherits@~2.0.3: version "2.0.4" resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.4.tgz#0fa2c64f932917c3433a0ded55363aae37416b7c" integrity sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ== @@ -5077,6 +5122,16 @@ mimic-fn@^2.1.0: resolved "https://registry.yarnpkg.com/mimic-fn/-/mimic-fn-2.1.0.tgz#7ed2c2ccccaf84d3ffcb7a69b57711fc2083401b" integrity sha512-OqbOk5oEQeAZ8WXWydlu9HJjz9WVdEIvamMCcXmuqUYjTknH/sqsWvhQ3vgwKFRR1HpjvNBKQ37nbJgYzGqGcg== +minimalistic-assert@^1.0.0, minimalistic-assert@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/minimalistic-assert/-/minimalistic-assert-1.0.1.tgz#2e194de044626d4a10e7f7fbc00ce73e83e4d5c7" + integrity sha512-UtJcAD4yEaGtjPezWuO9wC4nwUnVH/8/Im3yEHQP4b67cXlD/Qr9hdITCU1xDbSEXg2XKNaP8jsReV7vQd00/A== + +minimalistic-crypto-utils@^1.0.0, minimalistic-crypto-utils@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz#f6c00c1c0b082246e5c4d99dfb8c7c083b2b582a" + integrity sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo= + minimatch@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" @@ -5175,6 +5230,11 @@ nice-try@^1.0.4: resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366" integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ== +node-addon-api@^2.0.0: + version "2.0.2" + resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-2.0.2.tgz#432cfa82962ce494b132e9d72a15b29f71ff5d32" + integrity sha512-Ntyt4AIXyaLIuMHF6IOoTakB3K+RWxwtsHNRxllEoA6vPwP9o4866g6YWDLUdnucilZhmkxiHwHr11gAENw+QA== + node-environment-flags@^1.0.5: version "1.0.6" resolved "https://registry.yarnpkg.com/node-environment-flags/-/node-environment-flags-1.0.6.tgz#a30ac13621f6f7d674260a54dede048c3982c088" @@ -5183,6 +5243,11 @@ node-environment-flags@^1.0.5: object.getownpropertydescriptors "^2.0.3" semver "^5.7.0" +node-gyp-build@^4.2.0: + version "4.2.2" + resolved "https://registry.yarnpkg.com/node-gyp-build/-/node-gyp-build-4.2.2.tgz#3f44b65adaafd42fb6c3d81afd630e45c847eb66" + integrity sha512-Lqh7mrByWCM8Cf9UPqpeoVBBo5Ugx+RKu885GAzmLBVYjeywScxHXPGLa4JfYNZmcNGwzR0Glu5/9GaQZMFqyA== + node-int64@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/node-int64/-/node-int64-0.4.0.tgz#87a9065cdb355d3182d8f94ce11188b825c68a3b" @@ -6193,6 +6258,15 @@ saxes@^5.0.0: dependencies: xmlchars "^2.2.0" +secp256k1@^4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/secp256k1/-/secp256k1-4.0.1.tgz#b9570ca26ace9e74c3171512bba253da9c0b6d60" + integrity sha512-iGRjbGAKfXMqhtdkkuNxsgJQfJO8Oo78Rm7DAvsG3XKngq+nJIOGqrCSXcQqIVsmCj0wFanE5uTKFxV3T9j2wg== + dependencies: + elliptic "^6.5.2" + node-addon-api "^2.0.0" + node-gyp-build "^4.2.0" + "semver@2 || 3 || 4 || 5", semver@^5.3.0, semver@^5.4.1, semver@^5.5.0, semver@^5.5.1, semver@^5.6.0, semver@^5.7.0: version "5.7.1" resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.1.tgz#a954f931aeba508d307bbf069eff0c01c96116f7" @@ -6842,11 +6916,6 @@ tweetnacl@^0.14.3, tweetnacl@~0.14.0: resolved "https://registry.yarnpkg.com/tweetnacl/-/tweetnacl-0.14.5.tgz#5ae68177f192d4456269d108afa93ff8743f4f64" integrity sha1-WuaBd/GS1EViadEIr6k/+HQ/T2Q= -tweetnacl@^1.0.3: - version "1.0.3" - resolved "https://registry.yarnpkg.com/tweetnacl/-/tweetnacl-1.0.3.tgz#ac0af71680458d8a6378d0d0d050ab1407d35596" - integrity sha512-6rt+RN7aOi1nGMyC4Xa5DdYiukl2UWCbcJft7YhxReBGQD7OAM8Pbxw6YMo4r2diNEA8FEmu32YOn9rhaiE5yw== - type-check@^0.4.0, type-check@~0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/type-check/-/type-check-0.4.0.tgz#07b8203bfa7056c0657050e3ccd2c37730bab8f1"