Cleanup: Remove redundant (hmac, nonce) from codebase

Now that we have introduced an alternate mechanism for authentication
in the codebase, we can safely remove the now redundant (hmac, nonce)
fields from the MessageContext's while maintaining the security of the
onion messages.

Author: shaavan

lightning/src/blinded_path/message.rs

@@ -30,8 +30,6 @@ use crate::sign::{EntropySource, NodeSigner, ReceiveAuthKey, Recipient};
 use crate::types::payment::PaymentHash;
 use crate::util::scid_utils;
 use crate::util::ser::{FixedLengthReader, LengthReadableArgs, Readable, Writeable, Writer};
-use bitcoin::hashes::hmac::Hmac;
-use bitcoin::hashes::sha256::Hash as Sha256;
 
 use core::mem;
 use core::ops::Deref;
@@ -406,12 +404,6 @@ pub enum OffersContext {
 		/// [`Refund`]: crate::offers::refund::Refund
 		/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
 		nonce: Nonce,
-
-		/// Authentication code for the [`PaymentId`], which should be checked when the context is
-		/// used with an [`InvoiceError`].
-		///
-		/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
-		hmac: Option<Hmac<Sha256>>,
 	},
 	/// Context used by a [`BlindedMessagePath`] as a reply path for a [`Bolt12Invoice`].
 	///
@@ -424,19 +416,6 @@ pub enum OffersContext {
 		///
 		/// [`Bolt12Invoice::payment_hash`]: crate::offers::invoice::Bolt12Invoice::payment_hash
 		payment_hash: PaymentHash,
-
-		/// A nonce used for authenticating that a received [`InvoiceError`] is for a valid
-		/// sent [`Bolt12Invoice`].
-		///
-		/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
-		/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
-		nonce: Nonce,
-
-		/// Authentication code for the [`PaymentHash`], which should be checked when the context is
-		/// used to log the received [`InvoiceError`].
-		///
-		/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
-		hmac: Hmac<Sha256>,
 	},
 }
 
@@ -544,35 +523,12 @@ pub enum AsyncPaymentsContext {
 		///
 		/// [`Offer`]: crate::offers::offer::Offer
 		payment_id: PaymentId,
-		/// A nonce used for authenticating that a [`ReleaseHeldHtlc`] message is valid for a preceding
-		/// [`HeldHtlcAvailable`] message.
-		///
-		/// [`ReleaseHeldHtlc`]: crate::onion_message::async_payments::ReleaseHeldHtlc
-		/// [`HeldHtlcAvailable`]: crate::onion_message::async_payments::HeldHtlcAvailable
-		nonce: Nonce,
-		/// Authentication code for the [`PaymentId`].
-		///
-		/// Prevents the recipient from being able to deanonymize us by creating a blinded path to us
-		/// containing the expected [`PaymentId`].
-		hmac: Hmac<Sha256>,
 	},
 	/// Context contained within the [`BlindedMessagePath`]s we put in static invoices, provided back
 	/// to us in corresponding [`HeldHtlcAvailable`] messages.
 	///
 	/// [`HeldHtlcAvailable`]: crate::onion_message::async_payments::HeldHtlcAvailable
 	InboundPayment {
-		/// A nonce used for authenticating that a [`HeldHtlcAvailable`] message is valid for a
-		/// preceding static invoice.
-		///
-		/// [`HeldHtlcAvailable`]: crate::onion_message::async_payments::HeldHtlcAvailable
-		nonce: Nonce,
-		/// Authentication code for the [`HeldHtlcAvailable`] message.
-		///
-		/// Prevents nodes from creating their own blinded path to us, sending a [`HeldHtlcAvailable`]
-		/// message and trivially getting notified whenever we come online.
-		///
-		/// [`HeldHtlcAvailable`]: crate::onion_message::async_payments::HeldHtlcAvailable
-		hmac: Hmac<Sha256>,
 		/// The time as duration since the Unix epoch at which this path expires and messages sent over
 		/// it should be ignored. Without this, anyone with the path corresponding to this context is
 		/// able to trivially ask if we're online forever.
@@ -587,19 +543,27 @@ impl_writeable_tlv_based_enum!(MessageContext,
 	{3, DNSResolver} => (),
 );
 
+// NOTE:
+// Several TLV fields (`nonce`, `hmac`, etc.) were removed in LDK v0.2
+// following the introduction of `ReceiveAuthKey`-based authentication for
+// inbound `BlindedMessagePath`s. These fields are now commented out and
+// their `type` values must not be reused unless support for LDK v0.2
+// and earlier is fully dropped.
+//
+// For context-specific removals, see the commented-out fields within each enum variant.
 impl_writeable_tlv_based_enum!(OffersContext,
 	(0, InvoiceRequest) => {
 		(0, nonce, required),
 	},
 	(1, OutboundPayment) => {
 		(0, payment_id, required),
 		(1, nonce, required),
-		(2, hmac, option),
+		// Removed: (2, hmac, option)
 	},
 	(2, InboundPayment) => {
 		(0, payment_hash, required),
-		(1, nonce, required),
-		(2, hmac, required)
+		// Removed: (1, nonce, required),
+		// Removed: (2, hmac, required)
 	},
 	(3, StaticInvoiceRequested) => {
 		(0, recipient_id, required),
@@ -611,12 +575,12 @@ impl_writeable_tlv_based_enum!(OffersContext,
 impl_writeable_tlv_based_enum!(AsyncPaymentsContext,
 	(0, OutboundPayment) => {
 		(0, payment_id, required),
-		(2, nonce, required),
-		(4, hmac, required),
+		// Removed: (2, nonce, required),
+		// Removed: (4, hmac, required),
 	},
 	(1, InboundPayment) => {
-		(0, nonce, required),
-		(2, hmac, required),
+		// Removed: (0, nonce, required),
+		// Removed: (2, hmac, required),
 		(4, path_absolute_expiry, required),
 	},
 	(2, OfferPaths) => {

lightning/src/ln/channelmanager.rs

@@ -547,24 +547,6 @@ pub trait Verification {
 	) -> Result<(), ()>;
 }
 
-impl Verification for PaymentHash {
-	/// Constructs an HMAC to include in [`OffersContext::InboundPayment`] for the payment hash
-	/// along with the given [`Nonce`].
-	fn hmac_for_offer_payment(
-		&self, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
-	) -> Hmac<Sha256> {
-		signer::hmac_for_payment_hash(*self, nonce, expanded_key)
-	}
-
-	/// Authenticates the payment id using an HMAC and a [`Nonce`] taken from an
-	/// [`OffersContext::InboundPayment`].
-	fn verify_for_offer_payment(
-		&self, hmac: Hmac<Sha256>, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
-	) -> Result<(), ()> {
-		signer::verify_payment_hash(*self, hmac, nonce, expanded_key)
-	}
-}
-
 impl Verification for UnauthenticatedReceiveTlvs {
 	fn hmac_for_offer_payment(
 		&self, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
@@ -589,42 +571,6 @@ pub struct PaymentId(pub [u8; Self::LENGTH]);
 impl PaymentId {
 	/// Number of bytes in the id.
 	pub const LENGTH: usize = 32;
-
-	/// Constructs an HMAC to include in [`AsyncPaymentsContext::OutboundPayment`] for the payment id
-	/// along with the given [`Nonce`].
-	#[cfg(async_payments)]
-	pub fn hmac_for_async_payment(
-		&self, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
-	) -> Hmac<Sha256> {
-		signer::hmac_for_async_payment_id(*self, nonce, expanded_key)
-	}
-
-	/// Authenticates the payment id using an HMAC and a [`Nonce`] taken from an
-	/// [`AsyncPaymentsContext::OutboundPayment`].
-	#[cfg(async_payments)]
-	pub fn verify_for_async_payment(
-		&self, hmac: Hmac<Sha256>, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
-	) -> Result<(), ()> {
-		signer::verify_async_payment_id(*self, hmac, nonce, expanded_key)
-	}
-}
-
-impl Verification for PaymentId {
-	/// Constructs an HMAC to include in [`OffersContext::OutboundPayment`] for the payment id
-	/// along with the given [`Nonce`].
-	fn hmac_for_offer_payment(
-		&self, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
-	) -> Hmac<Sha256> {
-		signer::hmac_for_offer_payment_id(*self, nonce, expanded_key)
-	}
-
-	/// Authenticates the payment id using an HMAC and a [`Nonce`] taken from an
-	/// [`OffersContext::OutboundPayment`].
-	fn verify_for_offer_payment(
-		&self, hmac: Hmac<Sha256>, nonce: Nonce, expanded_key: &inbound_payment::ExpandedKey,
-	) -> Result<(), ()> {
-		signer::verify_offer_payment_id(*self, hmac, nonce, expanded_key)
-	}
 }
 
 impl PaymentId {
@@ -5291,10 +5237,7 @@ where
 				},
 			};
 
-			let entropy = &*self.entropy_source;
-
 			let enqueue_held_htlc_available_res = self.flow.enqueue_held_htlc_available(
-				entropy,
 				invoice,
 				payment_id,
 				self.get_peers_for_blinded_path(),
@@ -11824,7 +11767,7 @@ where
 
 				let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?;
 
-				self.flow.enqueue_invoice(entropy, invoice.clone(), refund, self.get_peers_for_blinded_path())?;
+				self.flow.enqueue_invoice(invoice.clone(), refund, self.get_peers_for_blinded_path())?;
 
 				Ok(invoice)
 			},
@@ -13825,8 +13768,6 @@ where
 	fn handle_message(
 		&self, message: OffersMessage, context: Option<OffersContext>, responder: Option<Responder>,
 	) -> Option<(OffersMessage, ResponseInstruction)> {
-		let expanded_key = &self.inbound_payment_key;
-
 		macro_rules! handle_pay_invoice_res {
 			($res: expr, $invoice: expr, $logger: expr) => {{
 				let error = match $res {
@@ -13942,38 +13883,26 @@ where
 			#[cfg(async_payments)]
 			OffersMessage::StaticInvoice(invoice) => {
 				let payment_id = match context {
-					Some(OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) }) => {
-						if payment_id.verify_for_offer_payment(hmac, nonce, expanded_key).is_err() {
-							return None
-						}
-						payment_id
-					},
+					Some(OffersContext::OutboundPayment { payment_id, .. }) => payment_id,
 					_ => return None
 				};
 				let res = self.initiate_async_payment(&invoice, payment_id);
 				handle_pay_invoice_res!(res, invoice, self.logger);
 			},
 			OffersMessage::InvoiceError(invoice_error) => {
 				let payment_hash = match context {
-					Some(OffersContext::InboundPayment { payment_hash, nonce, hmac }) => {
-						match payment_hash.verify_for_offer_payment(hmac, nonce, expanded_key) {
-							Ok(_) => Some(payment_hash),
-							Err(_) => None,
-						}
-					},
+					Some(OffersContext::InboundPayment { payment_hash }) => Some(payment_hash),
 					_ => None,
 				};
 
 				let logger = WithContext::from(&self.logger, None, None, payment_hash);
 				log_trace!(logger, "Received invoice_error: {}", invoice_error);
 
 				match context {
-					Some(OffersContext::OutboundPayment { payment_id, nonce, hmac: Some(hmac) }) => {
-						if let Ok(()) = payment_id.verify_for_offer_payment(hmac, nonce, expanded_key) {
-							self.abandon_payment_with_reason(
-								payment_id, PaymentFailureReason::InvoiceRequestRejected,
-							);
-						}
+					Some(OffersContext::OutboundPayment { payment_id, .. }) => {
+						self.abandon_payment_with_reason(
+							payment_id, PaymentFailureReason::InvoiceRequestRejected,
+						);
 					},
 					_ => {},
 				}
@@ -14122,15 +14051,18 @@ where
 	fn handle_release_held_htlc(&self, _message: ReleaseHeldHtlc, _context: AsyncPaymentsContext) {
 		#[cfg(async_payments)]
 		{
-			if let Ok(payment_id) = self.flow.verify_outbound_async_payment_context(_context) {
-				if let Err(e) = self.send_payment_for_static_invoice(payment_id) {
-					log_trace!(
-						self.logger,
-						"Failed to release held HTLC with payment id {}: {:?}",
-						payment_id,
-						e
-					);
-				}
+			let payment_id = match _context {
+				AsyncPaymentsContext::OutboundPayment { payment_id } => payment_id,
+				_ => return,
+			};
+
+			if let Err(e) = self.send_payment_for_static_invoice(payment_id) {
+				log_trace!(
+					self.logger,
+					"Failed to release held HTLC with payment id {}: {:?}",
+					payment_id,
+					e
+				);
 			}
 		}
 	}