diff --git a/deps/crypto/trezor-firmware b/deps/crypto/trezor-firmware index 45ecd8d8a..faf4f3fe3 160000 --- a/deps/crypto/trezor-firmware +++ b/deps/crypto/trezor-firmware @@ -1 +1 @@ -Subproject commit 45ecd8d8ab600a7b1aaa6428031a17042b7723a4 +Subproject commit faf4f3fe348a2c690ba6cd22b561313bcc792b98 diff --git a/deps/python-keepkey b/deps/python-keepkey index d88a073a5..7423fcfc6 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit d88a073a5af2d83e0f1f19574665ca1a44789414 +Subproject commit 7423fcfc647a4622628fe376f3faf2624a310468 diff --git a/lib/firmware/coins.c b/lib/firmware/coins.c index a231bc41a..964b9cf52 100644 --- a/lib/firmware/coins.c +++ b/lib/firmware/coins.c @@ -228,6 +228,22 @@ static bool path_mismatched(const CoinType* coin, const uint32_t* address_n, return mismatch; } + // m/86' : BIP86 Taproot + // m / purpose' / bip44_account_path' / account' / change / address_index + if (address_n[0] == (0x80000000 + 86)) { + mismatch |= !coin->has_segwit || !coin->segwit; + mismatch |= !coin->has_bech32_prefix; + mismatch |= !coin->has_taproot || !coin->taproot; + mismatch |= (address_n_count != (whole_account ? 3 : 5)); + mismatch |= (address_n[1] != coin->bip44_account_path); + mismatch |= (address_n[2] & 0x80000000) == 0; + if (!whole_account) { + mismatch |= (address_n[3] & 0x80000000) == 0x80000000; + mismatch |= (address_n[4] & 0x80000000) == 0x80000000; + } + return mismatch; + } + return false; } diff --git a/lib/firmware/fsm_msg_coin.h b/lib/firmware/fsm_msg_coin.h index 978c80712..d1b002a1d 100644 --- a/lib/firmware/fsm_msg_coin.h +++ b/lib/firmware/fsm_msg_coin.h @@ -188,6 +188,21 @@ static bool path_mismatched(const CoinType* coin, const GetAddress* msg) { return mismatch; } + // m/86' : BIP86 Taproot + // m / purpose' / bip44_account_path' / account' / change / address_index + if (msg->address_n[0] == (0x80000000 + 86)) { + mismatch |= (msg->script_type != InputScriptType_SPENDTAPROOT); + mismatch |= !coin->has_segwit || !coin->segwit; + mismatch |= !coin->has_bech32_prefix; + mismatch |= !coin->has_taproot || !coin->taproot; + mismatch |= (msg->address_n_count != 5); + mismatch |= (msg->address_n[1] != coin->bip44_account_path); + mismatch |= (msg->address_n[2] & 0x80000000) == 0; + mismatch |= (msg->address_n[3] & 0x80000000) == 0x80000000; + mismatch |= (msg->address_n[4] & 0x80000000) == 0x80000000; + return mismatch; + } + return false; } diff --git a/lib/firmware/signing.c b/lib/firmware/signing.c index a7b367e0f..c91945b4b 100644 --- a/lib/firmware/signing.c +++ b/lib/firmware/signing.c @@ -822,8 +822,11 @@ static bool signing_validate_output(const TxOutputType* txoutput) { return false; } + // has_taproot is the nanopb presence flag, not the value. Every coin in + // coins.def sets it, so testing it alone let all 41 taproot=false coins + // through and built a p2tr output for them. if (txoutput->script_type == OutputScriptType_PAYTOTAPROOT && - !coin->has_taproot) { + (!coin->has_taproot || !coin->taproot)) { fsm_sendFailure(FailureType_Failure_Other, _("Taproot not enabled on this coin.")); signing_abort(); diff --git a/lib/firmware/transaction.c b/lib/firmware/transaction.c index 9d30f1acd..aca2490e0 100644 --- a/lib/firmware/transaction.c +++ b/lib/firmware/transaction.c @@ -32,6 +32,7 @@ #include "keepkey/transport/interface.h" #include "trezor/crypto/address.h" #include "trezor/crypto/base58.h" +#include "trezor/crypto/bip340.h" #include "trezor/crypto/cash_addr.h" #include "trezor/crypto/ecdsa.h" #include "trezor/crypto/memzero.h" @@ -43,6 +44,7 @@ #define _(X) (X) #define SEGWIT_VERSION_0 0 +#define SEGWIT_VERSION_1 1 #define CASHADDR_P2KH (0) #define CASHADDR_P2SH (8) @@ -125,6 +127,11 @@ bool compute_address(const CoinType* coin, InputScriptType script_type, if (has_multisig) { size_t prelen; + // No taproot multisig. Without this the request would fall through to + // the p2sh branch below and hand back a p2sh address for a taproot ask. + if (script_type == InputScriptType_SPENDTAPROOT) { + return 0; + } if (cryptoMultisigPubkeyIndex(coin, multisig, node->public_key) < 0) { return 0; } @@ -186,9 +193,28 @@ bool compute_address(const CoinType* coin, InputScriptType script_type, return 0; } } else if (script_type == InputScriptType_SPENDTAPROOT) { - // we don't handle spendtaproot input types - return 0; - + // p2tr: the witness program is the BIP-86 tweaked output key, bech32m + // encoded at witness version 1. + if ((!coin->has_segwit || !coin->segwit) || !coin->has_bech32_prefix) { + return 0; + } + if (!coin->has_taproot || !coin->taproot) { + return 0; + } + uint8_t output_key[32]; + // node->public_key is compressed; bytes 1..33 are the x-only internal key. + // BIP-341 defines the internal key as x-only, so the odd-y case resolves + // to its even-y counterpart here and in the signer alike. + if (bip340_tweak_pubkey(curve->params, node->public_key + 1, output_key) != + 0) { + return 0; + } + // Exactly 32 bytes: segwit_addr_encode only length-checks the witness + // program for version 0, so a wrong length would encode silently. + if (!segwit_addr_encode(address, coin->bech32_prefix, SEGWIT_VERSION_1, + output_key, sizeof(output_key))) { + return 0; + } } else if (script_type == InputScriptType_SPENDP2SHWITNESS) { // segwit p2wpkh embedded in p2sh if (!coin->has_segwit || !coin->segwit) { diff --git a/unittests/crypto/bip340.cpp b/unittests/crypto/bip340.cpp index e449dbfb8..dd157cbb5 100644 --- a/unittests/crypto/bip340.cpp +++ b/unittests/crypto/bip340.cpp @@ -8,13 +8,18 @@ #include extern "C" { +#include "trezor/crypto/bip32.h" #include "trezor/crypto/bip340.h" +#include "trezor/crypto/bip39.h" +#include "trezor/crypto/curves.h" #include "trezor/crypto/ecdsa.h" #include "trezor/crypto/secp256k1.h" +#include "trezor/crypto/segwit_addr.h" } #include "gtest/gtest.h" +#include #include #include #include @@ -191,6 +196,107 @@ const Vector kVectors[] = { } // namespace +// Official BIP-86 test vectors, from +// https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki +// (mnemonic "abandon abandon ... about", account m/86'/0'/0'). +// HD derivation is covered at the firmware level; what is pinned here is the +// tweak and the bech32m encoding that turn an internal key into an address. +TEST(BIP340, BIP86Vectors) { + const struct { + const char *path; + const char *internal_key; + const char *output_key; + const char *address; + } vectors[] = { + {"m/86'/0'/0'/0/0", + "cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115", + "a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c", + "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr"}, + {"m/86'/0'/0'/0/1", + "83dfe85a3151d2517290da461fe2815591ef69f2b18a2ce63f01697a8b313145", + "a82f29944d65b86ae6b5e5cc75e294ead6c59391a1edc5e016e3498c67fc7bbb", + "bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh"}, + {"m/86'/0'/0'/1/0", + "399f1b2f4393f29a18c937859c5dd8a77350103157eb880f02e8c08214277cef", + "882d74e5d0572d5a816cef0041a96b6c1de832f6f9676d9605c44d5e9a97d3dc", + "bc1p3qkhfews2uk44qtvauqyr2ttdsw7svhkl9nkm9s9c3x4ax5h60wqwruhk7"}, + }; + + for (const auto &v : vectors) { + std::vector internal = unhex(v.internal_key); + uint8_t out[BIP340_XONLY_LENGTH] = {0}; + + ASSERT_EQ(0, bip340_tweak_pubkey(&secp256k1, internal.data(), out)) + << v.path; + + std::string got = hex(out, sizeof(out)); + std::transform(got.begin(), got.end(), got.begin(), ::tolower); + ASSERT_EQ(std::string(v.output_key), got) << v.path; + + // Witness version 1 + 32 bytes must come out bech32m, i.e. a bc1p address. + char address[MAX_ADDR_SIZE] = {0}; + ASSERT_EQ(1, segwit_addr_encode(address, "bc", 1, out, sizeof(out))) + << v.path; + ASSERT_EQ(std::string(v.address), std::string(address)) << v.path; + } +} + +// The same BIP-86 vectors driven from the mnemonic, so HD derivation and the +// x-only convention are covered too. compute_address() feeds +// node->public_key + 1 to bip340_tweak_pubkey(); this pins that the byte after +// the compressed prefix really is the internal key BIP-86 expects, which an +// off-by-one would otherwise turn into a valid-looking wrong address. +TEST(BIP340, BIP86FromMnemonic) { + const char *mnemonic = + "abandon abandon abandon abandon abandon abandon abandon abandon abandon " + "abandon abandon about"; + const struct { + uint32_t change; + uint32_t index; + const char *address; + } vectors[] = { + {0, 0, "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr"}, + {0, 1, "bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh"}, + {1, 0, "bc1p3qkhfews2uk44qtvauqyr2ttdsw7svhkl9nkm9s9c3x4ax5h60wqwruhk7"}, + }; + + uint8_t seed[64] = {0}; + mnemonic_to_seed(mnemonic, "", seed, nullptr); + + for (const auto &v : vectors) { + HDNode node = {0}; + ASSERT_EQ(1, hdnode_from_seed(seed, sizeof(seed), SECP256K1_NAME, &node)); + // m/86'/0'/0'/change/index + ASSERT_EQ(1, hdnode_private_ckd(&node, 0x80000000 + 86)); + ASSERT_EQ(1, hdnode_private_ckd(&node, 0x80000000 + 0)); + ASSERT_EQ(1, hdnode_private_ckd(&node, 0x80000000 + 0)); + ASSERT_EQ(1, hdnode_private_ckd(&node, v.change)); + ASSERT_EQ(1, hdnode_private_ckd(&node, v.index)); + hdnode_fill_public_key(&node); + + uint8_t out[BIP340_XONLY_LENGTH] = {0}; + ASSERT_EQ(0, bip340_tweak_pubkey(&secp256k1, node.public_key + 1, out)); + + char address[MAX_ADDR_SIZE] = {0}; + ASSERT_EQ(1, segwit_addr_encode(address, "bc", 1, out, sizeof(out))); + ASSERT_EQ(std::string(v.address), std::string(address)) + << "change=" << v.change << " index=" << v.index; + } +} + +TEST(BIP340, TweakRejectsInvalidInternalKey) { + // Vector 5's x coordinate, which is not on the curve. + std::vector bad = + unhex("EEFDEA4CDB677750A420FEE807EACF21EB9898AE79B9768766E4FAA04A2D4A34"); + // And an x coordinate past the field size (vector 14). + std::vector too_big = + unhex("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC30"); + uint8_t out[BIP340_XONLY_LENGTH] = {0}; + + ASSERT_NE(0, bip340_tweak_pubkey(&secp256k1, bad.data(), out)); + ASSERT_NE(0, bip340_tweak_pubkey(&secp256k1, too_big.data(), out)); +} + TEST(BIP340, TaggedHash) { // tagged_hash("BIP0340/challenge", "") == SHA256(h || h) where // h = SHA256("BIP0340/challenge"). Pins the double-tag construction.