Skip to content

Commit db7ff46

Browse files
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
1 parent 1f6700c commit db7ff46

File tree

3 files changed

+267
-28
lines changed

3 files changed

+267
-28
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ mod inbound_payment {
358358
// our payment, which we can use to decode errors or inform the user that the payment was sent.
359359

360360
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
361-
enum PendingHTLCRouting {
361+
pub(super) enum PendingHTLCRouting {
362362
Forward {
363363
onion_packet: msgs::OnionPacket,
364364
short_channel_id: u64, // This should be NonZero<u64> eventually when we bump MSRV
@@ -375,8 +375,8 @@ enum PendingHTLCRouting {
375375

376376
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
377377
pub(super) struct PendingHTLCInfo {
378-
routing: PendingHTLCRouting,
379-
incoming_shared_secret: [u8; 32],
378+
pub(super) routing: PendingHTLCRouting,
379+
pub(super) incoming_shared_secret: [u8; 32],
380380
payment_hash: PaymentHash,
381381
pub(super) amt_to_forward: u64,
382382
pub(super) outgoing_cltv_value: u32,
@@ -3011,45 +3011,59 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
30113011
HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
30123012
routing, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value },
30133013
prev_funding_outpoint } => {
3014+
let htlc_failure_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
3015+
short_channel_id: prev_short_channel_id,
3016+
outpoint: prev_funding_outpoint,
3017+
htlc_id: prev_htlc_id,
3018+
incoming_packet_shared_secret: incoming_shared_secret,
3019+
});
30143020
macro_rules! fail_forward {
30153021
($msg: expr, $err_code: expr, $err_data: expr) => {
30163022
{
30173023
log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
3018-
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
3019-
short_channel_id: short_chan_id,
3020-
outpoint: prev_funding_outpoint,
3021-
htlc_id: prev_htlc_id,
3022-
incoming_packet_shared_secret: incoming_shared_secret,
3023-
});
3024-
failed_forwards.push((htlc_source, payment_hash,
3024+
failed_forwards.push((htlc_failure_source, payment_hash,
30253025
HTLCFailReason::Reason { failure_code: $err_code, data: $err_data }
30263026
));
30273027
continue;
30283028
}
30293029
}
30303030
}
3031+
macro_rules! fail_phantom_forward {
3032+
($msg: expr, $err_code: expr, $err_data: expr, $phantom_shared_secret: expr) => {
3033+
{
3034+
log_info!(self.logger, "Failed to accept/forward incoming phantom node HTLC: {}", $msg);
3035+
let packet = onion_utils::build_failure_packet(&$phantom_shared_secret, $err_code, &$err_data[..]).encode();
3036+
let error_data = onion_utils::encrypt_failure_packet(&$phantom_shared_secret, &packet);
3037+
failed_forwards.push((htlc_failure_source, payment_hash,
3038+
HTLCFailReason::LightningError { err: error_data }
3039+
));
3040+
continue;
3041+
}
3042+
}
3043+
}
30313044
if let PendingHTLCRouting::Forward { onion_packet, .. } = routing {
30323045
let phantom_secret_res = self.keys_manager.get_node_secret(Recipient::PhantomNode);
30333046
if phantom_secret_res.is_ok() && fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, short_chan_id) {
3034-
let shared_secret = {
3047+
let phantom_shared_secret = {
30353048
let mut arr = [0; 32];
30363049
arr.copy_from_slice(&SharedSecret::new(&onion_packet.public_key.unwrap(), &phantom_secret_res.unwrap())[..]);
30373050
arr
30383051
};
3039-
let next_hop = match onion_utils::decode_next_hop(shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) {
3052+
let next_hop = match onion_utils::decode_next_hop(phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac, payment_hash) {
30403053
Ok(res) => res,
30413054
Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => {
3042-
fail_forward!(err_msg, err_code, Vec::new());
3055+
let sha256_of_onion = Sha256::hash(&onion_packet.hop_data).into_inner();
3056+
fail_forward!(err_msg, err_code, sha256_of_onion.to_vec());
30433057
},
30443058
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => {
3045-
fail_forward!(err_msg, err_code, Vec::new());
3059+
fail_phantom_forward!(err_msg, err_code, Vec::new(), phantom_shared_secret);
30463060
},
30473061
};
30483062
match next_hop {
30493063
onion_utils::Hop::Receive(hop_data) => {
3050-
match self.construct_recv_pending_htlc_info(hop_data, shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value) {
3064+
match self.construct_recv_pending_htlc_info(hop_data, phantom_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value) {
30513065
Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, vec![(info, prev_htlc_id)])),
3052-
Err(ReceiveError { err_code, err_data, msg }) => fail_forward!(msg, err_code, err_data)
3066+
Err(ReceiveError { err_code, err_data, msg }) => fail_phantom_forward!(msg, err_code, err_data, phantom_shared_secret)
30533067
}
30543068
},
30553069
_ => panic!(),

lightning/src/ln/msgs.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,10 +1291,6 @@ impl Readable for FinalOnionHopData {
12911291

12921292
impl Writeable for OnionHopData {
12931293
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
1294-
// Note that this should never be reachable if Rust-Lightning generated the message, as we
1295-
// check values are sane long before we get here, though its possible in the future
1296-
// user-generated messages may hit this.
1297-
if self.amt_to_forward > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
12981294
match self.format {
12991295
OnionHopDataFormat::Legacy { short_channel_id } => {
13001296
0u8.write(w)?;
@@ -1311,9 +1307,6 @@ impl Writeable for OnionHopData {
13111307
});
13121308
},
13131309
OnionHopDataFormat::FinalNode { ref payment_data, ref keysend_preimage } => {
1314-
if let Some(final_data) = payment_data {
1315-
if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
1316-
}
13171310
encode_varint_length_prefixed_tlv!(w, {
13181311
(2, HighZeroBytesDroppedVarInt(self.amt_to_forward), required),
13191312
(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value), required),

lightning/src/ln/onion_route_tests.rs

Lines changed: 237 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,28 @@
1212
//! returned errors decode to the correct thing.
1313
1414
use chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS};
15+
use chain::keysinterface::{KeysInterface, Recipient};
1516
use ln::{PaymentHash, PaymentSecret};
16-
use ln::channelmanager::{HTLCForwardInfo, CLTV_FAR_FAR_AWAY};
17+
use ln::channelmanager::{HTLCForwardInfo, CLTV_FAR_FAR_AWAY, MIN_CLTV_EXPIRY_DELTA, PendingHTLCInfo, PendingHTLCRouting};
1718
use ln::onion_utils;
18-
use routing::network_graph::NetworkUpdate;
19-
use routing::router::Route;
20-
use ln::features::InitFeatures;
19+
use routing::network_graph::{NetworkUpdate, RoutingFees};
20+
use routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop};
21+
use ln::features::{InitFeatures, InvoiceFeatures};
2122
use ln::msgs;
2223
use ln::msgs::{ChannelMessageHandler, ChannelUpdate, OptionalField};
2324
use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
2425
use util::ser::{Writeable, Writer};
26+
use util::{byte_utils, test_utils};
2527
use util::config::UserConfig;
2628

2729
use bitcoin::hash_types::BlockHash;
2830

2931
use bitcoin::hashes::Hash;
32+
use bitcoin::hashes::sha256::Hash as Sha256;
3033

3134
use bitcoin::secp256k1;
3235
use bitcoin::secp256k1::Secp256k1;
33-
use bitcoin::secp256k1::key::SecretKey;
36+
use bitcoin::secp256k1::key::{PublicKey, SecretKey};
3437

3538
use io;
3639
use prelude::*;
@@ -573,3 +576,232 @@ fn test_onion_failure() {
573576
nodes[2].node.fail_htlc_backwards(&payment_hash);
574577
}, true, Some(23), None, None);
575578
}
579+
580+
macro_rules! get_phantom_route {
581+
($nodes: expr, $amt: expr, $channel: expr) => {{
582+
let secp_ctx = Secp256k1::new();
583+
let phantom_secret = $nodes[1].keys_manager.get_node_secret(Recipient::PhantomNode).unwrap();
584+
let phantom_pubkey = PublicKey::from_secret_key(&secp_ctx, &phantom_secret);
585+
let phantom_route_hint = $nodes[1].node.get_phantom_route_hints();
586+
let payment_params = PaymentParameters::from_node_id(phantom_pubkey)
587+
.with_features(InvoiceFeatures::known())
588+
.with_route_hints(vec![RouteHint(vec![
589+
RouteHintHop {
590+
src_node_id: $nodes[0].node.get_our_node_id(),
591+
short_channel_id: $channel.0.contents.short_channel_id,
592+
fees: RoutingFees {
593+
base_msat: $channel.0.contents.fee_base_msat,
594+
proportional_millionths: $channel.0.contents.fee_proportional_millionths,
595+
},
596+
cltv_expiry_delta: $channel.0.contents.cltv_expiry_delta,
597+
htlc_minimum_msat: None,
598+
htlc_maximum_msat: None,
599+
},
600+
RouteHintHop {
601+
src_node_id: phantom_route_hint.real_node_pubkey,
602+
short_channel_id: phantom_route_hint.phantom_scid,
603+
fees: RoutingFees {
604+
base_msat: 0,
605+
proportional_millionths: 0,
606+
},
607+
cltv_expiry_delta: MIN_CLTV_EXPIRY_DELTA,
608+
htlc_minimum_msat: None,
609+
htlc_maximum_msat: None,
610+
}
611+
])]);
612+
let scorer = test_utils::TestScorer::with_penalty(0);
613+
(get_route(
614+
&$nodes[0].node.get_our_node_id(), &payment_params, $nodes[0].network_graph,
615+
Some(&$nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
616+
$amt, TEST_FINAL_CLTV, $nodes[0].logger, &scorer
617+
).unwrap(), phantom_route_hint.phantom_scid)
618+
}
619+
}}
620+
621+
#[test]
622+
fn test_phantom_onion_hmac_failure() {
623+
let chanmon_cfgs = create_chanmon_cfgs(2);
624+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
625+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
626+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
627+
628+
let channel = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
629+
630+
// Get the route.
631+
let recv_value_msat = 10_000;
632+
let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat));
633+
let (route, phantom_scid) = get_phantom_route!(nodes, recv_value_msat, channel);
634+
635+
// Route the HTLC through to the destination.
636+
nodes[0].node.send_payment(&route, payment_hash.clone(), &Some(payment_secret)).unwrap();
637+
check_added_monitors!(nodes[0], 1);
638+
let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
639+
let mut update_add = update_0.update_add_htlcs[0].clone();
640+
641+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
642+
commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true);
643+
644+
// Modify the payload so the phantom hop's HMAC is bogus.
645+
let sha256_of_onion = {
646+
let mut channel_state = nodes[1].node.channel_state.lock().unwrap();
647+
let mut pending_forward = channel_state.forward_htlcs.get_mut(&phantom_scid).unwrap();
648+
match pending_forward[0] {
649+
HTLCForwardInfo::AddHTLC {
650+
forward_info: PendingHTLCInfo {
651+
routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. },
652+
..
653+
}, ..
654+
} => {
655+
onion_packet.hmac[onion_packet.hmac.len() - 1] ^= 1;
656+
Sha256::hash(&onion_packet.hop_data).into_inner().to_vec()
657+
},
658+
_ => panic!("Unexpected forward"),
659+
}
660+
};
661+
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
662+
nodes[1].node.process_pending_htlc_forwards();
663+
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
664+
nodes[1].node.process_pending_htlc_forwards();
665+
let update_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
666+
check_added_monitors!(&nodes[1], 1);
667+
assert!(update_1.update_fail_htlcs.len() == 1);
668+
let fail_msg = update_1.update_fail_htlcs[0].clone();
669+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg);
670+
commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
671+
672+
// Ensure the payment fails with the expected error.
673+
let mut fail_conditions = PaymentFailedConditions::new()
674+
.blamed_scid(phantom_scid)
675+
.blamed_chan_closed(true)
676+
.expected_htlc_error_data(0x8000 | 0x4000 | 5, &sha256_of_onion);
677+
expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions);
678+
}
679+
680+
#[test]
681+
fn test_phantom_invalid_onion_payload() {
682+
let chanmon_cfgs = create_chanmon_cfgs(2);
683+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
684+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
685+
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
686+
687+
let channel = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
688+
689+
// Get the route.
690+
let recv_value_msat = 10_000;
691+
let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat));
692+
let (route, phantom_scid) = get_phantom_route!(nodes, recv_value_msat, channel);
693+
694+
// We'll use the session priv later when constructing an invalid onion packet.
695+
let session_priv = [3; 32];
696+
*nodes[0].keys_manager.override_session_priv.lock().unwrap() = Some(session_priv);
697+
nodes[0].node.send_payment(&route, payment_hash.clone(), &Some(payment_secret)).unwrap();
698+
check_added_monitors!(nodes[0], 1);
699+
let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
700+
let mut update_add = update_0.update_add_htlcs[0].clone();
701+
702+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
703+
commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true);
704+
705+
// Modify the onion packet to have an invalid payment amount.
706+
for (_, pending_forwards) in nodes[1].node.channel_state.lock().unwrap().forward_htlcs.iter_mut() {
707+
for f in pending_forwards.iter_mut() {
708+
match f {
709+
&mut HTLCForwardInfo::AddHTLC {
710+
forward_info: PendingHTLCInfo {
711+
routing: PendingHTLCRouting::Forward { ref mut onion_packet, .. },
712+
..
713+
}, ..
714+
} => {
715+
// Construct the onion payloads for the entire route and an invalid amount.
716+
let height = nodes[0].best_block_info().1;
717+
let session_priv = SecretKey::from_slice(&session_priv).unwrap();
718+
let mut onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
719+
let (mut onion_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], msgs::MAX_VALUE_MSAT + 1, &Some(payment_secret), height + 1, &None).unwrap();
720+
// We only want to construct the onion packet for the last hop, not the entire route, so
721+
// remove the first hop's payload and its keys.
722+
onion_keys.remove(0);
723+
onion_payloads.remove(0);
724+
725+
let new_onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
726+
onion_packet.hop_data = new_onion_packet.hop_data;
727+
onion_packet.hmac = new_onion_packet.hmac;
728+
},
729+
_ => panic!("Unexpected forward"),
730+
}
731+
}
732+
}
733+
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
734+
nodes[1].node.process_pending_htlc_forwards();
735+
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
736+
nodes[1].node.process_pending_htlc_forwards();
737+
let update_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
738+
check_added_monitors!(&nodes[1], 1);
739+
assert!(update_1.update_fail_htlcs.len() == 1);
740+
let fail_msg = update_1.update_fail_htlcs[0].clone();
741+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg);
742+
commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
743+
744+
// Ensure the payment fails with the expected error.
745+
let error_data = Vec::new();
746+
let mut fail_conditions = PaymentFailedConditions::new()
747+
.blamed_scid(phantom_scid)
748+
.blamed_chan_closed(true)
749+
.expected_htlc_error_data(0x4000 | 22, &error_data);
750+
expect_payment_failed_conditions!(nodes[0], payment_hash, true, fail_conditions);
751+
}
752+
753+
#[test]
754+
fn test_phantom_final_incorrect_cltv_expiry() {
755+
let chanmon_cfgs = create_chanmon_cfgs(2);
756+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
757+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
758+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
759+
760+
let channel = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
761+
762+
// Get the route.
763+
let recv_value_msat = 10_000;
764+
let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value_msat));
765+
let (route, phantom_scid) = get_phantom_route!(nodes, recv_value_msat, channel);
766+
767+
// Route the HTLC through to the destination.
768+
nodes[0].node.send_payment(&route, payment_hash.clone(), &Some(payment_secret)).unwrap();
769+
check_added_monitors!(nodes[0], 1);
770+
let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
771+
let mut update_add = update_0.update_add_htlcs[0].clone();
772+
773+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
774+
commitment_signed_dance!(nodes[1], nodes[0], &update_0.commitment_signed, false, true);
775+
776+
// Modify the payload so the phantom hop's HMAC is bogus.
777+
for (_, pending_forwards) in nodes[1].node.channel_state.lock().unwrap().forward_htlcs.iter_mut() {
778+
for f in pending_forwards.iter_mut() {
779+
match f {
780+
&mut HTLCForwardInfo::AddHTLC {
781+
forward_info: PendingHTLCInfo { ref mut outgoing_cltv_value, .. }, ..
782+
} => {
783+
*outgoing_cltv_value += 1;
784+
},
785+
_ => panic!("Unexpected forward"),
786+
}
787+
}
788+
}
789+
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
790+
nodes[1].node.process_pending_htlc_forwards();
791+
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
792+
nodes[1].node.process_pending_htlc_forwards();
793+
let update_1 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
794+
check_added_monitors!(&nodes[1], 1);
795+
assert!(update_1.update_fail_htlcs.len() == 1);
796+
let fail_msg = update_1.update_fail_htlcs[0].clone();
797+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_msg);
798+
commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
799+
800+
// Ensure the payment fails with the expected error.
801+
let expected_cltv = 82;
802+
let error_data = byte_utils::be32_to_array(expected_cltv).to_vec();
803+
let mut fail_conditions = PaymentFailedConditions::new()
804+
.blamed_scid(phantom_scid)
805+
.expected_htlc_error_data(18, &error_data);
806+
expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions);
807+
}

0 commit comments

Comments
 (0)