Make ReceiveTlvs::authentication required

Having ReceiveTlvs::authentication be an Option makes for an odd
interface as ReceiveTlvs first needs to be constructed with it set to
None. Afterward, the HMAC can be constructed from it and set on it as
Some. Instead, introduce an UnauthenticatedReceiveTlvs struct with a
method to convert to an HMAC'ed ReceiveTlvs. This prevents ReceiveTlvs
from being in an invalid state.

Author: jkczyz

fuzz/src/invoice_request_deser.rs

@@ -12,11 +12,13 @@ use bitcoin::secp256k1::{self, Keypair, Parity, PublicKey, Secp256k1, SecretKey}
 use core::convert::TryFrom;
 use lightning::blinded_path::payment::{
 	BlindedPaymentPath, Bolt12OfferContext, ForwardTlvs, PaymentConstraints, PaymentContext,
-	PaymentForwardNode, PaymentRelay, ReceiveTlvs,
+	PaymentForwardNode, PaymentRelay, UnauthenticatedReceiveTlvs,
 };
 use lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA;
+use lightning::ln::inbound_payment::ExpandedKey;
 use lightning::offers::invoice::UnsignedBolt12Invoice;
 use lightning::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields};
+use lightning::offers::nonce::Nonce;
 use lightning::offers::offer::OfferId;
 use lightning::offers::parse::Bolt12SemanticError;
 use lightning::sign::EntropySource;
@@ -80,7 +82,9 @@ fn privkey(byte: u8) -> SecretKey {
 fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
 	invoice_request: &InvoiceRequest, secp_ctx: &Secp256k1<T>,
 ) -> Result<UnsignedBolt12Invoice, Bolt12SemanticError> {
+	let expanded_key = ExpandedKey::new([42; 32]);
 	let entropy_source = Randomness {};
+	let nonce = Nonce::from_entropy_source(&entropy_source);
 	let payment_context = PaymentContext::Bolt12Offer(Bolt12OfferContext {
 		offer_id: OfferId([42; 32]),
 		invoice_request: InvoiceRequestFields {
@@ -92,15 +96,15 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
 			human_readable_name: None,
 		},
 	});
-	let payee_tlvs = ReceiveTlvs {
+	let payee_tlvs = UnauthenticatedReceiveTlvs {
 		payment_secret: PaymentSecret([42; 32]),
 		payment_constraints: PaymentConstraints {
 			max_cltv_expiry: 1_000_000,
 			htlc_minimum_msat: 1,
 		},
 		payment_context,
-		authentication: None,
 	};
+	let payee_tlvs = payee_tlvs.authenticate(nonce, &expanded_key);
 	let intermediate_nodes = [PaymentForwardNode {
 		tlvs: ForwardTlvs {
 			short_channel_id: 43,
@@ -110,7 +114,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
 				fee_base_msat: 1,
 			},
 			payment_constraints: PaymentConstraints {
-				max_cltv_expiry: payee_tlvs.payment_constraints.max_cltv_expiry + 40,
+				max_cltv_expiry: payee_tlvs.tlvs().payment_constraints.max_cltv_expiry + 40,
 				htlc_minimum_msat: 100,
 			},
 			features: BlindedHopFeatures::empty(),

fuzz/src/refund_deser.rs

@@ -12,10 +12,12 @@ use bitcoin::secp256k1::{self, Keypair, PublicKey, Secp256k1, SecretKey};
 use core::convert::TryFrom;
 use lightning::blinded_path::payment::{
 	BlindedPaymentPath, Bolt12RefundContext, ForwardTlvs, PaymentConstraints, PaymentContext,
-	PaymentForwardNode, PaymentRelay, ReceiveTlvs,
+	PaymentForwardNode, PaymentRelay, UnauthenticatedReceiveTlvs,
 };
 use lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA;
+use lightning::ln::inbound_payment::ExpandedKey;
 use lightning::offers::invoice::UnsignedBolt12Invoice;
+use lightning::offers::nonce::Nonce;
 use lightning::offers::parse::Bolt12SemanticError;
 use lightning::offers::refund::Refund;
 use lightning::sign::EntropySource;
@@ -67,17 +69,19 @@ fn privkey(byte: u8) -> SecretKey {
 fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
 	refund: &Refund, signing_pubkey: PublicKey, secp_ctx: &Secp256k1<T>,
 ) -> Result<UnsignedBolt12Invoice, Bolt12SemanticError> {
+	let expanded_key = ExpandedKey::new([42; 32]);
 	let entropy_source = Randomness {};
+	let nonce = Nonce::from_entropy_source(&entropy_source);
 	let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {});
-	let payee_tlvs = ReceiveTlvs {
+	let payee_tlvs = UnauthenticatedReceiveTlvs {
 		payment_secret: PaymentSecret([42; 32]),
 		payment_constraints: PaymentConstraints {
 			max_cltv_expiry: 1_000_000,
 			htlc_minimum_msat: 1,
 		},
 		payment_context,
-		authentication: None,
 	};
+	let payee_tlvs = payee_tlvs.authenticate(nonce, &expanded_key);
 	let intermediate_nodes = [PaymentForwardNode {
 		tlvs: ForwardTlvs {
 			short_channel_id: 43,
@@ -87,7 +91,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
 				fee_base_msat: 1,
 			},
 			payment_constraints: PaymentConstraints {
-				max_cltv_expiry: payee_tlvs.payment_constraints.max_cltv_expiry + 40,
+				max_cltv_expiry: payee_tlvs.tlvs().payment_constraints.max_cltv_expiry + 40,
 				htlc_minimum_msat: 100,
 			},
 			features: BlindedHopFeatures::empty(),

lightning/src/blinded_path/payment.rs

@@ -20,7 +20,9 @@ use crate::io;
 use crate::io::Cursor;
 use crate::types::payment::PaymentSecret;
 use crate::ln::channel_state::CounterpartyForwardingInfo;
+use crate::ln::channelmanager::Verification;
 use crate::types::features::BlindedHopFeatures;
+use crate::ln::inbound_payment::ExpandedKey;
 use crate::ln::msgs::DecodeError;
 use crate::ln::onion_utils;
 use crate::offers::invoice_request::InvoiceRequestFields;
@@ -117,7 +119,7 @@ impl BlindedPaymentPath {
 		let blinding_secret = SecretKey::from_slice(&blinding_secret_bytes[..]).expect("RNG is busted");
 
 		let blinded_payinfo = compute_payinfo(
-			intermediate_nodes, &payee_tlvs, htlc_maximum_msat, min_final_cltv_expiry_delta
+			intermediate_nodes, &payee_tlvs.tlvs, htlc_maximum_msat, min_final_cltv_expiry_delta
 		)?;
 		Ok(Self {
 			inner_path: BlindedPath {
@@ -255,16 +257,43 @@ pub struct ForwardTlvs {
 
 /// Data to construct a [`BlindedHop`] for receiving a payment. This payload is custom to LDK and
 /// may not be valid if received by another lightning implementation.
+///
+/// Can only be constructed by calling [`UnauthenticatedReceiveTlvs::authenticate`].
 #[derive(Clone, Debug)]
 pub struct ReceiveTlvs {
+	/// The TLVs for which the HMAC in `authentication` is derived.
+	pub(crate) tlvs: UnauthenticatedReceiveTlvs,
+	/// An HMAC of `tlvs` along with a nonce used to construct it.
+	pub(crate) authentication: (Hmac<Sha256>, Nonce),
+}
+
+impl ReceiveTlvs {
+	/// Returns the underlying TLVs.
+	pub fn tlvs(&self) -> &UnauthenticatedReceiveTlvs {
+		&self.tlvs
+	}
+}
+
+/// An unauthenticated [`ReceiveTlvs`].
+#[derive(Clone, Debug)]
+pub struct UnauthenticatedReceiveTlvs {
 	/// Used to authenticate the sender of a payment to the receiver and tie MPP HTLCs together.
 	pub payment_secret: PaymentSecret,
 	/// Constraints for the receiver of this payment.
 	pub payment_constraints: PaymentConstraints,
 	/// Context for the receiver of this payment.
 	pub payment_context: PaymentContext,
-	/// An HMAC of `payment_context` along with a nonce used to construct it.
-	pub authentication: Option<(Hmac<Sha256>, Nonce)>,
+}
+
+impl UnauthenticatedReceiveTlvs {
+	/// Creates an authenticated [`ReceiveTlvs`], which includes an HMAC and the provide [`Nonce`]
+	/// that can be use later to verify it authenticity.
+	pub fn authenticate(self, nonce: Nonce, expanded_key: &ExpandedKey) -> ReceiveTlvs {
+		ReceiveTlvs {
+			authentication: (self.hmac_for_offer_payment(nonce, expanded_key), nonce),
+			tlvs: self,
+		}
+	}
 }
 
 /// Data to construct a [`BlindedHop`] for sending a payment over.
@@ -392,12 +421,23 @@ impl Writeable for ForwardTlvs {
 }
 
 impl Writeable for ReceiveTlvs {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+		encode_tlv_stream!(w, {
+			(12, self.tlvs.payment_constraints, required),
+			(65536, self.tlvs.payment_secret, required),
+			(65537, self.tlvs.payment_context, required),
+			(65539, self.authentication, required),
+		});
+		Ok(())
+	}
+}
+
+impl Writeable for UnauthenticatedReceiveTlvs {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 		encode_tlv_stream!(w, {
 			(12, self.payment_constraints, required),
 			(65536, self.payment_secret, required),
 			(65537, self.payment_context, required),
-			(65539, self.authentication, option),
 		});
 		Ok(())
 	}
@@ -443,10 +483,12 @@ impl Readable for BlindedPaymentTlvs {
 		} else {
 			if payment_relay.is_some() || features.is_some() { return Err(DecodeError::InvalidValue) }
 			Ok(BlindedPaymentTlvs::Receive(ReceiveTlvs {
-				payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
-				payment_constraints: payment_constraints.0.unwrap(),
-				payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
-				authentication,
+				tlvs: UnauthenticatedReceiveTlvs {
+					payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
+					payment_constraints: payment_constraints.0.unwrap(),
+					payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
+				},
+				authentication: authentication.ok_or(DecodeError::InvalidValue)?,
 			}))
 		}
 	}
@@ -489,7 +531,7 @@ pub(crate) fn amt_to_forward_msat(inbound_amt_msat: u64, payment_relay: &Payment
 }
 
 pub(super) fn compute_payinfo(
-	intermediate_nodes: &[PaymentForwardNode], payee_tlvs: &ReceiveTlvs,
+	intermediate_nodes: &[PaymentForwardNode], payee_tlvs: &UnauthenticatedReceiveTlvs,
 	payee_htlc_maximum_msat: u64, min_final_cltv_expiry_delta: u16,
 ) -> Result<BlindedPayInfo, ()> {
 	let mut curr_base_fee: u64 = 0;
@@ -613,7 +655,7 @@ impl_writeable_tlv_based!(Bolt12RefundContext, {});
 #[cfg(test)]
 mod tests {
 	use bitcoin::secp256k1::PublicKey;
-	use crate::blinded_path::payment::{Bolt12RefundContext, PaymentForwardNode, ForwardTlvs, ReceiveTlvs, PaymentConstraints, PaymentContext, PaymentRelay};
+	use crate::blinded_path::payment::{Bolt12RefundContext, PaymentForwardNode, ForwardTlvs, PaymentConstraints, PaymentContext, PaymentRelay, UnauthenticatedReceiveTlvs};
 	use crate::types::payment::PaymentSecret;
 	use crate::types::features::BlindedHopFeatures;
 	use crate::ln::functional_test_utils::TEST_FINAL_CLTV;
@@ -658,14 +700,13 @@ mod tests {
 			},
 			htlc_maximum_msat: u64::max_value(),
 		}];
-		let recv_tlvs = ReceiveTlvs {
+		let recv_tlvs = UnauthenticatedReceiveTlvs {
 			payment_secret: PaymentSecret([0; 32]),
 			payment_constraints: PaymentConstraints {
 				max_cltv_expiry: 0,
 				htlc_minimum_msat: 1,
 			},
 			payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
-			authentication: None,
 		};
 		let htlc_maximum_msat = 100_000;
 		let blinded_payinfo = super::compute_payinfo(&intermediate_nodes[..], &recv_tlvs, htlc_maximum_msat, 12).unwrap();
@@ -678,14 +719,13 @@ mod tests {
 
 	#[test]
 	fn compute_payinfo_1_hop() {
-		let recv_tlvs = ReceiveTlvs {
+		let recv_tlvs = UnauthenticatedReceiveTlvs {
 			payment_secret: PaymentSecret([0; 32]),
 			payment_constraints: PaymentConstraints {
 				max_cltv_expiry: 0,
 				htlc_minimum_msat: 1,
 			},
 			payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
-			authentication: None,
 		};
 		let blinded_payinfo = super::compute_payinfo(&[], &recv_tlvs, 4242, TEST_FINAL_CLTV as u16).unwrap();
 		assert_eq!(blinded_payinfo.fee_base_msat, 0);
@@ -735,14 +775,13 @@ mod tests {
 			},
 			htlc_maximum_msat: u64::max_value()
 		}];
-		let recv_tlvs = ReceiveTlvs {
+		let recv_tlvs = UnauthenticatedReceiveTlvs {
 			payment_secret: PaymentSecret([0; 32]),
 			payment_constraints: PaymentConstraints {
 				max_cltv_expiry: 0,
 				htlc_minimum_msat: 3,
 			},
 			payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
-			authentication: None,
 		};
 		let htlc_maximum_msat = 100_000;
 		let blinded_payinfo = super::compute_payinfo(&intermediate_nodes[..], &recv_tlvs, htlc_maximum_msat, TEST_FINAL_CLTV as u16).unwrap();
@@ -789,14 +828,13 @@ mod tests {
 			},
 			htlc_maximum_msat: u64::max_value()
 		}];
-		let recv_tlvs = ReceiveTlvs {
+		let recv_tlvs = UnauthenticatedReceiveTlvs {
 			payment_secret: PaymentSecret([0; 32]),
 			payment_constraints: PaymentConstraints {
 				max_cltv_expiry: 0,
 				htlc_minimum_msat: 1,
 			},
 			payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
-			authentication: None,
 		};
 		let htlc_minimum_msat = 3798;
 		assert!(super::compute_payinfo(&intermediate_nodes[..], &recv_tlvs, htlc_minimum_msat - 1, TEST_FINAL_CLTV as u16).is_err());
@@ -847,14 +885,13 @@ mod tests {
 			},
 			htlc_maximum_msat: 10_000
 		}];
-		let recv_tlvs = ReceiveTlvs {
+		let recv_tlvs = UnauthenticatedReceiveTlvs {
 			payment_secret: PaymentSecret([0; 32]),
 			payment_constraints: PaymentConstraints {
 				max_cltv_expiry: 0,
 				htlc_minimum_msat: 1,
 			},
 			payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
-			authentication: None,
 		};
 
 		let blinded_payinfo = super::compute_payinfo(&intermediate_nodes[..], &recv_tlvs, 10_000, TEST_FINAL_CLTV as u16).unwrap();