diff --git a/CHANGELOG.md b/CHANGELOG.md index ca1b44ffc..e6ace8c3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately. ### [Unreleased] - Bitcoin: add support for sending to silent payment (BIP-352) addresses +- Bitcoin: add support for Taproot wallet policies and Miniscript on Taproot (MiniTapscript) - Bitcoin: add support for regtest - Cardano: add support for vote delegation diff --git a/src/keystore.c b/src/keystore.c index c4fad8f1e..f7ddf064c 100644 --- a/src/keystore.c +++ b/src/keystore.c @@ -941,9 +941,10 @@ bool keystore_secp256k1_schnorr_bip86_pubkey(const uint8_t* pubkey33, uint8_t* p return secp256k1_xonly_pubkey_serialize(ctx, pubkey_out, &tweaked_xonly_pubkey) == 1; } -static bool _schnorr_bip86_keypair( +static bool _schnorr_keypair( const uint32_t* keypath, size_t keypath_len, + const uint8_t* tweak, secp256k1_keypair* keypair_out, secp256k1_xonly_pubkey* pubkey_out) { @@ -959,20 +960,15 @@ static bool _schnorr_bip86_keypair( if (!secp256k1_keypair_create(ctx, keypair_out, secret_key)) { return false; } - if (!secp256k1_keypair_xonly_pub(ctx, pubkey_out, NULL, keypair_out)) { - return false; - } - uint8_t pubkey_serialized[32] = {0}; - if (!secp256k1_xonly_pubkey_serialize(ctx, pubkey_serialized, pubkey_out)) { - return false; + if (tweak != NULL) { + if (secp256k1_keypair_xonly_tweak_add(ctx, keypair_out, tweak) != 1) { + return false; + } } - uint8_t hash[32] = {0}; - _tagged_hash("TapTweak", pubkey_serialized, sizeof(pubkey_serialized), hash); - - if (secp256k1_keypair_xonly_tweak_add(ctx, keypair_out, hash) != 1) { + if (!secp256k1_keypair_xonly_pub(ctx, pubkey_out, NULL, keypair_out)) { return false; } - return secp256k1_keypair_xonly_pub(ctx, pubkey_out, NULL, keypair_out) == 1; + return true; } static void _cleanup_keypair(secp256k1_keypair* keypair) @@ -980,15 +976,16 @@ static void _cleanup_keypair(secp256k1_keypair* keypair) util_zero(keypair, sizeof(secp256k1_keypair)); } -bool keystore_secp256k1_schnorr_bip86_sign( +bool keystore_secp256k1_schnorr_sign( const uint32_t* keypath, size_t keypath_len, const uint8_t* msg32, + const uint8_t* tweak, uint8_t* sig64_out) { secp256k1_keypair __attribute__((__cleanup__(_cleanup_keypair))) keypair = {0}; secp256k1_xonly_pubkey pubkey = {0}; - if (!_schnorr_bip86_keypair(keypath, keypath_len, &keypair, &pubkey)) { + if (!_schnorr_keypair(keypath, keypath_len, tweak, &keypair, &pubkey)) { return false; } const secp256k1_context* ctx = wally_get_secp_context(); diff --git a/src/keystore.h b/src/keystore.h index 5e91c1ac2..ba2f37f9b 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -295,12 +295,15 @@ USE_RESULT bool keystore_secp256k1_schnorr_bip86_pubkey( * @param[in] keypath derivation keypath * @param[in] keypath_len number of elements in keypath * @param[in] msg32 32 byte message to sign + * @param[in] tweak 32 bytes, tweak private key before signing with this tweak. Use NULL to not + * tweak. * @param[out] sig64_out resulting 64 byte signature */ -USE_RESULT bool keystore_secp256k1_schnorr_bip86_sign( +USE_RESULT bool keystore_secp256k1_schnorr_sign( const uint32_t* keypath, size_t keypath_len, const uint8_t* msg32, + const uint8_t* tweak, uint8_t* sig64_out); /** diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin.rs index b03268a7c..4cafe7b96 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin.rs @@ -254,7 +254,8 @@ async fn address_policy( .await?; } - let address = common::Payload::from_policy(&parsed, keypath)?.address(coin_params)?; + let address = + common::Payload::from_policy(coin_params, &parsed, keypath)?.address(coin_params)?; if display { confirm::confirm(&confirm::Params { title, diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/bip341.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/bip341.rs index d0585558a..8341b5125 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/bip341.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/bip341.rs @@ -27,14 +27,16 @@ pub struct Args { pub hash_outputs: [u8; 32], // Data about this input: pub input_index: u32, + // tapleaf_hash as described in https://github.com/bitcoin/bips/blob/85cda4e225b4d5fd7aff403f69d827f23f6afbbc/bip-0342.mediawiki#common-signature-message-extension + // Providing this means we use the above tapscript message extension. + pub tapleaf_hash: Option<[u8; 32]>, } /// Compute the BIP341 signature hash. /// /// https://github.com/bitcoin/bips/blob/bb8dc57da9b3c6539b88378348728a2ff43f7e9c/bip-0341.mediawiki#common-signature-message /// -/// The hash_type is assumed 0 (`SIGHASH_DEFAULT`). The `ext_flag` is -/// assumed 0 and `annex` is assumed to be not present. +/// The hash_type is assumed 0 (`SIGHASH_DEFAULT`). `annex` is assumed to be not present. pub fn sighash(args: &Args) -> [u8; 32] { let tag = Sha256::digest(b"TapSighash"); let mut ctx = Sha256::new(); @@ -53,10 +55,28 @@ pub fn sighash(args: &Args) -> [u8; 32] { ctx.update(args.hash_sequences); ctx.update(args.hash_outputs); // spend_type is 0 because ext_flag is 0 and annex is absent. - ctx.update(0u8.to_le_bytes()); + let ext_flag = if args.tapleaf_hash.is_some() { + // ext_flag = 1 for Taproot leaf scripts + // See https://github.com/bitcoin/bips/blob/85cda4e225b4d5fd7aff403f69d827f23f6afbbc/bip-0342.mediawiki#common-signature-message-extension + 1 + } else { + 0 + }; + let spend_type: u8 = 2 * ext_flag; + ctx.update(spend_type.to_le_bytes()); // Data about this input: ctx.update(args.input_index.to_le_bytes()); + if let Some(hash) = args.tapleaf_hash.as_ref() { + // See https://github.com/bitcoin/bips/blob/85cda4e225b4d5fd7aff403f69d827f23f6afbbc/bip-0342.mediawiki#common-signature-message-extension + // tapleaf_hash + ctx.update(hash); + // keyversion + ctx.update(0u8.to_le_bytes()); + // codesep_pos - we do not use any OP_CODESEPARATORs. + let codesep_pos: u32 = 0xFFFFFFFF; + ctx.update(codesep_pos.to_le_bytes()); + } ctx.finalize().into() } @@ -79,7 +99,25 @@ mod tests { hash_sequences: *b"\x18\x95\x9c\x72\x21\xab\x5c\xe9\xe2\x6c\x3c\xd6\x7b\x22\xc2\x4f\x8b\xaa\x54\xba\xc2\x81\xd8\xe6\xb0\x5e\x40\x0e\x6c\x3a\x95\x7e", hash_outputs: *b"\xa2\xe6\xda\xb7\xc1\xf0\xdc\xd2\x97\xc8\xd6\x16\x47\xfd\x17\xd8\x21\x54\x1e\xa6\x9c\x3c\xc3\x7d\xcb\xad\x7f\x90\xd4\xeb\x4b\xc5", input_index: 4, + tapleaf_hash: None, }), *b"\x4f\x90\x0a\x0b\xae\x3f\x14\x46\xfd\x48\x49\x0c\x29\x58\xb5\xa0\x23\x22\x8f\x01\x66\x1c\xda\x34\x96\xa1\x1d\xa5\x02\xa7\xf7\xef"); } + + #[test] + fn test_sighash_tapleaf() { + assert_eq!( + sighash(&Args { + version: 2, + locktime: 500000000, + hash_prevouts: *b"\xe3\xb3\x3b\xb4\xef\x3a\x52\xad\x1f\xff\xb5\x55\xc0\xd8\x28\x28\xeb\x22\x73\x70\x36\xea\xeb\x02\xa2\x35\xd8\x2b\x90\x9c\x4c\x3f", + hash_amounts: *b"\x58\xa6\x96\x4a\x4f\x5f\x8f\x0b\x64\x2d\xed\x0a\x8a\x55\x3b\xe7\x62\x2a\x71\x9d\xa7\x1d\x1f\x5b\xef\xce\xfc\xde\xe8\xe0\xfd\xe6", + hash_scriptpubkeys: *b"\x23\xad\x0f\x61\xad\x2b\xca\x5b\xa6\xa7\x69\x3f\x50\xfc\xe9\x88\xe1\x7c\x37\x80\xbf\x2b\x1e\x72\x0c\xfb\xb3\x8f\xbd\xd5\x2e\x21", + hash_sequences: *b"\x18\x95\x9c\x72\x21\xab\x5c\xe9\xe2\x6c\x3c\xd6\x7b\x22\xc2\x4f\x8b\xaa\x54\xba\xc2\x81\xd8\xe6\xb0\x5e\x40\x0e\x6c\x3a\x95\x7e", + hash_outputs: *b"\xa2\xe6\xda\xb7\xc1\xf0\xdc\xd2\x97\xc8\xd6\x16\x47\xfd\x17\xd8\x21\x54\x1e\xa6\x9c\x3c\xc3\x7d\xcb\xad\x7f\x90\xd4\xeb\x4b\xc5", + input_index: 4, + tapleaf_hash: Some(*b"\x34\xe7\x21\x15\xc0\x9c\x91\x3c\x8b\xe1\x2e\x46\xfc\x14\x5f\xcf\x7c\x53\xca\xd9\xca\x2a\x05\xf9\x3a\x7c\xa2\xe0\xca\x88\xd0\x07"), + }), + *b"\xba\xe0\xaa\xcb\xa5\xae\xa9\xee\xbe\x19\xe1\x57\xa9\x8f\x1e\xe7\x0d\x7d\x28\x8c\x28\x0f\x27\x3e\x63\xbb\x8a\x85\xd1\xee\xf3\xc2"); + } } diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/common.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/common.rs index fe09a3027..83bc074e2 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/common.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/common.rs @@ -1,4 +1,4 @@ -// Copyright 2022 Shift Crypto AG +// Copyright 2022-2024 Shift Crypto AG // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -156,6 +156,7 @@ impl Payload { /// derived using keypath m/48'/1'/0'/3'/11/5 derives the payload for /// wsh(and_v(v:pk(@0/11/5),pk(@1/21/5))). pub fn from_policy( + params: &Params, policy: &super::policies::ParsedPolicy, keypath: &[u32], ) -> Result { @@ -165,6 +166,16 @@ impl Payload { data: Sha256::digest(wsh.witness_script()).to_vec(), output_type: BtcOutputType::P2wsh, }), + super::policies::Descriptor::Tr(tr) => { + if params.taproot_support { + Ok(Payload { + data: tr.output_key().to_vec(), + output_type: BtcOutputType::P2tr, + }) + } else { + Err(Error::InvalidInput) + } + } } } @@ -186,7 +197,7 @@ impl Payload { keypath[keypath.len() - 2], keypath[keypath.len() - 1], ), - ValidatedScriptConfig::Policy(policy) => Self::from_policy(policy, keypath), + ValidatedScriptConfig::Policy(policy) => Self::from_policy(params, policy, keypath), } } diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs index b8da788d3..d14acf93d 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs @@ -1,4 +1,4 @@ -// Copyright 2023 Shift Crypto AG +// Copyright 2023-2024 Shift Crypto AG // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -30,6 +30,9 @@ use miniscript::TranslatePk; use crate::bip32; use crate::workflow::confirm; +use crate::xpubcache::Bip32XpubCache; + +use bitcoin::taproot::{LeafVersion, TapLeafHash, TapTweakHash}; use sha2::{Digest, Sha256}; @@ -100,7 +103,8 @@ fn parse_wallet_policy_pk(pk: &str) -> Result<(usize, u32, u32), ()> { } /// Given policy pubkeys like `@0//*` and the keys list, determine if the given keypath -/// is valid and whether it points to a receive or change address. +/// is valid and whether it points to a receive or change address. We also return the matched +/// pubkey. /// /// Example: pubkeys "@0/<10;11>/*" and "@1/<20;21>/*", with our key [fp/48'/1'/0'/3']xpub...], /// derived using keypath m/48'/1'/0'/3'/11/5 means that this is the address index 5 at the change @@ -187,12 +191,74 @@ impl Wsh { } } +impl Wsh { + /// Iterates over all pubkey placeholders in this wsh descriptor. + /// Example: wsh(and_v(v:pk(A),pk(B))) iterates over A, B. + /// This iterates the keys "left-to-right" in the descriptor. + fn iter_pk(&self) -> impl Iterator + '_ { + self.miniscript_expr.iter_pk() + } +} + /// See `ParsedPolicy`. #[derive(Debug)] +pub struct Tr { + inner: miniscript::descriptor::Tr, +} + +impl Tr { + /// Returns the serialized Taproot output key. + pub fn output_key(&self) -> [u8; 32] { + self.inner.spend_info().output_key().serialize() + } + + /// Returns the tap leaf hash (as defined in BIP341) of the leaf whose script contains the given + /// pubkey (serialized as a compressed pubkey). If the pubkey is not present in any leaf script, + /// None is returned. + /// + /// Note that we assume that each pubkey is unique according to BIP-388 and validated by + /// `validate_keys()`, so the leaf is unique. + fn get_leaf_hash_by_pubkey(&self, pk: &[u8; 33]) -> Option { + for (_, ms) in self.inner.iter_scripts() { + if ms.iter_pk().any(|pk2| *pk == pk2.inner.serialize()) { + return Some(TapLeafHash::from_script( + &ms.encode(), + LeafVersion::TapScript, + )); + } + } + None + } +} + +impl Tr { + /// Iterates over the placeholder keys in the internal key and in each tapscript leaf, + /// i.e. "left-to-right" in the descriptor. + /// + /// Example: `tr(A,{pk(B),pk(C)}` iterates over A,B,C. + fn iter_pk(&self) -> impl Iterator + '_ { + core::iter::once(self.inner.internal_key().clone()) + .chain(self.inner.iter_scripts().flat_map(|(_, ms)| ms.iter_pk())) + } +} + +pub enum TaprootSpendInfo { + KeySpend(TapTweakHash), + ScriptSpend(TapLeafHash), +} + +/// See `ParsedPolicy`. +/// +/// We don't use `miniscript::descriptor::Descriptor` as it supports much more than what we want +/// (Bare, Sh, ...) and pulls in a lot of additional code to support them, their script generation, +/// etc., bloating the firmware binary size significantly (at least +50kB). By wrapping/parsing only +/// the descriptors we want to support, we avoid this binary bloat. +#[derive(Debug)] pub enum Descriptor { // `wsh(...)` policies Wsh(Wsh), - // `tr(...)` Taproot etc. in the future. + // `tr(...)` Taproot policies + Tr(Tr), } /// Result of `parse()`. @@ -208,50 +274,60 @@ pub struct ParsedPolicy<'a> { } impl<'a> ParsedPolicy<'a> { + /// Iterates over the placeholder keys in this descriptor. For tr() descriptors, this covers the + /// internal key and every key in every leaf script. + /// This iterates the keys "left-to-right" in the descriptor. + fn iter_pk(&self) -> alloc::boxed::Box + '_> { + match &self.descriptor { + Descriptor::Wsh(wsh) => alloc::boxed::Box::new(wsh.iter_pk()), + Descriptor::Tr(tr) => alloc::boxed::Box::new(tr.iter_pk()), + } + } + /// Check that it is impossible to create a derivation with duplicate pubkeys, assuming all the /// keys in the key vector are distinct. /// - /// Even though the rust-miniscript library checks for duplicate keys, it does so on the raw - /// miniscript, which would not catch e.g. that `wsh(or_b(pk(@0/<0;1>/*),s:pk(@0/<2;1>/*)))` has - /// a duplicate change derivation if we derive at the receive path. + /// Even though the rust-miniscript library checks for duplicate keys (per miniscript expr), it + /// does so on the raw miniscript, which would not catch e.g. that + /// `wsh(or_b(pk(@0/<0;1>/*),s:pk(@0/<2;1>/*)))` has a duplicate change derivation if we derive + /// at the receive path. + /// + /// For tr() descriptors, technically one can have duplicate keys as long as they are not in the + /// same leaf script, but BIP-388 prohibits duplicate keys across all parts for simplicity. /// /// Also checks that each key is used, e.g. if there are 3 keys in the key vector, @0, @1 and @2 /// must be present. fn validate_keys(&self) -> Result<(), Error> { - match &self.descriptor { - Descriptor::Wsh(Wsh { miniscript_expr }) => { - // in "@key_index/", keeps track of (key_index,left) and - // (key_index,right) to check for duplicates. - let mut derivations_seen: Vec<(usize, u32)> = Vec::new(); + // in "@key_index/", keeps track of (key_index,left) and + // (key_index,right) to check for duplicates. + let mut derivations_seen: Vec<(usize, u32)> = Vec::new(); - let mut keys_seen: Vec = vec![false; self.policy.keys.len()]; + let mut keys_seen: Vec = vec![false; self.policy.keys.len()]; - for pk in miniscript_expr.iter_pk() { - let (key_index, multipath_index_left, multipath_index_right) = - parse_wallet_policy_pk(&pk).or(Err(Error::InvalidInput))?; + for pk in self.iter_pk() { + let (key_index, multipath_index_left, multipath_index_right) = + parse_wallet_policy_pk(pk.as_ref()).or(Err(Error::InvalidInput))?; - if derivations_seen.contains(&(key_index, multipath_index_left)) { - return Err(Error::InvalidInput); - } - derivations_seen.push((key_index, multipath_index_left)); - if derivations_seen.contains(&(key_index, multipath_index_right)) { - return Err(Error::InvalidInput); - } - derivations_seen.push((key_index, multipath_index_right)); + if derivations_seen.contains(&(key_index, multipath_index_left)) { + return Err(Error::InvalidInput); + } + derivations_seen.push((key_index, multipath_index_left)); + if derivations_seen.contains(&(key_index, multipath_index_right)) { + return Err(Error::InvalidInput); + } + derivations_seen.push((key_index, multipath_index_right)); - *keys_seen.get_mut(key_index).ok_or(Error::InvalidInput)? = true; - } + *keys_seen.get_mut(key_index).ok_or(Error::InvalidInput)? = true; + } - if !keys_seen.into_iter().all(|b| b) { - return Err(Error::InvalidInput); - } - Ok(()) - } + if !keys_seen.into_iter().all(|b| b) { + return Err(Error::InvalidInput); } + Ok(()) } /// Validate a policy. - /// - Coin is supported (only Bitcoin testnet for now) + /// - Coin is supported /// - Number of keys /// - At least one of the keys is ours /// - There are no duplicate or missing xpubs @@ -340,8 +416,11 @@ impl<'a> ParsedPolicy<'a> { BtcCoin::Tbtc | BtcCoin::Rbtc | BtcCoin::Tltc => bip32::XPubType::Tpub, }; let num_keys = policy.keys.len(); + + let taproot_unspendable_internal_key_index = self.taproot_is_unspendable_internal_key()?; + for (i, key) in policy.keys.iter().enumerate() { - let key_str = match key { + let mut key_str = match key { pb::KeyOriginInfo { root_fingerprint, keypath, @@ -365,14 +444,14 @@ impl<'a> ParsedPolicy<'a> { } _ => return Err(Error::InvalidInput), }; + if self.is_our_key[i] { + key_str = format!("This device: {}", key_str) + } else if Some(i) == taproot_unspendable_internal_key_index { + key_str = format!("Provably unspendable: {}", key_str) + } confirm::confirm(&confirm::Params { title: &format!("Key {}/{}", i + 1, num_keys), - body: (if self.is_our_key[i] { - format!("This device: {}", key_str) - } else { - key_str - }) - .as_str(), + body: key_str.as_str(), scrollable: true, longtouch: i == num_keys - 1 && matches!(mode, Mode::Advanced), accept_is_nextarrow: true, @@ -409,6 +488,14 @@ impl<'a> ParsedPolicy<'a> { }; Ok(Descriptor::Wsh(Wsh { miniscript_expr })) } + Descriptor::Tr(Tr { inner }) => { + let derived = match inner.translate_pk(&mut translator) { + Ok(m) => m, + Err(miniscript::TranslateErr::TranslatorErr(e)) => return Err(e), + Err(miniscript::TranslateErr::OuterError(_)) => return Err(Error::Generic), + }; + Ok(Descriptor::Tr(Tr { inner: derived })) + } } } @@ -421,37 +508,138 @@ impl<'a> ParsedPolicy<'a> { &self, keypath: &[u32], ) -> Result, Error> { - match &self.descriptor { - Descriptor::Wsh(Wsh { miniscript_expr }) => { - let (is_change, address_index) = get_change_and_address_index( - miniscript_expr.iter_pk(), - &self.policy.keys, - &self.is_our_key, - keypath, - )?; - self.derive(is_change, address_index) - } - } + let (is_change, address_index) = get_change_and_address_index( + self.iter_pk(), + &self.policy.keys, + &self.is_our_key, + keypath, + )?; + self.derive(is_change, address_index) } /// Returns true if the address-level keypath points to a change address. pub fn is_change_keypath(&self, keypath: &[u32]) -> Result { + let (is_change, _) = get_change_and_address_index( + self.iter_pk(), + &self.policy.keys, + &self.is_our_key, + keypath, + )?; + Ok(is_change) + } + + /// Returns info needed to spend a Taproot UTXO at the given keypath. + /// + /// If the keypath points to the Taproot internal key, we return the necessary Taproot tweak to + /// spend using the Taproot key path. + /// + /// If th keypath points to a key used in a tap leaf script, we return the tap leaf hash (as + /// defined in BIP341), which is needed to in the sighash computation in the context of a + /// Taproot leaf script. + /// + /// This works because all keypaths are distinct per BIP-388, and checked by `validate_keys()`, + /// so they keypath alone is sufficient to figure out if we are using key path or script + /// path, and if the latter, which leaf exactly. + pub fn taproot_spend_info( + &self, + xpub_cache: &mut Bip32XpubCache, + keypath: &[u32], + ) -> Result { + match self.derive_at_keypath(keypath)? { + Descriptor::Tr(tr) => { + let xpub = xpub_cache.get_xpub(keypath)?; + let is_keypath_spend = + xpub.public_key() == tr.inner.internal_key().inner.serialize(); + + if is_keypath_spend { + Ok(TaprootSpendInfo::KeySpend( + tr.inner.spend_info().tap_tweak(), + )) + } else { + let leaf_hash = tr + .get_leaf_hash_by_pubkey(xpub.public_key().try_into().unwrap()) + .ok_or(Error::InvalidInput)?; + Ok(TaprootSpendInfo::ScriptSpend(leaf_hash)) + } + } + _ => Err(Error::Generic), + } + } + + /// Returns `Some(index of internal key)` if this is a Taproot policy and the Taproot internal + /// key is provably unspendable, and `None` otherwise. + /// + /// We consider it provably unspendable if the internal xpub's public key is the NUMS point and + /// the xpub's chain code is the sha256() of the concatenation of all the public keys (33 byte + /// compressed) in the taptree left-to-right. + /// + /// See https://delvingbitcoin.org/t/unspendable-keys-in-descriptors/304/21 + /// + /// This is not a standard yet, but it is provably unspendable in any case, so showing this info + /// to the user can't hurt. + fn taproot_is_unspendable_internal_key(&self) -> Result, Error> { match &self.descriptor { - Descriptor::Wsh(Wsh { miniscript_expr }) => { - let (is_change, _) = get_change_and_address_index( - miniscript_expr.iter_pk(), - &self.policy.keys, - &self.is_our_key, - keypath, - )?; - Ok(is_change) + Descriptor::Tr(tr) => { + let (internal_key_index, _, _) = parse_wallet_policy_pk(tr.inner.internal_key()) + .map_err(|_| Error::InvalidInput)?; + let internal_xpub = self + .policy + .keys + .get(internal_key_index) + .ok_or(Error::InvalidInput)? + .xpub + .as_ref() + .ok_or(Error::InvalidInput)?; + + // See + // https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#constructing-and-spending-taproot-outputs: + // > One example of such a point is H = + // > lift_x(0x50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0) which is constructed + // > by taking the hash of the standard uncompressed encoding of the secp256k1 base point G as X + // > coordinate. + const NUMS: [u8; 33] = [ + 0x02, 0x50, 0x92, 0x9b, 0x74, 0xc1, 0xa0, 0x49, 0x54, 0xb7, 0x8b, 0x4b, 0x60, + 0x35, 0xe9, 0x7a, 0x5e, 0x07, 0x8a, 0x5a, 0x0f, 0x28, 0xec, 0x96, 0xd5, 0x47, + 0xbf, 0xee, 0x9a, 0xce, 0x80, 0x3a, 0xc0, + ]; + + if internal_xpub.depth != [0u8; 1] + || internal_xpub.parent_fingerprint.as_slice() != [0u8; 4] + || internal_xpub.child_num != 0 + || internal_xpub.public_key.as_slice() != NUMS + { + return Ok(None); + } + + let chain_code: [u8; 32] = { + let mut hasher = Sha256::new(); + for pk in tr.inner.iter_scripts().flat_map(|(_, ms)| ms.iter_pk()) { + let (key_index, _, _) = + parse_wallet_policy_pk(&pk).map_err(|_| Error::InvalidInput)?; + let key_info = + self.policy.keys.get(key_index).ok_or(Error::InvalidInput)?; + hasher.update( + &key_info + .xpub + .as_ref() + .ok_or(Error::InvalidInput)? + .public_key, + ); + } + hasher.finalize().into() + }; + if chain_code != internal_xpub.chain_code.as_slice() { + return Ok(None); + } + Ok(Some(internal_key_index)) } + _ => Ok(None), } } } /// Parses a policy as specified by 'Wallet policies': https://github.com/bitcoin/bips/pull/1389. -/// Only `wsh()` is supported for now. +/// `wsh()` and `tr(KEY)` and `tr(KEY,TREE)` descriptors are supported. /// Example: `wsh(pk(@0/**))`. /// /// The parsed output keeps the key strings as is (e.g. "@0/**"). They will be processed and @@ -474,16 +662,35 @@ pub fn parse(policy: &Policy, coin: BtcCoin) -> Result { let parsed = match desc.as_bytes() { // Match wsh(...). [b'w', b's', b'h', b'(', .., b')'] => { + // `Miniscript::from_str` includes the equivalent of `miniscript_expr.sanity_check()`. + // We call it anyway below in case the miniscript library extends/changes the main + // sanity_check function. let miniscript_expr: miniscript::Miniscript = miniscript::Miniscript::from_str(&desc[4..desc.len() - 1]) .or(Err(Error::InvalidInput))?; - + miniscript_expr + .sanity_check() + .map_err(|_| Error::InvalidInput)?; ParsedPolicy { policy, is_our_key, descriptor: Descriptor::Wsh(Wsh { miniscript_expr }), } } + // Match tr(...). + [b't', b'r', b'(', .., b')'] => { + // During parsing, the leaf scripts are created using `Miniscript::from_str()`, which + // calls the equivalent of the sanity check. We call it anyway below in case the + // miniscript library extends/changes the main sanity_check function. + let tr = miniscript::descriptor::Tr::from_str(desc).map_err(|_| Error::InvalidInput)?; + tr.sanity_check().map_err(|_| Error::InvalidInput)?; + + ParsedPolicy { + policy, + is_our_key, + descriptor: Descriptor::Tr(Tr { inner: tr }), + } + } _ => return Err(Error::InvalidInput), }; parsed.validate()?; @@ -552,6 +759,7 @@ mod tests { use bitbox02::testing::{mock_unlocked, mock_unlocked_using_mnemonic}; const SOME_XPUB_1: &str = "tpubDFj9SBQssRHA5EB1ox58mcgF9sB61br9RGz6UrBukcNKmFe4fPgskZ4wigxQ1jSUzLdjnvvDHL8Z6L3ey5Ev5FNNqrDrePxwXsNHiLZhBTc"; + const SOME_XPUB_2: &str = "tpubDCmDXtvJLH9yHLNLnGVRoXBvvacvWskjV4hq4WAmGXcRbfa5uaiybZ7kjGRAFbLaoiw1LcwV56H88avibGh7GC7nqqz2Jcs1dWu33cRKYm4"; const KEYPATH_ACCOUNT: &[u32] = &[48 + HARDENED, 1 + HARDENED, 0 + HARDENED, 3 + HARDENED]; @@ -611,6 +819,45 @@ mod tests { assert!(parse_wallet_policy_pk("@0/<2147483648;100>/*").is_err()); } + // Tests that iter_pk() iterates the pubkeys from left to right as they appear in the + // descriptor. + #[test] + fn test_iter_pk_left_to_right() { + mock_unlocked(); + struct Test { + policy: &'static str, + expected_pks: &'static [&'static str], + } + let tests = &[ + Test { + policy: "wsh(andor(pk(@0/**),or_d(pk(@1/**),older(12960)),pk(@2/**)))", + expected_pks: &["@0/**", "@1/**", "@2/**"], + }, + Test { + policy: "tr(@0/<0;1>/*,{and_v(v:multi_a(1,@1/<2;3>/*,@2/<2;3>/*),older(2)),multi_a(2,@1/<0;1>/*,@2/<0;1>/*)})", + expected_pks: &[ + "@0/<0;1>/*", + "@1/<2;3>/*", + "@2/<2;3>/*", + "@1/<0;1>/*", + "@2/<0;1>/*", + ], + }, + ]; + for test in tests { + let policy = make_policy( + test.policy, + &[ + make_key(SOME_XPUB_1), + make_key(SOME_XPUB_2), + make_our_key(KEYPATH_ACCOUNT), + ], + ); + let pks: Vec = parse(&policy, BtcCoin::Tbtc).unwrap().iter_pk().collect(); + assert_eq!(pks.as_slice(), test.expected_pks); + } + } + #[test] fn test_parse_wsh_miniscript() { let coin = BtcCoin::Tbtc; @@ -626,6 +873,7 @@ mod tests { vec!["@0/**"] ); } + _ => panic!("expected wsh"), } // Parse another valid example and check that the keys are collected as is as strings. @@ -642,6 +890,7 @@ mod tests { vec!["@0/**", "@1/**"] ); } + _ => panic!("expected wsh"), } // Unknown top-level fragment. @@ -684,6 +933,20 @@ mod tests { // All good. assert!(parse(&make_policy("wsh(pk(@0/**))", &[our_key.clone()]), coin).is_ok()); + // All good, all keys are used across internal key & leaf scripts. + assert!(parse( + &make_policy( + "tr(@0/**,{pk(@1/**),pk(@2/**)})", + &[ + our_key.clone(), + make_key(SOME_XPUB_1), + make_key(SOME_XPUB_2) + ], + ), + coin + ) + .is_ok()); + // Unsupported coins for coin in [BtcCoin::Ltc, BtcCoin::Tltc] { assert!(matches!( @@ -710,7 +973,7 @@ mod tests { Err(Error::InvalidInput) )); - // Our key is not present - fingerprint and keypath exit but xpub does not match. + // Our key is not present - fingerprint and keypath exist but xpub does not match. let mut wrong_key = our_key.clone(); wrong_key.xpub = Some(parse_xpub(SOME_XPUB_1).unwrap()); assert!(matches!( @@ -770,7 +1033,7 @@ mod tests { } #[test] - fn test_parse_check_dups_in_policy() { + fn test_parse_check_dups_in_policy_wsh() { mock_unlocked(); let coin = BtcCoin::Tbtc; @@ -831,6 +1094,55 @@ mod tests { assert!(parse(&pol, coin).is_err()); } + #[test] + fn test_parse_check_dups_in_policy_tr() { + mock_unlocked(); + + let coin = BtcCoin::Tbtc; + let our_key = make_our_key(KEYPATH_ACCOUNT); + + // Ok, only internal key. + let pol = make_policy("tr(@0/**)", &[our_key.clone()]); + assert!(parse(&pol, coin).is_ok()); + + // Ok, one leaf with one key. + let pol = make_policy( + "tr(@0/**,pk(@1/**))", + &[our_key.clone(), make_key(SOME_XPUB_1)], + ); + assert!(parse(&pol, coin).is_ok()); + + // Ok, one leaf with two keys. + let pol = make_policy( + "tr(@0/**,or_b(pk(@1/**),s:pk(@2/**)))", + &[ + our_key.clone(), + make_key(SOME_XPUB_1), + make_key(SOME_XPUB_2), + ], + ); + assert!(parse(&pol, coin).is_ok()); + + // Duplicate keys across internal key and multiple leafs. Technically okay, but prohibited + // by BIP-388. + let pol = make_policy("tr(@0/**,pk(@0/**))", &[our_key.clone()]); + assert!(parse(&pol, coin).is_err()); + + // Duplicate key in one leaf script. + let pol = make_policy( + "tr(@0/**,or_b(pk(@1/**),s:pk(@1/**)))", + &[our_key.clone(), make_key(SOME_XPUB_1)], + ); + assert!(parse(&pol, coin).is_err()); + + // Duplicate key inside one leaf script, using same receive but different change. + let pol = make_policy( + "tr(@0/**,or_b(pk(@1/<0;1>/*),s:pk(@1/<0;2>/*)))", + &[our_key.clone(), make_key(SOME_XPUB_1)], + ); + assert!(parse(&pol, coin).is_err()); + } + #[test] fn test_get_change_and_address_index() { mock_unlocked(); @@ -854,6 +1166,23 @@ mod tests { Ok((false, 0)) ); + assert_eq!( + get_change_and_address_index( + ["@0/<10;11>/*", "@0/<20;21>/*"].iter(), + &[our_key.clone()], + &[true], + &[ + 48 + HARDENED, + 1 + HARDENED, + 0 + HARDENED, + 3 + HARDENED, + 20, + 0, + ], + ), + Ok((false, 0)) + ); + assert_eq!( get_change_and_address_index( ["@0/<10;11>/*", "@1/<20;21>/*"].iter(), @@ -968,6 +1297,7 @@ mod tests { .unwrap(); match derived { Descriptor::Wsh(wsh) => hex::encode(wsh.witness_script()), + _ => panic!("expected wsh"), } }; let witness_script_at_keypath = |pol: &str, keys: &[pb::KeyOriginInfo], keypath: &[u32]| { @@ -977,6 +1307,7 @@ mod tests { .unwrap(); match derived { Descriptor::Wsh(wsh) => hex::encode(wsh.witness_script()), + _ => panic!("expected wsh"), } }; @@ -1079,6 +1410,118 @@ mod tests { } } + // Test BIP-86 first test vector: + // https://github.com/bitcoin/bips/blob/85cda4e225b4d5fd7aff403f69d827f23f6afbbc/bip-0086.mediawiki#test-vectors + #[test] + fn test_tr_bip86() { + mock_unlocked_using_mnemonic( + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about", + "", + ); + let coin = BtcCoin::Tbtc; + let our_key = make_our_key(&[86 + HARDENED, HARDENED, HARDENED]); + + let (is_change, address_index) = (false, 0); + let derived = parse(&make_policy("tr(@0/**)", &[our_key.clone()]), coin) + .unwrap() + .derive(is_change, address_index) + .unwrap(); + match derived { + Descriptor::Tr(tr) => { + assert_eq!( + hex::encode(tr.output_key()), + "a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c" + ); + } + _ => panic!("expected tr"), + } + } + + #[test] + fn test_tr_output_key() { + mock_unlocked_using_mnemonic( + "sudden tenant fault inject concert weather maid people chunk youth stumble grit", + "", + ); + + let coin = BtcCoin::Tbtc; + let our_key = make_our_key(KEYPATH_ACCOUNT); + + let output_key = + |pol: &str, keys: &[pb::KeyOriginInfo], is_change: bool, address_index: u32| { + let derived = parse(&make_policy(pol, keys), coin) + .unwrap() + .derive(is_change, address_index) + .unwrap(); + match derived { + Descriptor::Tr(tr) => hex::encode(tr.output_key()), + _ => panic!("expected tr"), + } + }; + let output_key_at_keypath = |pol: &str, keys: &[pb::KeyOriginInfo], keypath: &[u32]| { + let derived = parse(&make_policy(pol, keys), coin) + .unwrap() + .derive_at_keypath(keypath) + .unwrap(); + match derived { + Descriptor::Tr(tr) => hex::encode(tr.output_key()), + _ => panic!("expected tr"), + } + }; + + // Test receive path and change path using relative and full keypaths. + { + const ADDRESS_INDEX: u32 = 5; + let expected_receive = + "7c8e93a04f41ee302ff08fd4f7348d600431cae1eabe170f287d903771a87395"; + let expected_change = + "b014ba52b642976b952dd028a763a05d039199e87e0c8e9559aa215793b77bd9"; + let desc = "tr(@0/<10;11>/*,{pk(@0/<20;21>/*),pk(@0/<30;31>/*)})"; + assert_eq!( + output_key(desc, &[our_key.clone()], false, ADDRESS_INDEX), + expected_receive + ); + assert_eq!( + output_key(desc, &[our_key.clone()], true, ADDRESS_INDEX), + expected_change + ); + for receive in [10, 20, 30] { + assert_eq!( + output_key_at_keypath( + desc, + &[our_key.clone()], + &[ + 48 + HARDENED, + 1 + HARDENED, + 0 + HARDENED, + 3 + HARDENED, + receive, + ADDRESS_INDEX, + ], + ), + expected_receive, + ); + } + for change in [11, 21, 31] { + assert_eq!( + output_key_at_keypath( + desc, + &[our_key.clone()], + &[ + 48 + HARDENED, + 1 + HARDENED, + 0 + HARDENED, + 3 + HARDENED, + change, + ADDRESS_INDEX, + ], + ), + expected_change, + ); + } + } + } + #[test] fn test_get_hash() { // Fixture below verified with: @@ -1130,4 +1573,48 @@ mod tests { "6160dc5cf72b79380e9e715c75ae54573b81dcb4ed8ab2e90fde5d661e443781", ); } + + #[test] + fn test_tr_unspendable_internal_key() { + mock_unlocked_using_mnemonic( + "sudden tenant fault inject concert weather maid people chunk youth stumble grit", + "", + ); + + let k0 = pb::KeyOriginInfo { + root_fingerprint: vec![], + keypath: vec![], + xpub: Some(parse_xpub("tpubD6NzVbkrYhZ4WNrreqKvZr3qeJR7meg2BgaGP9upLkt7bp5SY6AAhY8vaN8ThfCjVcK6ZzE6kZbinszppNoGKvypeTmhyQ6uvUptXEXqknv").unwrap()), + }; + let k1 = pb::KeyOriginInfo { + root_fingerprint: hex::decode("ffd63c8d").unwrap(), + keypath: vec![48 + HARDENED, 1 + HARDENED, 0 + HARDENED, 2 + HARDENED], + xpub: Some(parse_xpub("tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N").unwrap()), + }; + let k2 = make_our_key(KEYPATH_ACCOUNT); + + { + let policy_str = "tr(@0/<0;1>/*,{and_v(v:multi_a(1,@1/<2;3>/*,@2/<2;3>/*),older(2)),multi_a(2,@1/<0;1>/*,@2/<0;1>/*)})"; + let policy = make_policy(policy_str, &[k0.clone(), k1.clone(), k2.clone()]); + let parsed_policy = parse(&policy, BtcCoin::Tbtc).unwrap(); + assert_eq!( + parsed_policy.taproot_is_unspendable_internal_key(), + Ok(Some(0)) + ); + } + + { + // Different order is allowed, BIP-388 merely says "should" enforce ordered keys, not + // "must". + // See https://github.com/bitcoin/bips/blob/master/bip-0388.mediawiki#additional-rules + let policy_str = "tr(@1/<0;1>/*,{and_v(v:multi_a(1,@0/<2;3>/*,@2/<2;3>/*),older(2)),multi_a(2,@0/<0;1>/*,@2/<0;1>/*)})"; + + let policy = make_policy(policy_str, &[k1.clone(), k0.clone(), k2.clone()]); + let parsed_policy = parse(&policy, BtcCoin::Tbtc).unwrap(); + assert_eq!( + parsed_policy.taproot_is_unspendable_internal_key(), + Ok(Some(1)) + ); + } + } } diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs index af30e8b41..71842214b 100644 --- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs +++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs @@ -17,6 +17,7 @@ use super::Error; use super::common::format_amount; use super::payment_request; +use super::policies::TaprootSpendInfo; use super::script::serialize_varint; use super::script_configs::{ValidatedScriptConfig, ValidatedScriptConfigWithKeypath}; use super::{bip143, bip341, common, keypath}; @@ -250,6 +251,10 @@ fn is_taproot(script_config_account: &ValidatedScriptConfigWithKeypath) -> bool matches!( script_config_account.config, ValidatedScriptConfig::SimpleType(SimpleType::P2tr) + | ValidatedScriptConfig::Policy(super::policies::ParsedPolicy { + descriptor: super::policies::Descriptor::Tr(_), + .. + }) ) } @@ -293,6 +298,8 @@ fn sighash_script( .. } => match policy.derive_at_keypath(keypath)? { super::policies::Descriptor::Wsh(wsh) => Ok(wsh.witness_script()), + // This function is only called for SegWit v0 inputs. + _ => Err(Error::Generic), }, } } @@ -1041,6 +1048,28 @@ async fn _process(request: &pb::BtcSignInitRequest) -> Result { return Err(Error::InvalidInput); } + let spend_info = match &script_config_account.config { + ValidatedScriptConfig::SimpleType(SimpleType::P2tr) => { + // This is a BIP-86 spend, so we tweak the private key by the hash of the public + // key only, as there is no Taproot merkle root. + let xpub = xpub_cache.get_xpub(&tx_input.keypath)?; + let pubkey = bitcoin::PublicKey::from_slice(xpub.public_key()) + .map_err(|_| Error::Generic)?; + TaprootSpendInfo::KeySpend(bitcoin::TapTweakHash::from_key_and_tweak( + pubkey.into(), + None, + )) + } + ValidatedScriptConfig::Policy(policy) => { + // Get the Taproot tweak based on whether we spend using the internal key (key + // path spend) or if we spend using a leaf script. For key path spends, we must + // first tweak the private key to match the Taproot output key. For leaf + // scripts, we do not tweak. + + policy.taproot_spend_info(&mut xpub_cache, &tx_input.keypath)? + } + _ => return Err(Error::Generic), + }; let sighash = bip341::sighash(&bip341::Args { version: request.version, locktime: request.locktime, @@ -1050,11 +1079,24 @@ async fn _process(request: &pb::BtcSignInitRequest) -> Result { hash_sequences: hash_sequence.into(), hash_outputs: hash_outputs.into(), input_index, + tapleaf_hash: if let TaprootSpendInfo::ScriptSpend(leaf_hash) = &spend_info { + Some(leaf_hash.to_byte_array()) + } else { + None + }, }); + next_response.next.has_signature = true; - next_response.next.signature = - bitbox02::keystore::secp256k1_schnorr_bip86_sign(&tx_input.keypath, &sighash)? - .to_vec(); + next_response.next.signature = bitbox02::keystore::secp256k1_schnorr_sign( + &tx_input.keypath, + &sighash, + if let TaprootSpendInfo::KeySpend(tweak_hash) = &spend_info { + Some(tweak_hash.as_byte_array()) + } else { + None + }, + )? + .to_vec(); } else { // Sign all other supported inputs. @@ -3051,7 +3093,17 @@ mod tests { #[test] fn test_policy() { let transaction = alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new_policy())); - mock_host_responder(transaction.clone()); + // Check that previous transactions are streamed, as not all inputs are taproot. + static mut PREVTX_REQUESTED: bool = false; + let tx = transaction.clone(); + *crate::hww::MOCK_NEXT_REQUEST.0.borrow_mut() = + Some(Box::new(move |response: Response| { + let next = extract_next(&response); + if NextType::try_from(next.r#type).unwrap() == NextType::PrevtxInit { + unsafe { PREVTX_REQUESTED = true } + } + Ok(tx.borrow().make_host_request(response)) + })); static mut UI_COUNTER: u32 = 0; mock(Data { @@ -3135,6 +3187,7 @@ mod tests { "sudden tenant fault inject concert weather maid people chunk youth stumble grit", "", ); + bitbox02::random::mock_reset(); // For the policy registration below. mock_memory(); @@ -3176,6 +3229,169 @@ mod tests { unsafe { UI_COUNTER }, transaction.borrow().total_confirmations ); + assert!(unsafe { PREVTX_REQUESTED }); + } + + /// Same as `test_policy()`, but for a tr() Taproot policy. + /// We check that the previous transactions are not streamed as they are not needed for Taproot. + #[test] + fn test_policy_tr() { + let transaction = alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new_policy())); + + let tx = transaction.clone(); + // Check that previous transactions are not streamed, as all inputs are taproot. + static mut PREVTX_REQUESTED: bool = false; + *crate::hww::MOCK_NEXT_REQUEST.0.borrow_mut() = + Some(Box::new(move |response: Response| { + let next = extract_next(&response); + if NextType::try_from(next.r#type).unwrap() == NextType::PrevtxInit { + unsafe { PREVTX_REQUESTED = true } + } + Ok(tx.borrow().make_host_request(response)) + })); + + mock_default_ui(); + + mock_unlocked_using_mnemonic( + "sudden tenant fault inject concert weather maid people chunk youth stumble grit", + "", + ); + bitbox02::random::mock_reset(); + // For the policy registration below. + mock_memory(); + + let keypath_account = &[48 + HARDENED, 1 + HARDENED, 0 + HARDENED, 3 + HARDENED]; + + let policy = pb::btc_script_config::Policy { + policy: "tr(@0/**,pk(@1/**))".into(), + keys: vec![ + pb::KeyOriginInfo { + root_fingerprint: crate::keystore::root_fingerprint().unwrap(), + keypath: keypath_account.to_vec(), + xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()), + }, + pb::KeyOriginInfo { + root_fingerprint: vec![], + keypath: vec![], + xpub: Some(parse_xpub("tpubDFGkUYFfEhAALSXQ9VNssUq71HWYLWLK7sAEqFyqJBQxQ4uGSBW1RSBkoVfijE6iEHZFs2kZrVzzV1nZCSEXYKudtsfEWcWKVXvjjLeRyd8").unwrap()), + }, + ], + }; + + // Register policy. + let policy_hash = super::super::policies::get_hash(pb::BtcCoin::Tbtc, &policy).unwrap(); + bitbox02::memory::multisig_set_by_hash(&policy_hash, "test policy account name").unwrap(); + + let result = block_on(process( + &transaction + .borrow() + .init_request_policy(policy, keypath_account), + )); + match result { + Ok(Response::BtcSignNext(next)) => { + assert!(next.has_signature); + assert_eq!(&next.signature, b"\xf4\xb7\x60\xfa\x7f\x1c\xa8\xa0\x01\x49\xbf\x43\x9c\x07\xdc\xd3\xaa\xfe\x4c\x98\x11\x16\x07\xce\xce\x4b\x80\x06\x6f\x7e\xf2\xe4\x40\x6d\x18\x83\x19\x90\xde\xf0\xbf\x4a\x5b\x56\x47\xdc\x42\x6e\xf1\xf7\x49\x52\x4a\xdf\x0a\x68\x96\x84\x4c\xd9\x0b\x79\x60\x31"); + } + _ => panic!("wrong result"), + } + assert!(unsafe { !PREVTX_REQUESTED }); + } + + // Tests that unspendable internal Taproot keys are displayed as such. + #[test] + fn test_policy_tr_unspendable_internal_key() { + let transaction = alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new_policy())); + + mock_host_responder(transaction.clone()); + + let policy_str = "tr(@0/<0;1>/*,{and_v(v:multi_a(1,@1/<2;3>/*,@2/<2;3>/*),older(2)),multi_a(2,@1/<0;1>/*,@2/<0;1>/*)})"; + + static mut UI_COUNTER: u32 = 0; + mock(Data { + ui_confirm_create: Some(Box::new(move |params| { + match unsafe { + UI_COUNTER += 1; + UI_COUNTER + } { + 1 => { + assert_eq!(params.title, "Spend from"); + assert_eq!(params.body, "BTC Testnet\npolicy with\n3 keys"); + } + 2 => { + assert_eq!(params.title, "Name"); + assert_eq!(params.body, "test policy account name"); + } + 3 => { + assert_eq!(params.title, ""); + assert_eq!(params.body, "Show policy\ndetails?"); + } + 4 => { + assert_eq!(params.title, "Policy"); + assert_eq!(params.body, policy_str); + } + 5 => { + assert_eq!(params.title, "Key 1/3"); + assert_eq!(params.body, "Provably unspendable: tpubD6NzVbkrYhZ4WNrreqKvZr3qeJR7meg2BgaGP9upLkt7bp5SY6AAhY8vaN8ThfCjVcK6ZzE6kZbinszppNoGKvypeTmhyQ6uvUptXEXqknv"); + } + 6 => { + assert_eq!(params.title, "Key 2/3"); + assert_eq!(params.body, "[ffd63c8d/48'/1'/0'/2']tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N"); + } + 7 => { + assert_eq!(params.title, "Key 3/3"); + assert_eq!(params.body, "This device: [93531fa9/48'/1'/0'/3']tpubDEjJGD6BCCuA7VHrbk3gMeQ5HocbZ4eSQ121DcvCkC8xaeRFjyoJC9iVrSz1bWfNwAY5K2Vfz5bnHR3y4RrqVpkc5ikz4trfhSyosZPrcnk"); + } + _ => {} + } + true + })), + ui_transaction_address_create: Some(Box::new(move |_amount, _address| true)), + ui_transaction_fee_create: Some(Box::new(|_total, _fee, _longtouch| true)), + ..Default::default() + }); + + mock_unlocked_using_mnemonic( + "sudden tenant fault inject concert weather maid people chunk youth stumble grit", + "", + ); + bitbox02::random::mock_reset(); + // For the policy registration below. + mock_memory(); + + let keypath_account = &[48 + HARDENED, 1 + HARDENED, 0 + HARDENED, 3 + HARDENED]; + + let policy = pb::btc_script_config::Policy { + policy: policy_str.into(), + keys: vec![ + pb::KeyOriginInfo { + root_fingerprint: vec![], + keypath: vec![], + xpub: Some(parse_xpub("tpubD6NzVbkrYhZ4WNrreqKvZr3qeJR7meg2BgaGP9upLkt7bp5SY6AAhY8vaN8ThfCjVcK6ZzE6kZbinszppNoGKvypeTmhyQ6uvUptXEXqknv").unwrap()), + }, + pb::KeyOriginInfo { + root_fingerprint: hex::decode("ffd63c8d").unwrap(), + keypath: vec![48 + HARDENED, 1 + HARDENED, 0 + HARDENED, 2 + HARDENED], + xpub: Some(parse_xpub("tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N").unwrap()), + }, + pb::KeyOriginInfo { + root_fingerprint: crate::keystore::root_fingerprint().unwrap(), + keypath: keypath_account.to_vec(), + xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()), + }, + ], + }; + + // Register policy. + let policy_hash = super::super::policies::get_hash(pb::BtcCoin::Tbtc, &policy).unwrap(); + bitbox02::memory::multisig_set_by_hash(&policy_hash, "test policy account name").unwrap(); + + assert!(block_on(process( + &transaction + .borrow() + .init_request_policy(policy, keypath_account), + )) + .is_ok()); + assert!(unsafe { UI_COUNTER >= 7 }); } /// Test that a policy with derivations other than `/**` work. diff --git a/src/rust/bitbox02-sys/build.rs b/src/rust/bitbox02-sys/build.rs index f5382e98b..7bc80b41e 100644 --- a/src/rust/bitbox02-sys/build.rs +++ b/src/rust/bitbox02-sys/build.rs @@ -77,7 +77,7 @@ const ALLOWLIST_FNS: &[&str] = &[ "keystore_secp256k1_get_private_key", "keystore_secp256k1_nonce_commit", "keystore_secp256k1_schnorr_bip86_pubkey", - "keystore_secp256k1_schnorr_bip86_sign", + "keystore_secp256k1_schnorr_sign", "keystore_secp256k1_sign", "keystore_unlock", "keystore_unlock_bip39", diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs index 1eac60394..53076f686 100644 --- a/src/rust/bitbox02/src/keystore.rs +++ b/src/rust/bitbox02/src/keystore.rs @@ -343,13 +343,22 @@ pub fn bip85_ln(index: u32) -> Result, ()> { } } -pub fn secp256k1_schnorr_bip86_sign(keypath: &[u32], msg: &[u8; 32]) -> Result<[u8; 64], ()> { +pub fn secp256k1_schnorr_sign( + keypath: &[u32], + msg: &[u8; 32], + tweak: Option<&[u8; 32]>, +) -> Result<[u8; 64], ()> { let mut signature = [0u8; 64]; + match unsafe { - bitbox02_sys::keystore_secp256k1_schnorr_bip86_sign( + bitbox02_sys::keystore_secp256k1_schnorr_sign( keypath.as_ptr(), keypath.len() as _, msg.as_ptr(), + match tweak { + Some(t) => t.as_ptr(), + None => core::ptr::null() as *const _, + }, signature.as_mut_ptr(), ) } { diff --git a/test/unit-test/test_keystore.c b/test/unit-test/test_keystore.c index ed63c89be..1dc109a2f 100644 --- a/test/unit-test/test_keystore.c +++ b/test/unit-test/test_keystore.c @@ -697,13 +697,12 @@ static void _test_keystore_secp256k1_schnorr_bip86_pubkey(void** state) } } -static void _test_keystore_secp256k1_schnorr_bip86_sign(void** state) +static void _test_keystore_secp256k1_schnorr_sign(void** state) { _mock_with_mnemonic( "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon " "about", ""); - uint8_t pubkey[32] = {0}; const uint32_t keypath[] = { 86 + BIP32_INITIAL_HARDENED_CHILD, 0 + BIP32_INITIAL_HARDENED_CHILD, @@ -714,17 +713,33 @@ static void _test_keystore_secp256k1_schnorr_bip86_sign(void** state) struct ext_key xpub = {0}; assert_true(keystore_get_xpub(keypath, 5, &xpub)); - assert_true(keystore_secp256k1_schnorr_bip86_pubkey(xpub.pub_key, pubkey)); uint8_t msg[32] = {0}; memset(msg, 0x88, sizeof(msg)); uint8_t sig[64] = {0}; uint8_t mock_aux_rand[32] = {0}; + + // Test without tweak will_return(__wrap_random_32_bytes, mock_aux_rand); - assert_true(keystore_secp256k1_schnorr_bip86_sign(keypath, 5, msg, sig)); + assert_true(keystore_secp256k1_schnorr_sign(keypath, 5, msg, NULL, sig)); const secp256k1_context* ctx = wally_get_secp_context(); - secp256k1_xonly_pubkey pubkey_deserialized = {0}; - assert_true(secp256k1_xonly_pubkey_parse(ctx, &pubkey_deserialized, pubkey)); - assert_true(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &pubkey_deserialized)); + secp256k1_pubkey pubkey = {0}; + assert_true(secp256k1_ec_pubkey_parse(ctx, &pubkey, xpub.pub_key, sizeof(xpub.pub_key))); + secp256k1_xonly_pubkey xonly_pubkey = {0}; + assert_true(secp256k1_xonly_pubkey_from_pubkey(ctx, &xonly_pubkey, NULL, &pubkey)); + assert_true(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &xonly_pubkey)); + + // Test with tweak + const uint8_t tweak[32] = + "\xa3\x9f\xb1\x63\xdb\xd9\xb5\xe0\x84\x0a\xf3\xcc\x1e\xe4\x1d\x5b\x31\x24\x5c\x5d\xd8\xd6" + "\xbd\xc3\xd0\x26\xd0\x9b\x89\x64\x99\x7c"; + will_return(__wrap_random_32_bytes, mock_aux_rand); + assert_true(keystore_secp256k1_schnorr_sign(keypath, 5, msg, tweak, sig)); + secp256k1_pubkey tweaked_pubkey = {0}; + assert_true(secp256k1_xonly_pubkey_tweak_add(ctx, &tweaked_pubkey, &xonly_pubkey, tweak)); + secp256k1_xonly_pubkey tweaked_xonly_pubkey = {0}; + assert_true( + secp256k1_xonly_pubkey_from_pubkey(ctx, &tweaked_xonly_pubkey, NULL, &tweaked_pubkey)); + assert_true(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &tweaked_xonly_pubkey)); } int main(void) @@ -745,7 +760,7 @@ int main(void) cmocka_unit_test(_test_keystore_get_ed25519_seed), cmocka_unit_test(_test_secp256k1_schnorr_sign), cmocka_unit_test(_test_keystore_secp256k1_schnorr_bip86_pubkey), - cmocka_unit_test(_test_keystore_secp256k1_schnorr_bip86_sign), + cmocka_unit_test(_test_keystore_secp256k1_schnorr_sign), }; return cmocka_run_group_tests(tests, NULL, NULL); }