From 6dd1ec1feda39ee87f0b2cff7168751d827e0a43 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 29 Nov 2021 12:50:47 -0500 Subject: [PATCH 1/5] Add get_inbound_payment_key_material to KeysInterface This will allow us to retrieve key material for encrypting/decrypting inbound payment info, in upcoming commits --- fuzz/src/chanmon_consistency.rs | 6 +++++- fuzz/src/full_stack.rs | 12 ++++++++++-- lightning/src/chain/keysinterface.rs | 20 ++++++++++++++++++++ lightning/src/ln/channel.rs | 3 ++- lightning/src/util/test_utils.rs | 4 +++- 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index b1b8dfebbf5..f41b17a1430 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -34,7 +34,7 @@ use lightning::chain::{BestBlock, ChannelMonitorUpdateErr, chainmonitor, channel use lightning::chain::channelmonitor::{ChannelMonitor, MonitorEvent}; use lightning::chain::transaction::OutPoint; use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; -use lightning::chain::keysinterface::{KeysInterface, InMemorySigner}; +use lightning::chain::keysinterface::{KeyMaterial, KeysInterface, InMemorySigner}; use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use lightning::ln::channelmanager::{ChainParameters, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs}; use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; @@ -164,6 +164,10 @@ 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, 1, self.node_id]).unwrap() } + fn get_inbound_payment_key_material(&self) -> KeyMaterial { + KeyMaterial([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, 1, self.node_id]) + } + fn get_destination_script(&self) -> Script { let secp_ctx = Secp256k1::signing_only(); let channel_monitor_claim_key = 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, 2, self.node_id]).unwrap(); diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index e77410453e5..34c6f554c18 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -31,7 +31,7 @@ use lightning::chain::{BestBlock, Confirm, Listen}; use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; use lightning::chain::chainmonitor; use lightning::chain::transaction::OutPoint; -use lightning::chain::keysinterface::{InMemorySigner, KeysInterface}; +use lightning::chain::keysinterface::{InMemorySigner, KeyMaterial, KeysInterface}; use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use lightning::ln::channelmanager::{ChainParameters, ChannelManager}; use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,IgnoringMessageHandler}; @@ -56,6 +56,7 @@ use bitcoin::secp256k1::Secp256k1; use std::cell::RefCell; use std::collections::{HashMap, hash_map}; +use std::convert::TryInto; use std::cmp; use std::sync::{Arc, Mutex}; use std::sync::atomic::{AtomicU64,AtomicUsize,Ordering}; @@ -257,6 +258,7 @@ impl<'a> Drop for MoneyLossDetector<'a> { struct KeyProvider { node_secret: SecretKey, + inbound_payment_key: KeyMaterial, counter: AtomicU64, } impl KeysInterface for KeyProvider { @@ -266,6 +268,10 @@ impl KeysInterface for KeyProvider { self.node_secret.clone() } + fn get_inbound_payment_key_material(&self) -> KeyMaterial { + self.inbound_payment_key.clone() + } + fn get_destination_script(&self) -> Script { let secp_ctx = Secp256k1::signing_only(); let channel_monitor_claim_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(); @@ -365,11 +371,13 @@ pub fn do_test(data: &[u8], logger: &Arc) { Err(_) => return, }; + let inbound_payment_key = [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, 0, 42]; + let broadcast = Arc::new(TestBroadcaster{ txn_broadcasted: Mutex::new(Vec::new()) }); let monitor = Arc::new(chainmonitor::ChainMonitor::new(None, broadcast.clone(), Arc::clone(&logger), fee_est.clone(), Arc::new(TestPersister { update_ret: Mutex::new(Ok(())) }))); - let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), 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) }); let mut config = UserConfig::default(); config.channel_options.forwarding_fee_proportional_millionths = slice_to_be32(get_slice!(4)); config.channel_options.announced_channel = get_slice!(1)[0] != 0; diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 7be26634916..9e0cfb06e61 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -43,6 +43,12 @@ use core::sync::atomic::{AtomicUsize, Ordering}; use io::{self, Error}; use ln::msgs::{DecodeError, MAX_VALUE_MSAT}; +/// Used as initial key material, to be expanded into multiple secret keys (but not to be used +/// directly). This is used within LDK to encrypt/decrypt inbound payment data. +/// (C-not exported) as we just use [u8; 32] directly +#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] +pub struct KeyMaterial(pub [u8; 32]); + /// Information about a spendable output to a P2WSH script. See /// SpendableOutputDescriptor::DelayedPaymentOutput for more details on how to spend this. #[derive(Clone, Debug, PartialEq)] @@ -397,6 +403,11 @@ pub trait KeysInterface { /// this trait to parse the invoice and make sure they're signing what they expect, rather than /// blindly signing the hash. fn sign_invoice(&self, invoice_preimage: Vec) -> Result; + + /// Get secret key material as bytes for use in encrypting and decrypting inbound payment data. + /// + /// This method must return the same value each time it is called. + fn get_inbound_payment_key_material(&self) -> KeyMaterial; } #[derive(Clone)] @@ -766,6 +777,7 @@ impl Readable for InMemorySigner { pub struct KeysManager { secp_ctx: Secp256k1, node_secret: SecretKey, + inbound_payment_key: KeyMaterial, destination_script: Script, shutdown_pubkey: PublicKey, channel_master_key: ExtendedPrivKey, @@ -821,6 +833,9 @@ impl KeysManager { }; let channel_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(3).unwrap()).expect("Your RNG is busted"); let rand_bytes_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(4).unwrap()).expect("Your RNG is busted"); + let inbound_payment_key: SecretKey = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(5).unwrap()).expect("Your RNG is busted").private_key.key; + let mut inbound_pmt_key_bytes = [0; 32]; + inbound_pmt_key_bytes.copy_from_slice(&inbound_payment_key[..]); let mut rand_bytes_unique_start = Sha256::engine(); rand_bytes_unique_start.input(&byte_utils::be64_to_array(starting_time_secs)); @@ -830,6 +845,7 @@ impl KeysManager { let mut res = KeysManager { secp_ctx, node_secret, + inbound_payment_key: KeyMaterial(inbound_pmt_key_bytes), destination_script, shutdown_pubkey, @@ -1038,6 +1054,10 @@ impl KeysInterface for KeysManager { self.node_secret.clone() } + fn get_inbound_payment_key_material(&self) -> KeyMaterial { + self.inbound_payment_key.clone() + } + fn get_destination_script(&self) -> Script { self.destination_script.clone() } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0cdcb0c14c2..ca6c6781fde 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5816,7 +5816,7 @@ mod tests { use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT}; use chain::BestBlock; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; - use chain::keysinterface::{InMemorySigner, KeysInterface, BaseSign}; + use chain::keysinterface::{InMemorySigner, KeyMaterial, KeysInterface, BaseSign}; use chain::transaction::OutPoint; use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; @@ -5857,6 +5857,7 @@ mod tests { type Signer = InMemorySigner; fn get_node_secret(&self) -> SecretKey { panic!(); } + fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); } fn get_destination_script(&self) -> Script { let secp_ctx = Secp256k1::signing_only(); let channel_monitor_claim_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 6442c9cfa27..01c637b41ee 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -47,7 +47,7 @@ use core::time::Duration; use sync::{Mutex, Arc}; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use core::{cmp, mem}; -use chain::keysinterface::InMemorySigner; +use chain::keysinterface::{InMemorySigner, KeyMaterial}; pub struct TestVecWriter(pub Vec); impl Writer for TestVecWriter { @@ -71,6 +71,7 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface { type Signer = EnforcingSigner; fn get_node_secret(&self) -> SecretKey { unreachable!(); } + 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!(); } @@ -479,6 +480,7 @@ impl keysinterface::KeysInterface for TestKeysInterface { type Signer = EnforcingSigner; fn get_node_secret(&self) -> SecretKey { self.backing.get_node_secret() } + fn get_inbound_payment_key_material(&self) -> keysinterface::KeyMaterial { self.backing.get_inbound_payment_key_material() } fn get_destination_script(&self) -> Script { self.backing.get_destination_script() } fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { From 063b7583c13e17e339fc9f3083ca3d54e2d2d050 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 22 Nov 2021 16:53:18 -0500 Subject: [PATCH 2/5] Macro-ize checking that the total value of an MPP's parts is sane This DRY-ed code will be used in upcoming commits when we stop storing inbound payment data --- lightning/src/ln/channelmanager.rs | 102 ++++++++++++++++------------- 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1b0fa19e163..b9ee78499bc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2913,6 +2913,59 @@ impl ChannelMana } } + macro_rules! check_total_value { + ($payment_data_total_msat: expr, $payment_secret: expr, $payment_preimage: expr) => {{ + let mut total_value = 0; + let mut payment_received_generated = false; + let htlcs = channel_state.claimable_htlcs.entry(payment_hash) + .or_insert(Vec::new()); + if htlcs.len() == 1 { + if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { + log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0)); + fail_htlc!(claimable_htlc); + continue + } + } + htlcs.push(claimable_htlc); + for htlc in htlcs.iter() { + total_value += htlc.value; + match &htlc.onion_payload { + OnionPayload::Invoice(htlc_payment_data) => { + if htlc_payment_data.total_msat != $payment_data_total_msat { + log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})", + log_bytes!(payment_hash.0), $payment_data_total_msat, htlc_payment_data.total_msat); + total_value = msgs::MAX_VALUE_MSAT; + } + if total_value >= msgs::MAX_VALUE_MSAT { break; } + }, + _ => unreachable!(), + } + } + if total_value >= msgs::MAX_VALUE_MSAT || total_value > $payment_data_total_msat { + log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the total value {} ran over expected value {} (or HTLCs were inconsistent)", + log_bytes!(payment_hash.0), total_value, $payment_data_total_msat); + for htlc in htlcs.iter() { + fail_htlc!(htlc); + } + } else if total_value == $payment_data_total_msat { + new_events.push(events::Event::PaymentReceived { + payment_hash, + purpose: events::PaymentPurpose::InvoicePayment { + payment_preimage: $payment_preimage, + payment_secret: $payment_secret, + }, + amt: total_value, + }); + payment_received_generated = true; + } else { + // Nothing to do - we haven't reached the total + // payment value yet, wait until we receive more + // MPP parts. + } + payment_received_generated + }} + } + // Check that the payment hash and secret are known. Note that we // MUST take care to handle the "unknown payment hash" and // "incorrect payment secret" cases here identically or we'd expose @@ -2962,54 +3015,9 @@ impl ChannelMana log_bytes!(payment_hash.0), payment_data.total_msat, inbound_payment.get().min_value_msat.unwrap()); fail_htlc!(claimable_htlc); } else { - let mut total_value = 0; - let htlcs = channel_state.claimable_htlcs.entry(payment_hash) - .or_insert(Vec::new()); - if htlcs.len() == 1 { - if let OnionPayload::Spontaneous(_) = htlcs[0].onion_payload { - log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we already had an existing keysend HTLC with the same payment hash", log_bytes!(payment_hash.0)); - fail_htlc!(claimable_htlc); - continue - } - } - htlcs.push(claimable_htlc); - for htlc in htlcs.iter() { - total_value += htlc.value; - match &htlc.onion_payload { - OnionPayload::Invoice(htlc_payment_data) => { - if htlc_payment_data.total_msat != payment_data.total_msat { - log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the HTLCs had inconsistent total values (eg {} and {})", - log_bytes!(payment_hash.0), payment_data.total_msat, htlc_payment_data.total_msat); - total_value = msgs::MAX_VALUE_MSAT; - } - if total_value >= msgs::MAX_VALUE_MSAT { break; } - }, - _ => unreachable!(), - } - } - if total_value >= msgs::MAX_VALUE_MSAT || total_value > payment_data.total_msat { - log_trace!(self.logger, "Failing HTLCs with payment_hash {} as the total value {} ran over expected value {} (or HTLCs were inconsistent)", - log_bytes!(payment_hash.0), total_value, payment_data.total_msat); - for htlc in htlcs.iter() { - fail_htlc!(htlc); - } - } else if total_value == payment_data.total_msat { - new_events.push(events::Event::PaymentReceived { - payment_hash, - purpose: events::PaymentPurpose::InvoicePayment { - payment_preimage: inbound_payment.get().payment_preimage, - payment_secret: payment_data.payment_secret, - }, - amt: total_value, - }); - // Only ever generate at most one PaymentReceived - // per registered payment_hash, even if it isn't - // claimed. + let payment_received_generated = check_total_value!(payment_data.total_msat, payment_data.payment_secret, inbound_payment.get().payment_preimage); + if payment_received_generated { inbound_payment.remove_entry(); - } else { - // Nothing to do - we haven't reached the total - // payment value yet, wait until we receive more - // MPP parts. } } }, From 1d516a6fc505783ce752f0289322ce73ae10bcf4 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 29 Nov 2021 19:36:59 -0500 Subject: [PATCH 3/5] Add get_single_block to chacha20 module In the next commit, we'll want to get a single block from a chacha stream that takes a 16-byte nonce. --- lightning/src/util/chacha20.rs | 46 ++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/lightning/src/util/chacha20.rs b/lightning/src/util/chacha20.rs index 508ecd635c4..85c0a47d4a5 100644 --- a/lightning/src/util/chacha20.rs +++ b/lightning/src/util/chacha20.rs @@ -151,6 +151,14 @@ mod real_chacha { ChaCha20{ state: ChaCha20::expand(key, nonce), output: [0u8; BLOCK_SIZE], offset: 64 } } + /// Get one block from a ChaCha stream. + pub fn get_single_block(key: &[u8; 32], nonce: &[u8; 16]) -> [u8; 32] { + let mut chacha = ChaCha20 { state: ChaCha20::expand(key, nonce), output: [0u8; BLOCK_SIZE], offset: 64 }; + let mut chacha_bytes = [0; 32]; + chacha.process_in_place(&mut chacha_bytes); + chacha_bytes + } + fn expand(key: &[u8], nonce: &[u8]) -> ChaChaState { let constant = match key.len() { 16 => b"expand 16-byte k", @@ -256,6 +264,12 @@ mod real_chacha { self.offset += count; } } + + #[cfg(test)] + pub fn seek_to_block(&mut self, block_offset: u32) { + self.state.d.0 = block_offset; + self.update(); + } } } #[cfg(not(feature = "fuzztarget"))] @@ -272,6 +286,10 @@ mod fuzzy_chacha { Self {} } + pub fn get_single_block(_key: &[u8; 32], _nonce: &[u8; 16]) -> [u8; 32] { + [0; 32] + } + pub fn process(&mut self, input: &[u8], output: &mut [u8]) { output.copy_from_slice(input); } @@ -302,6 +320,7 @@ mod test { use core::iter::repeat; use super::ChaCha20; + use std::convert::TryInto; #[test] fn test_chacha20_256_tls_vectors() { @@ -572,4 +591,31 @@ mod test { assert_eq!(output, tv.keystream); } } + + #[test] + fn get_single_block() { + // Test that `get_single_block` (which takes a 16-byte nonce) is equivalent to getting a block + // using a 12-byte nonce, with the block starting at the counter offset given by the remaining 4 + // bytes. + let key = [ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, + ]; + let nonce_16bytes = [ + 0x00, 0x01, 0x02, 0x03, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b + ]; + let counter_pos = &nonce_16bytes[..4]; + let nonce_12bytes = &nonce_16bytes[4..]; + + // Initialize a ChaCha20 instance with its counter starting at 0. + let mut chacha20 = ChaCha20::new(&key, nonce_12bytes); + // Seek its counter to the block at counter_pos. + chacha20.seek_to_block(u32::from_le_bytes(counter_pos.try_into().unwrap())); + let mut block_bytes = [0; 32]; + chacha20.process_in_place(&mut block_bytes); + + assert_eq!(ChaCha20::get_single_block(&key, &nonce_16bytes), block_bytes); + } } From 846487555556d8465c5b7b811f976e78f265c48f Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 29 Nov 2021 19:59:18 -0500 Subject: [PATCH 4/5] Drop need to store pending inbound payments and replace payment_secret with encrypted metadata See docs on `inbound_payment::verify` for details Also add min_value checks to all create_inbound_payment* methods --- lightning-invoice/src/utils.rs | 2 +- lightning/src/ln/channelmanager.rs | 379 +++++++++++++++++++++++++-- lightning/src/ln/functional_tests.rs | 16 +- lightning/src/util/events.rs | 7 +- 4 files changed, 376 insertions(+), 28 deletions(-) diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 1aa3a8923d2..005be24c3b7 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -63,7 +63,7 @@ where let (payment_hash, payment_secret) = channelmanager.create_inbound_payment( amt_msat, DEFAULT_EXPIRY_TIME.try_into().unwrap(), - ); + ).unwrap(); let our_node_pubkey = channelmanager.get_our_node_id(); let mut invoice = InvoiceBuilder::new(network) .description(description) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b9ee78499bc..1bbd4a1bac7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -49,14 +49,14 @@ use routing::router::{Payee, Route, RouteHop, RoutePath, RouteParameters}; use ln::msgs; use ln::msgs::NetAddress; use ln::onion_utils; -use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, OptionalField}; +use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT, OptionalField}; use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner}; use util::config::UserConfig; use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use util::{byte_utils, events}; use util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer}; use util::chacha20::{ChaCha20, ChaChaReader}; -use util::logger::{Logger, Level}; +use util::logger::{Level, Logger}; use util::errors::APIError; use io; @@ -72,6 +72,258 @@ use core::ops::Deref; #[cfg(any(test, feature = "std"))] use std::time::Instant; +mod inbound_payment { + use bitcoin::hashes::{Hash, HashEngine}; + use bitcoin::hashes::cmp::fixed_time_eq; + use bitcoin::hashes::hmac::{Hmac, HmacEngine}; + use bitcoin::hashes::sha256::Hash as Sha256; + use chain::keysinterface::{KeyMaterial, KeysInterface, Sign}; + use ln::{PaymentHash, PaymentPreimage, PaymentSecret}; + use ln::msgs; + use ln::msgs::MAX_VALUE_MSAT; + use util::chacha20::ChaCha20; + use util::logger::Logger; + + use core::convert::TryInto; + use core::ops::Deref; + + const IV_LEN: usize = 16; + const METADATA_LEN: usize = 16; + const METADATA_KEY_LEN: usize = 32; + const AMT_MSAT_LEN: usize = 8; + // Used to shift the payment type bits to take up the top 3 bits of the metadata bytes, or to + // retrieve said payment type bits. + const METHOD_TYPE_OFFSET: usize = 5; + + /// A set of keys that were HKDF-expanded from an initial call to + /// [`KeysInterface::get_inbound_payment_key_material`]. + /// + /// [`KeysInterface::get_inbound_payment_key_material`]: crate::chain::keysinterface::KeysInterface::get_inbound_payment_key_material + pub(super) struct ExpandedKey { + /// The key used to encrypt the bytes containing the payment metadata (i.e. the amount and + /// expiry, included for payment verification on decryption). + metadata_key: [u8; 32], + /// The key used to authenticate an LDK-provided payment hash and metadata as previously + /// registered with LDK. + ldk_pmt_hash_key: [u8; 32], + /// The key used to authenticate a user-provided payment hash and metadata as previously + /// registered with LDK. + user_pmt_hash_key: [u8; 32], + } + + impl ExpandedKey { + pub(super) fn new(key_material: &KeyMaterial) -> ExpandedKey { + hkdf_extract_expand(&vec![0], &key_material) + } + } + + enum Method { + LdkPaymentHash = 0, + UserPaymentHash = 1, + } + + impl Method { + fn from_bits(bits: u8) -> Result { + match bits { + bits if bits == Method::LdkPaymentHash as u8 => Ok(Method::LdkPaymentHash), + bits if bits == Method::UserPaymentHash as u8 => Ok(Method::UserPaymentHash), + unknown => Err(unknown), + } + } + } + + pub(super) fn create(keys: &ExpandedKey, min_value_msat: Option, invoice_expiry_delta_secs: u32, keys_manager: &K, highest_seen_timestamp: u64) -> Result<(PaymentHash, PaymentSecret), ()> + where K::Target: KeysInterface + { + let metadata_bytes = construct_metadata_bytes(min_value_msat, Method::LdkPaymentHash, invoice_expiry_delta_secs, highest_seen_timestamp)?; + + let mut iv_bytes = [0 as u8; IV_LEN]; + let rand_bytes = keys_manager.get_secure_random_bytes(); + iv_bytes.copy_from_slice(&rand_bytes[..IV_LEN]); + + let mut hmac = HmacEngine::::new(&keys.ldk_pmt_hash_key); + hmac.input(&iv_bytes); + hmac.input(&metadata_bytes); + let payment_preimage_bytes = Hmac::from_engine(hmac).into_inner(); + + let ldk_pmt_hash = PaymentHash(Sha256::hash(&payment_preimage_bytes).into_inner()); + let payment_secret = construct_payment_secret(&iv_bytes, &metadata_bytes, &keys.metadata_key); + Ok((ldk_pmt_hash, payment_secret)) + } + + pub(super) fn create_from_hash(keys: &ExpandedKey, min_value_msat: Option, payment_hash: PaymentHash, invoice_expiry_delta_secs: u32, highest_seen_timestamp: u64) -> Result { + let metadata_bytes = construct_metadata_bytes(min_value_msat, Method::UserPaymentHash, invoice_expiry_delta_secs, highest_seen_timestamp)?; + + let mut hmac = HmacEngine::::new(&keys.user_pmt_hash_key); + hmac.input(&metadata_bytes); + hmac.input(&payment_hash.0); + let hmac_bytes = Hmac::from_engine(hmac).into_inner(); + + let mut iv_bytes = [0 as u8; IV_LEN]; + iv_bytes.copy_from_slice(&hmac_bytes[..IV_LEN]); + + Ok(construct_payment_secret(&iv_bytes, &metadata_bytes, &keys.metadata_key)) + } + + fn construct_metadata_bytes(min_value_msat: Option, payment_type: Method, invoice_expiry_delta_secs: u32, highest_seen_timestamp: u64) -> Result<[u8; METADATA_LEN], ()> { + if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT { + return Err(()); + } + + let mut min_amt_msat_bytes: [u8; AMT_MSAT_LEN] = match min_value_msat { + Some(amt) => amt.to_be_bytes(), + None => [0; AMT_MSAT_LEN], + }; + min_amt_msat_bytes[0] |= (payment_type as u8) << METHOD_TYPE_OFFSET; + + // We assume that highest_seen_timestamp is pretty close to the current time - it's updated when + // we receive a new block with the maximum time we've seen in a header. It should never be more + // than two hours in the future. Thus, we add two hours here as a buffer to ensure we + // absolutely never fail a payment too early. + // Note that we assume that received blocks have reasonably up-to-date timestamps. + let expiry_bytes = (highest_seen_timestamp + invoice_expiry_delta_secs as u64 + 7200).to_be_bytes(); + + let mut metadata_bytes: [u8; METADATA_LEN] = [0; METADATA_LEN]; + metadata_bytes[..AMT_MSAT_LEN].copy_from_slice(&min_amt_msat_bytes); + metadata_bytes[AMT_MSAT_LEN..].copy_from_slice(&expiry_bytes); + + Ok(metadata_bytes) + } + + fn construct_payment_secret(iv_bytes: &[u8; IV_LEN], metadata_bytes: &[u8; METADATA_LEN], metadata_key: &[u8; METADATA_KEY_LEN]) -> PaymentSecret { + let mut payment_secret_bytes: [u8; 32] = [0; 32]; + let (iv_slice, encrypted_metadata_slice) = payment_secret_bytes.split_at_mut(IV_LEN); + iv_slice.copy_from_slice(iv_bytes); + + let chacha_block = ChaCha20::get_single_block(metadata_key, iv_bytes); + for i in 0..METADATA_LEN { + encrypted_metadata_slice[i] = chacha_block[i] ^ metadata_bytes[i]; + } + PaymentSecret(payment_secret_bytes) + } + + /// Check that an inbound payment's `payment_data` field is sane. + /// + /// LDK does not store any data for pending inbound payments. Instead, we construct our payment + /// secret (and, if supplied by LDK, our payment preimage) to include encrypted metadata about the + /// payment. + /// + /// The metadata is constructed as: + /// payment method (3 bits) || payment amount (8 bytes - 3 bits) || expiry (8 bytes) + /// and encrypted using a key derived from [`KeysInterface::get_inbound_payment_key_material`]. + /// + /// Then on payment receipt, we verify in this method that the payment preimage and payment secret + /// match what was constructed. + /// + /// [`create_inbound_payment`] and [`create_inbound_payment_for_hash`] are called by the user to + /// construct the payment secret and/or payment hash that this method is verifying. If the former + /// method is called, then the payment method bits mentioned above are represented internally as + /// [`Method::LdkPaymentHash`]. If the latter, [`Method::UserPaymentHash`]. + /// + /// For the former method, the payment preimage is constructed as an HMAC of payment metadata and + /// random bytes. Because the payment secret is also encoded with these random bytes and metadata + /// (with the metadata encrypted with a block cipher), we're able to authenticate the preimage on + /// payment receipt. + /// + /// For the latter, the payment secret instead contains an HMAC of the user-provided payment hash + /// and payment metadata (encrypted with a block cipher), allowing us to authenticate the payment + /// hash and metadata on payment receipt. + /// + /// See [`ExpandedKey`] docs for more info on the individual keys used. + /// + /// [`KeysInterface::get_inbound_payment_key_material`]: crate::chain::keysinterface::KeysInterface::get_inbound_payment_key_material + /// [`create_inbound_payment`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment + /// [`create_inbound_payment_for_hash`]: crate::ln::channelmanager::ChannelManager::create_inbound_payment_for_hash + pub(super) fn verify(payment_hash: PaymentHash, payment_data: msgs::FinalOnionHopData, highest_seen_timestamp: u64, keys: &ExpandedKey, logger: &L) -> Result, ()> + where L::Target: Logger + { + let mut iv_bytes = [0; IV_LEN]; + let (iv_slice, encrypted_metadata_bytes) = payment_data.payment_secret.0.split_at(IV_LEN); + iv_bytes.copy_from_slice(iv_slice); + + let chacha_block = ChaCha20::get_single_block(&keys.metadata_key, &iv_bytes); + let mut metadata_bytes: [u8; METADATA_LEN] = [0; METADATA_LEN]; + for i in 0..METADATA_LEN { + metadata_bytes[i] = chacha_block[i] ^ encrypted_metadata_bytes[i]; + } + + let payment_type_res = Method::from_bits((metadata_bytes[0] & 0b1110_0000) >> METHOD_TYPE_OFFSET); + let mut amt_msat_bytes = [0; AMT_MSAT_LEN]; + amt_msat_bytes.copy_from_slice(&metadata_bytes[..AMT_MSAT_LEN]); + // Zero out the bits reserved to indicate the payment type. + amt_msat_bytes[0] &= 0b00011111; + let min_amt_msat: u64 = u64::from_be_bytes(amt_msat_bytes.into()); + let expiry = u64::from_be_bytes(metadata_bytes[AMT_MSAT_LEN..].try_into().unwrap()); + + // Make sure to check to check the HMAC before doing the other checks below, to mitigate timing + // attacks. + let mut payment_preimage = None; + match payment_type_res { + Ok(Method::UserPaymentHash) => { + let mut hmac = HmacEngine::::new(&keys.user_pmt_hash_key); + hmac.input(&metadata_bytes[..]); + hmac.input(&payment_hash.0); + if !fixed_time_eq(&iv_bytes, &Hmac::from_engine(hmac).into_inner().split_at_mut(IV_LEN).0) { + log_trace!(logger, "Failing HTLC with user-generated payment_hash {}: unexpected payment_secret", log_bytes!(payment_hash.0)); + return Err(()) + } + }, + Ok(Method::LdkPaymentHash) => { + let mut hmac = HmacEngine::::new(&keys.ldk_pmt_hash_key); + hmac.input(&iv_bytes); + hmac.input(&metadata_bytes); + let decoded_payment_preimage = Hmac::from_engine(hmac).into_inner(); + if !fixed_time_eq(&payment_hash.0, &Sha256::hash(&decoded_payment_preimage).into_inner()) { + log_trace!(logger, "Failing HTLC with payment_hash {}: payment preimage {} did not match", log_bytes!(payment_hash.0), log_bytes!(decoded_payment_preimage)); + return Err(()) + } + payment_preimage = Some(PaymentPreimage(decoded_payment_preimage)); + }, + Err(unknown_bits) => { + log_trace!(logger, "Failing HTLC with payment hash {} due to unknown payment type {}", log_bytes!(payment_hash.0), unknown_bits); + return Err(()); + } + } + + if payment_data.total_msat < min_amt_msat { + log_trace!(logger, "Failing HTLC with payment_hash {} due to total_msat {} being less than the minimum amount of {} msat", log_bytes!(payment_hash.0), payment_data.total_msat, min_amt_msat); + return Err(()) + } + + if expiry < highest_seen_timestamp { + log_trace!(logger, "Failing HTLC with payment_hash {}: expired payment", log_bytes!(payment_hash.0)); + return Err(()) + } + + Ok(payment_preimage) + } + + fn hkdf_extract_expand(salt: &[u8], ikm: &KeyMaterial) -> ExpandedKey { + let mut hmac = HmacEngine::::new(salt); + hmac.input(&ikm.0); + let prk = Hmac::from_engine(hmac).into_inner(); + let mut hmac = HmacEngine::::new(&prk[..]); + hmac.input(&[1; 1]); + let metadata_key = Hmac::from_engine(hmac).into_inner(); + + let mut hmac = HmacEngine::::new(&prk[..]); + hmac.input(&metadata_key); + hmac.input(&[2; 1]); + let ldk_pmt_hash_key = Hmac::from_engine(hmac).into_inner(); + + let mut hmac = HmacEngine::::new(&prk[..]); + hmac.input(&ldk_pmt_hash_key); + hmac.input(&[3; 1]); + let user_pmt_hash_key = Hmac::from_engine(hmac).into_inner(); + + ExpandedKey { + metadata_key, + ldk_pmt_hash_key, + user_pmt_hash_key, + } + } +} + // We hold various information about HTLC relay in the HTLC objects in Channel itself: // // Upon receipt of an HTLC from a peer, we'll give it a PendingHTLCStatus indicating if it should @@ -692,6 +944,8 @@ pub struct ChannelManager ChannelMana pub fn new(fee_est: F, chain_monitor: M, tx_broadcaster: T, logger: L, keys_manager: K, config: UserConfig, params: ChainParameters) -> Self { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes()); - + let inbound_pmt_key_material = keys_manager.get_inbound_payment_key_material(); + let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material); ChannelManager { default_configuration: config.clone(), genesis_hash: genesis_block(params.network).header.block_hash(), @@ -1409,6 +1664,8 @@ impl ChannelMana our_network_pubkey: PublicKey::from_secret_key(&secp_ctx, &keys_manager.get_node_secret()), secp_ctx, + inbound_payment_key: expanded_inbound_key, + last_node_announcement_serial: AtomicUsize::new(0), highest_seen_timestamp: AtomicUsize::new(0), @@ -2976,9 +3233,17 @@ impl ChannelMana match payment_secrets.entry(payment_hash) { hash_map::Entry::Vacant(_) => { match claimable_htlc.onion_payload { - OnionPayload::Invoice(_) => { - log_trace!(self.logger, "Failing new HTLC with payment_hash {} as we didn't have a corresponding inbound payment.", log_bytes!(payment_hash.0)); - fail_htlc!(claimable_htlc); + OnionPayload::Invoice(ref payment_data) => { + let payment_preimage = match inbound_payment::verify(payment_hash, payment_data.clone(), self.highest_seen_timestamp.load(Ordering::Acquire) as u64, &self.inbound_payment_key, &self.logger) { + Ok(payment_preimage) => payment_preimage, + Err(()) => { + fail_htlc!(claimable_htlc); + continue + } + }; + let payment_data_total_msat = payment_data.total_msat; + let payment_secret = payment_data.payment_secret.clone(); + check_total_value!(payment_data_total_msat, payment_secret, payment_preimage); }, OnionPayload::Spontaneous(preimage) => { match channel_state.claimable_htlcs.entry(payment_hash) { @@ -4681,6 +4946,10 @@ impl ChannelMana fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result { assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106 + if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT { + return Err(APIError::APIMisuseError { err: format!("min_value_msat of {} greater than total 21 million bitcoin supply", min_value_msat.unwrap()) }); + } + let payment_secret = PaymentSecret(self.keys_manager.get_secure_random_bytes()); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); @@ -4691,7 +4960,7 @@ impl ChannelMana payment_secret, min_value_msat, payment_preimage, user_payment_id: 0, // For compatibility with version 0.0.103 and earlier // We assume that highest_seen_timestamp is pretty close to the current time - - // its updated when we receive a new block with the maximum time we've seen in + // it's updated when we receive a new block with the maximum time we've seen in // a header. It should never be more than two hours in the future. // Thus, we add two hours here as a buffer to ensure we absolutely // never fail a payment too early. @@ -4709,7 +4978,7 @@ impl ChannelMana /// to pay us. /// /// This differs from [`create_inbound_payment_for_hash`] only in that it generates the - /// [`PaymentHash`] and [`PaymentPreimage`] for you, returning the first and storing the second. + /// [`PaymentHash`] and [`PaymentPreimage`] for you. /// /// The [`PaymentPreimage`] will ultimately be returned to you in the [`PaymentReceived`], which /// will have the [`PaymentReceived::payment_preimage`] field filled in. That should then be @@ -4717,17 +4986,37 @@ impl ChannelMana /// /// See [`create_inbound_payment_for_hash`] for detailed documentation on behavior and requirements. /// + /// Note that a malicious eavesdropper can intuit whether an inbound payment was created by + /// `create_inbound_payment` or `create_inbound_payment_for_hash` based on runtime. + /// + /// # Note + /// + /// If you register an inbound payment with this method, then serialize the `ChannelManager`, then + /// deserialize it with a node running 0.0.103 and earlier, the payment will fail to be received. + /// + /// Errors if `min_value_msat` is greater than total bitcoin supply. + /// /// [`claim_funds`]: Self::claim_funds /// [`PaymentReceived`]: events::Event::PaymentReceived /// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash - pub fn create_inbound_payment(&self, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) { + pub fn create_inbound_payment(&self, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result<(PaymentHash, PaymentSecret), ()> { + inbound_payment::create(&self.inbound_payment_key, min_value_msat, invoice_expiry_delta_secs, &self.keys_manager, self.highest_seen_timestamp.load(Ordering::Acquire) as u64) + } + + /// Legacy version of [`create_inbound_payment`]. Use this method if you wish to share + /// serialized state with LDK node(s) running 0.0.103 and earlier. + /// + /// # Note + /// This method is deprecated and will be removed soon. + /// + /// [`create_inbound_payment`]: Self::create_inbound_payment + #[deprecated] + pub fn create_inbound_payment_legacy(&self, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result<(PaymentHash, PaymentSecret), APIError> { let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes()); let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()); - - (payment_hash, - self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs) - .expect("RNG Generated Duplicate PaymentHash")) + let payment_secret = self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)?; + Ok((payment_hash, payment_secret)) } /// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is @@ -4757,18 +5046,36 @@ impl ChannelMana /// If you need exact expiry semantics, you should enforce them upon receipt of /// [`PaymentReceived`]. /// - /// Pending inbound payments are stored in memory and in serialized versions of this - /// [`ChannelManager`]. If potentially unbounded numbers of inbound payments may exist and - /// space is limited, you may wish to rate-limit inbound payment creation. - /// /// May panic if `invoice_expiry_delta_secs` is greater than one year. /// /// Note that invoices generated for inbound payments should have their `min_final_cltv_expiry` /// set to at least [`MIN_FINAL_CLTV_EXPIRY`]. /// + /// Note that a malicious eavesdropper can intuit whether an inbound payment was created by + /// `create_inbound_payment` or `create_inbound_payment_for_hash` based on runtime. + /// + /// # Note + /// + /// If you register an inbound payment with this method, then serialize the `ChannelManager`, then + /// deserialize it with a node running 0.0.103 and earlier, the payment will fail to be received. + /// + /// Errors if `min_value_msat` is greater than total bitcoin supply. + /// /// [`create_inbound_payment`]: Self::create_inbound_payment /// [`PaymentReceived`]: events::Event::PaymentReceived - pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result { + pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result { + inbound_payment::create_from_hash(&self.inbound_payment_key, min_value_msat, payment_hash, invoice_expiry_delta_secs, self.highest_seen_timestamp.load(Ordering::Acquire) as u64) + } + + /// Legacy version of [`create_inbound_payment_for_hash`]. Use this method if you wish to share + /// serialized state with LDK node(s) running 0.0.103 and earlier. + /// + /// # Note + /// This method is deprecated and will be removed soon. + /// + /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash + #[deprecated] + pub fn create_inbound_payment_for_hash_legacy(&self, payment_hash: PaymentHash, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result { self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs) } @@ -6219,6 +6526,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> pending_events_read.append(&mut channel_closures); } + let inbound_pmt_key_material = args.keys_manager.get_inbound_payment_key_material(); + let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material); let channel_manager = ChannelManager { genesis_hash, fee_estimator: args.fee_estimator, @@ -6234,6 +6543,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> claimable_htlcs, pending_msg_events: Vec::new(), }), + inbound_payment_key: expanded_inbound_key, pending_inbound_payments: Mutex::new(pending_inbound_payments), pending_outbound_payments: Mutex::new(pending_outbound_payments.unwrap()), @@ -6272,8 +6582,10 @@ mod tests { use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use core::time::Duration; + use core::sync::atomic::Ordering; use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use ln::channelmanager::{PaymentId, PaymentSendFailure}; + use ln::channelmanager::inbound_payment; use ln::features::InitFeatures; use ln::functional_test_utils::*; use ln::msgs; @@ -6288,7 +6600,7 @@ mod tests { fn test_wait_timeout() { use ln::channelmanager::PersistenceNotifier; use sync::Arc; - use core::sync::atomic::{AtomicBool, Ordering}; + use core::sync::atomic::AtomicBool; use std::thread; let persistence_notifier = Arc::new(PersistenceNotifier::new()); @@ -6734,6 +7046,35 @@ mod tests { _ => panic!("unexpected error") } } + + #[test] + fn bad_inbound_payment_hash() { + // Add coverage for checking that a user-provided payment hash matches the payment secret. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[0]); + let payment_data = msgs::FinalOnionHopData { + payment_secret, + total_msat: 100_000, + }; + + // Ensure that if the payment hash given to `inbound_payment::verify` differs from the original, + // payment verification fails as expected. + let mut bad_payment_hash = payment_hash.clone(); + bad_payment_hash.0[0] += 1; + match inbound_payment::verify(bad_payment_hash, payment_data.clone(), nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) { + Ok(_) => panic!("Unexpected ok"), + Err(()) => { + nodes[0].logger.assert_log_contains("lightning::ln::channelmanager::inbound_payment".to_string(), "Failing HTLC with user-generated payment_hash".to_string(), 1); + } + } + + // Check that using the original payment hash succeeds. + assert!(inbound_payment::verify(payment_hash, payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok()); + } } #[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))] diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 2fd5e7d2865..4b38cb32c27 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -8194,7 +8194,7 @@ fn test_preimage_storage() { create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id; { - let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200); + let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200).unwrap(); let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000); nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap(); check_added_monitors!(nodes[0], 1); @@ -8222,8 +8222,10 @@ fn test_preimage_storage() { } #[test] +#[allow(deprecated)] fn test_secret_timeout() { - // Simple test of payment secret storage time outs + // Simple test of payment secret storage time outs. After + // `create_inbound_payment(_for_hash)_legacy` is removed, this test will be removed as well. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -8231,11 +8233,11 @@ fn test_secret_timeout() { create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id; - let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment(Some(100_000), 2); + let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment_legacy(Some(100_000), 2).unwrap(); // We should fail to register the same payment hash twice, at least until we've connected a // block with time 7200 + CHAN_CONFIRM_DEPTH + 1. - if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2) { + if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2) { assert_eq!(err, "Duplicate payment hash"); } else { panic!(); } let mut block = { @@ -8250,7 +8252,7 @@ fn test_secret_timeout() { } }; connect_block(&nodes[1], &block); - if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2) { + if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2) { assert_eq!(err, "Duplicate payment hash"); } else { panic!(); } @@ -8259,7 +8261,7 @@ fn test_secret_timeout() { block.header.prev_blockhash = block.header.block_hash(); block.header.time += 1; connect_block(&nodes[1], &block); - let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(payment_hash, Some(100_000), 2).unwrap(); + let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2).unwrap(); assert_ne!(payment_secret_1, our_payment_secret); { @@ -8298,7 +8300,7 @@ fn test_bad_secret_hash() { let random_payment_hash = PaymentHash([42; 32]); let random_payment_secret = PaymentSecret([43; 32]); - let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2); + let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2).unwrap(); let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000); // All the below cases should end up being handled exactly identically, so we macro the diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 5184a02031d..a4a733c8e50 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -183,10 +183,15 @@ pub enum Event { /// [`ChannelManager::fail_htlc_backwards`] within the HTLC's timeout, the HTLC will be /// automatically failed. /// + /// # Note + /// LDK will not stop an inbound payment from being paid multiple times, so multiple + /// `PaymentReceived` events may be generated for the same payment. + /// /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds /// [`ChannelManager::fail_htlc_backwards`]: crate::ln::channelmanager::ChannelManager::fail_htlc_backwards PaymentReceived { - /// The hash for which the preimage should be handed to the ChannelManager. + /// The hash for which the preimage should be handed to the ChannelManager. Note that LDK will + /// not stop you from registering duplicate payment hashes for inbound payments. payment_hash: PaymentHash, /// The value, in thousandths of a satoshi, that this payment is for. amt: u64, From d41499a26084c828de6542380b0b615093bda954 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 9 Dec 2021 17:41:33 -0500 Subject: [PATCH 5/5] Add new invoice CreationError::InvalidAmount for use in checking `create_inbound_payment` in an invoice creation utility. Note that if the error type of `create_inbound_payment` ever changed, we'd be forced to update the invoice utility's callsite to handle the new error --- lightning-invoice/src/lib.rs | 4 ++++ lightning-invoice/src/utils.rs | 9 +++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 04ba08bd855..dd86c06f23f 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -1413,6 +1413,9 @@ pub enum CreationError { /// The supplied expiry time could cause an overflow if added to a `PositiveTimestamp` ExpiryTimeOutOfBounds, + + /// The supplied millisatoshi amount was greater than the total bitcoin supply. + InvalidAmount, } impl Display for CreationError { @@ -1422,6 +1425,7 @@ impl Display for CreationError { CreationError::RouteTooLong => f.write_str("The specified route has too many hops and can't be encoded"), CreationError::TimestampOutOfBounds => f.write_str("The unix timestamp of the supplied date is <0 or can't be represented as `SystemTime`"), CreationError::ExpiryTimeOutOfBounds => f.write_str("The supplied expiry time could cause an overflow if added to a `PositiveTimestamp`"), + CreationError::InvalidAmount => f.write_str("The supplied millisatoshi amount was greater than the total bitcoin supply"), } } } diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 005be24c3b7..5918753ae1c 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -1,6 +1,6 @@ //! Convenient utilities to create an invoice. -use {Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError, RawInvoice}; +use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError, RawInvoice}; use payment::{Payer, Router}; use bech32::ToBase32; @@ -60,10 +60,11 @@ where }])); } + // `create_inbound_payment` only returns an error if the amount is greater than the total bitcoin + // supply. let (payment_hash, payment_secret) = channelmanager.create_inbound_payment( - amt_msat, - DEFAULT_EXPIRY_TIME.try_into().unwrap(), - ).unwrap(); + amt_msat, DEFAULT_EXPIRY_TIME.try_into().unwrap()) + .map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?; let our_node_pubkey = channelmanager.get_our_node_id(); let mut invoice = InvoiceBuilder::new(network) .description(description)