diff --git a/fuzz/src/onion_hop_data.rs b/fuzz/src/onion_hop_data.rs index cc80ccf93..e7f51b991 100644 --- a/fuzz/src/onion_hop_data.rs +++ b/fuzz/src/onion_hop_data.rs @@ -16,16 +16,18 @@ use lightning::util::test_utils; #[inline] pub fn onion_hop_data_test(data: &[u8], _out: Out) { use lightning::util::ser::ReadableArgs; + use bitcoin::secp256k1::PublicKey; let mut r = ::std::io::Cursor::new(data); let node_signer = test_utils::TestNodeSigner::new(test_utils::privkey(42)); - let _ = >::read(&mut r, &&node_signer); + let _ = , &&test_utils::TestNodeSigner)>>::read(&mut r, (None, &&node_signer)); } #[no_mangle] pub extern "C" fn onion_hop_data_run(data: *const u8, datalen: usize) { use lightning::util::ser::ReadableArgs; + use bitcoin::secp256k1::PublicKey; let data = unsafe { std::slice::from_raw_parts(data, datalen) }; let mut r = ::std::io::Cursor::new(data); let node_signer = test_utils::TestNodeSigner::new(test_utils::privkey(42)); - let _ = >::read(&mut r, &&node_signer); + let _ = , &&test_utils::TestNodeSigner)>>::read(&mut r, (None, &&node_signer)); } diff --git a/lightning/src/blinded_path/mod.rs b/lightning/src/blinded_path/mod.rs index d75b4f25b..b1fc9e1c0 100644 --- a/lightning/src/blinded_path/mod.rs +++ b/lightning/src/blinded_path/mod.rs @@ -105,7 +105,7 @@ impl BlindedPath { /// /// [`ForwardTlvs`]: crate::blinded_path::payment::ForwardTlvs // TODO: make all payloads the same size with padding + add dummy hops - pub(crate) fn new_for_payment( + pub fn new_for_payment( intermediate_nodes: &[payment::ForwardNode], payee_node_id: PublicKey, payee_tlvs: payment::ReceiveTlvs, htlc_maximum_msat: u64, entropy_source: &ES, secp_ctx: &Secp256k1 diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 635057dea..9b580d1fa 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -10,7 +10,7 @@ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use crate::blinded_path::BlindedPath; use crate::blinded_path::payment::{ForwardNode, ForwardTlvs, PaymentConstraints, PaymentRelay, ReceiveTlvs}; -use crate::events::{HTLCDestination, MessageSendEventsProvider}; +use crate::events::{HTLCDestination, MessageSendEvent, MessageSendEventsProvider}; use crate::ln::PaymentSecret; use crate::ln::channelmanager; use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; @@ -22,7 +22,7 @@ use crate::ln::onion_utils; use crate::ln::onion_utils::INVALID_ONION_BLINDING; use crate::ln::outbound_payment::Retry; use crate::prelude::*; -use crate::routing::router::{PaymentParameters, RouteParameters}; +use crate::routing::router::{Payee, PaymentParameters, RouteParameters}; use crate::util::config::UserConfig; use crate::util::test_utils; @@ -281,7 +281,12 @@ fn failed_backwards_to_intro_node() { let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); let mut update_malformed = &mut updates.update_fail_malformed_htlcs[0]; - // Ensure the final hop does not correctly blind their error. + // Check that the final node encodes its failure correctly. + assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING); + assert_eq!(update_malformed.sha256_of_onion, [0; 32]); + + // Modify such the final hop does not correctly blind their error so we can ensure the intro node + // converts it to the correct error. update_malformed.sha256_of_onion = [1; 32]; 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.commitment_signed, true, false); @@ -377,6 +382,10 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck) #[test] fn blinded_intercept_payment() { + do_blinded_intercept_payment(true); + do_blinded_intercept_payment(false); +} +fn do_blinded_intercept_payment(intercept_node_fails: bool) { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut intercept_forwards_config = test_default_channel_config(); @@ -384,10 +393,13 @@ fn blinded_intercept_payment() { let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), None]); let 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 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents; + let (channel_id, chan_upd) = { + let chan = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0); + (chan.2, chan.0.contents) + }; let amt_msat = 5000; - let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None); + let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None); let intercept_scid = nodes[1].node.get_intercept_scid(); let mut intercept_chan_upd = chan_upd; intercept_chan_upd.short_channel_id = intercept_scid; @@ -408,27 +420,283 @@ fn blinded_intercept_payment() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); - let intercept_id = match events[0] { + let (intercept_id, expected_outbound_amount_msat) = match events[0] { crate::events::Event::HTLCIntercepted { intercept_id, payment_hash: pmt_hash, - requested_next_hop_scid: short_channel_id, .. + requested_next_hop_scid: short_channel_id, expected_outbound_amount_msat, .. } => { assert_eq!(pmt_hash, payment_hash); assert_eq!(short_channel_id, intercept_scid); - intercept_id + (intercept_id, expected_outbound_amount_msat) }, _ => panic!() }; - nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap(); - expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::UnknownNextHop { requested_forward_scid: intercept_scid }]); - nodes[1].node.process_pending_htlc_forwards(); - let update_fail = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + if intercept_node_fails { + nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap(); + expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::UnknownNextHop { requested_forward_scid: intercept_scid }]); + nodes[1].node.process_pending_htlc_forwards(); + let update_fail = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + check_added_monitors!(&nodes[1], 1); + assert!(update_fail.update_fail_htlcs.len() == 1); + let fail_msg = update_fail.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_fail.commitment_signed, false); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, + PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); + return + } + + nodes[1].node.forward_intercepted_htlc(intercept_id, &channel_id, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap(); + expect_pending_htlcs_forwardable!(nodes[1]); + + let payment_event = { + { + let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); + assert_eq!(added_monitors.len(), 1); + added_monitors.clear(); + } + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + SendEvent::from_event(events.remove(0)) + }; + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[1], &payment_event.commitment_msg, false, true); + expect_pending_htlcs_forwardable!(nodes[2]); + + expect_payment_claimable!(&nodes[2], payment_hash, payment_secret, amt_msat, None, nodes[2].node.get_our_node_id()); + do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[1], &nodes[2])[..]), false, payment_preimage); + expect_payment_sent(&nodes[0], payment_preimage, Some(Some(1000)), true, true); +} + +#[test] +fn two_hop_blinded_path_success() { + 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, None]); + 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 amt_msat = 5000; + let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None); + let route_params = get_blinded_route_parameters(amt_msat, payment_secret, + nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&chan_upd_1_2], + &chanmon_cfgs[2].keys_manager); + + nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap(); + check_added_monitors(&nodes[0], 1); + pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], amt_msat, payment_hash, payment_secret); + claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); +} + +#[derive(PartialEq)] +enum ReceiveCheckFail { + // The recipient fails the payment upon `PaymentClaimable`. + RecipientFail, + // Failure to decode the recipient's onion payload. + OnionDecodeFail, + // The incoming HTLC did not satisfy our requirements; in this case it underpaid us according to + // the expected receive amount in the onion. + ReceiveRequirements, + // The incoming HTLC errors when added to the Channel, in this case due to the HTLC being + // delivered out-of-order with a shutdown message. + ChannelCheck, + // The HTLC is successfully added to the inbound channel but fails receive checks in + // process_pending_htlc_forwards. + ProcessPendingHTLCsCheck, + // The HTLC violates the `PaymentConstraints` contained within the receiver's encrypted payload. + PaymentConstraints, +} + +#[test] +fn multi_hop_receiver_fail() { + do_multi_hop_receiver_fail(ReceiveCheckFail::RecipientFail); + do_multi_hop_receiver_fail(ReceiveCheckFail::OnionDecodeFail); + do_multi_hop_receiver_fail(ReceiveCheckFail::ReceiveRequirements); + do_multi_hop_receiver_fail(ReceiveCheckFail::ChannelCheck); + do_multi_hop_receiver_fail(ReceiveCheckFail::ProcessPendingHTLCsCheck); + do_multi_hop_receiver_fail(ReceiveCheckFail::PaymentConstraints); +} + +fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { + // Test that the receiver to a multihop blinded path fails back correctly. + 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, None]); + let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); + // We need the session priv to construct an invalid onion packet later. + let session_priv = [3; 32]; + *nodes[0].keys_manager.override_random_bytes.lock().unwrap() = Some(session_priv); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + let (chan_upd_1_2, chan_id_1_2) = { + let (chan_upd, _, channel_id, ..) = create_announced_chan_between_nodes_with_value( + &nodes, 1, 2, 1_000_000, 0 + ); + (chan_upd.contents, channel_id) + }; + + let amt_msat = 5000; + let final_cltv_delta = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck { + // Set the final CLTV expiry too low to trigger the failure in process_pending_htlc_forwards. + Some(TEST_FINAL_CLTV as u16 - 2) + } else { None }; + let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), final_cltv_delta); + let mut route_params = get_blinded_route_parameters(amt_msat, payment_secret, + nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&chan_upd_1_2], + &chanmon_cfgs[2].keys_manager); + + let route = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck { + let mut route = get_route(&nodes[0], &route_params).unwrap(); + // Set the final CLTV expiry too low to trigger the failure in process_pending_htlc_forwards. + route.paths[0].blinded_tail.as_mut().map(|bt| bt.excess_final_cltv_expiry_delta = TEST_FINAL_CLTV - 2); + route + } else if check == ReceiveCheckFail::PaymentConstraints { + // Create a blinded path where the receiver's encrypted payload has an htlc_minimum_msat that is + // violated by `amt_msat`, and stick it in the route_params without changing the corresponding + // BlindedPayInfo (to ensure pathfinding still succeeds). + let high_htlc_min_bp = { + let mut high_htlc_minimum_upd = chan_upd_1_2.clone(); + high_htlc_minimum_upd.htlc_minimum_msat = amt_msat + 1000; + let high_htlc_min_params = get_blinded_route_parameters(amt_msat, payment_secret, + nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&high_htlc_minimum_upd], + &chanmon_cfgs[2].keys_manager); + if let Payee::Blinded { route_hints, .. } = high_htlc_min_params.payment_params.payee { + route_hints[0].1.clone() + } else { panic!() } + }; + if let Payee::Blinded { ref mut route_hints, .. } = route_params.payment_params.payee { + route_hints[0].1 = high_htlc_min_bp; + } else { panic!() } + find_route(&nodes[0], &route_params).unwrap() + } else { + find_route(&nodes[0], &route_params).unwrap() + }; + node_cfgs[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); + nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap(); + check_added_monitors(&nodes[0], 1); + + let mut payment_event_0_1 = { + 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); + SendEvent::from_event(ev) + }; + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event_0_1.msgs[0]); + check_added_monitors!(nodes[1], 0); + do_commitment_signed_dance(&nodes[1], &nodes[0], &payment_event_0_1.commitment_msg, false, false); + expect_pending_htlcs_forwardable!(nodes[1]); check_added_monitors!(&nodes[1], 1); - assert!(update_fail.update_fail_htlcs.len() == 1); - let fail_msg = update_fail.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_fail.commitment_signed, false); + + let mut payment_event_1_2 = { + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); + SendEvent::from_event(ev) + }; + + match check { + ReceiveCheckFail::RecipientFail => { + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]); + check_added_monitors!(nodes[2], 0); + do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true); + expect_pending_htlcs_forwardable!(nodes[2]); + check_payment_claimable( + &nodes[2].node.get_and_clear_pending_events()[0], payment_hash, payment_secret, amt_msat, + None, nodes[2].node.get_our_node_id() + ); + nodes[2].node.fail_htlc_backwards(&payment_hash); + expect_pending_htlcs_forwardable_conditions( + nodes[2].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }] + ); + nodes[2].node.process_pending_htlc_forwards(); + check_added_monitors!(nodes[2], 1); + }, + ReceiveCheckFail::OnionDecodeFail => { + 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 cur_height = nodes[0].best_block_info().1; + let (mut onion_payloads, ..) = onion_utils::build_onion_payloads( + &route.paths[0], amt_msat, RecipientOnionFields::spontaneous_empty(), cur_height, &None).unwrap(); + + let update_add = &mut payment_event_1_2.msgs[0]; + onion_payloads.last_mut().map(|p| { + if let msgs::OutboundOnionPayload::BlindedReceive { ref mut intro_node_blinding_point, .. } = p { + // The receiver should error if both the update_add blinding_point and the + // intro_node_blinding_point are set. + assert!(intro_node_blinding_point.is_none() && update_add.blinding_point.is_some()); + *intro_node_blinding_point = Some(PublicKey::from_slice(&[2; 33]).unwrap()); + } else { panic!() } + }); + update_add.onion_routing_packet = onion_utils::construct_onion_packet( + vec![onion_payloads.pop().unwrap()], vec![onion_keys.pop().unwrap()], [0; 32], + &payment_hash + ).unwrap(); + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), update_add); + check_added_monitors!(nodes[2], 0); + do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true); + }, + ReceiveCheckFail::ReceiveRequirements => { + let update_add = &mut payment_event_1_2.msgs[0]; + update_add.amount_msat -= 1; + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), update_add); + check_added_monitors!(nodes[2], 0); + do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true); + }, + ReceiveCheckFail::ChannelCheck => { + nodes[2].node.close_channel(&chan_id_1_2, &nodes[1].node.get_our_node_id()).unwrap(); + let node_2_shutdown = get_event_msg!(nodes[2], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_shutdown(&nodes[2].node.get_our_node_id(), &node_2_shutdown); + let node_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[2].node.get_our_node_id()); + + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]); + nodes[2].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event_1_2.commitment_msg); + check_added_monitors!(nodes[2], 1); + + nodes[2].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &node_1_shutdown); + commitment_signed_dance!(nodes[2], nodes[1], (), false, true, false, false); + }, + ReceiveCheckFail::ProcessPendingHTLCsCheck => { + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]); + check_added_monitors!(nodes[2], 0); + do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true); + expect_pending_htlcs_forwardable!(nodes[2]); + expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[2], + vec![HTLCDestination::FailedPayment { payment_hash }]); + check_added_monitors!(nodes[2], 1); + }, + ReceiveCheckFail::PaymentConstraints => { + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]); + check_added_monitors!(nodes[2], 0); + do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true); + } + } + + 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 = if check == ReceiveCheckFail::ChannelCheck { + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + events.into_iter().find_map(|ev| { + match ev { + MessageSendEvent:: UpdateHTLCs { node_id, updates } => { + assert_eq!(node_id, nodes[0].node.get_our_node_id()); + return Some(updates) + }, + MessageSendEvent::SendClosingSigned { .. } => None, + _ => panic!() + } + }).unwrap() + } else { 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])); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8cff537bb..375beb6d6 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -259,6 +259,11 @@ enum HTLCUpdateAwaitingACK { htlc_id: u64, err_packet: msgs::OnionErrorPacket, }, + FailMalformedHTLC { + htlc_id: u64, + failure_code: u16, + sha256_of_onion: [u8; 32], + }, } macro_rules! define_state_flags { @@ -2518,6 +2523,64 @@ struct CommitmentTxInfoCached { feerate: u32, } +/// Contents of a wire message that fails an HTLC backwards. Useful for [`Channel::fail_htlc`] to +/// fail with either [`msgs::UpdateFailMalformedHTLC`] or [`msgs::UpdateFailHTLC`] as needed. +trait FailHTLCContents { + type Message: FailHTLCMessageName; + fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message; + fn to_inbound_htlc_state(self) -> InboundHTLCState; + fn to_htlc_update_awaiting_ack(self, htlc_id: u64) -> HTLCUpdateAwaitingACK; +} +impl FailHTLCContents for msgs::OnionErrorPacket { + type Message = msgs::UpdateFailHTLC; + fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message { + msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self } + } + fn to_inbound_htlc_state(self) -> InboundHTLCState { + InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self)) + } + fn to_htlc_update_awaiting_ack(self, htlc_id: u64) -> HTLCUpdateAwaitingACK { + HTLCUpdateAwaitingACK::FailHTLC { htlc_id, err_packet: self } + } +} +impl FailHTLCContents for (u16, [u8; 32]) { + type Message = msgs::UpdateFailMalformedHTLC; // (failure_code, sha256_of_onion) + fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message { + msgs::UpdateFailMalformedHTLC { + htlc_id, + channel_id, + failure_code: self.0, + sha256_of_onion: self.1 + } + } + fn to_inbound_htlc_state(self) -> InboundHTLCState { + InboundHTLCState::LocalRemoved( + InboundHTLCRemovalReason::FailMalformed((self.1, self.0)) + ) + } + fn to_htlc_update_awaiting_ack(self, htlc_id: u64) -> HTLCUpdateAwaitingACK { + HTLCUpdateAwaitingACK::FailMalformedHTLC { + htlc_id, + failure_code: self.0, + sha256_of_onion: self.1 + } + } +} + +trait FailHTLCMessageName { + fn name() -> &'static str; +} +impl FailHTLCMessageName for msgs::UpdateFailHTLC { + fn name() -> &'static str { + "update_fail_htlc" + } +} +impl FailHTLCMessageName for msgs::UpdateFailMalformedHTLC { + fn name() -> &'static str { + "update_fail_malformed_htlc" + } +} + impl Channel where SP::Target: SignerProvider, ::EcdsaSigner: WriteableEcdsaChannelSigner @@ -2719,7 +2782,9 @@ impl Channel where return UpdateFulfillFetch::DuplicateClaim {}; } }, - &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { + &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } | + &HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, .. } => + { if htlc_id_arg == htlc_id { log_warn!(logger, "Have preimage and want to fulfill HTLC with pending failure against channel {}", &self.context.channel_id()); // TODO: We may actually be able to switch to a fulfill here, though its @@ -2816,6 +2881,17 @@ impl Channel where .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) } + /// Used for failing back with [`msgs::UpdateFailMalformedHTLC`]. For now, this is used when we + /// want to fail blinded HTLCs where we are not the intro node. + /// + /// See [`Self::queue_fail_htlc`] for more info. + pub fn queue_fail_malformed_htlc( + &mut self, htlc_id_arg: u64, failure_code: u16, sha256_of_onion: [u8; 32], logger: &L + ) -> Result<(), ChannelError> where L::Target: Logger { + self.fail_htlc(htlc_id_arg, (failure_code, sha256_of_onion), true, logger) + .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) + } + /// We can only have one resolution per HTLC. In some cases around reconnect, we may fulfill /// an HTLC more than once or fulfill once and then attempt to fail after reconnect. We cannot, /// however, fail more than once as we wait for an upstream failure to be irrevocably committed @@ -2824,8 +2900,10 @@ impl Channel where /// If we do fail twice, we `debug_assert!(false)` and return `Ok(None)`. Thus, this will always /// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be /// [`ChannelError::Ignore`]. - fn fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, mut force_holding_cell: bool, logger: &L) - -> Result, ChannelError> where L::Target: Logger { + fn fail_htlc( + &mut self, htlc_id_arg: u64, err_packet: E, mut force_holding_cell: bool, + logger: &L + ) -> Result, ChannelError> where L::Target: Logger { if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { panic!("Was asked to fail an HTLC when channel was not in an operational state"); } @@ -2878,7 +2956,9 @@ impl Channel where return Ok(None); } }, - &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { + &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } | + &HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, .. } => + { if htlc_id_arg == htlc_id { debug_assert!(false, "Tried to fail an HTLC that was already failed"); return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); @@ -2888,24 +2968,18 @@ impl Channel where } } log_trace!(logger, "Placing failure for HTLC ID {} in holding cell in channel {}.", htlc_id_arg, &self.context.channel_id()); - self.context.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::FailHTLC { - htlc_id: htlc_id_arg, - err_packet, - }); + self.context.holding_cell_htlc_updates.push(err_packet.to_htlc_update_awaiting_ack(htlc_id_arg)); return Ok(None); } - log_trace!(logger, "Failing HTLC ID {} back with a update_fail_htlc message in channel {}.", htlc_id_arg, &self.context.channel_id()); + log_trace!(logger, "Failing HTLC ID {} back with {} message in channel {}.", htlc_id_arg, + E::Message::name(), &self.context.channel_id()); { let htlc = &mut self.context.pending_inbound_htlcs[pending_idx]; - htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(err_packet.clone())); + htlc.state = err_packet.clone().to_inbound_htlc_state(); } - Ok(Some(msgs::UpdateFailHTLC { - channel_id: self.context.channel_id(), - htlc_id: htlc_id_arg, - reason: err_packet - })) + Ok(Some(err_packet.to_message(htlc_id_arg, self.context.channel_id()))) } // Message handlers: @@ -3563,6 +3637,20 @@ impl Channel where } } }, + &HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => { + match self.fail_htlc(htlc_id, (failure_code, sha256_of_onion), false, logger) { + Ok(update_fail_malformed_opt) => { + debug_assert!(update_fail_malformed_opt.is_some()); // See above comment + update_fail_count += 1; + }, + Err(e) => { + if let ChannelError::Ignore(_) = e {} + else { + panic!("Got a non-IgnoreError action trying to fail holding cell HTLC"); + } + } + } + }, } } if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() { @@ -7433,6 +7521,8 @@ impl Writeable for Channel where SP::Target: SignerProvider { let mut holding_cell_skimmed_fees: Vec> = Vec::new(); let mut holding_cell_blinding_points: Vec> = Vec::new(); + // Vec of (htlc_id, failure_code, sha256_of_onion) + let mut malformed_htlcs: Vec<(u64, u16, [u8; 32])> = Vec::new(); (self.context.holding_cell_htlc_updates.len() as u64).write(writer)?; for update in self.context.holding_cell_htlc_updates.iter() { match update { @@ -7460,6 +7550,18 @@ impl Writeable for Channel where SP::Target: SignerProvider { htlc_id.write(writer)?; err_packet.write(writer)?; } + &HTLCUpdateAwaitingACK::FailMalformedHTLC { + htlc_id, failure_code, sha256_of_onion + } => { + // We don't want to break downgrading by adding a new variant, so write a dummy + // `::FailHTLC` variant and write the real malformed error as an optional TLV. + malformed_htlcs.push((htlc_id, failure_code, sha256_of_onion)); + + let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() }; + 2u8.write(writer)?; + htlc_id.write(writer)?; + dummy_err_packet.write(writer)?; + } } } @@ -7620,6 +7722,7 @@ impl Writeable for Channel where SP::Target: SignerProvider { (38, self.context.is_batch_funding, option), (39, pending_outbound_blinding_points, optional_vec), (41, holding_cell_blinding_points, optional_vec), + (43, malformed_htlcs, optional_vec), // Added in 0.0.119 }); Ok(()) @@ -7910,6 +8013,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch let mut pending_outbound_blinding_points_opt: Option>> = None; let mut holding_cell_blinding_points_opt: Option>> = None; + let mut malformed_htlcs: Option> = None; + read_tlv_fields!(reader, { (0, announcement_sigs, option), (1, minimum_depth, option), @@ -7938,6 +8043,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch (38, is_batch_funding, option), (39, pending_outbound_blinding_points_opt, optional_vec), (41, holding_cell_blinding_points_opt, optional_vec), + (43, malformed_htlcs, optional_vec), // Added in 0.0.119 }); let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id { @@ -8032,6 +8138,22 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch if iter.next().is_some() { return Err(DecodeError::InvalidValue) } } + if let Some(malformed_htlcs) = malformed_htlcs { + for (malformed_htlc_id, failure_code, sha256_of_onion) in malformed_htlcs { + let htlc_idx = holding_cell_htlc_updates.iter().position(|htlc| { + if let HTLCUpdateAwaitingACK::FailHTLC { htlc_id, err_packet } = htlc { + let matches = *htlc_id == malformed_htlc_id; + if matches { debug_assert!(err_packet.data.is_empty()) } + matches + } else { false } + }).ok_or(DecodeError::InvalidValue)?; + let malformed_htlc = HTLCUpdateAwaitingACK::FailMalformedHTLC { + htlc_id: malformed_htlc_id, failure_code, sha256_of_onion + }; + let _ = core::mem::replace(&mut holding_cell_htlc_updates[htlc_idx], malformed_htlc); + } + } + Ok(Channel { context: ChannelContext { user_id, @@ -8166,6 +8288,7 @@ mod tests { use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::blockdata::opcodes; use bitcoin::network::constants::Network; + use crate::ln::onion_utils::INVALID_ONION_BLINDING; use crate::ln::{PaymentHash, PaymentPreimage}; use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint}; use crate::ln::channelmanager::{self, HTLCSource, PaymentId}; @@ -8702,8 +8825,9 @@ mod tests { } #[test] - fn blinding_point_skimmed_fee_ser() { - // Ensure that channel blinding points and skimmed fees are (de)serialized properly. + fn blinding_point_skimmed_fee_malformed_ser() { + // Ensure that channel blinding points, skimmed fees, and malformed HTLCs are (de)serialized + // properly. let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; @@ -8768,13 +8892,19 @@ mod tests { payment_preimage: PaymentPreimage([42; 32]), htlc_id: 0, }; - let mut holding_cell_htlc_updates = Vec::with_capacity(10); - for i in 0..10 { - if i % 3 == 0 { + let dummy_holding_cell_failed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailHTLC { + htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } + }; + let dummy_holding_cell_malformed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailMalformedHTLC { + htlc_id, failure_code: INVALID_ONION_BLINDING, sha256_of_onion: [0; 32], + }; + let mut holding_cell_htlc_updates = Vec::with_capacity(12); + for i in 0..12 { + if i % 5 == 0 { holding_cell_htlc_updates.push(dummy_holding_cell_add_htlc.clone()); - } else if i % 3 == 1 { + } else if i % 5 == 1 { holding_cell_htlc_updates.push(dummy_holding_cell_claim_htlc.clone()); - } else { + } else if i % 5 == 2 { let mut dummy_add = dummy_holding_cell_add_htlc.clone(); if let HTLCUpdateAwaitingACK::AddHTLC { ref mut blinding_point, ref mut skimmed_fee_msat, .. @@ -8783,6 +8913,10 @@ mod tests { *skimmed_fee_msat = Some(42); } else { panic!() } holding_cell_htlc_updates.push(dummy_add); + } else if i % 5 == 3 { + holding_cell_htlc_updates.push(dummy_holding_cell_malformed_htlc(i as u64)); + } else { + holding_cell_htlc_updates.push(dummy_holding_cell_failed_htlc(i as u64)); } } chan.context.holding_cell_htlc_updates = holding_cell_htlc_updates.clone(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index da95e3b61..f08096426 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -111,6 +111,7 @@ use crate::ln::script::ShutdownScript; /// Information about where a received HTLC('s onion) has indicated the HTLC should go. #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug +#[cfg_attr(test, derive(Debug, PartialEq))] pub enum PendingHTLCRouting { /// An HTLC which should be forwarded on to another node. Forward { @@ -155,6 +156,8 @@ pub enum PendingHTLCRouting { /// [`Event::PaymentClaimable::onion_fields`] as /// [`RecipientOnionFields::custom_tlvs`]. custom_tlvs: Vec<(u64, Vec)>, + /// Set if this HTLC is the final hop in a multi-hop blinded path. + requires_blinded_error: bool, }, /// The onion indicates that this is for payment to us but which contains the preimage for /// claiming included, and is unrelated to any invoice we'd previously generated (aka a @@ -187,7 +190,7 @@ pub enum PendingHTLCRouting { } /// Information used to forward or fail this HTLC that is being forwarded within a blinded path. -#[derive(Clone, Copy, Hash, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] pub struct BlindedForward { /// The `blinding_point` that was set in the inbound [`msgs::UpdateAddHTLC`], or in the inbound /// onion payload if we're the introduction node. Useful for calculating the next hop's @@ -199,11 +202,11 @@ pub struct BlindedForward { impl PendingHTLCRouting { // Used to override the onion failure code and data if the HTLC is blinded. fn blinded_failure(&self) -> Option { - // TODO: needs update when we support receiving to multi-hop blinded paths - if let Self::Forward { blinded: Some(_), .. } = self { - Some(BlindedFailure::FromIntroductionNode) - } else { - None + // TODO: needs update when we support forwarding blinded HTLCs as non-intro node + match self { + Self::Forward { blinded: Some(_), .. } => Some(BlindedFailure::FromIntroductionNode), + Self::Receive { requires_blinded_error: true, .. } => Some(BlindedFailure::FromBlindedNode), + _ => None, } } } @@ -211,6 +214,7 @@ impl PendingHTLCRouting { /// Information about an incoming HTLC, including the [`PendingHTLCRouting`] describing where it /// should go next. #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug +#[cfg_attr(test, derive(Debug, PartialEq))] pub struct PendingHTLCInfo { /// Further routing details based on whether the HTLC is being forwarded or received. pub routing: PendingHTLCRouting, @@ -265,6 +269,7 @@ pub(super) enum PendingHTLCStatus { Fail(HTLCFailureMsg), } +#[cfg_attr(test, derive(Clone, Debug, PartialEq))] pub(super) struct PendingAddHTLCInfo { pub(super) forward_info: PendingHTLCInfo, @@ -280,19 +285,25 @@ pub(super) struct PendingAddHTLCInfo { prev_user_channel_id: u128, } +#[cfg_attr(test, derive(Clone, Debug, PartialEq))] pub(super) enum HTLCForwardInfo { AddHTLC(PendingAddHTLCInfo), FailHTLC { htlc_id: u64, err_packet: msgs::OnionErrorPacket, }, + FailMalformedHTLC { + htlc_id: u64, + failure_code: u16, + sha256_of_onion: [u8; 32], + }, } // Used for failing blinded HTLCs backwards correctly. -#[derive(Clone, Debug, Hash, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] enum BlindedFailure { FromIntroductionNode, - // Another variant will be added here for non-intro nodes. + FromBlindedNode, } /// Tracks the inbound corresponding to an outbound HTLC @@ -3016,11 +3027,12 @@ where msg, &self.node_signer, &self.logger, &self.secp_ctx )?; - let is_blinded = match next_hop { + let is_intro_node_forward = match next_hop { onion_utils::Hop::Forward { + // TODO: update this when we support blinded forwarding as non-intro node next_hop_data: msgs::InboundOnionPayload::BlindedForward { .. }, .. } => true, - _ => false, // TODO: update this when we support receiving to multi-hop blinded paths + _ => false, }; macro_rules! return_err { @@ -3030,7 +3042,17 @@ where WithContext::from(&self.logger, Some(*counterparty_node_id), Some(msg.channel_id)), "Failed to accept/forward incoming HTLC: {}", $msg ); - let (err_code, err_data) = if is_blinded { + // If `msg.blinding_point` is set, we must always fail with malformed. + if msg.blinding_point.is_some() { + return Err(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC { + channel_id: msg.channel_id, + htlc_id: msg.htlc_id, + sha256_of_onion: [0; 32], + failure_code: INVALID_ONION_BLINDING, + })); + } + + let (err_code, err_data) = if is_intro_node_forward { (INVALID_ONION_BLINDING, &[0; 32][..]) } else { ($err_code, $data) }; return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC { @@ -3183,6 +3205,16 @@ where { let logger = WithContext::from(&self.logger, Some(*counterparty_node_id), Some(msg.channel_id)); log_info!(logger, "Failed to accept/forward incoming HTLC: {}", $msg); + if msg.blinding_point.is_some() { + return PendingHTLCStatus::Fail(HTLCFailureMsg::Malformed( + msgs::UpdateFailMalformedHTLC { + channel_id: msg.channel_id, + htlc_id: msg.htlc_id, + sha256_of_onion: [0; 32], + failure_code: INVALID_ONION_BLINDING, + } + )) + } return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC { channel_id: msg.channel_id, htlc_id: msg.htlc_id, @@ -4246,7 +4278,7 @@ where let phantom_shared_secret = self.node_signer.ecdh(Recipient::PhantomNode, &onion_packet.public_key.unwrap(), None).unwrap().secret_bytes(); let next_hop = match onion_utils::decode_next_payment_hop( phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac, - payment_hash, &self.node_signer + payment_hash, None, &self.node_signer ) { Ok(res) => res, Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => { @@ -4282,7 +4314,7 @@ where fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None); } }, - HTLCForwardInfo::FailHTLC { .. } => { + HTLCForwardInfo::FailHTLC { .. } | HTLCForwardInfo::FailMalformedHTLC { .. } => { // Channel went away before we could fail it. This implies // the channel is now on chain and our counterparty is // trying to broadcast the HTLC-Timeout, but that's their @@ -4378,6 +4410,20 @@ where continue; } }, + HTLCForwardInfo::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => { + log_trace!(self.logger, "Failing malformed HTLC back to channel with short id {} (backward HTLC ID {}) after delay", short_chan_id, htlc_id); + if let Err(e) = chan.queue_fail_malformed_htlc(htlc_id, failure_code, sha256_of_onion, &self.logger) { + if let ChannelError::Ignore(msg) = e { + log_trace!(self.logger, "Failed to fail HTLC with ID {} backwards to short_id {}: {}", htlc_id, short_chan_id, msg); + } else { + panic!("Stated return value requirements in queue_fail_malformed_htlc() were not met"); + } + // fail-backs are best-effort, we probably already have one + // pending, and if not that's OK, if not, the channel is on + // the chain and sending the HTLC-Timeout is their problem. + continue; + } + }, } } } else { @@ -4396,7 +4442,10 @@ where }) => { let blinded_failure = routing.blinded_failure(); let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret, mut onion_fields) = match routing { - PendingHTLCRouting::Receive { payment_data, payment_metadata, incoming_cltv_expiry, phantom_shared_secret, custom_tlvs } => { + PendingHTLCRouting::Receive { + payment_data, payment_metadata, incoming_cltv_expiry, phantom_shared_secret, + custom_tlvs, requires_blinded_error: _ + } => { let _legacy_hop_data = Some(payment_data.clone()); let onion_fields = RecipientOnionFields { payment_secret: Some(payment_data.payment_secret), payment_metadata, custom_tlvs }; @@ -4455,7 +4504,7 @@ where htlc_id: $htlc.prev_hop.htlc_id, incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret, phantom_shared_secret, - blinded_failure: None, + blinded_failure, }), payment_hash, HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data), HTLCDestination::FailedPayment { payment_hash: $payment_hash }, @@ -4629,7 +4678,7 @@ where }, }; }, - HTLCForwardInfo::FailHTLC { .. } => { + HTLCForwardInfo::FailHTLC { .. } | HTLCForwardInfo::FailMalformedHTLC { .. } => { panic!("Got pending fail of our own HTLC"); } } @@ -5234,15 +5283,26 @@ where "Failing {}HTLC with payment_hash {} backwards from us: {:?}", if blinded_failure.is_some() { "blinded " } else { "" }, &payment_hash, onion_error ); - let err_packet = match blinded_failure { + let failure = match blinded_failure { Some(BlindedFailure::FromIntroductionNode) => { let blinded_onion_error = HTLCFailReason::reason(INVALID_ONION_BLINDING, vec![0; 32]); - blinded_onion_error.get_encrypted_failure_packet( + let err_packet = blinded_onion_error.get_encrypted_failure_packet( incoming_packet_shared_secret, phantom_shared_secret - ) + ); + HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet } + }, + Some(BlindedFailure::FromBlindedNode) => { + HTLCForwardInfo::FailMalformedHTLC { + htlc_id: *htlc_id, + failure_code: INVALID_ONION_BLINDING, + sha256_of_onion: [0; 32] + } }, None => { - onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret) + let err_packet = onion_error.get_encrypted_failure_packet( + incoming_packet_shared_secret, phantom_shared_secret + ); + HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet } } }; @@ -5253,10 +5313,10 @@ where } match forward_htlcs.entry(*short_channel_id) { hash_map::Entry::Occupied(mut entry) => { - entry.get_mut().push(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet }); + entry.get_mut().push(failure); }, hash_map::Entry::Vacant(entry) => { - entry.insert(vec!(HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet })); + entry.insert(vec!(failure)); } } mem::drop(forward_htlcs); @@ -6556,6 +6616,16 @@ where Err(e) => PendingHTLCStatus::Fail(e) }; let create_pending_htlc_status = |chan: &Channel, pending_forward_info: PendingHTLCStatus, error_code: u16| { + if msg.blinding_point.is_some() { + return PendingHTLCStatus::Fail(HTLCFailureMsg::Malformed( + msgs::UpdateFailMalformedHTLC { + channel_id: msg.channel_id, + htlc_id: msg.htlc_id, + sha256_of_onion: [0; 32], + failure_code: INVALID_ONION_BLINDING, + } + )) + } // If the update_add is completely bogus, the call will Err and we will close, // but if we've sent a shutdown and they haven't acknowledged it yet, we just // want to reject the new HTLC and fail it backwards instead of forwarding. @@ -9371,6 +9441,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting, (2, incoming_cltv_expiry, required), (3, payment_metadata, option), (5, custom_tlvs, optional_vec), + (7, requires_blinded_error, (default_value, false)), }, (2, ReceiveKeysend) => { (0, payment_preimage, required), @@ -9465,7 +9536,8 @@ impl_writeable_tlv_based_enum!(PendingHTLCStatus, ; ); impl_writeable_tlv_based_enum!(BlindedFailure, - (0, FromIntroductionNode) => {}, ; + (0, FromIntroductionNode) => {}, + (2, FromBlindedNode) => {}, ; ); impl_writeable_tlv_based!(HTLCPreviousHopData, { @@ -9629,13 +9701,68 @@ impl_writeable_tlv_based!(PendingAddHTLCInfo, { (6, prev_funding_outpoint, required), }); -impl_writeable_tlv_based_enum!(HTLCForwardInfo, - (1, FailHTLC) => { - (0, htlc_id, required), - (2, err_packet, required), - }; - (0, AddHTLC) -); +impl Writeable for HTLCForwardInfo { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + const FAIL_HTLC_VARIANT_ID: u8 = 1; + match self { + Self::AddHTLC(info) => { + 0u8.write(w)?; + info.write(w)?; + }, + Self::FailHTLC { htlc_id, err_packet } => { + FAIL_HTLC_VARIANT_ID.write(w)?; + write_tlv_fields!(w, { + (0, htlc_id, required), + (2, err_packet, required), + }); + }, + Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => { + // Since this variant was added in 0.0.119, write this as `::FailHTLC` with an empty error + // packet so older versions have something to fail back with, but serialize the real data as + // optional TLVs for the benefit of newer versions. + FAIL_HTLC_VARIANT_ID.write(w)?; + let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() }; + write_tlv_fields!(w, { + (0, htlc_id, required), + (1, failure_code, required), + (2, dummy_err_packet, required), + (3, sha256_of_onion, required), + }); + }, + } + Ok(()) + } +} + +impl Readable for HTLCForwardInfo { + fn read(r: &mut R) -> Result { + let id: u8 = Readable::read(r)?; + Ok(match id { + 0 => Self::AddHTLC(Readable::read(r)?), + 1 => { + _init_and_read_len_prefixed_tlv_fields!(r, { + (0, htlc_id, required), + (1, malformed_htlc_failure_code, option), + (2, err_packet, required), + (3, sha256_of_onion, option), + }); + if let Some(failure_code) = malformed_htlc_failure_code { + Self::FailMalformedHTLC { + htlc_id: _init_tlv_based_struct_field!(htlc_id, required), + failure_code, + sha256_of_onion: sha256_of_onion.ok_or(DecodeError::InvalidValue)?, + } + } else { + Self::FailHTLC { + htlc_id: _init_tlv_based_struct_field!(htlc_id, required), + err_packet: _init_tlv_based_struct_field!(err_packet, required), + } + } + }, + _ => return Err(DecodeError::InvalidValue), + }) + } +} impl_writeable_tlv_based!(PendingInboundPayment, { (0, payment_secret, required), @@ -10930,12 +11057,14 @@ mod tests { use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use crate::ln::ChannelId; - use crate::ln::channelmanager::{create_recv_pending_htlc_info, inbound_payment, PaymentId, PaymentSendFailure, RecipientOnionFields, InterceptId}; + use crate::ln::channelmanager::{create_recv_pending_htlc_info, HTLCForwardInfo, inbound_payment, PaymentId, PaymentSendFailure, RecipientOnionFields, InterceptId}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{self, ErrorAction}; use crate::ln::msgs::ChannelMessageHandler; + use crate::prelude::*; use crate::routing::router::{PaymentParameters, RouteParameters, find_route}; use crate::util::errors::APIError; + use crate::util::ser::Writeable; use crate::util::test_utils; use crate::util::config::{ChannelConfig, ChannelConfigUpdate}; use crate::sign::EntropySource; @@ -12210,6 +12339,63 @@ mod tests { check_spends!(txn[0], funding_tx); } } + + #[test] + fn test_malformed_forward_htlcs_ser() { + // Ensure that `HTLCForwardInfo::FailMalformedHTLC`s are (de)serialized properly. + let chanmon_cfg = create_chanmon_cfgs(1); + let node_cfg = create_node_cfgs(1, &chanmon_cfg); + let persister; + let chain_monitor; + let chanmgrs = create_node_chanmgrs(1, &node_cfg, &[None]); + let deserialized_chanmgr; + let mut nodes = create_network(1, &node_cfg, &chanmgrs); + + let dummy_failed_htlc = |htlc_id| { + HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }, } + }; + let dummy_malformed_htlc = |htlc_id| { + HTLCForwardInfo::FailMalformedHTLC { htlc_id, failure_code: 0x4000, sha256_of_onion: [0; 32] } + }; + + let dummy_htlcs_1: Vec = (1..10).map(|htlc_id| { + if htlc_id % 2 == 0 { + dummy_failed_htlc(htlc_id) + } else { + dummy_malformed_htlc(htlc_id) + } + }).collect(); + + let dummy_htlcs_2: Vec = (1..10).map(|htlc_id| { + if htlc_id % 2 == 1 { + dummy_failed_htlc(htlc_id) + } else { + dummy_malformed_htlc(htlc_id) + } + }).collect(); + + + let (scid_1, scid_2) = (42, 43); + let mut forward_htlcs = HashMap::new(); + forward_htlcs.insert(scid_1, dummy_htlcs_1.clone()); + forward_htlcs.insert(scid_2, dummy_htlcs_2.clone()); + + let mut chanmgr_fwd_htlcs = nodes[0].node.forward_htlcs.lock().unwrap(); + *chanmgr_fwd_htlcs = forward_htlcs.clone(); + core::mem::drop(chanmgr_fwd_htlcs); + + reload_node!(nodes[0], nodes[0].node.encode(), &[], persister, chain_monitor, deserialized_chanmgr); + + let mut deserialized_fwd_htlcs = nodes[0].node.forward_htlcs.lock().unwrap(); + for scid in [scid_1, scid_2].iter() { + let deserialized_htlcs = deserialized_fwd_htlcs.remove(scid).unwrap(); + assert_eq!(forward_htlcs.remove(scid).unwrap(), deserialized_htlcs); + } + assert!(deserialized_fwd_htlcs.is_empty()); + core::mem::drop(deserialized_fwd_htlcs); + + expect_pending_htlcs_forwardable!(nodes[0]); + } } #[cfg(ldk_bench)] diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d3d2e3322..5828ca803 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1971,6 +1971,18 @@ pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> Result Result { + let scorer = TestScorer::new(); + let keys_manager = TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet); + let random_seed_bytes = keys_manager.get_secure_random_bytes(); + router::find_route( + &send_node.node.get_our_node_id(), route_params, &send_node.network_graph, + Some(&send_node.node.list_usable_channels().iter().collect::>()), + send_node.logger, &scorer, &Default::default(), &random_seed_bytes + ) +} + /// Gets a route from the given sender to the node described in `payment_params`. /// /// Don't use this, use the identically-named function instead. diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index b877565e0..689058c7e 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1678,6 +1678,7 @@ mod fuzzy_internal_msgs { // These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize // them from untrusted input): #[derive(Clone)] + #[cfg_attr(test, derive(Debug, PartialEq))] pub struct FinalOnionHopData { pub payment_secret: PaymentSecret, /// The total value, in msat, of the payment as received by the ultimate recipient. @@ -1713,7 +1714,7 @@ mod fuzzy_internal_msgs { outgoing_cltv_value: u32, payment_secret: PaymentSecret, payment_constraints: PaymentConstraints, - intro_node_blinding_point: PublicKey, + intro_node_blinding_point: Option, } } @@ -2319,8 +2320,10 @@ impl Writeable for OutboundOnionPayload { } } -impl ReadableArgs<&NS> for InboundOnionPayload where NS::Target: NodeSigner { - fn read(r: &mut R, node_signer: &NS) -> Result { +impl ReadableArgs<(Option, &NS)> for InboundOnionPayload where NS::Target: NodeSigner { + fn read(r: &mut R, args: (Option, &NS)) -> Result { + let (update_add_blinding_point, node_signer) = args; + let mut amt = None; let mut cltv_value = None; let mut short_id: Option = None; @@ -2354,9 +2357,14 @@ impl ReadableArgs<&NS> for InboundOnionPayload where NS::Target: Node }); if amt.unwrap_or(0) > MAX_VALUE_MSAT { return Err(DecodeError::InvalidValue) } + if intro_node_blinding_point.is_some() && update_add_blinding_point.is_some() { + return Err(DecodeError::InvalidValue) + } - if let Some(blinding_point) = intro_node_blinding_point { - if short_id.is_some() || payment_data.is_some() || payment_metadata.is_some() { + if let Some(blinding_point) = intro_node_blinding_point.or(update_add_blinding_point) { + if short_id.is_some() || payment_data.is_some() || payment_metadata.is_some() || + keysend_preimage.is_some() + { return Err(DecodeError::InvalidValue) } let enc_tlvs = encrypted_tlvs_opt.ok_or(DecodeError::InvalidValue)?.0; @@ -2377,7 +2385,7 @@ impl ReadableArgs<&NS> for InboundOnionPayload where NS::Target: Node payment_relay, payment_constraints, features, - intro_node_blinding_point: blinding_point, + intro_node_blinding_point: intro_node_blinding_point.ok_or(DecodeError::InvalidValue)?, }) }, ChaChaPolyReadAdapter { readable: BlindedPaymentTlvs::Receive(ReceiveTlvs { @@ -2390,7 +2398,7 @@ impl ReadableArgs<&NS> for InboundOnionPayload where NS::Target: Node outgoing_cltv_value: cltv_value.ok_or(DecodeError::InvalidValue)?, payment_secret, payment_constraints, - intro_node_blinding_point: blinding_point, + intro_node_blinding_point, }) }, } @@ -3987,7 +3995,7 @@ mod tests { assert_eq!(encoded_value, target_value); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); - let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), &&node_signer).unwrap(); + let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &&node_signer)).unwrap(); if let msgs::InboundOnionPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } = inbound_msg { @@ -4012,7 +4020,7 @@ mod tests { assert_eq!(encoded_value, target_value); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); - let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), &&node_signer).unwrap(); + let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &&node_signer)).unwrap(); if let msgs::InboundOnionPayload::Receive { payment_data: None, amt_msat, outgoing_cltv_value, .. } = inbound_msg { @@ -4040,7 +4048,7 @@ mod tests { assert_eq!(encoded_value, target_value); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); - let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), &&node_signer).unwrap(); + let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &&node_signer)).unwrap(); if let msgs::InboundOnionPayload::Receive { payment_data: Some(FinalOnionHopData { payment_secret, @@ -4076,7 +4084,7 @@ mod tests { }; let encoded_value = msg.encode(); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); - assert!(msgs::InboundOnionPayload::read(&mut Cursor::new(&encoded_value[..]), &&node_signer).is_err()); + assert!(msgs::InboundOnionPayload::read(&mut Cursor::new(&encoded_value[..]), (None, &&node_signer)).is_err()); let good_type_range_tlvs = vec![ ((1 << 16) - 3, vec![42]), ((1 << 16) - 1, vec![42; 32]), @@ -4085,7 +4093,7 @@ mod tests { *custom_tlvs = good_type_range_tlvs.clone(); } let encoded_value = msg.encode(); - let inbound_msg = ReadableArgs::read(&mut Cursor::new(&encoded_value[..]), &&node_signer).unwrap(); + let inbound_msg = ReadableArgs::read(&mut Cursor::new(&encoded_value[..]), (None, &&node_signer)).unwrap(); match inbound_msg { msgs::InboundOnionPayload::Receive { custom_tlvs, .. } => assert!(custom_tlvs.is_empty()), _ => panic!(), @@ -4110,7 +4118,7 @@ mod tests { let target_value = >::from_hex("2e02080badf00d010203040404ffffffffff0000000146c6616b021234ff0000000146c6616f084242424242424242").unwrap(); assert_eq!(encoded_value, target_value); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); - let inbound_msg: msgs::InboundOnionPayload = ReadableArgs::read(&mut Cursor::new(&target_value[..]), &&node_signer).unwrap(); + let inbound_msg: msgs::InboundOnionPayload = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &&node_signer)).unwrap(); if let msgs::InboundOnionPayload::Receive { payment_data: None, payment_metadata: None, @@ -4275,8 +4283,8 @@ mod tests { let mut rd = Cursor::new(&big_payload[..]); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); - > - ::read(&mut rd, &&node_signer).unwrap(); + , &&test_utils::TestKeysInterface)>> + ::read(&mut rd, (None, &&node_signer)).unwrap(); } // see above test, needs to be a separate method for use of the serialization macros. fn encode_big_payload() -> Result, io::Error> { diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 7c0d32a52..5966dce91 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -3,9 +3,10 @@ //! Primarily features [`peel_payment_onion`], which allows the decoding of an onion statelessly //! and can be used to predict whether we'd accept a payment. -use bitcoin::hashes::Hash; +use bitcoin::hashes::{Hash, HashEngine}; +use bitcoin::hashes::hmac::{Hmac, HmacEngine}; use bitcoin::hashes::sha256::Hash as Sha256; -use bitcoin::secp256k1::{self, Secp256k1, PublicKey}; +use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1}; use crate::blinded_path; use crate::blinded_path::payment::{PaymentConstraints, PaymentRelay}; @@ -33,6 +34,15 @@ pub struct InboundOnionErr { pub msg: &'static str, } +fn check_blinded_payment_constraints( + amt_msat: u64, cltv_expiry: u32, constraints: &PaymentConstraints +) -> Result<(), ()> { + if amt_msat < constraints.htlc_minimum_msat || + cltv_expiry > constraints.max_cltv_expiry + { return Err(()) } + Ok(()) +} + fn check_blinded_forward( inbound_amt_msat: u64, inbound_cltv_expiry: u32, payment_relay: &PaymentRelay, payment_constraints: &PaymentConstraints, features: &BlindedHopFeatures @@ -43,9 +53,8 @@ fn check_blinded_forward( let outgoing_cltv_value = inbound_cltv_expiry.checked_sub( payment_relay.cltv_expiry_delta as u32 ).ok_or(())?; - if inbound_amt_msat < payment_constraints.htlc_minimum_msat || - outgoing_cltv_value > payment_constraints.max_cltv_expiry - { return Err(()) } + check_blinded_payment_constraints(inbound_amt_msat, outgoing_cltv_value, payment_constraints)?; + if features.requires_unknown_bits_from(&BlindedHopFeatures::empty()) { return Err(()) } Ok((amt_to_forward, outgoing_cltv_value)) } @@ -112,16 +121,30 @@ pub(super) fn create_recv_pending_htlc_info( amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool, counterparty_skimmed_fee_msat: Option, current_height: u32, accept_mpp_keysend: bool, ) -> Result { - let (payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, outgoing_cltv_value, payment_metadata) = match hop_data { + let ( + payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, outgoing_cltv_value, + payment_metadata, requires_blinded_error + ) = match hop_data { msgs::InboundOnionPayload::Receive { payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata, .. } => - (payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata), + (payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata, + false), msgs::InboundOnionPayload::BlindedReceive { - amt_msat, total_msat, outgoing_cltv_value, payment_secret, .. + amt_msat, total_msat, outgoing_cltv_value, payment_secret, intro_node_blinding_point, + payment_constraints, .. } => { + check_blinded_payment_constraints(amt_msat, cltv_expiry, &payment_constraints) + .map_err(|()| { + InboundOnionErr { + err_code: INVALID_ONION_BLINDING, + err_data: vec![0; 32], + msg: "Amount or cltv_expiry violated blinded payment constraints", + } + })?; let payment_data = msgs::FinalOnionHopData { payment_secret, total_msat }; - (Some(payment_data), None, Vec::new(), amt_msat, outgoing_cltv_value, None) + (Some(payment_data), None, Vec::new(), amt_msat, outgoing_cltv_value, None, + intro_node_blinding_point.is_none()) } msgs::InboundOnionPayload::Forward { .. } => { return Err(InboundOnionErr { @@ -208,6 +231,7 @@ pub(super) fn create_recv_pending_htlc_info( incoming_cltv_expiry: outgoing_cltv_value, phantom_shared_secret, custom_tlvs, + requires_blinded_error, } } else { return Err(InboundOnionErr { @@ -313,11 +337,16 @@ where ($msg: expr, $err_code: expr) => { { log_info!(logger, "Failed to accept/forward incoming HTLC: {}", $msg); + let (sha256_of_onion, failure_code) = if msg.blinding_point.is_some() { + ([0; 32], INVALID_ONION_BLINDING) + } else { + (Sha256::hash(&msg.onion_routing_packet.hop_data).to_byte_array(), $err_code) + }; return Err(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC { channel_id: msg.channel_id, htlc_id: msg.htlc_id, - sha256_of_onion: Sha256::hash(&msg.onion_routing_packet.hop_data).to_byte_array(), - failure_code: $err_code, + sha256_of_onion, + failure_code, })); } } @@ -327,8 +356,14 @@ where return_malformed_err!("invalid ephemeral pubkey", 0x8000 | 0x4000 | 6); } + let blinded_node_id_tweak = msg.blinding_point.map(|bp| { + let blinded_tlvs_ss = node_signer.ecdh(Recipient::Node, &bp, None).unwrap().secret_bytes(); + let mut hmac = HmacEngine::::new(b"blinded_node_id"); + hmac.input(blinded_tlvs_ss.as_ref()); + Scalar::from_be_bytes(Hmac::from_engine(hmac).to_byte_array()).unwrap() + }); let shared_secret = node_signer.ecdh( - Recipient::Node, &msg.onion_routing_packet.public_key.unwrap(), None + Recipient::Node, &msg.onion_routing_packet.public_key.unwrap(), blinded_node_id_tweak.as_ref() ).unwrap().secret_bytes(); if msg.onion_routing_packet.version != 0 { @@ -343,6 +378,10 @@ where macro_rules! return_err { ($msg: expr, $err_code: expr, $data: expr) => { { + if msg.blinding_point.is_some() { + return_malformed_err!($msg, INVALID_ONION_BLINDING) + } + log_info!(logger, "Failed to accept/forward incoming HTLC: {}", $msg); return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC { channel_id: msg.channel_id, @@ -356,7 +395,7 @@ where let next_hop = match onion_utils::decode_next_payment_hop( shared_secret, &msg.onion_routing_packet.hop_data[..], msg.onion_routing_packet.hmac, - msg.payment_hash, node_signer + msg.payment_hash, msg.blinding_point, node_signer ) { Ok(res) => res, Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => { diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index ea906c9e3..99e3e965a 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -940,9 +940,11 @@ pub(crate) enum OnionDecodeErr { pub(crate) fn decode_next_payment_hop( shared_secret: [u8; 32], hop_data: &[u8], hmac_bytes: [u8; 32], payment_hash: PaymentHash, - node_signer: &NS, + blinding_point: Option, node_signer: &NS, ) -> Result where NS::Target: NodeSigner { - match decode_next_hop(shared_secret, hop_data, hmac_bytes, Some(payment_hash), node_signer) { + match decode_next_hop( + shared_secret, hop_data, hmac_bytes, Some(payment_hash), (blinding_point, node_signer) + ) { Ok((next_hop_data, None)) => Ok(Hop::Receive(next_hop_data)), Ok((next_hop_data, Some((next_hop_hmac, FixedSizeOnionPacket(new_packet_bytes))))) => { Ok(Hop::Forward { diff --git a/pending_changelog/multihop-blinded-recv.txt b/pending_changelog/multihop-blinded-recv.txt new file mode 100644 index 000000000..8bfafc8fb --- /dev/null +++ b/pending_changelog/multihop-blinded-recv.txt @@ -0,0 +1,4 @@ +## Backwards Compatibility + +* Nodes that upgrade to 0.0.119 and subsequently downgrade after receiving a payment to a blinded + path may lose privacy if one or more of those HTLCs fails.