From a05d46525e880ec8125f74b6f4de464e5dfbb9eb Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 23 Feb 2022 16:40:02 -0500 Subject: [PATCH] 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. --- lightning/src/ln/channelmanager.rs | 28 +++++++++++----- lightning/src/ln/onion_route_tests.rs | 48 +++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e6fc17da..e8aee263 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/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 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 + payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result { // final_incorrect_cltv_expiry if hop_data.outgoing_cltv_value != cltv_expiry { @@ -2129,6 +2130,7 @@ impl 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 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 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 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 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) => { diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 5548a163..d2844c6e 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/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); +} -- 2.30.2