Drop need to store pending inbound payments

and replace payment_secret with encrypted payment metadata

See docs on create_inbound_payment and create_inbound_payment_for_hash for details

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -65,6 +65,7 @@ use core::{cmp, mem};
 use core::cell::RefCell;
 use io::{Cursor, Read};
 use sync::{Arc, Condvar, Mutex, MutexGuard, RwLock, RwLockReadGuard};
+use core::convert::TryInto;
 use core::sync::atomic::{AtomicUsize, Ordering};
 use core::time::Duration;
 #[cfg(any(test, feature = "allow_wallclock_use"))]
@@ -2586,6 +2587,62 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		}
 	}
 
+	// Check that an inbound payment's `payment_data` field is sane.
+	fn verify_inbound_payment_data(&self, payment_hash: PaymentHash, payment_data: msgs::FinalOnionHopData) -> Result<Option<PaymentPreimage>, ()> {
+		let (iv_bytes, encrypted_metadata_bytes) = payment_data.payment_secret.0.split_at(16);
+		let mut chacha = ChaCha20::new(&self.our_network_key[..], &iv_bytes[..12]);
+		let mut chacha_bytes = [0; 16];
+		chacha.process_in_place(&mut chacha_bytes);
+
+		let mut metadata_bytes: [u8; 16] = [0; 16];
+		for i in 0..16 {
+			metadata_bytes[i] = chacha_bytes[i] ^ encrypted_metadata_bytes[i];
+		}
+
+		let expiry = u64::from_be_bytes(metadata_bytes[8..].try_into().unwrap());
+		if expiry < self.highest_seen_timestamp.load(Ordering::Acquire) as u64 {
+			log_trace!(self.logger, "Failing HTLC with payment_hash {}: expired payment", log_bytes!(payment_hash.0));
+			return Err(())
+		}
+
+		let is_user_payment_hash = metadata_bytes[0] & 1 << 7 != 0;
+		let mut payment_preimage = None;
+		if is_user_payment_hash {
+			let mut hmac = HmacEngine::<Sha256>::new(&self.our_network_key[..]);
+			hmac.input(&metadata_bytes[..]);
+			hmac.input(&payment_hash.0[..]);
+			if iv_bytes != Hmac::from_engine(hmac).into_inner().split_at_mut(16).0 {
+				log_trace!(self.logger, "Failing HTLC with user-generated payment_hash {}: unexpected payment_secret", log_bytes!(payment_hash.0));
+				return Err(())
+			}
+			// Reset the bit that was set to indicate that the payment hash was user-generated.
+			let mut amt_msat_bytes = [0; 8];
+			amt_msat_bytes.copy_from_slice(&metadata_bytes[..8]);
+			amt_msat_bytes[0] ^= 1 << 7;
+			let min_amt_msat = u64::from_be_bytes(amt_msat_bytes.try_into().unwrap());
+			if payment_data.total_msat < min_amt_msat {
+				log_trace!(self.logger, "Failing HTLC with user-generated 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(())
+			}
+		} else {
+			let min_amt_msat = u64::from_be_bytes(metadata_bytes[..8].try_into().unwrap());
+			if payment_data.total_msat < min_amt_msat {
+				log_trace!(self.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(())
+			}
+			let mut hmac = HmacEngine::<Sha256>::new(&self.our_network_key[..]);
+			hmac.input(&iv_bytes);
+			hmac.input(&metadata_bytes);
+			let decoded_payment_preimage = Hmac::from_engine(hmac).into_inner();
+			if payment_hash.0 != Sha256::hash(&decoded_payment_preimage).into_inner() {
+				log_trace!(self.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));
+		}
+		Ok(payment_preimage)
+	}
+
 	/// Processes HTLCs which are pending waiting on random forward delay.
 	///
 	/// Should only really ever be called in response to a PendingHTLCsForwardable event.
@@ -2801,6 +2858,56 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 									}
 								}
 
+								macro_rules! check_total_value {
+									($payment_data_total_msat: expr, $payment_secret: expr, $payment_preimage: expr) => {
+										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: $payment_preimage,
+													payment_secret: $payment_secret,
+												},
+												amt: total_value,
+											});
+										} else {
+											// Nothing to do - we haven't reached the total
+											// payment value yet, wait until we receive more
+											// MPP parts.
+										}
+									}
+								}
+
 								// 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
@@ -2811,9 +2918,17 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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 self.verify_inbound_payment_data(payment_hash, payment_data.clone()) {
+													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) {
@@ -2850,55 +2965,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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.
-												inbound_payment.remove_entry();
-											} else {
-												// Nothing to do - we haven't reached the total
-												// payment value yet, wait until we receive more
-												// MPP parts.
-											}
+											check_total_value!(payment_data.total_msat, payment_data.payment_secret, inbound_payment.get().payment_preimage);
 										}
 									},
 								};
@@ -4522,38 +4589,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		}
 	}
 
-	fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
-		assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106
-
-		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);
-		let mut payment_secrets = self.pending_inbound_payments.lock().unwrap();
-		match payment_secrets.entry(payment_hash) {
-			hash_map::Entry::Vacant(e) => {
-				e.insert(PendingInboundPayment {
-					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
-					// 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.
-					expiry_time: self.highest_seen_timestamp.load(Ordering::Acquire) as u64 + invoice_expiry_delta_secs as u64 + 7200,
-				});
-			},
-			hash_map::Entry::Occupied(_) => return Err(APIError::APIMisuseError { err: "Duplicate payment hash".to_owned() }),
-		}
-		Ok(payment_secret)
-	}
-
 	/// Gets a payment secret and payment hash for use in an invoice given to a third party wishing
 	/// 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
@@ -4566,12 +4606,47 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// [`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<u64>, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) {
-		let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes());
-		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
+		let min_amt_msat_bytes: [u8; 8] = match min_value_msat {
+			Some(amt) => amt.to_be_bytes(),
+			None => [0; 8],
+		};
+		// 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 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 = (self.highest_seen_timestamp.load(Ordering::Acquire) as u64 + invoice_expiry_delta_secs as u64 + 7200).to_be_bytes();
+		let mut metadata_bytes: [u8; 16] = [0; 16];
+		{
+			let (min_amt_msat_slice, expiry_slice) = metadata_bytes.split_at_mut(8);
+			min_amt_msat_slice.copy_from_slice(&min_amt_msat_bytes);
+			expiry_slice.copy_from_slice(&expiry_bytes);
+		}
+
+		let rand_bytes = self.keys_manager.get_secure_random_bytes();
+		let iv_bytes = &rand_bytes[..16];
+
+		let mut hmac = HmacEngine::<Sha256>::new(&self.our_network_key[..]);
+		hmac.input(iv_bytes);
+		hmac.input(&metadata_bytes);
+		let payment_preimage_bytes = Hmac::from_engine(hmac).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 mut payment_secret_bytes: [u8; 32] = [0; 32];
+		{
+			let (iv_slice, encrypted_metadata_slice) = payment_secret_bytes.split_at_mut(16);
+			iv_slice.copy_from_slice(iv_bytes);
+
+			let mut chacha = ChaCha20::new(&self.our_network_key[..], &iv_bytes[..12]);
+			let mut chacha_bytes = [0; 16];
+			chacha.process_in_place(&mut chacha_bytes);
+
+			for i in 0..16 {
+				encrypted_metadata_slice[i] = chacha_bytes[i] ^ metadata_bytes[i];
+			}
+		}
+
+		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage_bytes).into_inner());
+		(payment_hash, PaymentSecret(payment_secret_bytes))
 	}
 
 	/// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is
@@ -4593,18 +4668,13 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// in excess of the current time. This should roughly match the expiry time set in the invoice.
 	/// After this many seconds, we will remove the inbound payment, resulting in any attempts to
 	/// pay the invoice failing. The BOLT spec suggests 3,600 secs as a default validity time for
-	/// invoices when no timeout is set.
-	///
-	/// Note that we use block header time to time-out pending inbound payments (with some margin
-	/// to compensate for the inaccuracy of block header timestamps). Thus, in practice we will
+	/// invoices when no timeout is set.  Note that we use block header time to time-out pending
+	/// inbound payments (with some margin to compensate for the inaccuracy of block header
+	/// timestamps). Thus, in practice we will
 	/// accept a payment and generate a [`PaymentReceived`] event for some time after the expiry.
 	/// 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`
@@ -4613,7 +4683,48 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// [`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<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
-		self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs)
+		let mut min_amt_msat_bytes: [u8; 8] = match min_value_msat {
+			Some(amt) => amt.to_be_bytes(),
+			None => [0; 8],
+		};
+		// Flip the highest bit of the min_amt_msat field to indicate that this payment has a
+		// user-generated PaymentHash.
+		min_amt_msat_bytes[0] |= 1 << 7;
+
+		// 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 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 = (self.highest_seen_timestamp.load(Ordering::Acquire) as u64 + invoice_expiry_delta_secs as u64 + 7200).to_be_bytes();
+
+		let mut metadata_bytes: [u8; 16] = [0; 16];
+		{
+			let (min_amt_msat_slice, expiry_slice) = metadata_bytes.split_at_mut(8);
+			min_amt_msat_slice.copy_from_slice(&min_amt_msat_bytes);
+			expiry_slice.copy_from_slice(&expiry_bytes);
+		}
+
+		let mut hmac = HmacEngine::<Sha256>::new(&self.our_network_key[..]);
+		hmac.input(&metadata_bytes);
+		hmac.input(&payment_hash.0);
+		let hmac_bytes = Hmac::from_engine(hmac).into_inner();
+		let iv_bytes = &hmac_bytes[..16];
+
+		let mut payment_secret_bytes: [u8; 32] = [0; 32];
+		{
+			let (iv_slice, encrypted_metadata_slice) = payment_secret_bytes.split_at_mut(16);
+			iv_slice.copy_from_slice(iv_bytes);
+
+			let mut chacha = ChaCha20::new(&self.our_network_key[..], &iv_bytes[..12]);
+			let mut chacha_bytes = [0; 16];
+			chacha.process_in_place(&mut chacha_bytes);
+
+			for i in 0..16 {
+				encrypted_metadata_slice[i] = chacha_bytes[i] ^ metadata_bytes[i];
+			}
+		}
+		Ok(PaymentSecret(payment_secret_bytes))
 	}
 
 	#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]