Verify blinded keysend payment secrets

If we're receiving a keysend to a blinded path, then we created the payment
secret within. Using our inbound_payment_key, we can decrypt the payment secret
bytes to get the payment's min_cltv_expiry_delta and min amount, to verify the
payment is valid. However, if we're receiving an MPP keysend *not* to a blinded
path, then we did not create the payment secret and shouldn't verify it since
it's only used to correlate MPP parts.

Therefore, store whether the payment secret is recipient-generated in our pending
inbound payment data so we know whether to verify it or not.

Author: valentinewallace

lightning/src/ln/blinded_payment_tests.rs

@@ -35,6 +35,10 @@ use crate::util::config::UserConfig;
 use crate::util::ser::WithoutLength;
 use crate::util::test_utils;
 use lightning_invoice::RawBolt11Invoice;
+#[cfg(async_payments)] use {
+	crate::ln::inbound_payment,
+	crate::types::payment::PaymentPreimage,
+};
 
 fn blinded_payment_path(
 	payment_secret: PaymentSecret, intro_node_min_htlc: u64, intro_node_max_htlc: u64,
@@ -1209,6 +1213,7 @@ fn conditionally_round_fwd_amt() {
 }
 
 #[test]
+#[cfg(async_payments)]
 fn blinded_keysend() {
 	let mut mpp_keysend_config = test_default_channel_config();
 	mpp_keysend_config.accept_mpp_keysend = true;
@@ -1219,8 +1224,15 @@ fn blinded_keysend() {
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
 	let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;
 
+	let inbound_payment_key = inbound_payment::ExpandedKey::new(
+		&nodes[2].keys_manager.get_inbound_payment_key_material()
+	);
+	let payment_secret = inbound_payment::create_for_spontaneous_payment(
+		&inbound_payment_key, None, u32::MAX, nodes[2].node.duration_since_epoch().as_secs(), None
+	).unwrap();
+
 	let amt_msat = 5000;
-	let (keysend_preimage, _, payment_secret) = get_payment_preimage_hash(&nodes[2], None, None);
+	let keysend_preimage = PaymentPreimage([42; 32]);
 	let route_params = get_blinded_route_parameters(amt_msat, payment_secret, 1,
 		1_0000_0000,
 		nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(),
@@ -1241,6 +1253,7 @@ fn blinded_keysend() {
 }
 
 #[test]
+#[cfg(async_payments)]
 fn blinded_mpp_keysend() {
 	let mut mpp_keysend_config = test_default_channel_config();
 	mpp_keysend_config.accept_mpp_keysend = true;
@@ -1254,8 +1267,15 @@ fn blinded_mpp_keysend() {
 	let chan_1_3 = create_announced_chan_between_nodes(&nodes, 1, 3);
 	let chan_2_3 = create_announced_chan_between_nodes(&nodes, 2, 3);
 
+	let inbound_payment_key = inbound_payment::ExpandedKey::new(
+		&nodes[3].keys_manager.get_inbound_payment_key_material()
+	);
+	let payment_secret = inbound_payment::create_for_spontaneous_payment(
+		&inbound_payment_key, None, u32::MAX, nodes[3].node.duration_since_epoch().as_secs(), None
+	).unwrap();
+
 	let amt_msat = 15_000_000;
-	let (keysend_preimage, _, payment_secret) = get_payment_preimage_hash(&nodes[3], None, None);
+	let keysend_preimage = PaymentPreimage([42; 32]);
 	let route_params = {
 		let pay_params = PaymentParameters::blinded(
 			vec![
@@ -1293,6 +1313,59 @@ fn blinded_mpp_keysend() {
 	);
 }
 
+#[test]
+#[cfg(async_payments)]
+fn invalid_keysend_payment_secret() {
+	let mut mpp_keysend_config = test_default_channel_config();
+	mpp_keysend_config.accept_mpp_keysend = true;
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, Some(mpp_keysend_config)]);
+	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+	let chan_upd_1_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents;
+
+	let invalid_payment_secret = PaymentSecret([42; 32]);
+	let amt_msat = 5000;
+	let keysend_preimage = PaymentPreimage([42; 32]);
+	let route_params = get_blinded_route_parameters(
+		amt_msat, invalid_payment_secret, 1, 1_0000_0000,
+		nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&chan_upd_1_2],
+		&chanmon_cfgs[2].keys_manager
+	);
+
+	let payment_hash = nodes[0].node.send_spontaneous_payment_with_retry(Some(keysend_preimage), RecipientOnionFields::spontaneous_empty(), PaymentId(keysend_preimage.0), route_params, Retry::Attempts(0)).unwrap();
+	check_added_monitors(&nodes[0], 1);
+
+	let expected_route: &[&[&Node]] = &[&[&nodes[1], &nodes[2]]];
+	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(events.len(), 1);
+
+	let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
+	let args = PassAlongPathArgs::new(
+		&nodes[0], &expected_route[0], amt_msat, payment_hash, ev.clone()
+	)
+		.with_payment_secret(invalid_payment_secret)
+		.with_payment_preimage(keysend_preimage)
+		.expect_failure(HTLCDestination::FailedPayment { payment_hash });
+	do_pass_along_path(args);
+
+	let updates_2_1 = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
+	assert_eq!(updates_2_1.update_fail_malformed_htlcs.len(), 1);
+	let update_malformed = &updates_2_1.update_fail_malformed_htlcs[0];
+	assert_eq!(update_malformed.sha256_of_onion, [0; 32]);
+	assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING);
+	nodes[1].node.handle_update_fail_malformed_htlc(nodes[2].node.get_our_node_id(), update_malformed);
+	do_commitment_signed_dance(&nodes[1], &nodes[2], &updates_2_1.commitment_signed, true, false);
+
+	let updates_1_0 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+	assert_eq!(updates_1_0.update_fail_htlcs.len(), 1);
+	nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates_1_0.update_fail_htlcs[0]);
+	do_commitment_signed_dance(&nodes[0], &nodes[1], &updates_1_0.commitment_signed, false, false);
+	expect_payment_failed_conditions(&nodes[0], payment_hash, false,
+		PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]));
+}
+
 #[test]
 fn custom_tlvs_to_blinded_path() {
 	let chanmon_cfgs = create_chanmon_cfgs(2);

lightning/src/ln/channelmanager.rs

@@ -220,6 +220,10 @@ pub enum PendingHTLCRouting {
 		custom_tlvs: Vec<(u64, Vec<u8>)>,
 		/// Set if this HTLC is the final hop in a multi-hop blinded path.
 		requires_blinded_error: bool,
+		/// Set if we are receiving a keysend to a blinded path, meaning we created the
+		/// [`PaymentSecret`] and should verify it using our
+		/// [`NodeSigner::get_inbound_payment_key_material`].
+		has_recipient_created_payment_secret: bool,
 	},
 }
 
@@ -5699,7 +5703,10 @@ where
 								}
 							}) => {
 								let blinded_failure = routing.blinded_failure();
-								let (cltv_expiry, onion_payload, payment_data, payment_context, phantom_shared_secret, mut onion_fields) = match routing {
+								let (
+									cltv_expiry, onion_payload, payment_data, payment_context, phantom_shared_secret,
+									mut onion_fields, has_recipient_created_payment_secret
+								) = match routing {
 									PendingHTLCRouting::Receive {
 										payment_data, payment_metadata, payment_context,
 										incoming_cltv_expiry, phantom_shared_secret, custom_tlvs,
@@ -5709,19 +5716,21 @@ where
 										let onion_fields = RecipientOnionFields { payment_secret: Some(payment_data.payment_secret),
 												payment_metadata, custom_tlvs };
 										(incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data },
-											Some(payment_data), payment_context, phantom_shared_secret, onion_fields)
+											Some(payment_data), payment_context, phantom_shared_secret, onion_fields,
+											true)
 									},
 									PendingHTLCRouting::ReceiveKeysend {
 										payment_data, payment_preimage, payment_metadata,
-										incoming_cltv_expiry, custom_tlvs, requires_blinded_error: _
+										incoming_cltv_expiry, custom_tlvs, requires_blinded_error: _,
+										has_recipient_created_payment_secret,
 									} => {
 										let onion_fields = RecipientOnionFields {
 											payment_secret: payment_data.as_ref().map(|data| data.payment_secret),
 											payment_metadata,
 											custom_tlvs,
 										};
 										(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage),
-											payment_data, None, None, onion_fields)
+											payment_data, None, None, onion_fields, has_recipient_created_payment_secret)
 									},
 									_ => {
 										panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
@@ -5886,9 +5895,8 @@ where
 								// that we are the ultimate recipient of the given payment hash.
 								// Further, we must not expose whether we have any other HTLCs
 								// associated with the same payment_hash pending or not.
-								match claimable_htlc.onion_payload {
-									OnionPayload::Invoice { .. } => {
-										let payment_data = payment_data.unwrap();
+								let payment_preimage = if has_recipient_created_payment_secret {
+									if let Some(ref payment_data) = payment_data {
 										let (payment_preimage, min_final_cltv_expiry_delta) = match inbound_payment::verify(payment_hash, &payment_data, self.highest_seen_timestamp.load(Ordering::Acquire) as u64, &self.inbound_payment_key, &self.logger) {
 											Ok(result) => result,
 											Err(()) => {
@@ -5904,6 +5912,12 @@ where
 												fail_htlc!(claimable_htlc, payment_hash);
 											}
 										}
+										payment_preimage
+									} else { fail_htlc!(claimable_htlc, payment_hash); }
+								} else { None };
+								match claimable_htlc.onion_payload {
+									OnionPayload::Invoice { .. } => {
+										let payment_data = payment_data.unwrap();
 										let purpose = events::PaymentPurpose::from_parts(
 											payment_preimage,
 											payment_data.payment_secret,
@@ -11422,6 +11436,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
 		(3, payment_metadata, option),
 		(4, payment_data, option), // Added in 0.0.116
 		(5, custom_tlvs, optional_vec),
+		(7, has_recipient_created_payment_secret, (default_value, false)),
 	},
 );
 

lightning/src/ln/functional_test_utils.rs

@@ -2587,6 +2587,7 @@ pub struct PassAlongPathArgs<'a, 'b, 'c, 'd> {
 	pub is_probe: bool,
 	pub custom_tlvs: Vec<(u64, Vec<u8>)>,
 	pub payment_metadata: Option<Vec<u8>>,
+	pub expected_failure: Option<HTLCDestination>,
 }
 
 impl<'a, 'b, 'c, 'd> PassAlongPathArgs<'a, 'b, 'c, 'd> {
@@ -2597,7 +2598,7 @@ impl<'a, 'b, 'c, 'd> PassAlongPathArgs<'a, 'b, 'c, 'd> {
 		Self {
 			origin_node, expected_path, recv_value, payment_hash, payment_secret: None, event,
 			payment_claimable_expected: true, clear_recipient_events: true, expected_preimage: None,
-			is_probe: false, custom_tlvs: Vec::new(), payment_metadata: None,
+			is_probe: false, custom_tlvs: Vec::new(), payment_metadata: None, expected_failure: None,
 		}
 	}
 	pub fn without_clearing_recipient_events(mut self) -> Self {
@@ -2629,13 +2630,19 @@ impl<'a, 'b, 'c, 'd> PassAlongPathArgs<'a, 'b, 'c, 'd> {
 		self.payment_metadata = Some(payment_metadata);
 		self
 	}
+	pub fn expect_failure(mut self, failure: HTLCDestination) -> Self {
+		self.payment_claimable_expected = false;
+		self.expected_failure = Some(failure);
+		self
+	}
 }
 
 pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option<Event> {
 	let PassAlongPathArgs {
 		origin_node, expected_path, recv_value, payment_hash: our_payment_hash,
 		payment_secret: our_payment_secret, event: ev, payment_claimable_expected,
 		clear_recipient_events, expected_preimage, is_probe, custom_tlvs, payment_metadata,
+		expected_failure
 	} = args;
 
 	let mut payment_event = SendEvent::from_event(ev);
@@ -2699,6 +2706,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(args: PassAlongPathArgs) -> Option<Event>
 					_ => panic!("Unexpected event"),
 				}
 				event = Some(events_2[0].clone());
+			} else if let Some(ref failure) = expected_failure {
+				assert_eq!(events_2.len(), 2);
+				expect_htlc_handling_failed_destinations!(events_2, &[failure]);
+				node.node.process_pending_htlc_forwards();
+				check_added_monitors!(node, 1);
 			} else {
 				assert!(events_2.is_empty());
 			}

lightning/src/ln/onion_payment.rs

@@ -135,14 +135,14 @@ pub(super) fn create_recv_pending_htlc_info(
 ) -> Result<PendingHTLCInfo, InboundHTLCErr> {
 	let (
 		payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, onion_cltv_expiry,
-		payment_metadata, payment_context, requires_blinded_error
+		payment_metadata, payment_context, requires_blinded_error, has_recipient_created_payment_secret
 	) = match hop_data {
 		msgs::InboundOnionPayload::Receive {
 			payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat,
 			cltv_expiry_height, payment_metadata, ..
 		} =>
 			(payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat,
-			 cltv_expiry_height, payment_metadata, None, false),
+			 cltv_expiry_height, payment_metadata, None, false, keysend_preimage.is_none()),
 		msgs::InboundOnionPayload::BlindedReceive {
 			sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, payment_secret,
 			intro_node_blinding_point, payment_constraints, payment_context, keysend_preimage,
@@ -161,7 +161,7 @@ pub(super) fn create_recv_pending_htlc_info(
 			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),
-			 intro_node_blinding_point.is_none())
+			 intro_node_blinding_point.is_none(), true)
 		}
 		msgs::InboundOnionPayload::Forward { .. } => {
 			return Err(InboundHTLCErr {
@@ -241,6 +241,7 @@ pub(super) fn create_recv_pending_htlc_info(
 			incoming_cltv_expiry: onion_cltv_expiry,
 			custom_tlvs,
 			requires_blinded_error,
+			has_recipient_created_payment_secret,
 		}
 	} else if let Some(data) = payment_data {
 		PendingHTLCRouting::Receive {