Correctly fail back phantom node payments that don't match inbound payment data

Previously, we failed back these payments without encrypting with the second-to-last-hop's
failure onion layer.

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -366,6 +366,7 @@ pub(super) enum PendingHTLCRouting {
 	Receive {
 		payment_data: msgs::FinalOnionHopData,
 		incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed
+		phantom_shared_secret: Option<[u8; 32]>,
 	},
 	ReceiveKeysend {
 		payment_preimage: PaymentPreimage,
@@ -2072,7 +2073,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	}
 
 	fn construct_recv_pending_htlc_info(&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32],
-		payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32) -> Result<PendingHTLCInfo, ReceiveError>
+		payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result<PendingHTLCInfo, ReceiveError>
 	{
 		// final_incorrect_cltv_expiry
 		if hop_data.outgoing_cltv_value != cltv_expiry {
@@ -2129,6 +2130,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 					PendingHTLCRouting::Receive {
 						payment_data: data,
 						incoming_cltv_expiry: hop_data.outgoing_cltv_value,
+						phantom_shared_secret,
 					}
 				} else if let Some(payment_preimage) = keysend_preimage {
 					// We need to check that the sender knows the keysend preimage before processing this
@@ -2232,7 +2234,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		let pending_forward_info = match next_hop {
 			onion_utils::Hop::Receive(next_hop_data) => {
 				// OUR PAYMENT!
-				match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry) {
+				match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None) {
 					Ok(info) => {
 						// Note that we could obviously respond immediately with an update_fulfill_htlc
 						// message, however that would leak that we are the recipient of this payment, so
@@ -3061,7 +3063,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 													};
 													match next_hop {
 														onion_utils::Hop::Receive(hop_data) => {
-															match self.construct_recv_pending_htlc_info(hop_data, phantom_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value) {
+															match self.construct_recv_pending_htlc_info(hop_data, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value, Some(phantom_shared_secret)) {
 																Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, vec![(info, prev_htlc_id)])),
 																Err(ReceiveError { err_code, err_data, msg }) => fail_phantom_forward!(msg, err_code, err_data, phantom_shared_secret)
 															}
@@ -3221,11 +3223,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 							HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
 									routing, incoming_shared_secret, payment_hash, amt_to_forward, .. },
 									prev_funding_outpoint } => {
-								let (cltv_expiry, onion_payload) = match routing {
-									PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry } =>
-										(incoming_cltv_expiry, OnionPayload::Invoice(payment_data)),
+								let (cltv_expiry, onion_payload, phantom_shared_secret) = match routing {
+									PendingHTLCRouting::Receive { payment_data, incoming_cltv_expiry, phantom_shared_secret } =>
+										(incoming_cltv_expiry, OnionPayload::Invoice(payment_data), phantom_shared_secret),
 									PendingHTLCRouting::ReceiveKeysend { payment_preimage, incoming_cltv_expiry } =>
-										(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage)),
+										(incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), None),
 									_ => {
 										panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
 									}
@@ -3248,13 +3250,20 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 										htlc_msat_height_data.extend_from_slice(
 											&byte_utils::be32_to_array(self.best_block.read().unwrap().height()),
 										);
+										let failure_code = 0x4000 | 15;
+										let failure_reason = if let Some(phantom_ss) = phantom_shared_secret {
+											let packet = onion_utils::build_failure_packet(&phantom_ss, failure_code, &htlc_msat_height_data[..]).encode();
+											let error_data =  onion_utils::encrypt_failure_packet(&phantom_ss, &packet);
+											HTLCFailReason::LightningError { err: error_data }
+										} else {
+											HTLCFailReason::Reason { failure_code, data: htlc_msat_height_data }
+										};
 										failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
 												short_channel_id: $htlc.prev_hop.short_channel_id,
 												outpoint: prev_funding_outpoint,
 												htlc_id: $htlc.prev_hop.htlc_id,
 												incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret,
-											}), payment_hash,
-											HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data }
+											}), payment_hash, failure_reason
 										));
 									}
 								}
@@ -5993,6 +6002,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
 	},
 	(1, Receive) => {
 		(0, payment_data, required),
+		(1, phantom_shared_secret, option),
 		(2, incoming_cltv_expiry, required),
 	},
 	(2, ReceiveKeysend) => {

lightning/src/ln/onion_route_tests.rs

@@ -850,3 +850,51 @@ fn test_phantom_failure_too_low_cltv() {
 		.expected_htlc_error_data(17, &error_data);
 	expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions);
 }
+
+#[test]
+fn test_phantom_failure_too_low_recv_amt() {
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let channel = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+
+	// Get the route with a too-low amount.
+	let recv_amt_msat = 10_000;
+	let bad_recv_amt_msat = recv_amt_msat - 10;
+	let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_amt_msat));
+	let (mut route, phantom_scid) = get_phantom_route!(nodes, bad_recv_amt_msat, channel);
+
+	// Route the HTLC through to the destination.
+	nodes[0].node.send_payment(&route, payment_hash.clone(), &Some(payment_secret)).unwrap();
+	check_added_monitors!(nodes[0], 1);
+	let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+	let mut update_add = update_0.update_add_htlcs[0].clone();
+
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
+	commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true);
+
+	expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+	nodes[1].node.process_pending_htlc_forwards();
+	expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+	nodes[1].node.process_pending_htlc_forwards();
+	expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+	nodes[1].node.process_pending_htlc_forwards();
+	let update_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+	check_added_monitors!(&nodes[1], 1);
+	assert!(update_1.update_fail_htlcs.len() == 1);
+	let fail_msg = update_1.update_fail_htlcs[0].clone();
+	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg);
+	commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
+
+	// Ensure the payment fails with the expected error.
+	let mut error_data = byte_utils::be64_to_array(bad_recv_amt_msat).to_vec();
+	error_data.extend_from_slice(
+		&byte_utils::be32_to_array(nodes[1].node.best_block.read().unwrap().height()),
+	);
+	let mut fail_conditions = PaymentFailedConditions::new()
+		.blamed_scid(phantom_scid)
+		.expected_htlc_error_data(0x4000 | 15, &error_data);
+	expect_payment_failed_conditions!(nodes[0], payment_hash, true, fail_conditions);
+}