Enforce Trampoline constraints

Ensure that the Trampolin onion's amount and CLTV values does not exceed
the limitations imposed by the outer onion.

Co-authored-by: Arik Sosman <git@arik.io>

Author: a-mpch

lightning/src/ln/blinded_payment_tests.rs

@@ -2086,7 +2086,7 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
 						pubkey: carol_node_id,
 						node_features: Features::empty(),
 						fee_msat: amt_msat,
-						cltv_expiry_delta: 24,
+						cltv_expiry_delta: 104,
 					},
 				],
 				hops: carol_blinded_hops,
@@ -2206,8 +2206,7 @@ fn test_trampoline_single_hop_receive() {
 	do_test_trampoline_single_hop_receive(false);
 }
 
-#[test]
-fn test_trampoline_unblinded_receive() {
+fn do_test_trampoline_unblinded_receive(underpay: bool) {
 	// Simulate a payment of A (0) -> B (1) -> C(Trampoline) (2)
 
 	const TOTAL_NODE_COUNT: usize = 3;
@@ -2277,7 +2276,7 @@ fn test_trampoline_unblinded_receive() {
 					node_features: NodeFeatures::empty(),
 					short_channel_id: bob_carol_scid,
 					channel_features: ChannelFeatures::empty(),
-					fee_msat: 0,
+					fee_msat: 0, // no routing fees because it's the final hop
 					cltv_expiry_delta: 48,
 					maybe_announced_channel: false,
 				}
@@ -2288,8 +2287,8 @@ fn test_trampoline_unblinded_receive() {
 					TrampolineHop {
 						pubkey: carol_node_id,
 						node_features: Features::empty(),
-						fee_msat: amt_msat,
-						cltv_expiry_delta: 24,
+						fee_msat: 0,
+						cltv_expiry_delta: 72,
 					},
 				],
 				hops: carol_blinded_hops,
@@ -2301,6 +2300,8 @@ fn test_trampoline_unblinded_receive() {
 		route_params: None,
 	};
 
+	let payment_id = PaymentId(payment_hash.0);
+
 	nodes[0].node.send_payment_with_route(route.clone(), payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0)).unwrap();
 
 	let replacement_onion = {
@@ -2311,17 +2312,18 @@ fn test_trampoline_unblinded_receive() {
 		let recipient_onion_fields = RecipientOnionFields::spontaneous_empty();
 
 		let blinded_tail = route.paths[0].blinded_tail.clone().unwrap();
-		let (mut trampoline_payloads, outer_total_msat, outer_starting_htlc_offset) = onion_utils::build_trampoline_onion_payloads(&blinded_tail, amt_msat, &recipient_onion_fields, 32, &None).unwrap();
 
+		let (mut trampoline_payloads, outer_total_msat, outer_starting_htlc_offset) = onion_utils::build_trampoline_onion_payloads(&blinded_tail, amt_msat, &recipient_onion_fields, 32, &None).unwrap();
 		// pop the last dummy hop
 		trampoline_payloads.pop();
+		let replacement_payload_amount = if underpay { amt_msat * 2 } else { amt_msat };
 
 		trampoline_payloads.push(msgs::OutboundTrampolinePayload::Receive {
 			payment_data: Some(msgs::FinalOnionHopData {
 				payment_secret,
-				total_msat: amt_msat,
+				total_msat: replacement_payload_amount,
 			}),
-			sender_intended_htlc_amt_msat: amt_msat,
+			sender_intended_htlc_amt_msat: replacement_payload_amount,
 			cltv_expiry_height: 104,
 		});
 
@@ -2334,10 +2336,20 @@ fn test_trampoline_unblinded_receive() {
 			None,
 		).unwrap();
 
-		// Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
-		// this and won't be able to decode the fulfill hold times.
-		let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
+		// Get the original inner session private key that the ChannelManager generated so we can
+		// re-use it for the outer session private key. This way HMAC validation in attributable
+		// errors do not makes the test fail.
+		let mut orig_inner_priv_bytes = [0u8; 32];
+		nodes[0].node.test_modify_pending_payment(&payment_id, |pmt| {
+			if let crate::ln::outbound_payment::PendingOutboundPayment::Retryable { session_privs, .. } = pmt {
+				orig_inner_priv_bytes = *session_privs.iter().next().unwrap();
+			}
+		});
+		let inner_session_priv = SecretKey::from_slice(&orig_inner_priv_bytes).unwrap();
 
+		// Derive the outer session private key from the inner one.
+		let outer_session_priv_hash = Sha256::hash(&inner_session_priv.secret_bytes());
+		let outer_session_priv = SecretKey::from_slice(&outer_session_priv_hash.to_byte_array()).unwrap();
 		let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();
 		let outer_onion_keys = onion_utils::construct_onion_keys(&secp_ctx, &route.clone().paths[0], &outer_session_priv);
 		let outer_packet = onion_utils::construct_onion_packet(
@@ -2367,11 +2379,51 @@ fn test_trampoline_unblinded_receive() {
 	});
 
 	let route: &[&Node] = &[&nodes[1], &nodes[2]];
-	let args = PassAlongPathArgs::new(&nodes[0], route, amt_msat, payment_hash, first_message_event)
-		.with_payment_secret(payment_secret);
+	let args = PassAlongPathArgs::new(&nodes[0], route, amt_msat, payment_hash, first_message_event);
+
+	let args = if underpay {
+		args.with_payment_preimage(payment_preimage)
+				.without_claimable_event()
+				.expect_failure(HTLCHandlingFailureType::Receive { payment_hash })
+	} else {
+			args.with_payment_secret(payment_secret)
+		};
+
 	do_pass_along_path(args);
 
-	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
+	if underpay {
+		{
+			let unblinded_node_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
+			nodes[1].node.handle_update_fail_htlc(
+				nodes[2].node.get_our_node_id(), &unblinded_node_updates.update_fail_htlcs[0]
+			);
+			do_commitment_signed_dance(&nodes[1], &nodes[2], &unblinded_node_updates.commitment_signed, true, false);
+		}
+		{
+			let unblinded_node_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+			nodes[0].node.handle_update_fail_htlc(
+				nodes[1].node.get_our_node_id(), &unblinded_node_updates.update_fail_htlcs[0]
+			);
+			do_commitment_signed_dance(&nodes[0], &nodes[1], &unblinded_node_updates.commitment_signed, false, false);
+		}
+		{
+		    let payment_failed_conditions = PaymentFailedConditions::new()
+				    .expected_htlc_error_data(LocalHTLCFailureReason::FinalIncorrectHTLCAmount, &[0, 0, 0, 0, 0, 0, 3, 232]);
+			  expect_payment_failed_conditions(&nodes[0], payment_hash, false, payment_failed_conditions);
+		}
+	} else {
+		claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
+	}
+}
+
+#[test]
+fn test_trampoline_unblinded_receive_underpay() {
+    do_test_trampoline_unblinded_receive(true);
+}
+
+#[test]
+fn test_trampoline_unblinded_receive_normal() {
+    do_test_trampoline_unblinded_receive(false);
 }
 
 #[test]

lightning/src/ln/channelmanager.rs

@@ -5099,7 +5099,7 @@ where
 		)
 	}
 
-	#[cfg(all(test, async_payments))]
+	#[cfg(all(test))]
 	pub(crate) fn test_modify_pending_payment<Fn>(&self, payment_id: &PaymentId, mut callback: Fn)
 	where
 		Fn: FnMut(&mut PendingOutboundPayment),

lightning/src/ln/onion_payment.rs

@@ -22,6 +22,7 @@ use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::BlindedHopFeatures;
 use crate::types::payment::PaymentHash;
 use crate::util::logger::Logger;
+use crate::util::ser::Writeable;
 
 #[allow(unused_imports)]
 use crate::prelude::*;
@@ -74,6 +75,24 @@ fn check_blinded_forward(
 	Ok((amt_to_forward, outgoing_cltv_value))
 }
 
+fn check_trampoline_onion_constraints(
+	outer_hop_data: &msgs::InboundTrampolineEntrypointPayload, trampoline_cltv_value: u32,
+	trampoline_amount: u64,
+) -> Result<(), LocalHTLCFailureReason> {
+	if outer_hop_data.outgoing_cltv_value < trampoline_cltv_value {
+		return Err(LocalHTLCFailureReason::FinalIncorrectCLTVExpiry);
+	}
+	let outgoing_amount = outer_hop_data
+		.multipath_trampoline_data
+		.as_ref()
+		.map_or(outer_hop_data.amt_to_forward, |mtd| mtd.total_msat);
+	if outgoing_amount < trampoline_amount {
+		return Err(LocalHTLCFailureReason::FinalIncorrectHTLCAmount);
+	}
+
+	Ok(())
+}
+
 enum RoutingInfo {
 	Direct {
 		short_channel_id: u64,
@@ -135,7 +154,25 @@ pub(super) fn create_fwd_pending_htlc_info(
 				reason: LocalHTLCFailureReason::InvalidOnionPayload,
 				err_data: Vec::new(),
 			}),
-		onion_utils::Hop::TrampolineForward { next_trampoline_hop_data, next_trampoline_hop_hmac, new_trampoline_packet_bytes, trampoline_shared_secret, .. } => {
+		onion_utils::Hop::TrampolineForward { ref outer_hop_data, next_trampoline_hop_data, next_trampoline_hop_hmac, new_trampoline_packet_bytes, trampoline_shared_secret, .. } => {
+			check_trampoline_onion_constraints(outer_hop_data, next_trampoline_hop_data.outgoing_cltv_value, next_trampoline_hop_data.amt_to_forward).map_err(|reason| {
+				let mut err_data = Vec::new();
+				match reason {
+					LocalHTLCFailureReason::FinalIncorrectCLTVExpiry => {
+						outer_hop_data.outgoing_cltv_value.write(&mut err_data).unwrap();
+					}
+					LocalHTLCFailureReason::FinalIncorrectHTLCAmount => {
+						outer_hop_data.amt_to_forward.write(&mut err_data).unwrap();
+					}
+					_ => unreachable!()
+				}
+				// The Trampoline onion's amt and CLTV values cannot exceed the outer onion's
+				InboundHTLCErr {
+					reason,
+					err_data,
+					msg: "Underflow calculating outbound amount or CLTV value for Trampoline forward",
+				}
+			})?;
 			(
 				RoutingInfo::Trampoline {
 					next_trampoline: next_trampoline_hop_data.next_trampoline,
@@ -150,7 +187,7 @@ pub(super) fn create_fwd_pending_htlc_info(
 				None
 			)
 		},
-		onion_utils::Hop::TrampolineBlindedForward { outer_hop_data, next_trampoline_hop_data, next_trampoline_hop_hmac, new_trampoline_packet_bytes, trampoline_shared_secret, .. } => {
+		onion_utils::Hop::TrampolineBlindedForward { ref outer_hop_data, next_trampoline_hop_data, next_trampoline_hop_hmac, new_trampoline_packet_bytes, trampoline_shared_secret, .. } => {
 			let (amt_to_forward, outgoing_cltv_value) = check_blinded_forward(
 				msg.amount_msat, msg.cltv_expiry, &next_trampoline_hop_data.payment_relay, &next_trampoline_hop_data.payment_constraints, &next_trampoline_hop_data.features
 			).map_err(|()| {
@@ -162,6 +199,15 @@ pub(super) fn create_fwd_pending_htlc_info(
 					err_data: vec![0; 32],
 				}
 			})?;
+			check_trampoline_onion_constraints(outer_hop_data, outgoing_cltv_value, amt_to_forward).map_err(|_| {
+				// The Trampoline onion's amt and CLTV values cannot exceed the outer onion's, but
+				// we're inside a blinded path
+				InboundHTLCErr {
+					reason: LocalHTLCFailureReason::InvalidOnionBlinding,
+					err_data: vec![0; 32],
+					msg: "Underflow calculating outbound amount or CLTV value for Trampoline forward",
+				}
+			})?;
 			(
 				RoutingInfo::Trampoline {
 					next_trampoline: next_trampoline_hop_data.next_trampoline,
@@ -281,14 +327,35 @@ pub(super) fn create_recv_pending_htlc_info(
 			 intro_node_blinding_point.is_none(), true, invoice_request)
 		}
 		onion_utils::Hop::TrampolineReceive {
+			ref outer_hop_data,
 			trampoline_hop_data: msgs::InboundOnionReceivePayload {
 				payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat,
 				cltv_expiry_height, payment_metadata, ..
 			}, ..
-		} =>
+		} => {
+			check_trampoline_onion_constraints(outer_hop_data, cltv_expiry_height, sender_intended_htlc_amt_msat).map_err(|reason| {
+				let mut err_data = Vec::new();
+				match reason {
+					LocalHTLCFailureReason::FinalIncorrectCLTVExpiry => {
+						outer_hop_data.outgoing_cltv_value.write(&mut err_data).unwrap();
+					}
+					LocalHTLCFailureReason::FinalIncorrectHTLCAmount => {
+						outer_hop_data.amt_to_forward.write(&mut err_data).unwrap();
+					}
+					_ => unreachable!()
+				}
+				// The Trampoline onion's amt and CLTV values cannot exceed the outer onion's
+				InboundHTLCErr {
+					reason,
+					err_data,
+					msg: "Underflow calculating skimmable amount or CLTV value for Trampoline receive",
+				}
+			})?;
 			(payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat,
-				cltv_expiry_height, payment_metadata, None, false, keysend_preimage.is_none(), None),
+				cltv_expiry_height, payment_metadata, None, false, keysend_preimage.is_none(), None)
+		}
 		onion_utils::Hop::TrampolineBlindedReceive {
+			ref outer_hop_data,
 			trampoline_hop_data: msgs::InboundOnionBlindedReceivePayload {
 				sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, payment_secret,
 				intro_node_blinding_point, payment_constraints, payment_context, keysend_preimage,
@@ -306,6 +373,15 @@ pub(super) fn create_recv_pending_htlc_info(
 					}
 				})?;
 			let payment_data = msgs::FinalOnionHopData { payment_secret, total_msat };
+			check_trampoline_onion_constraints(outer_hop_data, cltv_expiry_height, sender_intended_htlc_amt_msat).map_err(|_| {
+				// The Trampoline onion's amt and CLTV values cannot exceed the outer onion's, but
+				// we're inside a blinded path
+				InboundHTLCErr {
+					reason: LocalHTLCFailureReason::InvalidOnionBlinding,
+					err_data: vec![0; 32],
+					msg: "Underflow calculating skimmable amount or CLTV value for Trampoline receive",
+				}
+			})?;
 			(Some(payment_data), keysend_preimage, custom_tlvs,
 				sender_intended_htlc_amt_msat, cltv_expiry_height, None, Some(payment_context),
 				intro_node_blinding_point.is_none(), true, invoice_request)
@@ -602,6 +678,25 @@ where
 				outgoing_cltv_value,
 			})
 		}
+		onion_utils::Hop::TrampolineBlindedForward { next_trampoline_hop_data: msgs::InboundTrampolineBlindedForwardPayload { next_trampoline, ref payment_relay, ref payment_constraints, ref features, .. }, outer_shared_secret, trampoline_shared_secret, incoming_trampoline_public_key, .. } => {
+			let (amt_to_forward, outgoing_cltv_value) = match check_blinded_forward(
+				msg.amount_msat, msg.cltv_expiry, &payment_relay, &payment_constraints, &features
+			) {
+				Ok((amt, cltv)) => (amt, cltv),
+				Err(()) => {
+					return encode_relay_error("Underflow calculating outbound amount or cltv value for blinded forward",
+						LocalHTLCFailureReason::InvalidOnionBlinding, outer_shared_secret.secret_bytes(), Some(trampoline_shared_secret.secret_bytes()), &[0; 32]);
+				}
+			};
+			let next_trampoline_packet_pubkey = onion_utils::next_hop_pubkey(secp_ctx,
+				incoming_trampoline_public_key, &trampoline_shared_secret.secret_bytes());
+			Some(NextPacketDetails {
+				next_packet_pubkey: next_trampoline_packet_pubkey,
+				outgoing_connector: HopConnector::Trampoline(next_trampoline),
+				outgoing_amt_msat: amt_to_forward,
+				outgoing_cltv_value,
+			})
+		}
 		_ => None
 	};
 

lightning/src/ln/onion_utils.rs

@@ -1678,6 +1678,13 @@ pub enum LocalHTLCFailureReason {
 	HTLCMaximum,
 	/// The HTLC was failed because our remote peer is offline.
 	PeerOffline,
+	/// We have been unable to forward a payment to the next Trampoline node but may be able to
+	/// do it later.
+	TemporaryTrampolineFailure,
+	/// The amount or CLTV expiry were insufficient to route the payment to the next Trampoline.
+	TrampolineFeeOrExpiryInsufficient,
+	/// The specified next Trampoline node cannot be reached from our node.
+	UnkonwnNextTrampoline,
 }
 
 impl LocalHTLCFailureReason {
@@ -1718,6 +1725,9 @@ impl LocalHTLCFailureReason {
 			Self::InvalidOnionPayload | Self::InvalidTrampolinePayload => PERM | 22,
 			Self::MPPTimeout => 23,
 			Self::InvalidOnionBlinding => BADONION | PERM | 24,
+			Self::TemporaryTrampolineFailure => PERM | 25,
+			Self::TrampolineFeeOrExpiryInsufficient => PERM | 26,
+			Self::UnkonwnNextTrampoline => PERM | 27,
 			Self::UnknownFailureCode { code } => *code,
 		}
 	}
@@ -1852,6 +1862,9 @@ impl_writeable_tlv_based_enum!(LocalHTLCFailureReason,
 	(79, HTLCMinimum) => {},
 	(81, HTLCMaximum) => {},
 	(83, PeerOffline) => {},
+	(85, TemporaryTrampolineFailure) => {},
+	(87, TrampolineFeeOrExpiryInsufficient) => {},
+	(89, UnkonwnNextTrampoline) => {},
 );
 
 impl From<&HTLCFailReason> for HTLCHandlingFailureReason {
@@ -2018,6 +2031,11 @@ impl HTLCFailReason {
 					debug_assert!(false, "Unknown failure code: {}", code)
 				}
 			},
+			LocalHTLCFailureReason::TemporaryTrampolineFailure => debug_assert!(data.is_empty()),
+			LocalHTLCFailureReason::TrampolineFeeOrExpiryInsufficient => {
+				debug_assert!(data.is_empty())
+			},
+			LocalHTLCFailureReason::UnkonwnNextTrampoline => debug_assert!(data.is_empty()),
 		}
 
 		Self(HTLCFailReasonRepr::Reason { data, failure_reason })