From e1ed52fae77794e2c64adb8737c64be6e264a631 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 8 Oct 2023 17:02:16 -1000 Subject: [PATCH 01/27] Pass in update add blinding point on onion decode Will be used to read encrypted_tlvs on non-intro-node onion receipt. --- fuzz/src/onion_hop_data.rs | 6 ++++-- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/msgs.rs | 22 ++++++++++++---------- lightning/src/ln/onion_payment.rs | 2 +- lightning/src/ln/onion_utils.rs | 6 ++++-- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/fuzz/src/onion_hop_data.rs b/fuzz/src/onion_hop_data.rs index cc80ccf9320..e7f51b99167 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/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7c6ee1e429c..42f901ac78d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4248,7 +4248,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 }) => { diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index b877565e017..b987ab6db1e 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -2319,8 +2319,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; @@ -3987,7 +3989,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 +4014,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 +4042,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 +4078,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 +4087,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 +4112,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 +4277,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 06a9ae07e1e..f61c05df7a4 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -355,7 +355,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 051e78c46ee..b7da2fd882d 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 { From 87a25c7a5bbe9edf9e7ca3865d62f55aea039608 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 15:14:07 -0400 Subject: [PATCH 02/27] Support parsing blinded non-intro onion receive payloads. Support for receiving to multi-hop blinded payment paths will be completed in the next commit, sans error handling. --- lightning/src/ln/msgs.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index b987ab6db1e..db7579039cc 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1713,7 +1713,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, } } @@ -2356,8 +2356,11 @@ impl ReadableArgs<(Option, &NS)> for InboundOnionPayload w }); 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 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() { return Err(DecodeError::InvalidValue) } @@ -2379,7 +2382,7 @@ impl ReadableArgs<(Option, &NS)> for InboundOnionPayload w 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 { @@ -2392,7 +2395,7 @@ impl ReadableArgs<(Option, &NS)> for InboundOnionPayload w outgoing_cltv_value: cltv_value.ok_or(DecodeError::InvalidValue)?, payment_secret, payment_constraints, - intro_node_blinding_point: blinding_point, + intro_node_blinding_point, }) }, } From 51f41ce7ce1853a1385cac9216729706829f8479 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 15:16:42 -0400 Subject: [PATCH 03/27] Support receiving to multi-hop blinded payment paths. The only remaining step is to use the update_add blinding point in decoding inbound onion payloads. Error handling will be completed in upcoming commits. --- lightning/src/ln/onion_payment.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index f61c05df7a4..5f03b86e06f 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}; @@ -326,8 +327,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 { From 2a4650581587228624313c084533eb6a550dd390 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 15:17:04 -0400 Subject: [PATCH 04/27] Test successfully receiving to a multihop blinded path. --- lightning/src/ln/blinded_payment_tests.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 635057deab2..708a05d33db 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -432,3 +432,24 @@ fn blinded_intercept_payment() { expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); } + +#[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); +} From df2d0a47d528b00ee8da80917946beab746a7685 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 16:56:43 -0400 Subject: [PATCH 05/27] Add variant for non-intro-nodes to BlindedFailure enum For use in supporting receiving to multi-hop blinded paths. --- lightning/src/ln/channelmanager.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 42f901ac78d..f1dae2747b4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -292,7 +292,7 @@ pub(super) enum HTLCForwardInfo { #[derive(Clone, 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 @@ -5243,6 +5243,7 @@ where incoming_packet_shared_secret, phantom_shared_secret ) }, + Some(BlindedFailure::FromBlindedNode) => todo!(), None => { onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret) } @@ -9467,7 +9468,8 @@ impl_writeable_tlv_based_enum!(PendingHTLCStatus, ; ); impl_writeable_tlv_based_enum!(BlindedFailure, - (0, FromIntroductionNode) => {}, ; + (0, FromIntroductionNode) => {}, + (2, FromBlindedNode) => {}, ; ); impl_writeable_tlv_based!(HTLCPreviousHopData, { From 339f3fccf80c8ccfc8de936df8de65481586f833 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 17:14:40 -0400 Subject: [PATCH 06/27] Store whether a received HTLC is blinded in PendingHTLCInfo Useful for knowing to fail back these HTLCs with malformed and invalid_onion_blinding error per the BOLT 4 spec. --- lightning/src/ln/channelmanager.rs | 8 +++++++- lightning/src/ln/onion_payment.rs | 14 ++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f1dae2747b4..b925d842e95 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -155,6 +155,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 @@ -4398,7 +4400,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 }; @@ -9374,6 +9379,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), diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 5f03b86e06f..5ea07cfab30 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -112,16 +112,21 @@ 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, .. } => { 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 +213,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 { From e4485cffb3562e10de252894d18b4d4729cd2307 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 18:52:29 -0400 Subject: [PATCH 07/27] Set HTLCPreviousHopData::blinded for blinded received HTLCs. Will be used in the next commit(s) to let us know to fail blinded received HTLCs backwards with the malformed and invalid_onion_blinding error per BOLT 4. --- lightning/src/ln/channelmanager.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b925d842e95..107ef72a15f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -201,11 +201,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, } } } @@ -291,7 +291,7 @@ pub(super) enum HTLCForwardInfo { } // Used for failing blinded HTLCs backwards correctly. -#[derive(Clone, Debug, Hash, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] enum BlindedFailure { FromIntroductionNode, FromBlindedNode, @@ -4462,7 +4462,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 }, From af4d0df1bfcf1199772eb59fc8d4ab98798a69a4 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 20:11:00 -0400 Subject: [PATCH 08/27] Channel: add holding cell HTLC variant for blinded HTLCs. For context, blinded HTLCs where we are not the intro node must always be failed back with malformed and invalid_onion_blinding error per BOLT 4. Prior to supporting blinded payments, the only way for an update_malformed to be returned from Channel was if an onion was actually found to be malformed during initial update_add processing. This meant that any malformed HTLCs would never live in the holding cell but instead would be returned directly upon initial RAA processing. Now, we need to be able to store these HTLCs in the holding cell because the HTLC failure necessitating an update_malformed may come long after the RAA is initially processed, and we may not be a state to send the update_malformed message at that time. Therefore, add a new holding cell HTLC variant for blinded non-intro node HTLCs, which will signal to Channel to fail with malformed and the correct error code. --- lightning/src/ln/channel.rs | 50 +++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 582f67878e6..a1af5e6da68 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 { @@ -2719,7 +2724,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 @@ -2878,7 +2885,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())); @@ -3563,6 +3572,9 @@ impl Channel where } } }, + &HTLCUpdateAwaitingACK::FailMalformedHTLC { .. } => { + todo!() + }, } } if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() { @@ -7433,6 +7445,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 +7474,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 +7646,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 +7937,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 +7967,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 +8062,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, From 4ecf3f46dfd56e26d2bb3952d4c4ca3ef92f9221 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 19:47:31 -0400 Subject: [PATCH 09/27] Set up Channel::fail_htlc to be able to return update_malformed Currently it returns only update_fail, but we'll want it to be able to return update_malformed as well in upcoming commits. We'll use this for correctly failing blinded received HTLCs backwards with malformed and invalid_onion_blinding error per BOLT 4. --- lightning/src/ln/channel.rs | 52 +++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a1af5e6da68..44b09a861ff 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2523,6 +2523,36 @@ 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 } + } +} + +trait FailHTLCMessageName { + fn name() -> &'static str; +} +impl FailHTLCMessageName for msgs::UpdateFailHTLC { + fn name() -> &'static str { + "update_fail_htlc" + } +} + impl Channel where SP::Target: SignerProvider, ::EcdsaSigner: WriteableEcdsaChannelSigner @@ -2831,8 +2861,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"); } @@ -2897,24 +2929,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: From 846be8147fcf2a7a4802c1ae8e43483cdfc21333 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 4 Dec 2023 15:26:30 -0500 Subject: [PATCH 10/27] Adapt Channel::fail_htlc for failing with malformed OR update_fail_htlc. Useful for failing blinded payments back with malformed, and will also be useful in the future when we move onion decoding into process_pending_htlc_forwards, after which Channel::fail_htlc will be used for all malformed htlcs. --- lightning/src/ln/channel.rs | 43 +++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 44b09a861ff..a991c8eeda1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2543,6 +2543,29 @@ impl FailHTLCContents for msgs::OnionErrorPacket { 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; @@ -2552,6 +2575,11 @@ impl FailHTLCMessageName for msgs::UpdateFailHTLC { "update_fail_htlc" } } +impl FailHTLCMessageName for msgs::UpdateFailMalformedHTLC { + fn name() -> &'static str { + "update_fail_malformed_htlc" + } +} impl Channel where SP::Target: SignerProvider, @@ -3598,8 +3626,19 @@ impl Channel where } } }, - &HTLCUpdateAwaitingACK::FailMalformedHTLC { .. } => { - todo!() + &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"); + } + } + } }, } } From 7bb4a235bccc92e7edba42827a3f95644c3a02d9 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 26 Oct 2023 21:53:35 -0400 Subject: [PATCH 11/27] ChannelManager: add HTLCForwardInfo variant for blinded non-intro htlcs Necessary to tell the Channel how to fail these htlcs. --- lightning/src/ln/channelmanager.rs | 81 ++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 107ef72a15f..662b85bcca5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -288,6 +288,11 @@ pub(super) enum HTLCForwardInfo { 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. @@ -4286,7 +4291,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 @@ -4382,6 +4387,9 @@ where continue; } }, + HTLCForwardInfo::FailMalformedHTLC { .. } => { + todo!() + }, } } } else { @@ -4636,7 +4644,7 @@ where }, }; }, - HTLCForwardInfo::FailHTLC { .. } => { + HTLCForwardInfo::FailHTLC { .. } | HTLCForwardInfo::FailMalformedHTLC { .. } => { panic!("Got pending fail of our own HTLC"); } } @@ -9639,13 +9647,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), From 4198edaead6a170ef4a023562243bc9758dadc17 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 6 Dec 2023 15:14:19 -0500 Subject: [PATCH 12/27] Tweak initialization of HTLCForwardInfo in fail_htlc_backwards_internal Makes the next commit adding support for failing blinded HTLCs in said method easier to read. --- lightning/src/ln/channelmanager.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 662b85bcca5..aed58f84db1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5249,16 +5249,20 @@ 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) => todo!(), 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 } } }; @@ -5269,10 +5273,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); From b26480189e152784e1ff1f37e6519f83ad405e9f Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 6 Dec 2023 15:19:23 -0500 Subject: [PATCH 13/27] Support failing blinded non-intro HTLCs after RAA processing. If an HTLC fails after its RAA is processed, it is failed back with ChannelManager::fail_htlc_backwards_internal. This method will now correctly inform the channel that this HTLC is blinded and to construct an update_malformed message accordingly. --- lightning/src/ln/channel.rs | 11 +++++++++++ lightning/src/ln/channelmanager.rs | 23 ++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a991c8eeda1..bac45076891 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2881,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 diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index aed58f84db1..bf02015faf5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4387,8 +4387,19 @@ where continue; } }, - HTLCForwardInfo::FailMalformedHTLC { .. } => { - todo!() + 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; + } }, } } @@ -5257,7 +5268,13 @@ where ); HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet } }, - Some(BlindedFailure::FromBlindedNode) => todo!(), + Some(BlindedFailure::FromBlindedNode) => { + HTLCForwardInfo::FailMalformedHTLC { + htlc_id: *htlc_id, + failure_code: INVALID_ONION_BLINDING, + sha256_of_onion: [0; 32] + } + }, None => { let err_packet = onion_error.get_encrypted_failure_packet( incoming_packet_shared_secret, phantom_shared_secret From a2b4813c1f9421e3074145bf0ffd67e8636692a6 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 13 Oct 2023 15:53:45 -0400 Subject: [PATCH 14/27] Test recipient failing an HTLC received to a multi-hop blinded path --- lightning/src/ln/blinded_payment_tests.rs | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 708a05d33db..1494efdaad9 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -453,3 +453,45 @@ fn two_hop_blinded_path_success() { 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); } + +#[test] +fn multi_hop_receiver_fail() { + 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_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); + + 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); + + 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 = 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])); +} From d99089e16a6e7c4744af5dda0750a7c7a17caba6 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 16 Oct 2023 15:46:55 -0400 Subject: [PATCH 15/27] Fix blinded recipient fail on malformed HTLC If a blinded recipient to a multihop blinded path needs to fail back a malformed HTLC, they should use error code INVALID_ONION_BLINDING and a zeroed out onion hash per BOLT 4. --- lightning/src/ln/blinded_payment_tests.rs | 7 ++++++- lightning/src/ln/onion_payment.rs | 9 +++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 1494efdaad9..c14c75a5e02 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -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); diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 5ea07cfab30..f2488570542 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -319,11 +319,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, })); } } From fbe4bf1cdd8ffc4d8adc5a84a49bfc9f80e5d67d Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 22 Oct 2023 18:05:02 -0500 Subject: [PATCH 16/27] Add find_route test util And use it in the multihop blinded path receive failure test. Will be used in the next commit to test receiving an invalid blinded final onion payload. We can't use the existing get_route test util here because blinded payments rely on the sender adding a random shadow CLTV offset to the final hop; without this the payment will be failed with cltv-expiry-too-soon. --- lightning/src/ln/blinded_payment_tests.rs | 2 ++ lightning/src/ln/functional_test_utils.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index c14c75a5e02..c8b835debaf 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -474,6 +474,8 @@ fn multi_hop_receiver_fail() { nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&chan_upd_1_2], &chanmon_cfgs[2].keys_manager); + let route = 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); pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], amt_msat, payment_hash, payment_secret); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d3d2e3322ca..5828ca8038c 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. From 52f28e63e8d8ee8cd68ddf8aeacb401a47f0191d Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 24 Oct 2023 14:13:13 -0400 Subject: [PATCH 17/27] Fix blinded recipient fail on onion decode failure If a recipient behind a multihop blinded path fails to decode their onion payload, they should fail backwards with error code INVALID_ONION_BLINDING and a zeroed out onion hash per BOLT 4. --- lightning/src/ln/blinded_payment_tests.rs | 84 +++++++++++++++++++++-- lightning/src/ln/onion_payment.rs | 4 ++ 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index c8b835debaf..2ac66d5e826 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -459,12 +459,28 @@ fn two_hop_blinded_path_success() { claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); } +enum ReceiveCheckFail { + // The recipient fails the payment upon `PaymentClaimable`. + RecipientFail, + // Failure to decode the recipient's onion payload. + OnionDecodeFail, +} + #[test] fn multi_hop_receiver_fail() { + do_multi_hop_receiver_fail(ReceiveCheckFail::RecipientFail); + do_multi_hop_receiver_fail(ReceiveCheckFail::OnionDecodeFail); +} + +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 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents; @@ -478,14 +494,68 @@ fn multi_hop_receiver_fail() { 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); - pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], amt_msat, payment_hash, payment_secret); - 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); + 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); + + 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); + } + } 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); diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index f2488570542..c92c8fd24cd 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -360,6 +360,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, From eca4dc0799889432bfd2a65d763074f38000ffce Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 24 Oct 2023 16:45:24 -0400 Subject: [PATCH 18/27] Fix blinded recipient fail on receive reqs violation If a blinded HTLC does not satisfy the receiver's requirements, e.g. bad CLTV or amount, they should malformed-fail backwards with error code INVALID_ONION_BLINDING and a zeroed out onion hash per BOLt 4. --- lightning/src/ln/blinded_payment_tests.rs | 13 ++++++++++++- lightning/src/ln/channelmanager.rs | 10 ++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 2ac66d5e826..72df4e15851 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -464,12 +464,16 @@ enum ReceiveCheckFail { 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, } #[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); } fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { @@ -554,7 +558,14 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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); + }, } let updates_2_1 = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index bf02015faf5..153e3991199 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3192,6 +3192,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, From 85d3cb802c21c72b293db9eab45edf481548d697 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 24 Oct 2023 19:17:29 -0400 Subject: [PATCH 19/27] Fix blinded recipient fail on Channel error If a blinded HTLC errors when added to a Channel, such as if the recipient has already sent a shutdown message, they should malformed-fail backwards with error code INVALID_ONION_BLINDING and a zeroed out onion hash per BOLT 4. --- lightning/src/ln/blinded_payment_tests.rs | 42 +++++++++++++++++++++-- lightning/src/ln/channelmanager.rs | 10 ++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 72df4e15851..8922506d28d 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}; @@ -459,6 +459,7 @@ fn two_hop_blinded_path_success() { claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); } +#[derive(PartialEq)] enum ReceiveCheckFail { // The recipient fails the payment upon `PaymentClaimable`. RecipientFail, @@ -467,6 +468,9 @@ enum ReceiveCheckFail { // 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, } #[test] @@ -474,6 +478,7 @@ 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); } fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { @@ -486,7 +491,12 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents; + 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 (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None); @@ -566,6 +576,19 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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); + } } let updates_2_1 = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); @@ -576,7 +599,20 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + 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); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 153e3991199..e8b8f43dae9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6603,6 +6603,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. From a351301362508bf0fd4b444c3b37ec41d2c22450 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 30 Oct 2023 21:36:13 -0400 Subject: [PATCH 20/27] Test successful intercept payment to 2-hop blinded path --- lightning/src/ln/blinded_payment_tests.rs | 63 +++++++++++++++++------ 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 8922506d28d..e08ec16d5db 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -382,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(); @@ -389,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; @@ -413,29 +420,53 @@ 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()); - 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])); + 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] From 93ef850670b9531b47a0aca8187e01fcb822e3fa Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 31 Oct 2023 17:09:44 -0400 Subject: [PATCH 21/27] Test received blinded HTLC failure in process_pending_htlc_forwards --- lightning/src/ln/blinded_payment_tests.rs | 28 +++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index e08ec16d5db..0558de52ec0 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -502,6 +502,9 @@ enum ReceiveCheckFail { // 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, } #[test] @@ -510,6 +513,7 @@ fn multi_hop_receiver_fail() { 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); } fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { @@ -530,12 +534,23 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { }; let amt_msat = 5000; - let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None); + 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 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 = find_route(&nodes[0], &route_params).unwrap(); + 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 { + 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); @@ -619,6 +634,15 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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); } } From 41808037ac7111024fe5da30ea16e6c0bcc4c3d3 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 9 Nov 2023 15:12:09 -0500 Subject: [PATCH 22/27] Fail blinded received HTLCs if they violate PaymentConstraints .. contained within their encrypted payload. --- lightning/src/ln/blinded_payment_tests.rs | 32 ++++++++++++++++++++--- lightning/src/ln/onion_payment.rs | 25 +++++++++++++++--- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 0558de52ec0..9b580d1fa75 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -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; @@ -505,6 +505,8 @@ enum ReceiveCheckFail { // 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] @@ -514,6 +516,7 @@ fn multi_hop_receiver_fail() { 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) { @@ -539,7 +542,7 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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 route_params = get_blinded_route_parameters(amt_msat, payment_secret, + 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); @@ -548,7 +551,25 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { // 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 { + } 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())); @@ -643,6 +664,11 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { 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); } } diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index c92c8fd24cd..8767e2500bd 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -33,6 +33,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 +52,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)) } @@ -122,8 +130,17 @@ pub(super) fn create_recv_pending_htlc_info( (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, intro_node_blinding_point, .. + 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, intro_node_blinding_point.is_none()) From ae08d0c86a81a0d287f466c8ce97d616885c5d8b Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 9 Nov 2023 15:15:15 -0500 Subject: [PATCH 23/27] Make BlindedPath::new_for_payment pub Because we now support receiving to multi-hop blinded paths. --- lightning/src/blinded_path/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/blinded_path/mod.rs b/lightning/src/blinded_path/mod.rs index d75b4f25b36..b1fc9e1c0a4 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 From 11bdcdaa08e5e56fadc54857d49412341a528b76 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 4 Dec 2023 16:27:18 -0500 Subject: [PATCH 24/27] Add redundant blinded HTLC failure check for posterity. Although this new check is unreachable right now, it helps prevent potential future errors where we incorrectly fail blinded HTLCs with an unblinded error. --- lightning/src/ln/channelmanager.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e8b8f43dae9..5bee15c9724 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3025,11 +3025,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 { @@ -3039,7 +3040,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 { From 63ebde1d2e90293cc24ab8bff04bd98db41dff76 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 8 Dec 2023 14:31:01 -0500 Subject: [PATCH 25/27] Add test coverage for serialization of malformed HTLCs. in Channel and ChannelManager. --- lightning/src/ln/channel.rs | 26 ++++++++---- lightning/src/ln/channelmanager.rs | 67 +++++++++++++++++++++++++++++- lightning/src/ln/msgs.rs | 1 + 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index bac45076891..19e4b966658 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8288,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}; @@ -8824,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]; @@ -8890,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, .. @@ -8905,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 5bee15c9724..da5f4394a4e 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 { @@ -189,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 @@ -213,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, @@ -267,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, @@ -282,6 +285,7 @@ pub(super) struct PendingAddHTLCInfo { prev_user_channel_id: u128, } +#[cfg_attr(test, derive(Clone, Debug, PartialEq))] pub(super) enum HTLCForwardInfo { AddHTLC(PendingAddHTLCInfo), FailHTLC { @@ -11056,12 +11060,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; @@ -12336,6 +12342,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/msgs.rs b/lightning/src/ln/msgs.rs index db7579039cc..6c4a324a906 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. From ecd8238592e99c69c906224e5d549d4bedbc78f2 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 11 Dec 2023 15:38:47 -0500 Subject: [PATCH 26/27] Add release note for blinded HTLC serialization. --- pending_changelog/multihop-blinded-recv.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 pending_changelog/multihop-blinded-recv.txt diff --git a/pending_changelog/multihop-blinded-recv.txt b/pending_changelog/multihop-blinded-recv.txt new file mode 100644 index 00000000000..8bfafc8fb4f --- /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. From 6b66271acfdd0e3963ee0b112cd0d9aecd76ca11 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 12 Dec 2023 18:42:38 -0500 Subject: [PATCH 27/27] Add missing keysend preimage check on inbound onion read. --- lightning/src/ln/msgs.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 6c4a324a906..689058c7e4a 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -2362,7 +2362,9 @@ impl ReadableArgs<(Option, &NS)> for InboundOnionPayload w } 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() { + 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;