Enforce Trampoline constraints

Trampoline onion amount/CLTV may not exceed 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,11 +2336,46 @@ 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(0x2000 | 26, &[0; 0]);
+			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]

lightning/src/ln/onion_payment.rs

@@ -61,6 +61,16 @@ fn check_blinded_forward(
 	Ok((amt_to_forward, outgoing_cltv_value))
 }
 
+fn check_trampoline_sanity(outer_hop_data: &msgs::InboundTrampolineEntrypointPayload, trampoline_cltv_value: u32, trampoline_amount: u64) -> Result<(), ()> {
+	if outer_hop_data.outgoing_cltv_value < trampoline_cltv_value {
+		return Err(());
+	}
+	if outer_hop_data.amt_to_forward < trampoline_amount {
+		return Err(());
+	}
+	Ok(())
+}
+
 enum RoutingInfo {
 	Direct {
 		short_channel_id: u64,
@@ -121,7 +131,15 @@ pub(super) fn create_fwd_pending_htlc_info(
 				err_code: 0x4000 | 22,
 				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_sanity(outer_hop_data, next_trampoline_hop_data.outgoing_cltv_value, next_trampoline_hop_data.amt_to_forward).map_err(|()| {
+				// The Trampoline onion's amt and CLTV values cannot exceed the outer onion's
+				InboundHTLCErr {
+					msg: "Underflow calculating outbound amount or CLTV value for Trampoline forward",
+					err_code: 0x2000 | 26,
+					err_data: Vec::new(),
+				}
+			})?;
 			(
 				RoutingInfo::Trampoline {
 					next_trampoline: next_trampoline_hop_data.next_trampoline,
@@ -136,7 +154,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(|()| {
@@ -148,6 +166,15 @@ pub(super) fn create_fwd_pending_htlc_info(
 					err_data: vec![0; 32],
 				}
 			})?;
+			check_trampoline_sanity(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 {
+					msg: "Underflow calculating outbound amount or CLTV value for Trampoline forward",
+					err_code: INVALID_ONION_BLINDING,
+					err_data: vec![0; 32],
+				}
+			})?;
 			(
 				RoutingInfo::Trampoline {
 					next_trampoline: next_trampoline_hop_data.next_trampoline,
@@ -266,14 +293,25 @@ 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_sanity(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
+				InboundHTLCErr {
+					msg: "Underflow calculating skimmable amount or CLTV value for Trampoline receive",
+					err_code: 0x2000 | 26,
+					err_data: Vec::new(),
+				}
+			})?;
 			(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,
@@ -290,6 +328,15 @@ pub(super) fn create_recv_pending_htlc_info(
 						msg: "Amount or cltv_expiry violated blinded payment constraints within Trampoline onion",
 					}
 				})?;
+			check_trampoline_sanity(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 {
+					msg: "Underflow calculating skimmable amount or CLTV value for Trampoline receive",
+					err_code: INVALID_ONION_BLINDING,
+					err_data: vec![0; 32],
+				}
+			})?;
 			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),

lightning/src/ln/onion_utils.rs

@@ -1580,6 +1580,7 @@ impl HTLCFailReason {
 		else if failure_code == 21 { debug_assert!(data.is_empty()) }
 		else if failure_code == 22 | PERM { debug_assert!(data.len() <= 11) }
 		else if failure_code == 23 { debug_assert!(data.is_empty()) }
+		else if failure_code == 26 | NODE { debug_assert!(data.is_empty()) }
 		else if failure_code == INVALID_ONION_BLINDING { debug_assert_eq!(data.len(), 32) }
 		else if failure_code & BADONION != 0 {
 			// We set some bogus BADONION failure codes in test, so ignore unknown ones.

lightning/src/util/errors.rs

@@ -123,6 +123,7 @@ pub(crate) fn get_onion_error_description(error_code: u16) -> (&'static str, &'s
 	match error_code {
 		_c if _c == PERM|1 => ("The realm byte was not understood by the processing node", "invalid_realm"),
 		_c if _c == NODE|2 => ("Node indicated temporary node failure", "temporary_node_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"),
 		_c if _c == PERM|NODE|2 => ("Node indicated permanent node failure", "permanent_node_failure"),
 		_c if _c == PERM|NODE|3 => ("Node indicated the required node feature is missing in the onion", "required_node_feature_missing"),
 		_c if _c == BADONION|PERM|4 => ("Node indicated the version by is not understood", "invalid_onion_version"),