Elide metadata from Offer with derived keys

When an Offer uses blinded paths, its metadata consists of a nonce used
to derive its signing keys. Now that the blinded paths contain this
nonce, elide the metadata as it is now redundant. This saves space and
also makes it impossible to derive the signing keys if an invoice
request is received with the incorrect nonce. The nonce shouldn't be
revealed in this case either to prevent de-anonymization attacks.

Author: jkczyz

lightning/src/offers/invoice.rs

@@ -1776,15 +1776,17 @@ mod tests {
 			.sign(payer_sign).unwrap();
 
 		if let Err(e) = invoice_request.clone()
-			.verify(&expanded_key, &secp_ctx).unwrap()
+			.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).unwrap()
 			.respond_using_derived_keys_no_std(payment_paths(), payment_hash(), now()).unwrap()
 			.build_and_sign(&secp_ctx)
 		{
 			panic!("error building invoice: {:?}", e);
 		}
 
 		let expanded_key = ExpandedKey::new(&KeyMaterial([41; 32]));
-		assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err());
+		assert!(
+			invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err()
+		);
 
 		let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx)
 			.amount_msats(1000)

lightning/src/offers/invoice_request.rs

@@ -605,8 +605,9 @@ pub struct InvoiceRequest {
 	signature: Signature,
 }
 
-/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify`] and exposes different
-/// ways to respond depending on whether the signing keys were derived.
+/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify`] or
+/// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending
+/// on whether the signing keys were derived.
 #[derive(Clone, Debug)]
 pub struct VerifiedInvoiceRequest {
 	/// The identifier of the [`Offer`] for which the [`InvoiceRequest`] was made.
@@ -737,7 +738,8 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { (
 	/// # Note
 	///
 	/// If the originating [`Offer`] was created using [`OfferBuilder::deriving_signing_pubkey`],
-	/// then use [`InvoiceRequest::verify`] and [`VerifiedInvoiceRequest`] methods instead.
+	/// then first use [`InvoiceRequest::verify`] or [`InvoiceRequest::verify_using_recipient_data`]
+	/// and then [`VerifiedInvoiceRequest`] methods instead.
 	///
 	/// [`Bolt12Invoice::created_at`]: crate::offers::invoice::Bolt12Invoice::created_at
 	/// [`OfferBuilder::deriving_signing_pubkey`]: crate::offers::offer::OfferBuilder::deriving_signing_pubkey

lightning/src/offers/offer.rs

@@ -249,9 +249,12 @@ macro_rules! offer_derived_metadata_builder_methods { ($secp_context: ty) => {
 	///
 	/// Also, sets the metadata when [`OfferBuilder::build`] is called such that it can be used by
 	/// [`InvoiceRequest::verify`] to determine if the request was produced for the offer given an
-	/// [`ExpandedKey`].
+	/// [`ExpandedKey`]. However, if [`OfferBuilder::path`] is called, then the metadata will not be
+	/// set and must be included in each [`BlindedPath`] instead. In this case, use
+	/// [`InvoiceRequest::verify_using_recipient_data`].
 	///
 	/// [`InvoiceRequest::verify`]: crate::offers::invoice_request::InvoiceRequest::verify
+	/// [`InvoiceRequest::verify_using_recipient_data`]: crate::offers::invoice_request::InvoiceRequest::verify_using_recipient_data
 	/// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey
 	pub fn deriving_signing_pubkey(
 		node_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce,
@@ -384,9 +387,12 @@ macro_rules! offer_builder_methods { (
 	}
 
 	fn build_without_checks($($self_mut)* $self: $self_type) -> Offer {
-		// Create the metadata for stateless verification of an InvoiceRequest.
 		if let Some(mut metadata) = $self.offer.metadata.take() {
+			// Create the metadata for stateless verification of an InvoiceRequest.
 			if metadata.has_derivation_material() {
+
+				// Don't derive keys if no blinded paths were given since this means the signing
+				// pubkey must be the node id of an announced node.
 				if $self.offer.paths.is_none() {
 					metadata = metadata.without_keys();
 				}
@@ -398,14 +404,17 @@ macro_rules! offer_builder_methods { (
 					tlv_stream.node_id = None;
 				}
 
+				// Either replace the signing pubkey with the derived pubkey or include the metadata
+				// for verification. In the former case, the blinded paths must include
+				// `OffersContext::InvoiceRequest` instead.
 				let (derived_metadata, keys) = metadata.derive_from(tlv_stream, $self.secp_ctx);
-				metadata = derived_metadata;
-				if let Some(keys) = keys {
-					$self.offer.signing_pubkey = Some(keys.public_key());
+				match keys {
+					Some(keys) => $self.offer.signing_pubkey = Some(keys.public_key()),
+					None => $self.offer.metadata = Some(derived_metadata),
 				}
+			} else {
+				$self.offer.metadata = Some(metadata);
 			}
-
-			$self.offer.metadata = Some(metadata);
 		}
 
 		let mut bytes = Vec::new();
@@ -667,9 +676,9 @@ impl Offer {
 
 	#[cfg(async_payments)]
 	pub(super) fn verify<T: secp256k1::Signing>(
-		&self, key: &ExpandedKey, secp_ctx: &Secp256k1<T>
+		&self, nonce: Nonce, key: &ExpandedKey, secp_ctx: &Secp256k1<T>
 	) -> Result<(OfferId, Option<Keypair>), ()> {
-		self.contents.verify(&self.bytes, key, secp_ctx)
+		self.contents.verify_using_recipient_data(&self.bytes, nonce, key, secp_ctx)
 	}
 }
 
@@ -1296,6 +1305,7 @@ mod tests {
 		let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx)
 			.amount_msats(1000)
 			.build().unwrap();
+		assert!(offer.metadata().is_some());
 		assert_eq!(offer.signing_pubkey(), Some(node_id));
 
 		let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
@@ -1365,23 +1375,22 @@ mod tests {
 			.amount_msats(1000)
 			.path(blinded_path)
 			.build().unwrap();
+		assert!(offer.metadata().is_none());
 		assert_ne!(offer.signing_pubkey(), Some(node_id));
 
 		let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.build().unwrap()
 			.sign(payer_sign).unwrap();
-		match invoice_request.verify(&expanded_key, &secp_ctx) {
+		match invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) {
 			Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()),
 			Err(_) => panic!("unexpected error"),
 		}
 
+		// Fails verification when using the wrong method
 		let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.build().unwrap()
 			.sign(payer_sign).unwrap();
-		match invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) {
-			Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()),
-			Err(_) => panic!("unexpected error"),
-		}
+		assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err());
 
 		// Fails verification with altered offer field
 		let mut tlv_stream = offer.as_tlv_stream();
@@ -1394,7 +1403,9 @@ mod tests {
 			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.build().unwrap()
 			.sign(payer_sign).unwrap();
-		assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err());
+		assert!(
+			invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err()
+		);
 
 		// Fails verification with altered signing pubkey
 		let mut tlv_stream = offer.as_tlv_stream();
@@ -1408,7 +1419,9 @@ mod tests {
 			.request_invoice(vec![1; 32], payer_pubkey()).unwrap()
 			.build().unwrap()
 			.sign(payer_sign).unwrap();
-		assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err());
+		assert!(
+			invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err()
+		);
 	}
 
 	#[test]

lightning/src/offers/static_invoice.rs

@@ -22,6 +22,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
 use crate::offers::merkle::{
 	self, SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash,
 };
+use crate::offers::nonce::Nonce;
 use crate::offers::offer::{
 	Amount, Offer, OfferContents, OfferTlvStream, OfferTlvStreamRef, Quantity,
 };
@@ -97,7 +98,7 @@ impl<'a> StaticInvoiceBuilder<'a> {
 	pub fn for_offer_using_derived_keys<T: secp256k1::Signing>(
 		offer: &'a Offer, payment_paths: Vec<(BlindedPayInfo, BlindedPath)>,
 		message_paths: Vec<BlindedPath>, created_at: Duration, expanded_key: &ExpandedKey,
-		secp_ctx: &Secp256k1<T>,
+		nonce: Nonce, secp_ctx: &Secp256k1<T>,
 	) -> Result<Self, Bolt12SemanticError> {
 		if offer.chains().len() > 1 {
 			return Err(Bolt12SemanticError::UnexpectedChain);
@@ -111,7 +112,7 @@ impl<'a> StaticInvoiceBuilder<'a> {
 			offer.signing_pubkey().ok_or(Bolt12SemanticError::MissingSigningPubkey)?;
 
 		let keys = offer
-			.verify(&expanded_key, &secp_ctx)
+			.verify(nonce, &expanded_key, &secp_ctx)
 			.map_err(|()| Bolt12SemanticError::InvalidMetadata)?
 			.1
 			.ok_or(Bolt12SemanticError::MissingSigningPubkey)?;
@@ -623,6 +624,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		)
 		.unwrap()
@@ -662,6 +664,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		)
 		.unwrap()
@@ -672,7 +675,7 @@ mod tests {
 		invoice.write(&mut buffer).unwrap();
 
 		assert_eq!(invoice.bytes, buffer.as_slice());
-		assert!(invoice.metadata().is_some());
+		assert_eq!(invoice.metadata(), None);
 		assert_eq!(invoice.amount(), None);
 		assert_eq!(invoice.description(), None);
 		assert_eq!(invoice.offer_features(), &OfferFeatures::empty());
@@ -698,13 +701,12 @@ mod tests {
 		);
 
 		let paths = vec![blinded_path()];
-		let metadata = vec![42; 16];
 		assert_eq!(
 			invoice.as_tlv_stream(),
 			(
 				OfferTlvStreamRef {
 					chains: None,
-					metadata: Some(&metadata),
+					metadata: None,
 					currency: None,
 					amount: None,
 					description: None,
@@ -762,6 +764,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		)
 		.unwrap()
@@ -782,6 +785,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		)
 		.unwrap()
@@ -815,6 +819,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		) {
 			assert_eq!(e, Bolt12SemanticError::MissingPaths);
@@ -829,6 +834,7 @@ mod tests {
 			Vec::new(),
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		) {
 			assert_eq!(e, Bolt12SemanticError::MissingPaths);
@@ -849,6 +855,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		) {
 			assert_eq!(e, Bolt12SemanticError::MissingPaths);
@@ -886,6 +893,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		) {
 			assert_eq!(e, Bolt12SemanticError::MissingSigningPubkey);
@@ -906,6 +914,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		) {
 			assert_eq!(e, Bolt12SemanticError::InvalidMetadata);
@@ -937,6 +946,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		) {
 			assert_eq!(e, Bolt12SemanticError::UnexpectedChain);
@@ -967,6 +977,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		)
 		.unwrap()
@@ -1007,6 +1018,7 @@ mod tests {
 			vec![blinded_path()],
 			now,
 			&expanded_key,
+			nonce,
 			&secp_ctx,
 		)
 		.unwrap()