From e885d0a7747cfc3b89a3c2765a8c0dd174e3889a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 6 Feb 2021 13:11:23 -0500 Subject: [PATCH] Swap key_derivation_params (u64, u64) for channel_keys_id [u8; 32] Instead of `key_derivation_params` being a rather strange type, we call it `channel_keys_id` and give it a generic 32 byte array. This should be much clearer for users and also more flexible. --- fuzz/src/chanmon_consistency.rs | 2 +- fuzz/src/full_stack.rs | 4 +- lightning/src/chain/channelmonitor.rs | 20 +++---- lightning/src/chain/keysinterface.rs | 62 ++++++++++----------- lightning/src/ln/channel.rs | 2 +- lightning/src/ln/functional_tests.rs | 8 +-- lightning/src/util/enforcing_trait_impls.rs | 2 +- lightning/src/util/test_utils.rs | 4 +- 8 files changed, 52 insertions(+), 52 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 8e79ac5f9..4cf90b1dc 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -179,7 +179,7 @@ impl KeysInterface for KeyProvider { SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, self.node_id]).unwrap(), [id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, self.node_id], channel_value_satoshis, - (0, 0), + [0; 32], ); let revoked_commitment = self.make_revoked_commitment_cell(keys.commitment_seed); EnforcingChannelKeys::new_with_revoked(keys, revoked_commitment, false) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 901c9ef1e..a1e1b7599 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -279,7 +279,7 @@ impl KeysInterface for KeyProvider { SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, ctr]).unwrap(), [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, ctr], channel_value_satoshis, - (0, 0), + [0; 32] ) } else { InMemoryChannelKeys::new( @@ -291,7 +291,7 @@ impl KeysInterface for KeyProvider { SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 11, ctr]).unwrap(), [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 12, ctr], channel_value_satoshis, - (0, 0), + [0; 32] ) }) } diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 95495d1bf..a8258f564 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -632,7 +632,7 @@ pub struct ChannelMonitor { counterparty_payment_script: Script, shutdown_script: Script, - key_derivation_params: (u64, u64), + channel_keys_id: [u8; 32], holder_revocation_basepoint: PublicKey, funding_info: (OutPoint, Script), current_counterparty_commitment_txid: Option, @@ -728,7 +728,7 @@ impl PartialEq for ChannelMonitor { self.destination_script != other.destination_script || self.broadcasted_holder_revokable_script != other.broadcasted_holder_revokable_script || self.counterparty_payment_script != other.counterparty_payment_script || - self.key_derivation_params != other.key_derivation_params || + self.channel_keys_id != other.channel_keys_id || self.holder_revocation_basepoint != other.holder_revocation_basepoint || self.funding_info != other.funding_info || self.current_counterparty_commitment_txid != other.current_counterparty_commitment_txid || @@ -786,7 +786,7 @@ impl Writeable for ChannelMonitor { self.counterparty_payment_script.write(writer)?; self.shutdown_script.write(writer)?; - self.key_derivation_params.write(writer)?; + self.channel_keys_id.write(writer)?; self.holder_revocation_basepoint.write(writer)?; writer.write_all(&self.funding_info.0.txid[..])?; writer.write_all(&byte_utils::be16_to_array(self.funding_info.0.index))?; @@ -967,7 +967,7 @@ impl ChannelMonitor { let counterparty_htlc_base_key = counterparty_channel_parameters.pubkeys.htlc_basepoint; let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() }; - let key_derivation_params = keys.key_derivation_params(); + let channel_keys_id = keys.channel_keys_id(); let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint; let secp_ctx = Secp256k1::new(); @@ -1006,7 +1006,7 @@ impl ChannelMonitor { counterparty_payment_script, shutdown_script, - key_derivation_params, + channel_keys_id, holder_revocation_basepoint, funding_info, current_counterparty_commitment_txid: None, @@ -2206,7 +2206,7 @@ impl ChannelMonitor { per_commitment_point: broadcasted_holder_revokable_script.1, to_self_delay: self.on_holder_tx_csv, output: outp.clone(), - key_derivation_params: self.key_derivation_params, + channel_keys_id: self.channel_keys_id, revocation_pubkey: broadcasted_holder_revokable_script.2.clone(), }); break; @@ -2215,7 +2215,7 @@ impl ChannelMonitor { spendable_output = Some(SpendableOutputDescriptor::StaticOutputCounterpartyPayment { outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), - key_derivation_params: self.key_derivation_params, + channel_keys_id: self.channel_keys_id, }); break; } else if outp.script_pubkey == self.shutdown_script { @@ -2332,7 +2332,7 @@ impl<'a, ChanSigner: ChannelKeys, K: KeysInterface> let counterparty_payment_script = Readable::read(reader)?; let shutdown_script = Readable::read(reader)?; - let key_derivation_params = Readable::read(reader)?; + let channel_keys_id = Readable::read(reader)?; let holder_revocation_basepoint = Readable::read(reader)?; // Technically this can fail and serialize fail a round-trip, but only for serialization of // barely-init'd ChannelMonitors that we can't do anything with. @@ -2547,7 +2547,7 @@ impl<'a, ChanSigner: ChannelKeys, K: KeysInterface> counterparty_payment_script, shutdown_script, - key_derivation_params, + channel_keys_id, holder_revocation_basepoint, funding_info, current_counterparty_commitment_txid, @@ -2675,7 +2675,7 @@ mod tests { SecretKey::from_slice(&[41; 32]).unwrap(), [41; 32], 0, - (0, 0) + [0; 32] ); let counterparty_pubkeys = ChannelPublicKeys { diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index e56ac96f0..f713d70ca 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -100,7 +100,7 @@ pub enum SpendableOutputDescriptor { output: TxOut, /// The channel keys state used to proceed to derivation of signing key. Must /// be pass to KeysInterface::derive_channel_keys. - key_derivation_params: (u64, u64), + channel_keys_id: [u8; 32], /// The revocation_pubkey used to derive witnessScript revocation_pubkey: PublicKey }, @@ -118,7 +118,7 @@ pub enum SpendableOutputDescriptor { output: TxOut, /// The channel keys state used to proceed to derivation of signing key. Must /// be pass to KeysInterface::derive_channel_keys. - key_derivation_params: (u64, u64), + channel_keys_id: [u8; 32], } } @@ -130,22 +130,20 @@ impl Writeable for SpendableOutputDescriptor { outpoint.write(writer)?; output.write(writer)?; }, - &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref revocation_pubkey } => { + &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref channel_keys_id, ref revocation_pubkey } => { 1u8.write(writer)?; outpoint.write(writer)?; per_commitment_point.write(writer)?; to_self_delay.write(writer)?; output.write(writer)?; - key_derivation_params.0.write(writer)?; - key_derivation_params.1.write(writer)?; + channel_keys_id.write(writer)?; revocation_pubkey.write(writer)?; }, - &SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref key_derivation_params } => { + &SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref channel_keys_id } => { 2u8.write(writer)?; outpoint.write(writer)?; output.write(writer)?; - key_derivation_params.0.write(writer)?; - key_derivation_params.1.write(writer)?; + channel_keys_id.write(writer)?; }, } Ok(()) @@ -164,13 +162,13 @@ impl Readable for SpendableOutputDescriptor { per_commitment_point: Readable::read(reader)?, to_self_delay: Readable::read(reader)?, output: Readable::read(reader)?, - key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?), + channel_keys_id: Readable::read(reader)?, revocation_pubkey: Readable::read(reader)?, }), 2u8 => Ok(SpendableOutputDescriptor::StaticOutputCounterpartyPayment { outpoint: Readable::read(reader)?, output: Readable::read(reader)?, - key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?), + channel_keys_id: Readable::read(reader)?, }), _ => Err(DecodeError::InvalidValue), } @@ -221,10 +219,10 @@ pub trait ChannelKeys : Send+Clone + Writeable { fn release_commitment_secret(&self, idx: u64) -> [u8; 32]; /// Gets the holder's channel public keys and basepoints fn pubkeys(&self) -> &ChannelPublicKeys; - /// Gets arbitrary identifiers describing the set of keys which are provided back to you in - /// some SpendableOutputDescriptor types. These should be sufficient to identify this + /// Gets an arbitrary identifier describing the set of keys which are provided back to you in + /// some SpendableOutputDescriptor types. This should be sufficient to identify this /// ChannelKeys object uniquely and lookup or re-derive its keys. - fn key_derivation_params(&self) -> (u64, u64); + fn channel_keys_id(&self) -> [u8; 32]; /// Create a signature for a counterparty's commitment transaction and associated HTLC transactions. /// @@ -375,7 +373,7 @@ pub struct InMemoryChannelKeys { /// The total value of this channel channel_value_satoshis: u64, /// Key derivation parameters - key_derivation_params: (u64, u64), + channel_keys_id: [u8; 32], } impl InMemoryChannelKeys { @@ -389,7 +387,7 @@ impl InMemoryChannelKeys { htlc_base_key: SecretKey, commitment_seed: [u8; 32], channel_value_satoshis: u64, - key_derivation_params: (u64, u64)) -> InMemoryChannelKeys { + channel_keys_id: [u8; 32]) -> InMemoryChannelKeys { let holder_channel_pubkeys = InMemoryChannelKeys::make_holder_keys(secp_ctx, &funding_key, &revocation_base_key, &payment_key, &delayed_payment_base_key, @@ -404,7 +402,7 @@ impl InMemoryChannelKeys { channel_value_satoshis, holder_channel_pubkeys, channel_parameters: None, - key_derivation_params, + channel_keys_id, } } @@ -468,7 +466,7 @@ impl ChannelKeys for InMemoryChannelKeys { } fn pubkeys(&self) -> &ChannelPublicKeys { &self.holder_channel_pubkeys } - fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params } + fn channel_keys_id(&self) -> [u8; 32] { self.channel_keys_id } fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { let trusted_tx = commitment_tx.trust(); @@ -600,8 +598,7 @@ impl Writeable for InMemoryChannelKeys { self.commitment_seed.write(writer)?; self.channel_parameters.write(writer)?; self.channel_value_satoshis.write(writer)?; - self.key_derivation_params.0.write(writer)?; - self.key_derivation_params.1.write(writer)?; + self.channel_keys_id.write(writer)?; Ok(()) } @@ -622,8 +619,7 @@ impl Readable for InMemoryChannelKeys { InMemoryChannelKeys::make_holder_keys(&secp_ctx, &funding_key, &revocation_base_key, &payment_key, &delayed_payment_base_key, &htlc_base_key); - let params_1 = Readable::read(reader)?; - let params_2 = Readable::read(reader)?; + let keys_id = Readable::read(reader)?; Ok(InMemoryChannelKeys { funding_key, @@ -635,7 +631,7 @@ impl Readable for InMemoryChannelKeys { channel_value_satoshis, holder_channel_pubkeys, channel_parameters: counterparty_channel_data, - key_derivation_params: (params_1, params_2), + channel_keys_id: keys_id, }) } } @@ -731,19 +727,19 @@ impl KeysManager { /// Derive an old set of ChannelKeys for per-channel secrets based on a key derivation /// parameters. /// Key derivation parameters are accessible through a per-channel secrets - /// ChannelKeys::key_derivation_params and is provided inside DynamicOuputP2WSH in case of + /// ChannelKeys::channel_keys_id and is provided inside DynamicOuputP2WSH in case of /// onchain output detection for which a corresponding delayed_payment_key must be derived. - pub fn derive_channel_keys(&self, channel_value_satoshis: u64, params_1: u64, params_2: u64) -> InMemoryChannelKeys { - let chan_id = ((params_1 & 0xFFFF_FFFF_0000_0000) >> 32) as u32; + pub fn derive_channel_keys(&self, channel_value_satoshis: u64, params: &[u8; 32]) -> InMemoryChannelKeys { + let chan_id = byte_utils::slice_to_be64(¶ms[0..8]); + assert!(chan_id <= std::u32::MAX as u64); // Otherwise the params field wasn't created by us let mut unique_start = Sha256::engine(); - unique_start.input(&byte_utils::be64_to_array(params_2)); - unique_start.input(&byte_utils::be32_to_array(params_1 as u32)); + unique_start.input(params); unique_start.input(&self.seed); // We only seriously intend to rely on the channel_master_key for true secure // entropy, everything else just ensures uniqueness. We rely on the unique_start (ie // starting_time provided in the constructor) to be unique. - let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(chan_id).expect("key space exhausted")).expect("Your RNG is busted"); + let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(chan_id as u32).expect("key space exhausted")).expect("Your RNG is busted"); unique_start.input(&child_privkey.private_key.key[..]); let seed = Sha256::from_engine(unique_start).into_inner(); @@ -778,7 +774,7 @@ impl KeysManager { htlc_base_key, commitment_seed, channel_value_satoshis, - (params_1, params_2), + params.clone() ) } } @@ -800,8 +796,12 @@ impl KeysInterface for KeysManager { fn get_channel_keys(&self, _inbound: bool, channel_value_satoshis: u64) -> Self::ChanKeySigner { let child_ix = self.channel_child_index.fetch_add(1, Ordering::AcqRel); - let ix_and_nanos: u64 = (child_ix as u64) << 32 | (self.starting_time_nanos as u64); - self.derive_channel_keys(channel_value_satoshis, ix_and_nanos, self.starting_time_secs) + assert!(child_ix <= std::u32::MAX as usize); + let mut id = [0; 32]; + id[0..8].copy_from_slice(&byte_utils::be64_to_array(child_ix as u64)); + id[8..16].copy_from_slice(&byte_utils::be64_to_array(self.starting_time_nanos as u64)); + id[16..24].copy_from_slice(&byte_utils::be64_to_array(self.starting_time_secs)); + self.derive_channel_keys(channel_value_satoshis, &id) } fn get_secure_random_bytes(&self) -> [u8; 32] { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 08faea160..e700b1f3b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4657,7 +4657,7 @@ mod tests { // These aren't set in the test vectors: [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff], 10_000_000, - (0, 0) + [0; 32] ); assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..], diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 77b791e01..4c15a95eb 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4662,7 +4662,7 @@ macro_rules! check_spendable_outputs { Event::SpendableOutputs { ref outputs } => { for outp in outputs { match *outp { - SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref key_derivation_params } => { + SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref channel_keys_id } => { let input = TxIn { previous_output: outpoint.into_bitcoin_outpoint(), script_sig: Script::new(), @@ -4681,7 +4681,7 @@ macro_rules! check_spendable_outputs { }; spend_tx.output[0].value -= (spend_tx.get_weight() + 2 + 1 + 73 + 35 + 3) as u64 / 4; // (Max weight + 3 (to round up)) / 4 let secp_ctx = Secp256k1::new(); - let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1); + let keys = $keysinterface.derive_channel_keys($chan_value, channel_keys_id); let remotepubkey = keys.pubkeys().payment_point; let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey(); let sighash = Message::from_slice(&bip143::SigHashCache::new(&spend_tx).signature_hash(0, &witness_script, output.value, SigHashType::All)[..]).unwrap(); @@ -4691,7 +4691,7 @@ macro_rules! check_spendable_outputs { spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec()); txn.push(spend_tx); }, - SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref revocation_pubkey } => { + SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref channel_keys_id, ref revocation_pubkey } => { let input = TxIn { previous_output: outpoint.into_bitcoin_outpoint(), script_sig: Script::new(), @@ -4709,7 +4709,7 @@ macro_rules! check_spendable_outputs { output: vec![outp], }; let secp_ctx = Secp256k1::new(); - let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1); + let keys = $keysinterface.derive_channel_keys($chan_value, channel_keys_id); if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) { let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key); diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 88c1754f1..f5d531ca3 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -89,7 +89,7 @@ impl ChannelKeys for EnforcingChannelKeys { } fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() } - fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() } + fn channel_keys_id(&self) -> [u8; 32] { self.inner.channel_keys_id() } fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 4bf8aa39d..b758f5c65 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -482,8 +482,8 @@ impl TestKeysInterface { revoked_commitments: Mutex::new(HashMap::new()), } } - pub fn derive_channel_keys(&self, channel_value_satoshis: u64, user_id_1: u64, user_id_2: u64) -> EnforcingChannelKeys { - let keys = self.backing.derive_channel_keys(channel_value_satoshis, user_id_1, user_id_2); + pub fn derive_channel_keys(&self, channel_value_satoshis: u64, id: &[u8; 32]) -> EnforcingChannelKeys { + let keys = self.backing.derive_channel_keys(channel_value_satoshis, id); let revoked_commitment = self.make_revoked_commitment_cell(keys.commitment_seed); EnforcingChannelKeys::new_with_revoked(keys, revoked_commitment, self.disable_revocation_policy_check) } -- 2.39.5