Encrypt payment_metadata when we build the payment secret

fuzz/src/chanmon_consistency.rs

@@ -1369,7 +1369,7 @@ impl PaymentTracker {
 		let mut payment_preimage = PaymentPreimage([0; 32]);
 		payment_preimage.0[0..8].copy_from_slice(&self.payment_ctr.to_be_bytes());
 		let hash = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array());
-		let secret = dest
+		let (secret, _no_metadata) = dest
 			.create_inbound_payment_for_hash(hash, None, 3600, None, None)
 			.expect("create_inbound_payment_for_hash failed");
 		assert!(self.payment_preimages.insert(hash, payment_preimage).is_none());

lightning-liquidity/tests/lsps2_integration_tests.rs

@@ -120,7 +120,7 @@ fn create_jit_invoice(
 ) -> Result<Bolt11Invoice, ()> {
 	// LSPS2 requires min_final_cltv_expiry_delta to be at least 2 more than usual.
 	let min_final_cltv_expiry_delta = MIN_FINAL_CLTV_EXPIRY_DELTA + 2;
-	let (payment_hash, payment_secret) = node
+	let (payment_hash, payment_secret, _) = node
 		.node
 		.create_inbound_payment(None, expiry_secs, Some(min_final_cltv_expiry_delta), None)
 		.map_err(|e| {

lightning/src/crypto/utils.rs

@@ -3,6 +3,8 @@ use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::secp256k1::{ecdsa::Signature, Message, Secp256k1, SecretKey, Signing};
 
+use chacha20_poly1305::chacha20::{ChaCha20, Key, Nonce};
+
 use crate::sign::EntropySource;
 
 macro_rules! hkdf_extract_expand {
@@ -22,7 +24,7 @@ macro_rules! hkdf_extract_expand {
 		let (k1, k2, _) = hkdf_extract_expand!($salt, $ikm);
 		(k1, k2)
 	}};
-	($salt: expr, $ikm: expr, 7) => {{
+	($salt: expr, $ikm: expr, 8) => {{
 		let (k1, k2, prk) = hkdf_extract_expand!($salt, $ikm);
 
 		let mut hmac = HmacEngine::<Sha256>::new(&prk[..]);
@@ -50,18 +52,23 @@ macro_rules! hkdf_extract_expand {
 		hmac.input(&[7; 1]);
 		let k7 = Hmac::from_engine(hmac).to_byte_array();
 
-		(k1, k2, k3, k4, k5, k6, k7)
+		let mut hmac = HmacEngine::<Sha256>::new(&prk[..]);
+		hmac.input(&k7);
+		hmac.input(&[8; 1]);
+		let k8 = Hmac::from_engine(hmac).to_byte_array();
+
+		(k1, k2, k3, k4, k5, k6, k7, k8)
 	}};
 }
 
 pub fn hkdf_extract_expand_twice(salt: &[u8], ikm: &[u8]) -> ([u8; 32], [u8; 32]) {
 	hkdf_extract_expand!(salt, ikm, 2)
 }
 
-pub fn hkdf_extract_expand_7x(
+pub fn hkdf_extract_expand_8x(
 	salt: &[u8], ikm: &[u8],
-) -> ([u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32]) {
-	hkdf_extract_expand!(salt, ikm, 7)
+) -> ([u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32], [u8; 32]) {
+	hkdf_extract_expand!(salt, ikm, 8)
 }
 
 #[inline]
@@ -91,3 +98,12 @@ pub fn sign_with_aux_rand<C: Signing, ES: EntropySource>(
 	let sig = sign(ctx, msg, sk);
 	sig
 }
+
+pub fn apply_chacha20(key: [u8; 32], nonce: [u8; 16], data: &mut [u8]) {
+	ChaCha20::new_from_block(
+		Key::new(key),
+		Nonce::new(nonce[4..].try_into().unwrap()),
+		u32::from_le_bytes(nonce[..4].try_into().unwrap()),
+	)
+	.apply_keystream(data);
+}

lightning/src/ln/bolt11_payment_tests.rs

@@ -30,8 +30,10 @@ fn payment_metadata_end_to_end_for_invoice_with_amount() {
 
 	let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42];
 
-	let (payment_hash, payment_secret) =
-		nodes[1].node.create_inbound_payment(None, 7200, None, Some(&payment_metadata)).unwrap();
+	let (payment_hash, payment_secret, encrypted_metadata) = nodes[1]
+		.node
+		.create_inbound_payment(None, 7200, None, Some(payment_metadata.clone()))
+		.unwrap();
 
 	let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
 	let invoice = InvoiceBuilder::new(Currency::Bitcoin)
@@ -41,7 +43,7 @@ fn payment_metadata_end_to_end_for_invoice_with_amount() {
 		.duration_since_epoch(timestamp)
 		.min_final_cltv_expiry_delta(144)
 		.amount_milli_satoshis(50_000)
-		.payment_metadata(payment_metadata.clone())
+		.payment_metadata(encrypted_metadata.unwrap())
 		.build_raw()
 		.unwrap();
 	let sig = nodes[1].keys_manager.backing.sign_invoice(&invoice, Recipient::Node).unwrap();
@@ -97,8 +99,10 @@ fn payment_metadata_end_to_end_for_invoice_with_no_amount() {
 
 	let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42];
 
-	let (payment_hash, payment_secret) =
-		nodes[1].node.create_inbound_payment(None, 7200, None, Some(&payment_metadata)).unwrap();
+	let (payment_hash, payment_secret, encrypted_metadata) = nodes[1]
+		.node
+		.create_inbound_payment(None, 7200, None, Some(payment_metadata.clone()))
+		.unwrap();
 
 	let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
 	let invoice = InvoiceBuilder::new(Currency::Bitcoin)
@@ -107,7 +111,7 @@ fn payment_metadata_end_to_end_for_invoice_with_no_amount() {
 		.payment_secret(payment_secret)
 		.duration_since_epoch(timestamp)
 		.min_final_cltv_expiry_delta(144)
-		.payment_metadata(payment_metadata.clone())
+		.payment_metadata(encrypted_metadata.unwrap())
 		.build_raw()
 		.unwrap();
 	let sig = nodes[1].keys_manager.backing.sign_invoice(&invoice, Recipient::Node).unwrap();

lightning/src/ln/channelmanager.rs

@@ -8462,7 +8462,7 @@ impl<
 						payment_data,
 						payment_context,
 						phantom_shared_secret,
-						onion_fields,
+						mut onion_fields,
 						has_recipient_created_payment_secret,
 						invoice_request_opt,
 						trampoline_shared_secret,
@@ -8603,7 +8603,7 @@ impl<
 							let verify_res = inbound_payment::verify(
 								payment_hash,
 								&payment_data,
-								onion_fields.payment_metadata.as_deref(),
+								onion_fields.payment_metadata.as_mut(),
 								self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
 								&self.inbound_payment_key,
 								&self.logger,
@@ -14372,24 +14372,24 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 			}
 		}
 
-		let (payment_hash, payment_secret) = match payment_hash {
+		let (payment_hash, payment_secret, payment_metadata) = match payment_hash {
 			Some(payment_hash) => {
-				let payment_secret = self
+				let (payment_secret, payment_metadata) = self
 					.create_inbound_payment_for_hash(
 						payment_hash, amount_msats,
 						invoice_expiry_delta_secs.unwrap_or(DEFAULT_EXPIRY_TIME as u32),
 						min_final_cltv_expiry_delta,
-						payment_metadata.as_deref(),
+						payment_metadata,
 					)
 					.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?;
-				(payment_hash, payment_secret)
+				(payment_hash, payment_secret, payment_metadata)
 			},
 			None => {
 				self
 					.create_inbound_payment(
 						amount_msats, invoice_expiry_delta_secs.unwrap_or(DEFAULT_EXPIRY_TIME as u32),
 						min_final_cltv_expiry_delta,
-						payment_metadata.as_deref(),
+						payment_metadata,
 					)
 					.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?
 			},
@@ -14516,8 +14516,7 @@ pub struct Bolt11InvoiceParameters {
 	/// onion by the sender, available as [`RecipientOnionFields::payment_metadata`] via
 	/// [`Event::PaymentClaimable::onion_fields`].
 	///
-	/// Note that because it is exposed to the sender in the invoice you should consider encrypting
-	/// it. It is committed to, however, so cannot be modified by the sender.
+	/// The metadata itself is encrypted and HMAC'd before being stored in the BOLT 11 invoice.
 	pub payment_metadata: Option<Vec<u8>>,
 }
 
@@ -15023,6 +15022,7 @@ impl<
 			|amount_msats, relative_expiry| {
 				self.create_inbound_payment(Some(amount_msats), relative_expiry, None, None)
 					.map_err(|()| Bolt12SemanticError::InvalidAmount)
+					.map(|(preimage, secret, _no_metadata)| (preimage, secret))
 			},
 			None,
 		)?;
@@ -15033,8 +15033,8 @@ impl<
 		Ok(invoice)
 	}
 
-	/// Gets a payment secret and payment hash for use in an invoice given to a third party wishing
-	/// to pay us.
+	/// Gets a payment secret, payment hash, and encrypts the `payment_metadata` 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.
@@ -15065,8 +15065,8 @@ impl<
 	/// [`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,
-		min_final_cltv_expiry_delta: Option<u16>, payment_metadata: Option<&[u8]>,
-	) -> Result<(PaymentHash, PaymentSecret), ()> {
+		min_final_cltv_expiry_delta: Option<u16>, payment_metadata: Option<Vec<u8>>,
+	) -> Result<(PaymentHash, PaymentSecret, Option<Vec<u8>>), ()> {
 		inbound_payment::create(
 			&self.inbound_payment_key,
 			min_value_msat,
@@ -15078,8 +15078,8 @@ impl<
 		)
 	}
 
-	/// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is
-	/// stored external to LDK.
+	/// Gets a [`PaymentSecret`] for a given [`PaymentHash`] (for which the payment preimage is
+	/// stored external to LDK) and encrypts the `payment_metadata`.
 	///
 	/// A [`PaymentClaimable`] event will only be generated if the [`PaymentSecret`] matches a
 	/// payment secret fetched via this method or [`create_inbound_payment`], and which is at least
@@ -15115,41 +15115,34 @@ impl<
 	/// 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.
 	///
-	/// If `min_final_cltv_expiry_delta` is set to some value, then the payment will not be receivable
-	/// on versions of LDK prior to 0.0.114.
-	///
 	/// [`create_inbound_payment`]: Self::create_inbound_payment
 	/// [`PaymentClaimable`]: events::Event::PaymentClaimable
 	pub fn create_inbound_payment_for_hash(
 		&self, payment_hash: PaymentHash, min_value_msat: Option<u64>,
 		invoice_expiry_delta_secs: u32, min_final_cltv_expiry: Option<u16>,
-		payment_metadata: Option<&[u8]>,
-	) -> Result<PaymentSecret, ()> {
+		payment_metadata: Option<Vec<u8>>,
+	) -> Result<(PaymentSecret, Option<Vec<u8>>), ()> {
 		inbound_payment::create_from_hash(
 			&self.inbound_payment_key,
 			min_value_msat,
 			payment_hash,
 			invoice_expiry_delta_secs,
+			&self.entropy_source,
 			self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
 			min_final_cltv_expiry,
 			payment_metadata,
 		)
 	}
 
-	/// Gets an LDK-generated payment preimage from a payment hash, metadata and secret that were
-	/// previously returned from [`create_inbound_payment`].
+	/// Gets an LDK-generated payment preimage from a payment hash and secret and decrypts the
+	/// metadata (if any) that were previously returned from [`create_inbound_payment`].
 	///
 	/// [`create_inbound_payment`]: Self::create_inbound_payment
-	pub fn get_payment_preimage(
+	pub fn get_payment_preimage_decrypt_metadata(
 		&self, payment_hash: PaymentHash, payment_secret: PaymentSecret,
-		payment_metadata: Option<&[u8]>,
+		payment_metadata: Option<&mut [u8]>,
 	) -> Result<PaymentPreimage, APIError> {
 		let expanded_key = &self.inbound_payment_key;
 		inbound_payment::get_payment_preimage(
@@ -17235,7 +17228,9 @@ impl<
 						relative_expiry,
 						None,
 						None,
-					).map_err(|_| Bolt12SemanticError::InvalidAmount)
+					)
+					.map_err(|_| Bolt12SemanticError::InvalidAmount)
+					.map(|(preimage, secret, _no_metadata)| (preimage, secret))
 				};
 
 				let (result, context) = match invoice_request {
@@ -22137,7 +22132,8 @@ pub mod bench {
 				payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
 				payment_count += 1;
 				let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
-				let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, None, None).unwrap();
+				let (payment_secret, _no_payment_metadata) =
+					$node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, None, None).unwrap();
 
 				$node_a.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret, 10_000),
 					PaymentId(payment_hash.0),

lightning/src/ln/functional_test_utils.rs

@@ -2800,7 +2800,7 @@ pub fn get_payment_preimage_hash(
 	let payment_preimage = PaymentPreimage([*payment_count; 32]);
 	*payment_count += 1;
 	let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
-	let payment_secret = recipient
+	let (payment_secret, _) = recipient
 		.node
 		.create_inbound_payment_for_hash(
 			payment_hash,