From: Valentine Wallace Date: Tue, 22 Feb 2022 17:17:47 +0000 (-0500) Subject: Test failing phantom payments X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=edba328859469313e42f1d75e5be5eaa7d0fb543;p=rust-lightning Test failing phantom payments Also fix a bug where we were failing back phantom payments with the wrong scid, causing them to never actually be failed backwards. Finally, drop some unnecessary panics when reading OnionHopData objects. This also enables one of the phantom failure tests because we can construct OnionHopDatas with invalid amounts --- diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0a0407eac..e6fc17da4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -358,7 +358,7 @@ mod inbound_payment { // our payment, which we can use to decode errors or inform the user that the payment was sent. #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug -enum PendingHTLCRouting { +pub(super) enum PendingHTLCRouting { Forward { onion_packet: msgs::OnionPacket, short_channel_id: u64, // This should be NonZero eventually when we bump MSRV @@ -375,8 +375,8 @@ enum PendingHTLCRouting { #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug pub(super) struct PendingHTLCInfo { - routing: PendingHTLCRouting, - incoming_shared_secret: [u8; 32], + pub(super) routing: PendingHTLCRouting, + pub(super) incoming_shared_secret: [u8; 32], payment_hash: PaymentHash, pub(super) amt_to_forward: u64, pub(super) outgoing_cltv_value: u32, @@ -3011,45 +3011,59 @@ impl ChannelMana HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { routing, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value }, prev_funding_outpoint } => { + let htlc_failure_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { + short_channel_id: prev_short_channel_id, + outpoint: prev_funding_outpoint, + htlc_id: prev_htlc_id, + incoming_packet_shared_secret: incoming_shared_secret, + }); macro_rules! fail_forward { ($msg: expr, $err_code: expr, $err_data: expr) => { { log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg); - let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { - short_channel_id: short_chan_id, - outpoint: prev_funding_outpoint, - htlc_id: prev_htlc_id, - incoming_packet_shared_secret: incoming_shared_secret, - }); - failed_forwards.push((htlc_source, payment_hash, + failed_forwards.push((htlc_failure_source, payment_hash, HTLCFailReason::Reason { failure_code: $err_code, data: $err_data } )); continue; } } } + macro_rules! fail_phantom_forward { + ($msg: expr, $err_code: expr, $err_data: expr, $phantom_shared_secret: expr) => { + { + log_info!(self.logger, "Failed to accept/forward incoming phantom node HTLC: {}", $msg); + let packet = onion_utils::build_failure_packet(&$phantom_shared_secret, $err_code, &$err_data[..]).encode(); + let error_data = onion_utils::encrypt_failure_packet(&$phantom_shared_secret, &packet); + failed_forwards.push((htlc_failure_source, payment_hash, + HTLCFailReason::LightningError { err: error_data } + )); + continue; + } + } + } if let PendingHTLCRouting::Forward { onion_packet, .. } = routing { let phantom_secret_res = self.keys_manager.get_node_secret(Recipient::PhantomNode); if phantom_secret_res.is_ok() && fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, short_chan_id) { - let shared_secret = { + let phantom_shared_secret = { let mut arr = [0; 32]; arr.copy_from_slice(&SharedSecret::new(&onion_packet.public_key.unwrap(), &phantom_secret_res.unwrap())[..]); arr }; - let next_hop = match onion_utils::decode_next_hop(shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) { + let next_hop = match onion_utils::decode_next_hop(phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) { Ok(res) => res, Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => { - fail_forward!(err_msg, err_code, Vec::new()); + let sha256_of_onion = Sha256::hash(&onion_packet.hop_data).into_inner(); + fail_forward!(err_msg, err_code, sha256_of_onion.to_vec()); }, Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => { - fail_forward!(err_msg, err_code, Vec::new()); + fail_phantom_forward!(err_msg, err_code, Vec::new(), phantom_shared_secret); }, }; match next_hop { onion_utils::Hop::Receive(hop_data) => { - match self.construct_recv_pending_htlc_info(hop_data, shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value) { + match self.construct_recv_pending_htlc_info(hop_data, phantom_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value) { 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_forward!(msg, err_code, err_data) + Err(ReceiveError { err_code, err_data, msg }) => fail_phantom_forward!(msg, err_code, err_data, phantom_shared_secret) } }, _ => panic!(), diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 203e2426f..131c4fb54 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1291,10 +1291,6 @@ impl Readable for FinalOnionHopData { impl Writeable for OnionHopData { fn write(&self, w: &mut W) -> Result<(), io::Error> { - // Note that this should never be reachable if Rust-Lightning generated the message, as we - // check values are sane long before we get here, though its possible in the future - // user-generated messages may hit this. - if self.amt_to_forward > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); } match self.format { OnionHopDataFormat::Legacy { short_channel_id } => { 0u8.write(w)?; @@ -1311,9 +1307,6 @@ impl Writeable for OnionHopData { }); }, OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => { - if let Some(final_data) = payment_data { - if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); } - } encode_varint_length_prefixed_tlv!(w, { (2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required), (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required), diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 4feae104d..5548a163d 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -12,25 +12,28 @@ //! returned errors decode to the correct thing. use chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS}; +use chain::keysinterface::{KeysInterface, Recipient}; use ln::{PaymentHash, PaymentSecret}; -use ln::channelmanager::{HTLCForwardInfo, CLTV_FAR_FAR_AWAY}; +use ln::channelmanager::{HTLCForwardInfo, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingHTLCInfo, PendingHTLCRouting}; use ln::onion_utils; -use routing::network_graph::NetworkUpdate; -use routing::router::Route; -use ln::features::InitFeatures; +use routing::network_graph::{NetworkUpdate, RoutingFees}; +use routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop}; +use ln::features::{InitFeatures, InvoiceFeatures}; use ln::msgs; use ln::msgs::{ChannelMessageHandler, ChannelUpdate, OptionalField}; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; use util::ser::{Writeable, Writer}; +use util::{byte_utils, test_utils}; use util::config::UserConfig; use bitcoin::hash_types::BlockHash; use bitcoin::hashes::Hash; +use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::secp256k1; use bitcoin::secp256k1::Secp256k1; -use bitcoin::secp256k1::key::SecretKey; +use bitcoin::secp256k1::key::{PublicKey, SecretKey}; use io; use prelude::*; @@ -573,3 +576,277 @@ fn test_onion_failure() { nodes[2].node.fail_htlc_backwards(&payment_hash); }, true, Some(23), None, None); } + +macro_rules! get_phantom_route { + ($nodes: expr, $amt: expr, $channel: expr) => {{ + let secp_ctx = Secp256k1::new(); + let phantom_secret = $nodes[1].keys_manager.get_node_secret(Recipient::PhantomNode).unwrap(); + let phantom_pubkey = PublicKey::from_secret_key(&secp_ctx, &phantom_secret); + let phantom_route_hint = $nodes[1].node.get_phantom_route_hints(); + let payment_params = PaymentParameters::from_node_id(phantom_pubkey) + .with_features(InvoiceFeatures::known()) + .with_route_hints(vec![RouteHint(vec![ + RouteHintHop { + src_node_id: $nodes[0].node.get_our_node_id(), + short_channel_id: $channel.0.contents.short_channel_id, + fees: RoutingFees { + base_msat: $channel.0.contents.fee_base_msat, + proportional_millionths: $channel.0.contents.fee_proportional_millionths, + }, + cltv_expiry_delta: $channel.0.contents.cltv_expiry_delta, + htlc_minimum_msat: None, + htlc_maximum_msat: None, + }, + RouteHintHop { + src_node_id: phantom_route_hint.real_node_pubkey, + short_channel_id: phantom_route_hint.phantom_scid, + fees: RoutingFees { + base_msat: 0, + proportional_millionths: 0, + }, + cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA, + htlc_minimum_msat: None, + htlc_maximum_msat: None, + } + ])]); + let scorer = test_utils::TestScorer::with_penalty(0); + (get_route( + &$nodes[0].node.get_our_node_id(), &payment_params, $nodes[0].network_graph, + Some(&$nodes[0].node.list_usable_channels().iter().collect::>()), + $amt, TEST_FINAL_CLTV, $nodes[0].logger, &scorer + ).unwrap(), phantom_route_hint.phantom_scid) + } +}} + +#[test] +fn test_phantom_onion_hmac_failure() { + 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. + let recv_value_msat = 10_000; + let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat)); + let (route, phantom_scid) = get_phantom_route!(nodes, recv_value_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); + + // Modify the payload so the phantom hop's HMAC is bogus. + let sha256_of_onion = { + let mut channel_state = nodes[1].node.channel_state.lock().unwrap(); + let mut pending_forward = channel_state.forward_htlcs.get_mut(&phantom_scid).unwrap(); + match pending_forward[0] { + HTLCForwardInfo::AddHTLC { + forward_info: PendingHTLCInfo { + routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. }, + .. + }, .. + } => { + onion_packet.hmac[onion_packet.hmac.len() - 1] ^= 1; + Sha256::hash(&onion_packet.hop_data).into_inner().to_vec() + }, + _ => panic!("Unexpected forward"), + } + }; + 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 fail_conditions = PaymentFailedConditions::new() + .blamed_scid(phantom_scid) + .blamed_chan_closed(true) + .expected_htlc_error_data(0x8000 | 0x4000 | 5, &sha256_of_onion); + expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); +} + +#[test] +fn test_phantom_invalid_onion_payload() { + 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 mut 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. + let recv_value_msat = 10_000; + let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat)); + let (route, phantom_scid) = get_phantom_route!(nodes, recv_value_msat, channel); + + // We'll use the session priv later when constructing an invalid onion packet. + let session_priv = [3; 32]; + *nodes[0].keys_manager.override_session_priv.lock().unwrap() = Some(session_priv); + 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); + + // Modify the onion packet to have an invalid payment amount. + for (_, pending_forwards) in nodes[1].node.channel_state.lock().unwrap().forward_htlcs.iter_mut() { + for f in pending_forwards.iter_mut() { + match f { + &mut HTLCForwardInfo::AddHTLC { + forward_info: PendingHTLCInfo { + routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. }, + .. + }, .. + } => { + // Construct the onion payloads for the entire route and an invalid amount. + let height = nodes[0].best_block_info().1; + let session_priv = SecretKey::from_slice(&session_priv).unwrap(); + let mut onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap(); + let (mut onion_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], msgs::MAX_VALUE_MSAT + 1, &Some(payment_secret), height + 1, &None).unwrap(); + // We only want to construct the onion packet for the last hop, not the entire route, so + // remove the first hop's payload and its keys. + onion_keys.remove(0); + onion_payloads.remove(0); + + let new_onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash); + onion_packet.hop_data = new_onion_packet.hop_data; + onion_packet.hmac = new_onion_packet.hmac; + }, + _ => panic!("Unexpected forward"), + } + } + } + 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 error_data = Vec::new(); + let mut fail_conditions = PaymentFailedConditions::new() + .blamed_scid(phantom_scid) + .blamed_chan_closed(true) + .expected_htlc_error_data(0x4000 | 22, &error_data); + expect_payment_failed_conditions!(nodes[0], payment_hash, true, fail_conditions); +} + +#[test] +fn test_phantom_final_incorrect_cltv_expiry() { + 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. + let recv_value_msat = 10_000; + let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat)); + let (route, phantom_scid) = get_phantom_route!(nodes, recv_value_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); + + // Modify the payload so the phantom hop's HMAC is bogus. + for (_, pending_forwards) in nodes[1].node.channel_state.lock().unwrap().forward_htlcs.iter_mut() { + for f in pending_forwards.iter_mut() { + match f { + &mut HTLCForwardInfo::AddHTLC { + forward_info: PendingHTLCInfo { ref mut outgoing_cltv_value, .. }, .. + } => { + *outgoing_cltv_value += 1; + }, + _ => panic!("Unexpected forward"), + } + } + } + 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 expected_cltv = 82; + let error_data = byte_utils::be32_to_array(expected_cltv).to_vec(); + let mut fail_conditions = PaymentFailedConditions::new() + .blamed_scid(phantom_scid) + .expected_htlc_error_data(18, &error_data); + expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); +} + +#[test] +fn test_phantom_failure_too_low_cltv() { + 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. + let recv_value_msat = 10_000; + let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat)); + let (mut route, phantom_scid) = get_phantom_route!(nodes, recv_value_msat, channel); + + // Modify the route to have a too-low cltv. + route.paths[0][1].cltv_expiry_delta = 5; + + // 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(); + 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 error_data = Vec::new(); + let mut fail_conditions = PaymentFailedConditions::new() + .blamed_scid(phantom_scid) + .expected_htlc_error_data(17, &error_data); + expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); +}