Enforce Trampoline constraints

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

Author: arik-so

lightning/src/ln/blinded_payment_tests.rs

@@ -2061,7 +2061,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: 39,
 					},
 				],
 				hops: carol_blinded_hops,
@@ -2175,8 +2175,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;
@@ -2246,7 +2245,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,
 				}
@@ -2257,8 +2256,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,
@@ -2270,6 +2269,8 @@ fn test_trampoline_unblinded_receive() {
 		route_params: None,
 	};
 
+	// outer 56
+
 	nodes[0].node.send_payment_with_route(route.clone(), payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0)).unwrap();
 
 	let replacement_onion = {
@@ -2285,12 +2286,13 @@ fn test_trampoline_unblinded_receive() {
 		// 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,15 +2336,50 @@ 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(HTLCDestination::FailedPayment { 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() {
+	do_test_trampoline_unblinded_receive(true);
+	do_test_trampoline_unblinded_receive(false);
 }
 
 #[test]
-fn test_trampoline_forward_rejection() {
+fn test_trampoline_constraint_enforcement() {
 	const TOTAL_NODE_COUNT: usize = 3;
 
 	let chanmon_cfgs = create_chanmon_cfgs(TOTAL_NODE_COUNT);
@@ -2435,7 +2472,7 @@ fn test_trampoline_forward_rejection() {
 	let args = PassAlongPathArgs::new(&nodes[0], route, amt_msat, payment_hash, first_message_event)
 		.with_payment_preimage(payment_preimage)
 		.without_claimable_event()
-		.expect_failure(HTLCDestination::FailedPayment { payment_hash });
+		.expect_failure(HTLCDestination::InvalidOnion);
 	do_pass_along_path(args);
 
 	{
@@ -2455,7 +2492,7 @@ fn test_trampoline_forward_rejection() {
 	{
 		// Expect UnknownNextPeer error while we are unable to route forwarding Trampoline payments.
 		let payment_failed_conditions = PaymentFailedConditions::new()
-			.expected_htlc_error_data(LocalHTLCFailureReason::UnknownNextPeer, &[0; 0]);
+			.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);
 	}
 }

lightning/src/ln/onion_payment.rs

@@ -19,6 +19,7 @@ use crate::ln::onion_utils;
 use crate::ln::onion_utils::{HTLCFailReason, ONION_DATA_LEN, LocalHTLCFailureReason};
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::logger::Logger;
+use crate::util::ser::Writeable;
 
 #[allow(unused_imports)]
 use crate::prelude::*;
@@ -69,6 +70,16 @@ 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);
+	}
+	if outer_hop_data.multipath_trampoline_data.as_ref().map_or(outer_hop_data.amt_to_forward, |mtd| mtd.total_msat) < trampoline_amount {
+		return Err(LocalHTLCFailureReason::FinalIncorrectHTLCAmount);
+	}
+	Ok(())
+}
+
 enum RoutingInfo {
 	Direct {
 		short_channel_id: u64,
@@ -129,7 +140,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,
@@ -144,7 +173,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(|()| {
@@ -156,6 +185,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,
@@ -274,14 +312,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,
@@ -298,6 +357,15 @@ pub(super) fn create_recv_pending_htlc_info(
 						msg: "Amount or cltv_expiry violated blinded payment constraints within Trampoline onion",
 					}
 				})?;
+			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",
+				}
+			})?;
 			let payment_data = msgs::FinalOnionHopData { payment_secret, total_msat };
 			(Some(payment_data), keysend_preimage, custom_tlvs,
 				sender_intended_htlc_amt_msat, cltv_expiry_height, None, Some(payment_context),
@@ -583,7 +651,54 @@ where
 				outgoing_cltv_value
 			})
 		}
-		onion_utils::Hop::TrampolineForward { next_trampoline_hop_data: msgs::InboundTrampolineForwardPayload { amt_to_forward, outgoing_cltv_value, next_trampoline }, trampoline_shared_secret, incoming_trampoline_public_key, .. } => {
+		onion_utils::Hop::TrampolineForward { ref outer_hop_data, next_trampoline_hop_data: msgs::InboundTrampolineForwardPayload { amt_to_forward, outgoing_cltv_value, next_trampoline }, outer_shared_secret, trampoline_shared_secret, incoming_trampoline_public_key, .. } => {
+			if let Err(reason) = check_trampoline_onion_constraints(outer_hop_data, outgoing_cltv_value, amt_to_forward) {
+				let mut data = Vec::new();
+				match reason {
+					LocalHTLCFailureReason::FinalIncorrectCLTVExpiry => {
+						outer_hop_data.outgoing_cltv_value.write(&mut data).unwrap();
+					}
+					LocalHTLCFailureReason::FinalIncorrectHTLCAmount => {
+						outer_hop_data.amt_to_forward.write(&mut data).unwrap();
+					}
+					_ => unreachable!()
+				}
+				return encode_relay_error("Underflow calculating outbound amount or CLTV value for Trampoline forward",
+					reason, outer_shared_secret.secret_bytes(), Some(trampoline_shared_secret.secret_bytes()), &data);
+			}
+			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,
+			})
+		}
+		onion_utils::Hop::TrampolineBlindedForward { ref outer_hop_data, 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]);
+				}
+			};
+			if let Err(reason) = check_trampoline_onion_constraints(outer_hop_data, outgoing_cltv_value, amt_to_forward) {
+				let mut data = Vec::new();
+				match reason {
+					LocalHTLCFailureReason::FinalIncorrectCLTVExpiry => {
+						outer_hop_data.outgoing_cltv_value.write(&mut data).unwrap();
+					}
+					LocalHTLCFailureReason::FinalIncorrectHTLCAmount => {
+						outer_hop_data.amt_to_forward.write(&mut data).unwrap();
+					}
+					_ => unreachable!()
+				}
+				return encode_relay_error("Underflow calculating outbound amount or CLTV value for Trampoline forward",
+					reason, outer_shared_secret.secret_bytes(), Some(trampoline_shared_secret.secret_bytes()), &data);
+			}
 			let next_trampoline_packet_pubkey = onion_utils::next_hop_pubkey(secp_ctx,
 				incoming_trampoline_public_key, &trampoline_shared_secret.secret_bytes());
 			Some(NextPacketDetails {

lightning/src/ln/onion_utils.rs

@@ -1606,6 +1606,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
+	/// later.
+	TemporaryTrampolineFailure,
+	/// The amount or CLTV expiry were insufficient to route the payment to the next Trampoline node.
+	TrampolineFeeOrExpiryInsufficient,
+	/// The specified next Trampoline node cannot be reached from our node.
+	UnknownNextTrampoline,
 }
 
 impl LocalHTLCFailureReason {
@@ -1647,6 +1654,9 @@ impl LocalHTLCFailureReason {
 			Self::InvalidOnionPayload | Self::InvalidTrampolinePayload => PERM | 22,
 			Self::MPPTimeout => 23,
 			Self::InvalidOnionBlinding => BADONION | PERM | 24,
+			Self::TemporaryTrampolineFailure => NODE | 25,
+			Self::TrampolineFeeOrExpiryInsufficient => NODE | 26,
+			Self::UnknownNextTrampoline => PERM | 27,
 			Self::UnknownFailureCode { code } => *code,
 		}
 	}
@@ -1707,6 +1717,12 @@ impl From<u16> for LocalHTLCFailureReason {
 			LocalHTLCFailureReason::MPPTimeout
 		} else if value == (BADONION | PERM | 24) {
 			LocalHTLCFailureReason::InvalidOnionBlinding
+		} else if value == (NODE | 25) {
+			LocalHTLCFailureReason::TemporaryTrampolineFailure
+		} else if value == (NODE | 26) {
+			LocalHTLCFailureReason::TrampolineFeeOrExpiryInsufficient
+		} else if value == (PERM | 27) {
+			LocalHTLCFailureReason::UnknownNextTrampoline
 		} else {
 			LocalHTLCFailureReason::UnknownFailureCode { code: value }
 		}
@@ -1759,6 +1775,9 @@ impl_writeable_tlv_based_enum!(LocalHTLCFailureReason,
 	(81, HTLCMinimum) => {},
 	(83, HTLCMaximum) => {},
 	(85, PeerOffline) => {},
+	(87, TemporaryTrampolineFailure) => {},
+	(89, TrampolineFeeOrExpiryInsufficient) => {},
+	(91, UnknownNextTrampoline) => {},
 );
 
 #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
@@ -1915,6 +1934,9 @@ impl HTLCFailReason {
 					debug_assert!(false, "Unknown failure code: {}", code)
 				}
 			},
+			LocalHTLCFailureReason::TemporaryTrampolineFailure => debug_assert!(data.is_empty()),
+			LocalHTLCFailureReason::TrampolineFeeOrExpiryInsufficient => debug_assert_eq!(data.len(), 10),
+			LocalHTLCFailureReason::UnknownNextTrampoline => debug_assert!(data.is_empty()),
 		}
 
 		Self(HTLCFailReasonRepr::Reason { data, failure_reason })

lightning/src/util/errors.rs

@@ -145,6 +145,8 @@ pub(crate) fn get_onion_error_description(error_code: u16) -> (&'static str, &'s
 		_c if _c == 21 => ("Node indicated the CLTV expiry in the HTLC is too far in the future", "expiry_too_far"),
 		_c if _c == PERM|22 => ("Node indicated that the decrypted onion per-hop payload was not understood by it or is incomplete", "invalid_onion_payload"),
 		_c if _c == 23 => ("The final node indicated the complete amount of the multi-part payment was not received within a reasonable time", "mpp_timeout"),
+		_c if _c == NODE|25 => ("The Trampoline node was unable to relay the payment to the subsequent Trampoline node.", "temporary_trampoline_failure"),
+		_c if _c == NODE|26 => ("Node indicated the fee amount or CLTV value was below that required by the Trampoline node", "trampoline_fee_or_expiry_insufficient"),
 		_ => ("Unknown", ""),
 	}
 }