From f560320b5fb257b1863eaf33f50aca373a70c9f9 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Thu, 18 May 2023 23:16:29 -0500 Subject: [PATCH] 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 7b0558881..5f23842df 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 aedd1e76e..b6ec022dc 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 f3579a56b..7e0ccbe96 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 1744b923d..41185b179 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, -- 2.39.5