Correct blinded path forwarding CLTV expiry check

The `PaymentConstraints::max_cltv_expiry` field exists to ensure
a blinded path expires across the entire path at once - once the
path is expired it will be rejected by the introduction node rather
than traversing the entire path and failing at the destination.

This was broken by the fact that we were checking the outgoing CLTV
value rather than the incoming one, which admittedly isn't clear in
the spec but is somewhat implied. Here we fix this, updating a test
which was actually (kinda) exploiting this privacy loss rather than
allowing the HTLC to fail at the introduction node.

This, of course, does not risk funds loss as our own CLTV policy is
still enforced on top. The only impact it could have is a recipient
which was relying on blinded path expiry to avoid some cost (e.g.
LSPS5 node wakeup cost) involved in receiving an HTLC they
ultimately fail, though I'm not aware of any practical deployment
where that is a concern.

Reported by Jordan Mecom of Block's Security Team

Author: TheBlueMatt

lightning/src/ln/async_payments_tests.rs

@@ -1886,8 +1886,9 @@ fn expired_static_invoice_payment_path() {
 		}
 	};
 
-	// Mine a bunch of blocks so the hardcoded path's `max_cltv_expiry` is expired at the recipient's
-	// end by the time the payment arrives.
+	// Mine a bunch of blocks on the sender so the hardcoded path's `max_cltv_expiry` is expired.
+	// Note that the path expires "all at once" and will be invalid at the intro point so will be
+	// rejected before it reaches the destination.
 	let min_cltv_expiry_delta = test_default_channel_config().channel_config.cltv_expiry_delta;
 	connect_blocks(
 		&nodes[0],
@@ -1902,7 +1903,6 @@ fn expired_static_invoice_payment_path() {
 		&nodes[1],
 		final_max_cltv_expiry
 			- nodes[1].best_block_info().1
-			// Don't expire the path for nodes[1]
 			- min_cltv_expiry_delta as u32
 			- HTLC_FAIL_BACK_BUFFER
 			- LATENCY_GRACE_PERIOD_BLOCKS
@@ -1939,18 +1939,17 @@ fn expired_static_invoice_payment_path() {
 	let payment_hash = extract_payment_hash(&ev);
 	check_added_monitors(&nodes[0], 1);
 
-	let route: &[&[&Node]] = &[&[&nodes[1], &nodes[2]]];
-	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev)
-		.without_claimable_event()
-		.expect_failure(HTLCHandlingFailureType::Receive { payment_hash })
-		.with_dummy_tlvs(&[DummyTlvs::default(); DEFAULT_PAYMENT_DUMMY_HOPS]);
-	do_pass_along_path(args);
-	fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1], &nodes[2]], false);
-	nodes[2].logger.assert_log_contains(
-		"lightning::ln::channelmanager",
-		"violated blinded payment constraints",
-		1,
+	let payment_event = SendEvent::from_event(ev);
+	nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+	check_added_monitors(&nodes[1], 0);
+	do_commitment_signed_dance(&nodes[1], &nodes[0], &payment_event.commitment_msg, false, true);
+	expect_and_process_pending_htlcs(&nodes[1], false);
+	expect_htlc_handling_failed_destinations!(
+		nodes[1].node.get_and_clear_pending_events(),
+		&[HTLCHandlingFailureType::InvalidOnion]
 	);
+	check_added_monitors(&nodes[1], 1);
+	fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1]], false);
 }
 
 #[cfg_attr(feature = "std", ignore)]

lightning/src/ln/onion_payment.rs

@@ -66,7 +66,7 @@ fn check_blinded_forward(
 	let outgoing_cltv_value = inbound_cltv_expiry.checked_sub(
 		payment_relay.cltv_expiry_delta as u32
 	).ok_or(())?;
-	check_blinded_payment_constraints(inbound_amt_msat, outgoing_cltv_value, payment_constraints)?;
+	check_blinded_payment_constraints(inbound_amt_msat, inbound_cltv_expiry, payment_constraints)?;
 
 	if features.requires_unknown_bits_from(&BlindedHopFeatures::empty()) { return Err(()) }
 	Ok((amt_to_forward, outgoing_cltv_value))