From b72f56c4ae021ca5a26f78eb8e9808a43aecd19e Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Mon, 12 Apr 2021 13:47:03 +0200 Subject: [PATCH 1/4] BIP32 extended keys are using Scep256k1 keys instead of bitcoin ECDSA According to #588, BIP32 does not support uncompressed keys and using type with compression flag is a mistake --- examples/bip32.rs | 4 ++-- src/util/bip32.rs | 52 +++++++++++++++++++++++++++++--------------- src/util/psbt/mod.rs | 30 +++++++++++++++---------- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/examples/bip32.rs b/examples/bip32.rs index 0b0d1f27ef..0ae3bce352 100644 --- a/examples/bip32.rs +++ b/examples/bip32.rs @@ -4,7 +4,7 @@ use std::{env, process}; use std::str::FromStr; use bitcoin::secp256k1::Secp256k1; -use bitcoin::util::ecdsa::PrivateKey; +use bitcoin::{PrivateKey, PublicKey}; use bitcoin::util::bip32::ExtendedPrivKey; use bitcoin::util::bip32::ExtendedPubKey; use bitcoin::util::bip32::DerivationPath; @@ -58,7 +58,7 @@ fn main() { let public_key = xpub.derive_pub(&secp, &vec![zero, zero]) .unwrap() .public_key; - let address = Address::p2wpkh(&public_key, network).unwrap(); + let address = Address::p2wpkh(&PublicKey::new(public_key), network).unwrap(); println!("First receiving address: {}", address); } diff --git a/src/util/bip32.rs b/src/util/bip32.rs index 9675a3cfe8..77b7de284b 100644 --- a/src/util/bip32.rs +++ b/src/util/bip32.rs @@ -28,8 +28,9 @@ use hashes::{sha512, Hash, HashEngine, Hmac, HmacEngine}; use secp256k1::{self, Secp256k1}; use network::constants::Network; -use util::{base58, endian, key}; -use util::ecdsa::{PublicKey, PrivateKey}; +use util::{base58, endian}; +use util::key; +use io::Write; /// A chain code #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -44,7 +45,8 @@ impl_array_newtype!(Fingerprint, u8, 4); impl_bytes_newtype!(Fingerprint, 4); /// Extended private key -#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "std", derive(Debug))] pub struct ExtendedPrivKey { /// The network this key is to be used on pub network: Network, @@ -55,12 +57,26 @@ pub struct ExtendedPrivKey { /// Child number of the key used to derive from parent (0 for master) pub child_number: ChildNumber, /// Private key - pub private_key: PrivateKey, + pub private_key: secp256k1::SecretKey, /// Chain code pub chain_code: ChainCode } serde_string_impl!(ExtendedPrivKey, "a BIP-32 extended private key"); +#[cfg(not(feature = "std"))] +#[cfg_attr(docsrs, doc(cfg(not(feature = "std"))))] +impl fmt::Debug for ExtendedPrivKey { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("ExtendedPrivKey") + .field("network", &self.network) + .field("depth", &self.depth) + .field("parent_fingerprint", &self.parent_fingerprint) + .field("child_number", &self.child_number) + .field("chain_code", &self.chain_code) + .finish_non_exhaustive() + } +} + /// Extended public key #[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, Hash)] pub struct ExtendedPubKey { @@ -73,7 +89,7 @@ pub struct ExtendedPubKey { /// Child number of the key used to derive from parent (0 for master) pub child_number: ChildNumber, /// Public key - pub public_key: PublicKey, + pub public_key: secp256k1::PublicKey, /// Chain code pub chain_code: ChainCode } @@ -506,7 +522,7 @@ impl ExtendedPrivKey { depth: 0, parent_fingerprint: Default::default(), child_number: ChildNumber::from_normal_idx(0)?, - private_key: PrivateKey::from_slice(&hmac_result[..32], network)?, + private_key: secp256k1::SecretKey::from_slice(&hmac_result[..32])?, chain_code: ChainCode::from(&hmac_result[32..]), }) } @@ -532,7 +548,7 @@ impl ExtendedPrivKey { match i { ChildNumber::Normal { .. } => { // Non-hardened key: compute public data and use that - hmac_engine.input(&PublicKey::from_private_key(secp, &self.private_key).key.serialize()[..]); + hmac_engine.input(&secp256k1::PublicKey::from_secret_key(secp, &self.private_key).serialize()[..]); } ChildNumber::Hardened { .. } => { // Hardened key: use only secret data to prevent public derivation @@ -543,8 +559,8 @@ impl ExtendedPrivKey { hmac_engine.input(&endian::u32_to_array_be(u32::from(i))); let hmac_result: Hmac = Hmac::from_engine(hmac_engine); - let mut sk = PrivateKey::from_slice(&hmac_result[..32], self.network)?; - sk.key.add_assign(&self.private_key[..])?; + let mut sk = secp256k1::SecretKey::from_slice(&hmac_result[..32])?; + sk.add_assign(&self.private_key[..])?; Ok(ExtendedPrivKey { network: self.network, @@ -578,7 +594,7 @@ impl ExtendedPrivKey { parent_fingerprint: Fingerprint::from(&data[5..9]), child_number: endian::slice_to_u32_be(&data[9..13]).into(), chain_code: ChainCode::from(&data[13..45]), - private_key: PrivateKey::from_slice(&data[46..78], network)?, + private_key: secp256k1::SecretKey::from_slice(&data[46..78])?, }) } @@ -617,7 +633,7 @@ impl ExtendedPubKey { depth: sk.depth, parent_fingerprint: sk.parent_fingerprint, child_number: sk.child_number, - public_key: PublicKey::from_private_key(secp, &sk.private_key), + public_key: secp256k1::PublicKey::from_secret_key(secp, &sk.private_key), chain_code: sk.chain_code } } @@ -638,19 +654,19 @@ impl ExtendedPubKey { } /// Compute the scalar tweak added to this key to get a child key - pub fn ckd_pub_tweak(&self, i: ChildNumber) -> Result<(PrivateKey, ChainCode), Error> { + pub fn ckd_pub_tweak(&self, i: ChildNumber) -> Result<(secp256k1::SecretKey, ChainCode), Error> { match i { ChildNumber::Hardened { .. } => { Err(Error::CannotDeriveFromHardenedKey) } ChildNumber::Normal { index: n } => { let mut hmac_engine: HmacEngine = HmacEngine::new(&self.chain_code[..]); - hmac_engine.input(&self.public_key.key.serialize()[..]); + hmac_engine.input(&self.public_key.serialize()[..]); hmac_engine.input(&endian::u32_to_array_be(n)); let hmac_result: Hmac = Hmac::from_engine(hmac_engine); - let private_key = PrivateKey::from_slice(&hmac_result[..32], self.network)?; + let private_key = secp256k1::SecretKey::from_slice(&hmac_result[..32])?; let chain_code = ChainCode::from(&hmac_result[32..]); Ok((private_key, chain_code)) } @@ -665,7 +681,7 @@ impl ExtendedPubKey { ) -> Result { let (sk, chain_code) = self.ckd_pub_tweak(i)?; let mut pk = self.public_key; - pk.key.add_exp_assign(secp, &sk[..])?; + pk.add_exp_assign(secp, &sk[..])?; Ok(ExtendedPubKey { network: self.network, @@ -697,7 +713,7 @@ impl ExtendedPubKey { parent_fingerprint: Fingerprint::from(&data[5..9]), child_number: endian::slice_to_u32_be(&data[9..13]).into(), chain_code: ChainCode::from(&data[13..45]), - public_key: PublicKey::from_slice(&data[45..78])?, + public_key: secp256k1::PublicKey::from_slice(&data[45..78])?, }) } @@ -712,14 +728,14 @@ impl ExtendedPubKey { ret[5..9].copy_from_slice(&self.parent_fingerprint[..]); ret[9..13].copy_from_slice(&endian::u32_to_array_be(u32::from(self.child_number))); ret[13..45].copy_from_slice(&self.chain_code[..]); - ret[45..78].copy_from_slice(&self.public_key.key.serialize()[..]); + ret[45..78].copy_from_slice(&self.public_key.serialize()[..]); ret } /// Returns the HASH160 of the chaincode pub fn identifier(&self) -> XpubIdentifier { let mut engine = XpubIdentifier::engine(); - self.public_key.write_into(&mut engine).expect("engines don't error"); + engine.write(&self.public_key.serialize()).expect("engines don't error"); XpubIdentifier::from_engine(engine) } diff --git a/src/util/psbt/mod.rs b/src/util/psbt/mod.rs index 6e3b4a038d..401929b5cb 100644 --- a/src/util/psbt/mod.rs +++ b/src/util/psbt/mod.rs @@ -243,24 +243,23 @@ impl Decodable for PartiallySignedTransaction { #[cfg(test)] mod tests { + use super::PartiallySignedTransaction; + use hashes::hex::FromHex; use hashes::{sha256, hash160, Hash, ripemd160}; use hash_types::Txid; - - use secp256k1::Secp256k1; + use secp256k1::{Secp256k1, self}; use blockdata::script::Script; use blockdata::transaction::{Transaction, TxIn, TxOut, OutPoint}; use network::constants::Network::Bitcoin; use consensus::encode::{deserialize, serialize, serialize_hex}; use util::bip32::{ChildNumber, ExtendedPrivKey, ExtendedPubKey, Fingerprint, KeySource}; - use util::ecdsa::PublicKey; + use util::ecdsa; use util::psbt::map::{Output, Input}; use util::psbt::raw; - use super::PartiallySignedTransaction; - use util::psbt::raw::ProprietaryKey; use std::collections::BTreeMap; use blockdata::witness::Witness; @@ -292,7 +291,7 @@ mod tests { let secp = &Secp256k1::new(); let seed = Vec::from_hex("000102030405060708090a0b0c0d0e0f").unwrap(); - let mut hd_keypaths: BTreeMap = Default::default(); + let mut hd_keypaths: BTreeMap = Default::default(); let mut sk: ExtendedPrivKey = ExtendedPrivKey::new_master(Bitcoin, &seed).unwrap(); @@ -322,7 +321,10 @@ mod tests { witness_script: Some(hex_script!( "a9143545e6e33b832c47050f24d3eeb93c9c03948bc787" )), - bip32_derivation: hd_keypaths, + bip32_derivation: hd_keypaths.into_iter().map(|(key, src)| (ecdsa::PublicKey { + compressed: true, + key, + }, src)).collect(), ..Default::default() }; @@ -435,7 +437,7 @@ mod tests { vec![3, 4 ,5], )].into_iter().collect(); let key_source = ("deadbeef".parse().unwrap(), "m/0'/1".parse().unwrap()); - let keypaths: BTreeMap = vec![( + let keypaths: BTreeMap = vec![( "0339880dc92394b7355e3d0439fa283c31de7590812ea011c4245c0674a685e883".parse().unwrap(), key_source.clone(), )].into_iter().collect(); @@ -479,7 +481,10 @@ mod tests { "0339880dc92394b7355e3d0439fa283c31de7590812ea011c4245c0674a685e883".parse().unwrap(), vec![8, 5, 4], )].into_iter().collect(), - bip32_derivation: keypaths.clone(), + bip32_derivation: keypaths.clone().into_iter().map(|(key, src)| (ecdsa::PublicKey { + compressed: true, + key, + }, src)).collect(), final_script_witness: Some(vec![vec![1, 3], vec![5]]), ripemd160_preimages: vec![(ripemd160::Hash::hash(&[]), vec![1, 2])].into_iter().collect(), sha256_preimages: vec![(sha256::Hash::hash(&[]), vec![1, 2])].into_iter().collect(), @@ -490,7 +495,10 @@ mod tests { ..Default::default() }], outputs: vec![Output { - bip32_derivation: keypaths.clone(), + bip32_derivation: keypaths.into_iter().map(|(key, src)| (ecdsa::PublicKey { + compressed: true, + key, + }, src)).collect(), proprietary: proprietary.clone(), unknown: unknown.clone(), ..Default::default() @@ -1012,7 +1020,7 @@ mod tests { #[test] fn serialize_and_deserialize_proprietary() { let mut psbt: PartiallySignedTransaction = hex_psbt!("70736274ff0100a00200000002ab0949a08c5af7c49b8212f417e2f15ab3f5c33dcf153821a8139f877a5b7be40000000000feffffffab0949a08c5af7c49b8212f417e2f15ab3f5c33dcf153821a8139f877a5b7be40100000000feffffff02603bea0b000000001976a914768a40bbd740cbe81d988e71de2a4d5c71396b1d88ac8e240000000000001976a9146f4620b553fa095e721b9ee0efe9fa039cca459788ac000000000001076a47304402204759661797c01b036b25928948686218347d89864b719e1f7fcf57d1e511658702205309eabf56aa4d8891ffd111fdf1336f3a29da866d7f8486d75546ceedaf93190121035cdc61fc7ba971c0b501a646a2a83b102cb43881217ca682dc86e2d73fa882920001012000e1f5050000000017a9143545e6e33b832c47050f24d3eeb93c9c03948bc787010416001485d13537f2e265405a34dbafa9e3dda01fb82308000000").unwrap(); - psbt.proprietary.insert(ProprietaryKey { + psbt.proprietary.insert(raw::ProprietaryKey { prefix: b"test".to_vec(), subtype: 0u8, key: b"test".to_vec(), From e6a3d603c9a3e062e38d4eb03bbc8effd64b6319 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sun, 1 Aug 2021 21:03:58 +0200 Subject: [PATCH 2/4] BIP32 extended key `to_ecdsa()` and `to_schnorr()` methods --- examples/bip32.rs | 2 +- src/util/bip32.rs | 47 ++++++++++++++++++++++++++++++++++++++------ src/util/psbt/mod.rs | 2 +- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/examples/bip32.rs b/examples/bip32.rs index 0ae3bce352..ea4b876281 100644 --- a/examples/bip32.rs +++ b/examples/bip32.rs @@ -49,7 +49,7 @@ fn main() { let path = DerivationPath::from_str("m/84h/0h/0h").unwrap(); let child = root.derive_priv(&secp, &path).unwrap(); println!("Child at {}: {}", path, child); - let xpub = ExtendedPubKey::from_private(&secp, &child); + let xpub = ExtendedPubKey::from_priv(&secp, &child); println!("Public key at {}: {}", path, xpub); // generate first receiving address at m/0/0 diff --git a/src/util/bip32.rs b/src/util/bip32.rs index 77b7de284b..248dc4761f 100644 --- a/src/util/bip32.rs +++ b/src/util/bip32.rs @@ -25,11 +25,11 @@ use core::{fmt, str::FromStr, default::Default}; use hash_types::XpubIdentifier; use hashes::{sha512, Hash, HashEngine, Hmac, HmacEngine}; -use secp256k1::{self, Secp256k1}; +use secp256k1::{self, Secp256k1, XOnlyPublicKey}; use network::constants::Network; use util::{base58, endian}; -use util::key; +use util::{key, ecdsa, schnorr}; use io::Write; /// A chain code @@ -527,6 +527,21 @@ impl ExtendedPrivKey { }) } + /// Constructs ECDSA compressed private key matching internal secret key representation. + pub fn to_priv(&self) -> ecdsa::PrivateKey { + ecdsa::PrivateKey { + compressed: true, + network: self.network, + key: self.private_key + } + } + + /// Constructs BIP340 keypair for Schnorr signatures and Taproot use matching the internal + /// secret key representation. + pub fn to_keypair(&self, secp: &Secp256k1) -> schnorr::KeyPair { + schnorr::KeyPair::from_seckey_slice(secp, &self.private_key[..]).expect("BIP32 internal private key representation is broken") + } + /// Attempts to derive an extended private key from a path. /// /// The `path` argument can be both of type `DerivationPath` or `Vec`. @@ -616,7 +631,7 @@ impl ExtendedPrivKey { /// Returns the HASH160 of the public key belonging to the xpriv pub fn identifier(&self, secp: &Secp256k1) -> XpubIdentifier { - ExtendedPubKey::from_private(secp, self).identifier() + ExtendedPubKey::from_priv(secp, self).identifier() } /// Returns the first four bytes of the identifier @@ -627,7 +642,13 @@ impl ExtendedPrivKey { impl ExtendedPubKey { /// Derives a public key from a private key + #[deprecated(since = "0.28.0", note = "use ExtendedPubKey::from_priv")] pub fn from_private(secp: &Secp256k1, sk: &ExtendedPrivKey) -> ExtendedPubKey { + ExtendedPubKey::from_priv(secp, sk) + } + + /// Derives a public key from a private key + pub fn from_priv(secp: &Secp256k1, sk: &ExtendedPrivKey) -> ExtendedPubKey { ExtendedPubKey { network: sk.network, depth: sk.depth, @@ -638,6 +659,20 @@ impl ExtendedPubKey { } } + /// Constructs ECDSA compressed public key matching internal public key representation. + pub fn to_pub(&self) -> ecdsa::PublicKey { + ecdsa::PublicKey { + compressed: true, + key: self.public_key + } + } + + /// Constructs BIP340 x-only public key for BIP-340 signatures and Taproot use matching + /// the internal public key representation. + pub fn to_x_only_pub(&self) -> XOnlyPublicKey { + XOnlyPublicKey::from(self.public_key) + } + /// Attempts to derive an extended public key from a path. /// /// The `path` argument can be both of type `DerivationPath` or `Vec`. @@ -869,7 +904,7 @@ mod tests { expected_pk: &str) { let mut sk = ExtendedPrivKey::new_master(network, seed).unwrap(); - let mut pk = ExtendedPubKey::from_private(secp, &sk); + let mut pk = ExtendedPubKey::from_priv(secp, &sk); // Check derivation convenience method for ExtendedPrivKey assert_eq!( @@ -897,7 +932,7 @@ mod tests { match num { Normal {..} => { let pk2 = pk.ckd_pub(secp, num).unwrap(); - pk = ExtendedPubKey::from_private(secp, &sk); + pk = ExtendedPubKey::from_priv(secp, &sk); assert_eq!(pk, pk2); } Hardened {..} => { @@ -905,7 +940,7 @@ mod tests { pk.ckd_pub(secp, num), Err(Error::CannotDeriveFromHardenedKey) ); - pk = ExtendedPubKey::from_private(secp, &sk); + pk = ExtendedPubKey::from_priv(secp, &sk); } } } diff --git a/src/util/psbt/mod.rs b/src/util/psbt/mod.rs index 401929b5cb..c6c4e46c89 100644 --- a/src/util/psbt/mod.rs +++ b/src/util/psbt/mod.rs @@ -310,7 +310,7 @@ mod tests { sk = sk.derive_priv(secp, &dpath).unwrap(); - let pk: ExtendedPubKey = ExtendedPubKey::from_private(&secp, &sk); + let pk: ExtendedPubKey = ExtendedPubKey::from_priv(&secp, &sk); hd_keypaths.insert(pk.public_key, (fprint, dpath.into())); From b65a6ae49b64df0487de595f97beb31c6bb0c524 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Thu, 30 Dec 2021 01:53:06 +0100 Subject: [PATCH 3/4] Test for extended private key keypair generation f5875a --- src/util/bip32.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/util/bip32.rs b/src/util/bip32.rs index 248dc4761f..96ea64a51e 100644 --- a/src/util/bip32.rs +++ b/src/util/bip32.rs @@ -1131,5 +1131,42 @@ mod tests { assert_eq!("42", &format!("{}", ChildNumber::from_normal_idx(42).unwrap())); assert_eq!("000042", &format!("{:06}", ChildNumber::from_normal_idx(42).unwrap())); } + + #[test] + #[should_panic(expected = "Secp256k1(InvalidSecretKey)")] + fn schnorr_broken_privkey_zeros() { + /* this is how we generate key: + let mut sk = secp256k1::key::ONE_KEY; + + let zeros = [0u8; 32]; + unsafe { + sk.as_mut_ptr().copy_from(zeros.as_ptr(), 32); + } + + let xpriv = ExtendedPrivKey { + network: Network::Bitcoin, + depth: 0, + parent_fingerprint: Default::default(), + child_number: ChildNumber::Normal { index: 0 }, + private_key: sk, + chain_code: ChainCode::from(&[0u8; 32][..]) + }; + + println!("{}", xpriv); + */ + + // Xpriv having secret key set to all zeros + let xpriv_str = "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzF93Y5wvzdUayhgkkFoicQZcP3y52uPPxFnfoLZB21Teqt1VvEHx"; + ExtendedPrivKey::from_str(xpriv_str).unwrap(); + } + + + #[test] + #[should_panic(expected = "Secp256k1(InvalidSecretKey)")] + fn schnorr_broken_privkey_ffs() { + // Xpriv having secret key set to all 0xFF's + let xpriv_str = "xprv9s21ZrQH143K24Mfq5zL5MhWK9hUhhGbd45hLXo2Pq2oqzMMo63oStZzFAzHGBP2UuGCqWLTAPLcMtD9y5gkZ6Eq3Rjuahrv17fENZ3QzxW"; + ExtendedPrivKey::from_str(xpriv_str).unwrap(); + } } From cf0c48cc86b1f9710588987db8137362275166ad Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sat, 8 Jan 2022 20:46:52 +0100 Subject: [PATCH 4/4] Improve Debug for PrivateKey --- src/util/ecdsa.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/ecdsa.rs b/src/util/ecdsa.rs index 9cd4bb2cfa..b887fa7ff7 100644 --- a/src/util/ecdsa.rs +++ b/src/util/ecdsa.rs @@ -173,6 +173,7 @@ impl FromStr for PublicKey { /// A Bitcoin ECDSA private key #[derive(Copy, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "std", derive(Debug))] pub struct PrivateKey { /// Whether this private key should be serialized as compressed pub compressed: bool, @@ -279,6 +280,7 @@ impl fmt::Display for PrivateKey { } } +#[cfg(not(feature = "std"))] impl fmt::Debug for PrivateKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "[private key data]")