From 039b1c8d1007cb7d42f594b0349cbbaf110a31a8 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Tue, 16 May 2023 17:56:28 -0500 Subject: [PATCH 1/7] Allow users to provide custom TLVs through `RecipientOnionFields` Custom TLVs allow users to send extra application-specific data with a payment. These have the additional flexibility compared to `payment_metadata` that they don't have to reflect recipient generated data provided in an invoice, in which `payment_metadata` could be reused. We ensure provided type numbers are unique, increasing, and within the experimental range with the `RecipientOnionFields::with_custom_tlvs` method. This begins sender-side support for custom TLVs. --- lightning-invoice/src/payment.rs | 6 +-- lightning/src/ln/channelmanager.rs | 7 +-- lightning/src/ln/outbound_payment.rs | 69 +++++++++++++++++++++++++++- lightning/src/ln/payment_tests.rs | 2 +- 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index 42408540ee4..b67bac13f34 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -146,10 +146,8 @@ fn pay_invoice_using_amount( payer: P ) -> Result<(), PaymentError> where P::Target: Payer { let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner()); - let recipient_onion = RecipientOnionFields { - payment_secret: Some(*invoice.payment_secret()), - payment_metadata: invoice.payment_metadata().map(|v| v.clone()), - }; + let mut recipient_onion = RecipientOnionFields::secret_only(*invoice.payment_secret()); + recipient_onion.payment_metadata = invoice.payment_metadata().map(|v| v.clone()); let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32) .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs()) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3757b06b3c9..374368e83e5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3943,15 +3943,16 @@ where 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 } => { let _legacy_hop_data = Some(payment_data.clone()); - let onion_fields = - RecipientOnionFields { payment_secret: Some(payment_data.payment_secret), payment_metadata }; + let onion_fields = RecipientOnionFields { payment_secret: Some(payment_data.payment_secret), + payment_metadata, custom_tlvs: vec![] }; (incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data }, Some(payment_data), phantom_shared_secret, onion_fields) }, PendingHTLCRouting::ReceiveKeysend { payment_data, payment_preimage, payment_metadata, incoming_cltv_expiry } => { let onion_fields = RecipientOnionFields { payment_secret: payment_data.as_ref().map(|data| data.payment_secret), - payment_metadata + payment_metadata, + custom_tlvs: vec![], }; (incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), payment_data, None, onion_fields) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 24d683e5fdc..45c723b8525 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -431,10 +431,13 @@ pub struct RecipientOnionFields { /// [`Self::payment_secret`] and while nearly all lightning senders support secrets, metadata /// may not be supported as universally. pub payment_metadata: Option>, + /// See [`Self::custom_tlvs`] for more info. + pub(super) custom_tlvs: Vec<(u64, Vec)>, } impl_writeable_tlv_based!(RecipientOnionFields, { (0, payment_secret, option), + (1, custom_tlvs, optional_vec), (2, payment_metadata, option), }); @@ -443,7 +446,7 @@ impl RecipientOnionFields { /// set of onion fields for today's BOLT11 invoices - most nodes require a [`PaymentSecret`] /// but do not require or provide any further data. pub fn secret_only(payment_secret: PaymentSecret) -> Self { - Self { payment_secret: Some(payment_secret), payment_metadata: None } + Self { payment_secret: Some(payment_secret), payment_metadata: None, custom_tlvs: Vec::new() } } /// Creates a new [`RecipientOnionFields`] with no fields. This generally does not create @@ -455,7 +458,46 @@ impl RecipientOnionFields { /// [`ChannelManager::send_spontaneous_payment`]: super::channelmanager::ChannelManager::send_spontaneous_payment /// [`RecipientOnionFields::secret_only`]: RecipientOnionFields::secret_only pub fn spontaneous_empty() -> Self { - Self { payment_secret: None, payment_metadata: None } + Self { payment_secret: None, payment_metadata: None, custom_tlvs: Vec::new() } + } + + /// Creates a new [`RecipientOnionFields`] from an existing one, adding custom TLVs. Each + /// TLV is provided as a `(u64, Vec)` for the type number and serialized value + /// respectively. TLV type numbers must be unique and within the range + /// reserved for custom types, i.e. >= 2^16, otherwise this method will return `Err(())`. + /// + /// This method will also error for types in the experimental range which have been + /// standardized within the protocol, which only includes 5482373484 (keysend) for now. + /// + /// See [`Self::custom_tlvs`] for more info. + pub fn with_custom_tlvs(mut self, mut custom_tlvs: Vec<(u64, Vec)>) -> Result { + custom_tlvs.sort_unstable_by_key(|(typ, _)| *typ); + let mut prev_type = None; + for (typ, _) in custom_tlvs.iter() { + if *typ < 1 << 16 { return Err(()); } + if *typ == 5482373484 { return Err(()); } // keysend + match prev_type { + Some(prev) if prev >= *typ => return Err(()), + _ => {}, + } + prev_type = Some(*typ); + } + self.custom_tlvs = custom_tlvs; + Ok(self) + } + + /// Gets the custom TLVs that will be sent or have been received. + /// + /// Custom TLVs allow sending extra application-specific data with a payment. They provide + /// additional flexibility on top of payment metadata, as while other implementations may + /// require `payment_metadata` to reflect metadata provided in an invoice, custom TLVs + /// do not have this restriction. + /// + /// Note that if this field is non-empty, it will contain strictly increasing TLVs, each + /// represented by a `(u64, Vec)` for its type number and serialized value respectively. + /// This is validated when setting this field using [`Self::with_custom_tlvs`]. + pub fn custom_tlvs(&self) -> &Vec<(u64, Vec)> { + &self.custom_tlvs } /// When we have received some HTLC(s) towards an MPP payment, as we receive further HTLC(s) we @@ -773,6 +815,7 @@ impl OutboundPayments { (*total_msat, RecipientOnionFields { payment_secret: *payment_secret, payment_metadata: payment_metadata.clone(), + custom_tlvs: Vec::new(), }, *keysend_preimage) }, PendingOutboundPayment::Legacy { .. } => { @@ -1450,6 +1493,28 @@ mod tests { use alloc::collections::VecDeque; + #[test] + fn test_recipient_onion_fields_with_custom_tlvs() { + let onion_fields = RecipientOnionFields::spontaneous_empty(); + + let bad_type_range_tlvs = vec![ + (0, vec![42]), + (1, vec![42; 32]), + ]; + assert!(onion_fields.clone().with_custom_tlvs(bad_type_range_tlvs).is_err()); + + let keysend_tlv = vec![ + (5482373484, vec![42; 32]), + ]; + assert!(onion_fields.clone().with_custom_tlvs(keysend_tlv).is_err()); + + let good_tlvs = vec![ + ((1 << 16) + 1, vec![42]), + ((1 << 16) + 3, vec![42; 32]), + ]; + assert!(onion_fields.with_custom_tlvs(good_tlvs).is_ok()); + } + #[test] #[cfg(feature = "std")] fn fails_paying_after_expiration() { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 7d639611df2..26ece931770 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -3424,7 +3424,7 @@ fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) { // Send the MPP payment, delivering the updated commitment state to nodes[1]. nodes[0].node.send_payment(payment_hash, RecipientOnionFields { - payment_secret: Some(payment_secret), payment_metadata: Some(payment_metadata), + payment_secret: Some(payment_secret), payment_metadata: Some(payment_metadata), custom_tlvs: vec![], }, payment_id, route_params.clone(), Retry::Attempts(1)).unwrap(); check_added_monitors!(nodes[0], 2); From d2e9cb4bcd4234e22f731b6e39f178e10f5eaee7 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Mon, 24 Jul 2023 14:04:18 -0500 Subject: [PATCH 2/7] Add custom tlvs to `PendingOutboundPayment::Retryable` --- lightning/src/ln/channelmanager.rs | 1 + lightning/src/ln/outbound_payment.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 374368e83e5..7b0558881ba 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8751,6 +8751,7 @@ where payment_secret: None, // only used for retries, and we'll never retry on startup payment_metadata: None, // only used for retries, and we'll never retry on startup keysend_preimage: None, // only used for retries, and we'll never retry on startup + custom_tlvs: Vec::new(), // only used for retries, and we'll never retry on startup pending_amt_msat: path_amt, pending_fee_msat: Some(path_fee), total_msat: path_amt, diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 45c723b8525..0e3c15c8066 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -47,6 +47,7 @@ pub(crate) enum PendingOutboundPayment { payment_secret: Option, payment_metadata: Option>, keysend_preimage: Option, + custom_tlvs: Vec<(u64, Vec)>, pending_amt_msat: u64, /// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+. pending_fee_msat: Option, @@ -804,7 +805,8 @@ impl OutboundPayments { hash_map::Entry::Occupied(mut payment) => { let res = match payment.get() { PendingOutboundPayment::Retryable { - total_msat, keysend_preimage, payment_secret, payment_metadata, pending_amt_msat, .. + total_msat, keysend_preimage, payment_secret, payment_metadata, + custom_tlvs, pending_amt_msat, .. } => { let retry_amt_msat = route.get_total_amount(); if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 { @@ -815,7 +817,7 @@ impl OutboundPayments { (*total_msat, RecipientOnionFields { payment_secret: *payment_secret, payment_metadata: payment_metadata.clone(), - custom_tlvs: Vec::new(), + custom_tlvs: custom_tlvs.clone(), }, *keysend_preimage) }, PendingOutboundPayment::Legacy { .. } => { @@ -1014,6 +1016,7 @@ impl OutboundPayments { payment_secret: recipient_onion.payment_secret, payment_metadata: recipient_onion.payment_metadata, keysend_preimage, + custom_tlvs: recipient_onion.custom_tlvs, starting_block_height: best_block_height, total_msat: route.get_total_amount(), }); @@ -1463,6 +1466,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (6, total_msat, required), (7, payment_metadata, option), (8, pending_amt_msat, required), + (9, custom_tlvs, optional_vec), (10, starting_block_height, required), (not_written, retry_strategy, (static_value, None)), (not_written, attempts, (static_value, PaymentAttempts::new())), From f560320b5fb257b1863eaf33f50aca373a70c9f9 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Thu, 18 May 2023 23:16:29 -0500 Subject: [PATCH 3/7] De/serialize custom TLVs on `{Inbound,Outbound}OnionPayload` When serialized, the TLVs in `OutboundOnionPayload`, unlike a normal TLV stream, are prefixed with the length of the stream. To allow a user to add arbitrary custom TLVs, we aren't able to communicate to our serialization macros exactly which fields to expect, so this commit adds new macro variants to allow appending an extra set of bytes (and modifying the prefixed length accordingly). Because the keysend preimage TLV has a type number in the custom type range, and a user's TLVs may have type numbers above and/or below keysend's type number, and because TLV streams must be serialized in increasing order by type number, this commit also ensures the keysend TLV is properly sorted/serialized amongst the custom TLVs. --- lightning/src/ln/channelmanager.rs | 8 ++- lightning/src/ln/msgs.rs | 103 +++++++++++++++++++++++++++-- lightning/src/ln/onion_utils.rs | 1 + lightning/src/util/ser_macros.rs | 42 ++++++++++-- 4 files changed, 141 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7b0558881ba..5f23842dfb7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2674,11 +2674,11 @@ where amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool, counterparty_skimmed_fee_msat: Option, ) -> Result { - let (payment_data, keysend_preimage, 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) = match hop_data { msgs::InboundOnionPayload::Receive { - payment_data, keysend_preimage, amt_msat, outgoing_cltv_value, payment_metadata, .. + payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata, .. } => - (payment_data, keysend_preimage, amt_msat, outgoing_cltv_value, payment_metadata), + (payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata), _ => return Err(InboundOnionErr { err_code: 0x4000|22, @@ -10094,6 +10094,7 @@ mod tests { payment_data: Some(msgs::FinalOnionHopData { payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat, }), + custom_tlvs: Vec::new(), }; // Check that if the amount we received + the penultimate hop extra fee is less than the sender // intended amount, we fail the payment. @@ -10113,6 +10114,7 @@ mod tests { payment_data: Some(msgs::FinalOnionHopData { payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat, }), + custom_tlvs: Vec::new(), }; assert!(node[0].node.construct_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]), sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat)).is_ok()); diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index aedd1e76e5b..b6ec022dc14 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -43,7 +43,7 @@ use crate::io_extras::read_to_end; use crate::events::{MessageSendEventsProvider, OnionMessageProvider}; use crate::util::logger; -use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, WithoutLength, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, TransactionU16LenLimited}; +use crate::util::ser::{LengthReadable, Readable, ReadableArgs, Writeable, Writer, WithoutLength, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, TransactionU16LenLimited, BigSize}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; @@ -1441,6 +1441,7 @@ mod fuzzy_internal_msgs { payment_data: Option, payment_metadata: Option>, keysend_preimage: Option, + custom_tlvs: Vec<(u64, Vec)>, amt_msat: u64, outgoing_cltv_value: u32, }, @@ -1457,6 +1458,7 @@ mod fuzzy_internal_msgs { payment_data: Option, payment_metadata: Option>, keysend_preimage: Option, + custom_tlvs: Vec<(u64, Vec)>, amt_msat: u64, outgoing_cltv_value: u32, }, @@ -1979,15 +1981,23 @@ impl Writeable for OutboundOnionPayload { }); }, Self::Receive { - ref payment_data, ref payment_metadata, ref keysend_preimage, amt_msat, outgoing_cltv_value + ref payment_data, ref payment_metadata, ref keysend_preimage, amt_msat, + outgoing_cltv_value, ref custom_tlvs, } => { + // We need to update [`ln::outbound_payment::RecipientOnionFields::with_custom_tlvs`] + // to reject any reserved types in the experimental range if new ones are ever + // standardized. + let preimage = if let Some(ref preimage) = keysend_preimage { + Some((5482373484, preimage.encode())) + } else { None }; + let mut custom_tlvs: Vec<&(u64, Vec)> = custom_tlvs.iter().chain(preimage.iter()).collect(); + custom_tlvs.sort_unstable_by_key(|(typ, _)| *typ); _encode_varint_length_prefixed_tlv!(w, { (2, HighZeroBytesDroppedBigSize(*amt_msat), required), (4, HighZeroBytesDroppedBigSize(*outgoing_cltv_value), required), (8, payment_data, option), - (16, payment_metadata.as_ref().map(|m| WithoutLength(m)), option), - (5482373484, keysend_preimage, option) - }); + (16, payment_metadata.as_ref().map(|m| WithoutLength(m)), option) + }, custom_tlvs.iter()); }, } Ok(()) @@ -2002,7 +2012,11 @@ impl Readable for InboundOnionPayload { let mut payment_data: Option = None; let mut payment_metadata: Option>> = None; let mut keysend_preimage: Option = None; - read_tlv_fields!(r, { + let mut custom_tlvs = Vec::new(); + + let tlv_len = BigSize::read(r)?; + let rd = FixedLengthReader::new(r, tlv_len.0); + decode_tlv_stream_with_custom_tlv_decode!(rd, { (2, amt, required), (4, cltv_value, required), (6, short_id, option), @@ -2010,6 +2024,12 @@ impl Readable for InboundOnionPayload { (16, payment_metadata, option), // See https://github.com/lightning/blips/blob/master/blip-0003.md (5482373484, keysend_preimage, option) + }, |msg_type: u64, msg_reader: &mut FixedLengthReader<_>| -> Result { + if msg_type < 1 << 16 { return Ok(false) } + let mut value = Vec::new(); + msg_reader.read_to_end(&mut value)?; + custom_tlvs.push((msg_type, value)); + Ok(true) }); if amt.0 > MAX_VALUE_MSAT { return Err(DecodeError::InvalidValue) } @@ -2033,6 +2053,7 @@ impl Readable for InboundOnionPayload { keysend_preimage, amt_msat: amt.0, outgoing_cltv_value: cltv_value.0, + custom_tlvs, }) } } @@ -3566,6 +3587,7 @@ mod tests { keysend_preimage: None, amt_msat: 0x0badf00d01020304, outgoing_cltv_value: 0xffffffff, + custom_tlvs: vec![], }; let encoded_value = outbound_msg.encode(); let target_value = hex::decode("1002080badf00d010203040404ffffffff").unwrap(); @@ -3590,6 +3612,7 @@ mod tests { keysend_preimage: None, amt_msat: 0x0badf00d01020304, outgoing_cltv_value: 0xffffffff, + custom_tlvs: vec![], }; let encoded_value = outbound_msg.encode(); let target_value = hex::decode("3602080badf00d010203040404ffffffff082442424242424242424242424242424242424242424242424242424242424242421badca1f").unwrap(); @@ -3604,10 +3627,78 @@ mod tests { amt_msat, outgoing_cltv_value, payment_metadata: None, keysend_preimage: None, + custom_tlvs, } = inbound_msg { assert_eq!(payment_secret, expected_payment_secret); assert_eq!(amt_msat, 0x0badf00d01020304); assert_eq!(outgoing_cltv_value, 0xffffffff); + assert_eq!(custom_tlvs, vec![]); + } else { panic!(); } + } + + #[test] + fn encoding_final_onion_hop_data_with_bad_custom_tlvs() { + // If custom TLVs have type number within the range reserved for protocol, treat them as if + // they're unknown + let bad_type_range_tlvs = vec![ + ((1 << 16) - 4, vec![42]), + ((1 << 16) - 2, vec![42; 32]), + ]; + let mut msg = msgs::OutboundOnionPayload::Receive { + payment_data: None, + payment_metadata: None, + keysend_preimage: None, + custom_tlvs: bad_type_range_tlvs, + amt_msat: 0x0badf00d01020304, + outgoing_cltv_value: 0xffffffff, + }; + let encoded_value = msg.encode(); + assert!(msgs::InboundOnionPayload::read(&mut Cursor::new(&encoded_value[..])).is_err()); + let good_type_range_tlvs = vec![ + ((1 << 16) - 3, vec![42]), + ((1 << 16) - 1, vec![42; 32]), + ]; + if let msgs::OutboundOnionPayload::Receive { ref mut custom_tlvs, .. } = msg { + *custom_tlvs = good_type_range_tlvs.clone(); + } + let encoded_value = msg.encode(); + let inbound_msg = Readable::read(&mut Cursor::new(&encoded_value[..])).unwrap(); + match inbound_msg { + msgs::InboundOnionPayload::Receive { custom_tlvs, .. } => assert!(custom_tlvs.is_empty()), + _ => panic!(), + } + } + + #[test] + fn encoding_final_onion_hop_data_with_custom_tlvs() { + let expected_custom_tlvs = vec![ + (5482373483, vec![0x12, 0x34]), + (5482373487, vec![0x42u8; 8]), + ]; + let msg = msgs::OutboundOnionPayload::Receive { + payment_data: None, + payment_metadata: None, + keysend_preimage: None, + custom_tlvs: expected_custom_tlvs.clone(), + amt_msat: 0x0badf00d01020304, + outgoing_cltv_value: 0xffffffff, + }; + let encoded_value = msg.encode(); + let target_value = hex::decode("2e02080badf00d010203040404ffffffffff0000000146c6616b021234ff0000000146c6616f084242424242424242").unwrap(); + assert_eq!(encoded_value, target_value); + let inbound_msg: msgs::InboundOnionPayload = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + if let msgs::InboundOnionPayload::Receive { + payment_data: None, + payment_metadata: None, + keysend_preimage: None, + custom_tlvs, + amt_msat, + outgoing_cltv_value, + .. + } = inbound_msg { + assert_eq!(custom_tlvs, expected_custom_tlvs); + assert_eq!(amt_msat, 0x0badf00d01020304); + assert_eq!(outgoing_cltv_value, 0xffffffff); } else { panic!(); } } diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index f3579a56b8d..7e0ccbe9652 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -171,6 +171,7 @@ pub(super) fn build_onion_payloads(path: &Path, total_msat: u64, mut recipient_o } else { None }, payment_metadata: recipient_onion.payment_metadata.take(), keysend_preimage: *keysend_preimage, + custom_tlvs: recipient_onion.custom_tlvs.clone(), amt_msat: value_msat, outgoing_cltv_value: cltv, } diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 1744b923d5e..41185b179ff 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -132,6 +132,16 @@ macro_rules! _check_encoded_tlv_order { /// [`Writer`]: crate::util::ser::Writer #[macro_export] macro_rules! encode_tlv_stream { + ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { + $crate::_encode_tlv_stream!($stream, {$(($type, $field, $fieldty)),*}) + } +} + +/// Implementation of [`encode_tlv_stream`]. +/// This is exported for use by other exported macros, do not use directly. +#[doc(hidden)] +#[macro_export] +macro_rules! _encode_tlv_stream { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { { #[allow(unused_imports)] use $crate::{ @@ -153,7 +163,21 @@ macro_rules! encode_tlv_stream { $crate::_check_encoded_tlv_order!(last_seen, $type, $fieldty); )* } - } } + } }; + ($stream: expr, $tlvs: expr) => { { + for tlv in $tlvs { + let (typ, value): &&(u64, Vec) = tlv; + $crate::_encode_tlv!($stream, *typ, *value, required_vec); + } + + #[cfg(debug_assertions)] { + let mut last_seen: Option = None; + for tlv in $tlvs { + let (typ, _): &&(u64, Vec) = tlv; + $crate::_check_encoded_tlv_order!(last_seen, *typ, required_vec); + } + } + } }; } /// Adds the length of the serialized field to a [`LengthCalculatingWriter`]. @@ -210,18 +234,27 @@ macro_rules! _get_varint_length_prefixed_tlv_length { #[macro_export] macro_rules! _encode_varint_length_prefixed_tlv { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),*}) => { { + _encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*}, &[]) + } }; + ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),*}, $extra_tlvs: expr) => { { use $crate::util::ser::BigSize; + use alloc::vec::Vec; let len = { #[allow(unused_mut)] let mut len = $crate::util::ser::LengthCalculatingWriter(0); $( $crate::_get_varint_length_prefixed_tlv_length!(len, $type, $field, $fieldty); )* + for tlv in $extra_tlvs { + let (typ, value): &&(u64, Vec) = tlv; + $crate::_get_varint_length_prefixed_tlv_length!(len, *typ, *value, required_vec); + } len.0 }; BigSize(len as u64).write($stream)?; - $crate::encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }); - } } + $crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }); + $crate::_encode_tlv_stream!($stream, $extra_tlvs); + } }; } /// Errors if there are missing required TLV types between the last seen type and the type currently being processed. @@ -785,7 +818,8 @@ macro_rules! _init_and_read_tlv_fields { /// /// For example, /// ``` -/// # use lightning::impl_writeable_tlv_based; +/// # use lightning::{impl_writeable_tlv_based, _encode_varint_length_prefixed_tlv}; +/// # extern crate alloc; /// struct LightningMessage { /// tlv_integer: u32, /// tlv_default_integer: u32, From 0a2dbdf2475b2ed8c7b8f53c17837f894ab2d802 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Wed, 17 May 2023 18:40:18 -0500 Subject: [PATCH 4/7] Handle receiving custom HTLC TLVs This completes basic receiver-side support for custom TLVs and adds functional testing for sending and receiving. --- lightning/src/ln/channelmanager.rs | 16 +- lightning/src/ln/payment_tests.rs | 166 ++++++++++++++++++++- pending_changelog/custom_tlv_downgrade.txt | 3 + 3 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 pending_changelog/custom_tlv_downgrade.txt diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5f23842dfb7..d2a62b8973e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -110,6 +110,8 @@ pub(super) enum PendingHTLCRouting { payment_metadata: Option>, incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed phantom_shared_secret: Option<[u8; 32]>, + /// See [`RecipientOnionFields::custom_tlvs`] for more info. + custom_tlvs: Vec<(u64, Vec)>, }, ReceiveKeysend { /// This was added in 0.0.116 and will break deserialization on downgrades. @@ -117,6 +119,8 @@ pub(super) enum PendingHTLCRouting { payment_preimage: PaymentPreimage, payment_metadata: Option>, incoming_cltv_expiry: u32, // Used to track when we should expire pending HTLCs that go unclaimed + /// See [`RecipientOnionFields::custom_tlvs`] for more info. + custom_tlvs: Vec<(u64, Vec)>, }, } @@ -2748,6 +2752,7 @@ where payment_preimage, payment_metadata, incoming_cltv_expiry: outgoing_cltv_value, + custom_tlvs, } } else if let Some(data) = payment_data { PendingHTLCRouting::Receive { @@ -2755,6 +2760,7 @@ where payment_metadata, incoming_cltv_expiry: outgoing_cltv_value, phantom_shared_secret, + custom_tlvs, } } else { return Err(InboundOnionErr { @@ -3941,18 +3947,18 @@ where } }) => { 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 } => { + PendingHTLCRouting::Receive { payment_data, payment_metadata, incoming_cltv_expiry, phantom_shared_secret, custom_tlvs } => { let _legacy_hop_data = Some(payment_data.clone()); let onion_fields = RecipientOnionFields { payment_secret: Some(payment_data.payment_secret), - payment_metadata, custom_tlvs: vec![] }; + payment_metadata, custom_tlvs }; (incoming_cltv_expiry, OnionPayload::Invoice { _legacy_hop_data }, Some(payment_data), phantom_shared_secret, onion_fields) }, - PendingHTLCRouting::ReceiveKeysend { payment_data, payment_preimage, payment_metadata, incoming_cltv_expiry } => { + PendingHTLCRouting::ReceiveKeysend { payment_data, payment_preimage, payment_metadata, incoming_cltv_expiry, custom_tlvs } => { let onion_fields = RecipientOnionFields { payment_secret: payment_data.as_ref().map(|data| data.payment_secret), payment_metadata, - custom_tlvs: vec![], + custom_tlvs, }; (incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), payment_data, None, onion_fields) @@ -7639,12 +7645,14 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting, (1, phantom_shared_secret, option), (2, incoming_cltv_expiry, required), (3, payment_metadata, option), + (5, custom_tlvs, optional_vec), }, (2, ReceiveKeysend) => { (0, payment_preimage, required), (2, incoming_cltv_expiry, required), (3, payment_metadata, option), (4, payment_data, option), // Added in 0.0.116 + (5, custom_tlvs, optional_vec), }, ;); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 26ece931770..86f29d96034 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -15,7 +15,7 @@ use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::sign::EntropySource; use crate::chain::transaction::OutPoint; -use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason}; +use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason, PaymentPurpose}; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; use crate::ln::features::Bolt11InvoiceFeatures; @@ -3385,6 +3385,170 @@ fn claim_from_closed_chan() { do_claim_from_closed_chan(false); } +#[test] +fn test_custom_tlvs() { + do_test_custom_tlvs(true); + do_test_custom_tlvs(false); +} + +fn do_test_custom_tlvs(spontaneous: bool) { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None; 2]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1); + + let amt_msat = 100_000; + let (mut route, our_payment_hash, our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(&nodes[0], &nodes[1], amt_msat); + let payment_id = PaymentId(our_payment_hash.0); + let custom_tlvs = vec![ + (5482373483, vec![1, 2, 3, 4]), + (5482373487, vec![0x42u8; 16]), + ]; + let onion_fields = RecipientOnionFields { + payment_secret: if spontaneous { None } else { Some(our_payment_secret) }, + payment_metadata: None, + custom_tlvs: custom_tlvs.clone() + }; + if spontaneous { + nodes[0].node.send_spontaneous_payment(&route, Some(our_payment_preimage), onion_fields, payment_id).unwrap(); + } else { + nodes[0].node.send_payment_with_route(&route, our_payment_hash, onion_fields, payment_id).unwrap(); + } + check_added_monitors(&nodes[0], 1); + + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); + let mut payment_event = SendEvent::from_event(ev); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + check_added_monitors!(&nodes[1], 0); + commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[1]); + + let events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentClaimable { ref purpose, amount_msat, ref onion_fields, .. } => { + match &purpose { + PaymentPurpose::InvoicePayment { payment_secret, .. } => { + assert_eq!(our_payment_secret, *payment_secret); + assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret); + }, + PaymentPurpose::SpontaneousPayment(payment_preimage) => { + assert_eq!(our_payment_preimage, *payment_preimage); + }, + } + assert_eq!(amount_msat, amt_msat); + assert_eq!(onion_fields.clone().unwrap().custom_tlvs().clone(), custom_tlvs); + }, + _ => panic!("Unexpected event"), + } + + claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage); +} + +#[test] +fn test_retry_custom_tlvs() { + // Test that custom TLVs are successfully sent on retries + 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 nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1); + let (chan_2_update, _, chan_2_id, _) = create_announced_chan_between_nodes(&nodes, 2, 1); + + // Rebalance + send_payment(&nodes[2], &vec!(&nodes[1])[..], 1_500_000); + + let amt_msat = 1_000_000; + let (route, payment_hash, payment_preimage, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat); + + // Initiate the payment + let payment_id = PaymentId(payment_hash.0); + let mut route_params = RouteParameters { + payment_params: route.payment_params.clone().unwrap(), + final_value_msat: amt_msat, + }; + + let custom_tlvs = vec![((1 << 16) + 1, vec![0x42u8; 16])]; + let onion_fields = RecipientOnionFields::secret_only(payment_secret); + let onion_fields = onion_fields.with_custom_tlvs(custom_tlvs.clone()).unwrap(); + + nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); + nodes[0].node.send_payment(payment_hash, onion_fields, + payment_id, route_params.clone(), Retry::Attempts(1)).unwrap(); + check_added_monitors!(nodes[0], 1); // one monitor per path + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + + // Add the HTLC along the first hop. + let fail_path_msgs_1 = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); + let (update_add, commitment_signed) = match fail_path_msgs_1 { + MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate { + ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, + ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } + } => { + assert_eq!(update_add_htlcs.len(), 1); + assert!(update_fail_htlcs.is_empty()); + assert!(update_fulfill_htlcs.is_empty()); + assert!(update_fail_malformed_htlcs.is_empty()); + assert!(update_fee.is_none()); + (update_add_htlcs[0].clone(), commitment_signed.clone()) + }, + _ => panic!("Unexpected event"), + }; + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add); + commitment_signed_dance!(nodes[1], nodes[0], commitment_signed, false); + + // Attempt to forward the payment and complete the path's failure. + expect_pending_htlcs_forwardable!(&nodes[1]); + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(&nodes[1], + vec![HTLCDestination::NextHopChannel { + node_id: Some(nodes[2].node.get_our_node_id()), + channel_id: chan_2_id + }]); + let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + assert!(htlc_updates.update_add_htlcs.is_empty()); + assert_eq!(htlc_updates.update_fail_htlcs.len(), 1); + assert!(htlc_updates.update_fulfill_htlcs.is_empty()); + assert!(htlc_updates.update_fail_malformed_htlcs.is_empty()); + check_added_monitors!(nodes[1], 1); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), + &htlc_updates.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false); + let mut events = nodes[0].node.get_and_clear_pending_events(); + match events[1] { + Event::PendingHTLCsForwardable { .. } => {}, + _ => panic!("Unexpected event") + } + events.remove(1); + expect_payment_failed_conditions_event(events, payment_hash, false, + PaymentFailedConditions::new().mpp_parts_remain()); + + // Rebalance the channel so the retry of the payment can succeed. + send_payment(&nodes[2], &vec!(&nodes[1])[..], 1_500_000); + + // Retry the payment and make sure it succeeds + route_params.payment_params.previously_failed_channels.push(chan_2_update.contents.short_channel_id); + nodes[0].router.expect_find_route(route_params, Ok(route)); + nodes[0].node.process_pending_htlc_forwards(); + check_added_monitors!(nodes[0], 1); + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let payment_claimable = pass_along_path(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000, + payment_hash, Some(payment_secret), events.pop().unwrap(), true, None).unwrap(); + let onion_fields = match payment_claimable { + Event::PaymentClaimable { onion_fields, .. } => onion_fields, + _ => panic!("Unexpected event"), + }; + assert_eq!(onion_fields.unwrap().custom_tlvs(), &custom_tlvs); + claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage); +} + fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) { // Check that a payment metadata received on one HTLC that doesn't match the one received on // another results in the HTLC being rejected. diff --git a/pending_changelog/custom_tlv_downgrade.txt b/pending_changelog/custom_tlv_downgrade.txt new file mode 100644 index 00000000000..54a36ca28e0 --- /dev/null +++ b/pending_changelog/custom_tlv_downgrade.txt @@ -0,0 +1,3 @@ +## Backwards Compatibility + +* Since the addition of custom HTLC TLV support in 0.0.117, if you downgrade you may unintentionally accept payments with features you don't understand. From e84fb067aa0e48f5851cb491354cdcdc94d063c7 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Fri, 19 May 2023 15:37:47 -0500 Subject: [PATCH 5/7] Drop non-matching custom TLVs when receiving MPP Upon receiving multiple payment parts with custom TLVs, we fail payments if they have any non-matching or missing even TLVs, and otherwise just drop non-matching TLVs if they're odd. --- lightning/src/ln/outbound_payment.rs | 12 +- lightning/src/ln/payment_tests.rs | 161 +++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 0e3c15c8066..9c8d6645873 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -511,7 +511,17 @@ impl RecipientOnionFields { pub(super) fn check_merge(&mut self, further_htlc_fields: &mut Self) -> Result<(), ()> { if self.payment_secret != further_htlc_fields.payment_secret { return Err(()); } if self.payment_metadata != further_htlc_fields.payment_metadata { return Err(()); } - // For custom TLVs we should just drop non-matching ones, but not reject the payment. + + let tlvs = &mut self.custom_tlvs; + let further_tlvs = &mut further_htlc_fields.custom_tlvs; + + let even_tlvs: Vec<&(u64, Vec)> = tlvs.iter().filter(|(typ, _)| *typ % 2 == 0).collect(); + let further_even_tlvs: Vec<&(u64, Vec)> = further_tlvs.iter().filter(|(typ, _)| *typ % 2 == 0).collect(); + if even_tlvs != further_even_tlvs { return Err(()) } + + tlvs.retain(|tlv| further_tlvs.iter().any(|further_tlv| tlv == further_tlv)); + further_tlvs.retain(|further_tlv| tlvs.iter().any(|tlv| tlv == further_tlv)); + Ok(()) } } diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 86f29d96034..fc1b181e9e7 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -3549,6 +3549,167 @@ fn test_retry_custom_tlvs() { claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], false, payment_preimage); } +#[test] +fn test_custom_tlvs_consistency() { + let even_type_1 = 1 << 16; + let odd_type_1 = (1 << 16)+ 1; + let even_type_2 = (1 << 16) + 2; + let odd_type_2 = (1 << 16) + 3; + let value_1 = || vec![1, 2, 3, 4]; + let differing_value_1 = || vec![1, 2, 3, 5]; + let value_2 = || vec![42u8; 16]; + + // Drop missing odd tlvs + do_test_custom_tlvs_consistency( + vec![(odd_type_1, value_1()), (odd_type_2, value_2())], + vec![(odd_type_1, value_1())], + Some(vec![(odd_type_1, value_1())]), + ); + // Drop non-matching odd tlvs + do_test_custom_tlvs_consistency( + vec![(odd_type_1, value_1()), (odd_type_2, value_2())], + vec![(odd_type_1, differing_value_1()), (odd_type_2, value_2())], + Some(vec![(odd_type_2, value_2())]), + ); + // Fail missing even tlvs + do_test_custom_tlvs_consistency( + vec![(odd_type_1, value_1()), (even_type_2, value_2())], + vec![(odd_type_1, value_1())], + None, + ); + // Fail non-matching even tlvs + do_test_custom_tlvs_consistency( + vec![(even_type_1, value_1()), (odd_type_2, value_2())], + vec![(even_type_1, differing_value_1()), (odd_type_2, value_2())], + None, + ); +} + +fn do_test_custom_tlvs_consistency(first_tlvs: Vec<(u64, Vec)>, second_tlvs: Vec<(u64, Vec)>, + expected_receive_tlvs: Option)>>) { + + let chanmon_cfgs = create_chanmon_cfgs(4); + let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]); + let nodes = create_network(4, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); + create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 100_000, 0); + create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0); + let chan_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0); + + let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_bolt11_features(nodes[3].node.invoice_features()).unwrap(); + let mut route = get_route!(nodes[0], payment_params, 15_000_000).unwrap(); + assert_eq!(route.paths.len(), 2); + route.paths.sort_by(|path_a, _| { + // Sort the path so that the path through nodes[1] comes first + if path_a.hops[0].pubkey == nodes[1].node.get_our_node_id() { + core::cmp::Ordering::Less } else { core::cmp::Ordering::Greater } + }); + + let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[3]); + let payment_id = PaymentId([42; 32]); + let amt_msat = 15_000_000; + + // Send first part + let onion_fields = RecipientOnionFields { + payment_secret: Some(our_payment_secret), + payment_metadata: None, + custom_tlvs: first_tlvs + }; + let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash, + onion_fields.clone(), payment_id, &route).unwrap(); + let cur_height = nodes[0].best_block_info().1; + nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash, + onion_fields.clone(), amt_msat, cur_height, payment_id, + &None, session_privs[0]).unwrap(); + check_added_monitors!(nodes[0], 1); + + { + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], amt_msat, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), false, None); + } + assert!(nodes[3].node.get_and_clear_pending_events().is_empty()); + + // Send second part + let onion_fields = RecipientOnionFields { + payment_secret: Some(our_payment_secret), + payment_metadata: None, + custom_tlvs: second_tlvs + }; + nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash, + onion_fields.clone(), amt_msat, cur_height, payment_id, &None, session_privs[1]).unwrap(); + check_added_monitors!(nodes[0], 1); + + { + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let payment_event = SendEvent::from_event(events.pop().unwrap()); + + nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[2], nodes[0], payment_event.commitment_msg, false); + + expect_pending_htlcs_forwardable!(nodes[2]); + check_added_monitors!(nodes[2], 1); + + let mut events = nodes[2].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let payment_event = SendEvent::from_event(events.pop().unwrap()); + + nodes[3].node.handle_update_add_htlc(&nodes[2].node.get_our_node_id(), &payment_event.msgs[0]); + check_added_monitors!(nodes[3], 0); + commitment_signed_dance!(nodes[3], nodes[2], payment_event.commitment_msg, true, true); + } + expect_pending_htlcs_forwardable_ignore!(nodes[3]); + nodes[3].node.process_pending_htlc_forwards(); + + if let Some(expected_tlvs) = expected_receive_tlvs { + // Claim and match expected + let events = nodes[3].node.get_and_clear_pending_events(); + println!("events: {:?}", events); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentClaimable { ref purpose, amount_msat, ref onion_fields, .. } => { + match &purpose { + PaymentPurpose::InvoicePayment { payment_secret, .. } => { + assert_eq!(our_payment_secret, *payment_secret); + assert_eq!(Some(*payment_secret), onion_fields.as_ref().unwrap().payment_secret); + }, + PaymentPurpose::SpontaneousPayment(payment_preimage) => { + assert_eq!(our_payment_preimage, *payment_preimage); + }, + } + assert_eq!(amount_msat, amt_msat); + assert_eq!(onion_fields.clone().unwrap().custom_tlvs, expected_tlvs); + }, + _ => panic!("Unexpected event"), + } + + do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage); + expect_payment_sent(&nodes[0], our_payment_preimage, Some(Some(2000)), true); + } else { + // Expect fail back + let expected_destinations = vec![HTLCDestination::FailedPayment { payment_hash: our_payment_hash }]; + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[3], expected_destinations); + check_added_monitors!(nodes[3], 1); + + let fail_updates_1 = get_htlc_update_msgs!(nodes[3], nodes[2].node.get_our_node_id()); + nodes[2].node.handle_update_fail_htlc(&nodes[3].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[2], nodes[3], fail_updates_1.commitment_signed, false); + + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[2], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_2_3.2 }]); + check_added_monitors!(nodes[2], 1); + + let fail_updates_2 = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &fail_updates_2.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[2], fail_updates_2.commitment_signed, false); + + expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain()); + } +} + fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) { // Check that a payment metadata received on one HTLC that doesn't match the one received on // another results in the HTLC being rejected. From 8ff16046475b39c280b85dede5f0912269aed28c Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Wed, 7 Jun 2023 23:17:09 -0500 Subject: [PATCH 6/7] Add `FailureCode::InvalidOnionPayload` variant When a user decodes custom TLVs, if they fail to recognize even type numbers they should fail back with the correct failure code and fail data. This new variant adds the proper failure variant for the user to pass into `ChannelManager::fail_htlc_backwards_with_reason`. Note that the enum discriminants were removed because when adding a struct variant we can no longer make use of the discriminant through casting like we previously did, and instead have to manually define the associated failure code anyway. --- lightning/src/ln/channelmanager.rs | 36 ++++++++++++++++++++++----- lightning/src/ln/onion_route_tests.rs | 12 +++++++-- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d2a62b8973e..68e5451d8b2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -359,15 +359,32 @@ struct InboundOnionErr { pub enum FailureCode { /// We had a temporary error processing the payment. Useful if no other error codes fit /// and you want to indicate that the payer may want to retry. - TemporaryNodeFailure = 0x2000 | 2, + TemporaryNodeFailure, /// We have a required feature which was not in this onion. For example, you may require /// some additional metadata that was not provided with this payment. - RequiredNodeFeatureMissing = 0x4000 | 0x2000 | 3, + RequiredNodeFeatureMissing, /// You may wish to use this when a `payment_preimage` is unknown, or the CLTV expiry of /// the HTLC is too close to the current block height for safe handling. /// Using this failure code in [`ChannelManager::fail_htlc_backwards_with_reason`] is /// equivalent to calling [`ChannelManager::fail_htlc_backwards`]. - IncorrectOrUnknownPaymentDetails = 0x4000 | 15, + IncorrectOrUnknownPaymentDetails, + /// We failed to process the payload after the onion was decrypted. You may wish to + /// use this when receiving custom HTLC TLVs with even type numbers that you don't recognize. + /// + /// If available, the tuple data may include the type number and byte offset in the + /// decrypted byte stream where the failure occurred. + InvalidOnionPayload(Option<(u64, u16)>), +} + +impl Into for FailureCode { + fn into(self) -> u16 { + match self { + FailureCode::TemporaryNodeFailure => 0x2000 | 2, + FailureCode::RequiredNodeFeatureMissing => 0x4000 | 0x2000 | 3, + FailureCode::IncorrectOrUnknownPaymentDetails => 0x4000 | 15, + FailureCode::InvalidOnionPayload(_) => 0x4000 | 22, + } + } } /// Error type returned across the peer_state mutex boundary. When an Err is generated for a @@ -4583,12 +4600,19 @@ where /// Gets error data to form an [`HTLCFailReason`] given a [`FailureCode`] and [`ClaimableHTLC`]. fn get_htlc_fail_reason_from_failure_code(&self, failure_code: FailureCode, htlc: &ClaimableHTLC) -> HTLCFailReason { match failure_code { - FailureCode::TemporaryNodeFailure => HTLCFailReason::from_failure_code(failure_code as u16), - FailureCode::RequiredNodeFeatureMissing => HTLCFailReason::from_failure_code(failure_code as u16), + FailureCode::TemporaryNodeFailure => HTLCFailReason::from_failure_code(failure_code.into()), + FailureCode::RequiredNodeFeatureMissing => HTLCFailReason::from_failure_code(failure_code.into()), FailureCode::IncorrectOrUnknownPaymentDetails => { let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec(); htlc_msat_height_data.extend_from_slice(&self.best_block.read().unwrap().height().to_be_bytes()); - HTLCFailReason::reason(failure_code as u16, htlc_msat_height_data) + HTLCFailReason::reason(failure_code.into(), htlc_msat_height_data) + }, + FailureCode::InvalidOnionPayload(data) => { + let fail_data = match data { + Some((typ, offset)) => [BigSize(typ).encode(), offset.encode()].concat(), + None => Vec::new(), + }; + HTLCFailReason::reason(failure_code.into(), fail_data) } } } diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 888ac7dca10..c6ac77ac378 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -24,7 +24,7 @@ use crate::ln::features::{InitFeatures, Bolt11InvoiceFeatures}; use crate::ln::msgs; use crate::ln::msgs::{ChannelMessageHandler, ChannelUpdate}; use crate::ln::wire::Encode; -use crate::util::ser::{Writeable, Writer}; +use crate::util::ser::{Writeable, Writer, BigSize}; use crate::util::test_utils; use crate::util::config::{UserConfig, ChannelConfig, MaxDustHTLCExposure}; use crate::util::errors::APIError; @@ -942,10 +942,16 @@ fn do_test_fail_htlc_backwards_with_reason(failure_code: FailureCode) { let mut htlc_msat_height_data = (payment_amount as u64).to_be_bytes().to_vec(); htlc_msat_height_data.extend_from_slice(&CHAN_CONFIRM_DEPTH.to_be_bytes()); htlc_msat_height_data + }, + FailureCode::InvalidOnionPayload(data) => { + match data { + Some((typ, offset)) => [BigSize(typ).encode(), offset.encode()].concat(), + None => Vec::new(), + } } }; - let failure_code = failure_code as u16; + let failure_code = failure_code.into(); let permanent_flag = 0x4000; let permanent_fail = (failure_code & permanent_flag) != 0; expect_payment_failed!(nodes[0], payment_hash, permanent_fail, failure_code, failure_data); @@ -957,6 +963,8 @@ fn test_fail_htlc_backwards_with_reason() { do_test_fail_htlc_backwards_with_reason(FailureCode::TemporaryNodeFailure); do_test_fail_htlc_backwards_with_reason(FailureCode::RequiredNodeFeatureMissing); do_test_fail_htlc_backwards_with_reason(FailureCode::IncorrectOrUnknownPaymentDetails); + do_test_fail_htlc_backwards_with_reason(FailureCode::InvalidOnionPayload(Some((1 << 16, 42)))); + do_test_fail_htlc_backwards_with_reason(FailureCode::InvalidOnionPayload(None)); } macro_rules! get_phantom_route { From dec3fb316a7905284d64bb5bad88a898d04744d1 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Thu, 8 Jun 2023 12:08:25 -0500 Subject: [PATCH 7/7] Enforce explicit claims on payments with even custom TLVs Because we don't know which custom TLV type numbers the user is expecting (and it would be cumbersome for them to tell us), instead of failing unknown even custom TLVs on deserialization, we accept all custom TLVs, and pass them to the user to check whether they recognize them and choose to fail back if they don't. However, a user may not check for custom TLVs, in which case we should reject any even custom TLVs as unknown. This commit makes sure a user must explicitly accept a payment with even custom TLVs, by (1) making the default `ChannelManager::claim_funds` fail if the payment had even custom TLVs and (2) adding a new function `ChannelManager::claim_funds_with_known_custom_tlvs` that accepts them. This commit also refactors our custom TLVs test and updates various documentation to account for this. --- lightning/src/events/mod.rs | 18 +++++++++-- lightning/src/ln/channelmanager.rs | 39 +++++++++++++++++++++++ lightning/src/ln/functional_test_utils.rs | 3 ++ lightning/src/ln/payment_tests.rs | 35 ++++++++++++++++---- 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index d08e563cbf6..f8d251c91c7 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -356,9 +356,19 @@ pub enum Event { /// Note that if the preimage is not known, you should call /// [`ChannelManager::fail_htlc_backwards`] or [`ChannelManager::fail_htlc_backwards_with_reason`] /// to free up resources for this HTLC and avoid network congestion. - /// If you fail to call either [`ChannelManager::claim_funds`], [`ChannelManager::fail_htlc_backwards`], - /// or [`ChannelManager::fail_htlc_backwards_with_reason`] within the HTLC's timeout, the HTLC will be - /// automatically failed. + /// + /// If [`Event::PaymentClaimable::onion_fields`] is `Some`, and includes custom TLVs with even type + /// numbers, you should use [`ChannelManager::fail_htlc_backwards_with_reason`] with + /// [`FailureCode::InvalidOnionPayload`] if you fail to understand and handle the contents, or + /// [`ChannelManager::claim_funds_with_known_custom_tlvs`] upon successful handling. + /// If you don't intend to check for custom TLVs, you can simply use + /// [`ChannelManager::claim_funds`], which will automatically fail back even custom TLVs. + /// + /// If you fail to call [`ChannelManager::claim_funds`], + /// [`ChannelManager::claim_funds_with_known_custom_tlvs`], + /// [`ChannelManager::fail_htlc_backwards`], or + /// [`ChannelManager::fail_htlc_backwards_with_reason`] within the HTLC's timeout, the HTLC will + /// be automatically failed. /// /// # Note /// LDK will not stop an inbound payment from being paid multiple times, so multiple @@ -370,6 +380,8 @@ pub enum Event { /// This event used to be called `PaymentReceived` in LDK versions 0.0.112 and earlier. /// /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds + /// [`ChannelManager::claim_funds_with_known_custom_tlvs`]: crate::ln::channelmanager::ChannelManager::claim_funds_with_known_custom_tlvs + /// [`FailureCode::InvalidOnionPayload`]: crate::ln::channelmanager::FailureCode::InvalidOnionPayload /// [`ChannelManager::fail_htlc_backwards`]: crate::ln::channelmanager::ChannelManager::fail_htlc_backwards /// [`ChannelManager::fail_htlc_backwards_with_reason`]: crate::ln::channelmanager::ChannelManager::fail_htlc_backwards_with_reason PaymentClaimable { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 68e5451d8b2..ae9744cb71c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4759,13 +4759,35 @@ where /// event matches your expectation. If you fail to do so and call this method, you may provide /// the sender "proof-of-payment" when they did not fulfill the full expected payment. /// + /// This function will fail the payment if it has custom TLVs with even type numbers, as we + /// will assume they are unknown. If you intend to accept even custom TLVs, you should use + /// [`claim_funds_with_known_custom_tlvs`]. + /// /// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable /// [`Event::PaymentClaimable::claim_deadline`]: crate::events::Event::PaymentClaimable::claim_deadline /// [`Event::PaymentClaimed`]: crate::events::Event::PaymentClaimed /// [`process_pending_events`]: EventsProvider::process_pending_events /// [`create_inbound_payment`]: Self::create_inbound_payment /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash + /// [`claim_funds_with_known_custom_tlvs`]: Self::claim_funds_with_known_custom_tlvs pub fn claim_funds(&self, payment_preimage: PaymentPreimage) { + self.claim_payment_internal(payment_preimage, false); + } + + /// This is a variant of [`claim_funds`] that allows accepting a payment with custom TLVs with + /// even type numbers. + /// + /// # Note + /// + /// You MUST check you've understood all even TLVs before using this to + /// claim, otherwise you may unintentionally agree to some protocol you do not understand. + /// + /// [`claim_funds`]: Self::claim_funds + pub fn claim_funds_with_known_custom_tlvs(&self, payment_preimage: PaymentPreimage) { + self.claim_payment_internal(payment_preimage, true); + } + + fn claim_payment_internal(&self, payment_preimage: PaymentPreimage, custom_tlvs_known: bool) { let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); @@ -4792,6 +4814,23 @@ where log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug", log_bytes!(payment_hash.0)); } + + if let Some(RecipientOnionFields { ref custom_tlvs, .. }) = payment.onion_fields { + if !custom_tlvs_known && custom_tlvs.iter().any(|(typ, _)| typ % 2 == 0) { + log_info!(self.logger, "Rejecting payment with payment hash {} as we cannot accept payment with unknown even TLVs: {}", + log_bytes!(payment_hash.0), log_iter!(custom_tlvs.iter().map(|(typ, _)| typ).filter(|typ| *typ % 2 == 0))); + claimable_payments.pending_claiming_payments.remove(&payment_hash); + mem::drop(claimable_payments); + for htlc in payment.htlcs { + let reason = self.get_htlc_fail_reason_from_failure_code(FailureCode::InvalidOnionPayload(None), &htlc); + let source = HTLCSource::PreviousHopData(htlc.prev_hop); + let receiver = HTLCDestination::FailedPayment { payment_hash }; + self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver); + } + return; + } + } + payment.htlcs } else { return; } }; diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 39396685f00..d51eceac21c 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2250,7 +2250,10 @@ pub fn do_claim_payment_along_route_with_extra_penultimate_hop_fees<'a, 'b, 'c>( assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id()); } expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage); + pass_claimed_payment_along_route(origin_node, expected_paths, expected_extra_fees, skip_last, our_payment_preimage) +} +pub fn pass_claimed_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], expected_extra_fees: &[u32], skip_last: bool, our_payment_preimage: PaymentPreimage) -> u64 { let claim_event = expected_paths[0].last().unwrap().node.get_and_clear_pending_events(); assert_eq!(claim_event.len(), 1); match claim_event[0] { diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index fc1b181e9e7..1725fd301fa 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -3386,12 +3386,20 @@ fn claim_from_closed_chan() { } #[test] -fn test_custom_tlvs() { - do_test_custom_tlvs(true); - do_test_custom_tlvs(false); +fn test_custom_tlvs_basic() { + do_test_custom_tlvs(false, false, false); + do_test_custom_tlvs(true, false, false); } -fn do_test_custom_tlvs(spontaneous: bool) { +#[test] +fn test_custom_tlvs_explicit_claim() { + // Test that when receiving even custom TLVs the user must explicitly accept in case they + // are unknown. + do_test_custom_tlvs(false, true, false); + do_test_custom_tlvs(false, true, true); +} + +fn do_test_custom_tlvs(spontaneous: bool, even_tlvs: bool, known_tlvs: bool) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None; 2]); @@ -3403,7 +3411,7 @@ fn do_test_custom_tlvs(spontaneous: bool) { let (mut route, our_payment_hash, our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(&nodes[0], &nodes[1], amt_msat); let payment_id = PaymentId(our_payment_hash.0); let custom_tlvs = vec![ - (5482373483, vec![1, 2, 3, 4]), + (if even_tlvs { 5482373482 } else { 5482373483 }, vec![1, 2, 3, 4]), (5482373487, vec![0x42u8; 16]), ]; let onion_fields = RecipientOnionFields { @@ -3446,7 +3454,22 @@ fn do_test_custom_tlvs(spontaneous: bool) { _ => panic!("Unexpected event"), } - claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage); + match (known_tlvs, even_tlvs) { + (true, _) => { + nodes[1].node.claim_funds_with_known_custom_tlvs(our_payment_preimage); + let expected_total_fee_msat = pass_claimed_payment_along_route(&nodes[0], &[&[&nodes[1]]], &[0; 1], false, our_payment_preimage); + expect_payment_sent!(&nodes[0], our_payment_preimage, Some(expected_total_fee_msat)); + }, + (false, false) => { + claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage); + }, + (false, true) => { + nodes[1].node.claim_funds(our_payment_preimage); + let expected_destinations = vec![HTLCDestination::FailedPayment { payment_hash: our_payment_hash }]; + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], expected_destinations); + pass_failed_payment_back(&nodes[0], &[&[&nodes[1]]], false, our_payment_hash, PaymentFailureReason::RecipientRejected); + } + } } #[test]