Merge pull request #1177 from valentinewallace/2021-11-derivable-payment-secret

Stop storing pending inbound payment data; instead make it derivable on receive

Author: TheBlueMatt

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();

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<dyn Logger>) {
 		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;

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"),
 		}
 	}
 }

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(),
-	);
+		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)

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<u8>) -> Result<RecoverableSignature, ()>;
+
+	/// 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<secp256k1::All>,
 	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()
 	}

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();