diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 6c1d4348dc1..37c0e8ee840 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -193,9 +193,14 @@ impl KeysInterface for KeyProvider { ShutdownScript::new_p2wpkh(&pubkey_hash) } - fn get_channel_signer(&self, _inbound: bool, channel_value_satoshis: u64) -> EnforcingSigner { + fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] { + let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed) as u8; + [id; 32] + } + + fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer { let secp_ctx = Secp256k1::signing_only(); - let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed); + let id = channel_keys_id[0]; let keys = InMemorySigner::new( &secp_ctx, self.get_node_secret(Recipient::Node).unwrap(), @@ -204,9 +209,9 @@ 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, 6, self.node_id]).unwrap(), 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, 7, self.node_id]).unwrap(), 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 as u8, 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], + [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; 32], + channel_keys_id, ); let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed); EnforcingSigner::new_with_revoked(keys, revoked_commitment, false) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index b6292c608b5..fbe854e01c0 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -263,6 +263,7 @@ struct KeyProvider { node_secret: SecretKey, inbound_payment_key: KeyMaterial, counter: AtomicU64, + signer_state: RefCell>)>> } impl KeysInterface for KeyProvider { type Signer = EnforcingSigner; @@ -297,10 +298,17 @@ impl KeysInterface for KeyProvider { ShutdownScript::new_p2wpkh(&pubkey_hash) } - fn get_channel_signer(&self, inbound: bool, channel_value_satoshis: u64) -> EnforcingSigner { + fn generate_channel_keys_id(&self, inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] { let ctr = self.counter.fetch_add(1, Ordering::Relaxed) as u8; + self.signer_state.borrow_mut().insert(ctr, (inbound, Arc::new(Mutex::new(EnforcementState::new())))); + [ctr; 32] + } + + fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer { let secp_ctx = Secp256k1::signing_only(); - EnforcingSigner::new(if inbound { + let ctr = channel_keys_id[0]; + let (inbound, state) = self.signer_state.borrow().get(&ctr).unwrap().clone(); + EnforcingSigner::new_with_revoked(if inbound { InMemorySigner::new( &secp_ctx, self.node_secret.clone(), @@ -311,7 +319,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; 32], + channel_keys_id, ) } else { InMemorySigner::new( @@ -324,9 +332,9 @@ 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; 32], + channel_keys_id, ) - }) + }, state, false) } fn get_secure_random_bytes(&self) -> [u8; 32] { @@ -390,7 +398,12 @@ pub fn do_test(data: &[u8], logger: &Arc) { let monitor = Arc::new(chainmonitor::ChainMonitor::new(None, broadcast.clone(), Arc::clone(&logger), fee_est.clone(), Arc::new(TestPersister { update_ret: Mutex::new(ChannelMonitorUpdateStatus::Completed) }))); - let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()), counter: AtomicU64::new(0) }); + let keys_manager = Arc::new(KeyProvider { + node_secret: our_network_key.clone(), + inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()), + counter: AtomicU64::new(0), + signer_state: RefCell::new(HashMap::new()) + }); let mut config = UserConfig::default(); config.channel_config.forwarding_fee_proportional_millionths = slice_to_be32(get_slice!(4)); config.channel_handshake_config.announced_channel = get_slice!(1)[0] != 0; diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 8fa6e4231be..b1f8fc33637 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -111,7 +111,9 @@ impl KeysInterface for KeyProvider { fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { unreachable!() } - fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> EnforcingSigner { + fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] { unreachable!() } + + fn derive_channel_signer(&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32]) -> Self::Signer { unreachable!() } diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 9f81976906c..9f3cfc531e2 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -402,7 +402,7 @@ pub enum Recipient { /// A trait to describe an object which can get user secrets and key material. pub trait KeysInterface { - /// A type which implements Sign which will be returned by get_channel_signer. + /// A type which implements Sign which will be returned by derive_channel_signer. type Signer : Sign; /// Get node secret key based on the provided [`Recipient`]. @@ -445,11 +445,20 @@ pub trait KeysInterface { /// This method should return a different value each time it is called, to avoid linking /// on-chain funds across channels as controlled to the same user. fn get_shutdown_scriptpubkey(&self) -> ShutdownScript; - /// Get a new set of Sign for per-channel secrets. These MUST be unique even if you - /// restarted with some stale data! + /// Generates a unique `channel_keys_id` that can be used to obtain a `Signer` through + /// [`KeysInterface::derive_channel_signer`]. The `user_channel_id` is provided to allow + /// implementations of `KeysInterface` to maintain a mapping between it and the generated + /// `channel_keys_id`. /// /// This method must return a different value each time it is called. - fn get_channel_signer(&self, inbound: bool, channel_value_satoshis: u64) -> Self::Signer; + fn generate_channel_keys_id(&self, inbound: bool, channel_value_satoshis: u64, user_channel_id: u128) -> [u8; 32]; + /// Derives the private key material backing a `Signer`. + /// + /// To derive a new `Signer`, a fresh `channel_keys_id` should be obtained through + /// [`KeysInterface::generate_channel_keys_id`]. Otherwise, an existing `Signer` can be + /// re-derived from its `channel_keys_id`, which can be obtained through its trait method + /// [`BaseSign::channel_keys_id`]. + fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer; /// Gets a unique, cryptographically-secure, random 32 byte value. This is used for encrypting /// onion packets and for temporary channel IDs. There is no requirement that these be /// persisted anywhere, though they must be unique across restarts. @@ -463,6 +472,9 @@ pub trait KeysInterface { /// The bytes are exactly those which `::write()` writes, and /// contain no versioning scheme. You may wish to include your own version prefix and ensure /// you've read all of the provided bytes to ensure no corruption occurred. + /// + /// This method is slowly being phased out -- it will only be called when reading objects + /// written by LDK versions prior to 0.0.113. fn read_chan_signer(&self, reader: &[u8]) -> Result; /// Sign an invoice. @@ -987,13 +999,8 @@ impl KeysManager { } } /// Derive an old Sign containing per-channel secrets based on a key derivation parameters. - /// - /// Key derivation parameters are accessible through a per-channel secrets - /// Sign::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: &[u8; 32]) -> InMemorySigner { let chan_id = byte_utils::slice_to_be64(¶ms[0..8]); - assert!(chan_id <= core::u32::MAX as u64); // Otherwise the params field wasn't created by us let mut unique_start = Sha256::engine(); unique_start.input(params); unique_start.input(&self.seed); @@ -1209,14 +1216,19 @@ impl KeysInterface for KeysManager { ShutdownScript::new_p2wpkh_from_pubkey(self.shutdown_pubkey.clone()) } - fn get_channel_signer(&self, _inbound: bool, channel_value_satoshis: u64) -> Self::Signer { - let child_ix = self.channel_child_index.fetch_add(1, Ordering::AcqRel); - assert!(child_ix <= core::u32::MAX as usize); + fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, user_channel_id: u128) -> [u8; 32] { + let child_idx = self.channel_child_index.fetch_add(1, Ordering::AcqRel); + assert!(child_idx <= core::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) + id[0..4].copy_from_slice(&(child_idx as u32).to_be_bytes()); + id[4..8].copy_from_slice(&self.starting_time_nanos.to_be_bytes()); + id[8..16].copy_from_slice(&self.starting_time_secs.to_be_bytes()); + id[16..32].copy_from_slice(&user_channel_id.to_be_bytes()); + id + } + + fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer { + self.derive_channel_keys(channel_value_satoshis, &channel_keys_id) } fn get_secure_random_bytes(&self) -> [u8; 32] { @@ -1309,8 +1321,12 @@ impl KeysInterface for PhantomKeysManager { self.inner.get_shutdown_scriptpubkey() } - fn get_channel_signer(&self, inbound: bool, channel_value_satoshis: u64) -> Self::Signer { - self.inner.get_channel_signer(inbound, channel_value_satoshis) + fn generate_channel_keys_id(&self, inbound: bool, channel_value_satoshis: u64, user_channel_id: u128) -> [u8; 32] { + self.inner.generate_channel_keys_id(inbound, channel_value_satoshis, user_channel_id) + } + + fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer { + self.inner.derive_channel_signer(channel_value_satoshis, channel_keys_id) } fn get_secure_random_bytes(&self) -> [u8; 32] { diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 7fc817995fb..e03989513a4 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1627,8 +1627,8 @@ mod tests { let seed = [42; 32]; let network = Network::Testnet; let keys_provider = test_utils::TestKeysInterface::new(&seed, network); - let signer = keys_provider.get_channel_signer(false, 3000); - let counterparty_signer = keys_provider.get_channel_signer(false, 3000); + let signer = keys_provider.derive_channel_signer(3000, keys_provider.generate_channel_keys_id(false, 1_000_000, 0)); + let counterparty_signer = keys_provider.derive_channel_signer(3000, keys_provider.generate_channel_keys_id(true, 1_000_000, 1)); let delayed_payment_base = &signer.pubkeys().delayed_payment_basepoint; let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap(); let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret); @@ -1724,9 +1724,9 @@ mod tests { assert_eq!(tx.built.transaction.output[0].script_pubkey, get_htlc_redeemscript(&received_htlc, false, &keys).to_v0_p2wsh()); assert_eq!(tx.built.transaction.output[1].script_pubkey, get_htlc_redeemscript(&offered_htlc, false, &keys).to_v0_p2wsh()); assert_eq!(get_htlc_redeemscript(&received_htlc, false, &keys).to_v0_p2wsh().to_hex(), - "002085cf52e41ba7c099a39df504e7b61f6de122971ceb53b06731876eaeb85e8dc5"); + "0020e43a7c068553003fe68fcae424fb7b28ec5ce48cd8b6744b3945631389bad2fb"); assert_eq!(get_htlc_redeemscript(&offered_htlc, false, &keys).to_v0_p2wsh().to_hex(), - "002049f0736bb335c61a04d2623a24df878a7592a3c51fa7258d41b2c85318265e73"); + "0020215d61bba56b19e9eadb6107f5a85d7f99c40f65992443f69229c290165bc00d"); // Generate broadcaster output and received and offered HTLC outputs, with anchors channel_parameters.opt_anchors = Some(()); @@ -1743,9 +1743,9 @@ mod tests { assert_eq!(tx.built.transaction.output[2].script_pubkey, get_htlc_redeemscript(&received_htlc, true, &keys).to_v0_p2wsh()); assert_eq!(tx.built.transaction.output[3].script_pubkey, get_htlc_redeemscript(&offered_htlc, true, &keys).to_v0_p2wsh()); assert_eq!(get_htlc_redeemscript(&received_htlc, true, &keys).to_v0_p2wsh().to_hex(), - "002067114123af3f95405bae4fd930fc95de03e3c86baaee8b2dd29b43dd26cf613c"); + "0020b70d0649c72b38756885c7a30908d912a7898dd5d79457a7280b8e9a20f3f2bc"); assert_eq!(get_htlc_redeemscript(&offered_htlc, true, &keys).to_v0_p2wsh().to_hex(), - "0020a06e3b0d4fcf704f2b9c41e16a70099e39989466c3142b8573a1154542f28f57"); + "002087a3faeb1950a469c0e2db4a79b093a41b9526e5a6fc6ef5cb949bde3be379c7"); } #[test] diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5cc4f4a364b..cd6448d088c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -909,7 +909,8 @@ impl Channel { let opt_anchors = false; // TODO - should be based on features let holder_selected_contest_delay = config.channel_handshake_config.our_to_self_delay; - let holder_signer = keys_provider.get_channel_signer(false, channel_value_satoshis); + let channel_keys_id = keys_provider.generate_channel_keys_id(false, channel_value_satoshis, user_id); + let holder_signer = keys_provider.derive_channel_signer(channel_value_satoshis, channel_keys_id); let pubkeys = holder_signer.pubkeys().clone(); if !their_features.supports_wumbo() && channel_value_satoshis > MAX_FUNDING_SATOSHIS_NO_WUMBO { @@ -1153,7 +1154,8 @@ impl Channel { return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned())); } - let holder_signer = keys_provider.get_channel_signer(true, msg.funding_satoshis); + let channel_keys_id = keys_provider.generate_channel_keys_id(true, msg.funding_satoshis, user_id); + let holder_signer = keys_provider.derive_channel_signer(msg.funding_satoshis, channel_keys_id); let pubkeys = holder_signer.pubkeys().clone(); let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.funding_pubkey, @@ -6735,7 +6737,7 @@ mod tests { use crate::ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight}; use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget}; - use crate::chain::keysinterface::{InMemorySigner, Recipient, KeyMaterial, KeysInterface}; + use crate::chain::keysinterface::{BaseSign, InMemorySigner, Recipient, KeyMaterial, KeysInterface}; use crate::chain::transaction::OutPoint; use crate::util::config::UserConfig; use crate::util::enforcing_trait_impls::EnforcingSigner; @@ -6803,7 +6805,10 @@ mod tests { ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key)) } - fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> InMemorySigner { + fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] { + self.signer.channel_keys_id() + } + fn derive_channel_signer(&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32]) -> Self::Signer { self.signer.clone() } fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index c0d6eb2cd69..4dac879f20f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4803,7 +4803,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { assert_eq!(htlc_success_txn[2], commitment_txn[0]); assert_eq!(htlc_success_txn[3], htlc_success_txn[0]); assert_eq!(htlc_success_txn[4], htlc_success_txn[1]); - assert_ne!(htlc_success_txn[0].input[0].previous_output, htlc_timeout_tx.input[0].previous_output); + assert_ne!(htlc_success_txn[1].input[0].previous_output, htlc_timeout_tx.input[0].previous_output); mine_transaction(&nodes[1], &htlc_timeout_tx); connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1); @@ -4826,7 +4826,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { // Solve 2nd HTLC by broadcasting on B's chain HTLC-Success Tx from C // Note that the fee paid is effectively double as the HTLC value (including the nodes[1] fee // and nodes[2] fee) is rounded down and then claimed in full. - mine_transaction(&nodes[1], &htlc_success_txn[0]); + mine_transaction(&nodes[1], &htlc_success_txn[1]); expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(196*2), true, true); let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); assert!(updates.update_add_htlcs.is_empty()); diff --git a/lightning/src/util/byte_utils.rs b/lightning/src/util/byte_utils.rs index 1ab6384e3b8..eac00241db2 100644 --- a/lightning/src/util/byte_utils.rs +++ b/lightning/src/util/byte_utils.rs @@ -73,7 +73,7 @@ pub fn be64_to_array(u: u64) -> [u8; 8] { #[cfg(test)] mod tests { use super::*; - + #[test] fn test_all() { assert_eq!(slice_to_be48(&[0xde, 0xad, 0xbe, 0xef, 0x1b, 0xad]), 0xdeadbeef1bad); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 89bf27de280..17bdb06c73f 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -80,7 +80,8 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface { fn get_inbound_payment_key_material(&self) -> KeyMaterial { unreachable!(); } fn get_destination_script(&self) -> Script { unreachable!(); } fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { unreachable!(); } - fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> EnforcingSigner { unreachable!(); } + fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] { unreachable!(); } + fn derive_channel_signer(&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32]) -> Self::Signer { unreachable!(); } fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] } fn read_chan_signer(&self, mut reader: &[u8]) -> Result { @@ -629,8 +630,12 @@ impl keysinterface::KeysInterface for TestKeysInterface { } } - fn get_channel_signer(&self, inbound: bool, channel_value_satoshis: u64) -> EnforcingSigner { - let keys = self.backing.get_channel_signer(inbound, channel_value_satoshis); + fn generate_channel_keys_id(&self, inbound: bool, channel_value_satoshis: u64, user_channel_id: u128) -> [u8; 32] { + self.backing.generate_channel_keys_id(inbound, channel_value_satoshis, user_channel_id) + } + + fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> EnforcingSigner { + let keys = self.backing.derive_channel_signer(channel_value_satoshis, channel_keys_id); let state = self.make_enforcement_state_cell(keys.commitment_seed); EnforcingSigner::new_with_revoked(keys, state, self.disable_revocation_policy_check) }