diff --git a/lightning/src/blinded_path/payment.rs b/lightning/src/blinded_path/payment.rs index 03b676adc92..63fab5a1e64 100644 --- a/lightning/src/blinded_path/payment.rs +++ b/lightning/src/blinded_path/payment.rs @@ -632,7 +632,7 @@ impl<'a> Writeable for BlindedPaymentTlvsRef<'a> { impl Readable for BlindedPaymentTlvs { fn read(r: &mut R) -> Result { - _init_and_read_tlv_stream!(r, { + _init_and_read_tlv_stream_with_custom_tlv_decode!(r, { // Reasoning: Padding refers to filler data added to a packet to increase // its size and obscure its actual length. Since padding contains no meaningful // information, we can safely omit reading it here. @@ -645,6 +645,14 @@ impl Readable for BlindedPaymentTlvs { (65536, payment_secret, option), (65537, payment_context, option), (65539, is_dummy, option) + }, |typ: u64, _reader: &mut FixedLengthReader<_>| -> Result { + if typ == 1 { + return Ok(true); + } + if typ % 2 == 1 { + return Err(DecodeError::UnknownRequiredFeature); + } + Ok(false) }); match ( @@ -685,7 +693,7 @@ impl Readable for BlindedPaymentTlvs { impl Readable for BlindedTrampolineTlvs { fn read(r: &mut R) -> Result { - _init_and_read_tlv_stream!(r, { + _init_and_read_tlv_stream_with_custom_tlv_decode!(r, { (4, next_trampoline, option), (8, next_blinding_override, option), (10, payment_relay, option), @@ -693,6 +701,14 @@ impl Readable for BlindedTrampolineTlvs { (14, features, (option, encoding: (BlindedHopFeatures, WithoutLength))), (65536, payment_secret, option), (65537, payment_context, option), + }, |typ: u64, _reader: &mut FixedLengthReader<_>| -> Result { + if typ == 1 { + return Ok(true); + } + if typ % 2 == 1 { + return Err(DecodeError::UnknownRequiredFeature); + } + Ok(false) }); if let Some(next_trampoline) = next_trampoline { @@ -964,12 +980,15 @@ impl_writeable_tlv_based!(Bolt12RefundContext, {}); #[cfg(test)] mod tests { use crate::blinded_path::payment::{ - Bolt12RefundContext, ForwardTlvs, PaymentConstraints, PaymentContext, PaymentForwardNode, - PaymentRelay, ReceiveTlvs, + BlindedPaymentTlvs, BlindedTrampolineTlvs, Bolt12RefundContext, ForwardTlvs, + PaymentConstraints, PaymentContext, PaymentForwardNode, PaymentRelay, ReceiveTlvs, }; + use crate::io::Cursor; use crate::ln::functional_test_utils::TEST_FINAL_CLTV; + use crate::ln::msgs::DecodeError; use crate::types::features::BlindedHopFeatures; use crate::types::payment::PaymentSecret; + use crate::util::ser::{BigSize, Readable, Writeable}; use bitcoin::secp256k1::PublicKey; #[test] @@ -1237,4 +1256,42 @@ mod tests { .unwrap(); assert_eq!(blinded_payinfo.htlc_maximum_msat, 3997); } + + #[test] + fn blinded_payment_tlvs_reject_unknown_odd_types() { + let receive_tlvs = ReceiveTlvs { + payment_secret: PaymentSecret([0; 32]), + payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 }, + payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}), + }; + + let mut encoded = Vec::new(); + receive_tlvs.write(&mut encoded).unwrap(); + BigSize(65541).write(&mut encoded).unwrap(); + BigSize(0).write(&mut encoded).unwrap(); + + assert!(matches!( + BlindedPaymentTlvs::read(&mut Cursor::new(&encoded)), + Err(DecodeError::UnknownRequiredFeature), + )); + } + + #[test] + fn blinded_trampoline_tlvs_reject_unknown_odd_types() { + let receive_tlvs = ReceiveTlvs { + payment_secret: PaymentSecret([0; 32]), + payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 }, + payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}), + }; + + let mut encoded = Vec::new(); + receive_tlvs.write(&mut encoded).unwrap(); + BigSize(65541).write(&mut encoded).unwrap(); + BigSize(0).write(&mut encoded).unwrap(); + + assert!(matches!( + BlindedTrampolineTlvs::read(&mut Cursor::new(&encoded)), + Err(DecodeError::UnknownRequiredFeature), + )); + } } diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index d62f79957eb..1d7f43a0cc1 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -1412,7 +1412,7 @@ fn custom_tlvs_to_blinded_path() { let chan_upd = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0).0.contents; let amt_msat = 5000; - let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[1], Some(amt_msat), None); + let (_payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[1], Some(amt_msat), None); let payee_tlvs = ReceiveTlvs { payment_secret, payment_constraints: PaymentConstraints { @@ -1448,12 +1448,23 @@ fn custom_tlvs_to_blinded_path() { let path = &[&nodes[1]]; let args = PassAlongPathArgs::new(&nodes[0], path, amt_msat, payment_hash, ev) .with_payment_secret(payment_secret) - .with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone()); + .with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone()) + .without_clearing_recipient_events() + .without_claimable_event(); do_pass_along_path(args); - claim_payment_along_route( - ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1]]], payment_preimage) - .with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone()) + check_added_monitors(&nodes[1], 1); + + expect_htlc_handling_failed_destinations!( + nodes[1].node.get_and_clear_pending_events(), + &[HTLCHandlingFailureType::InvalidOnion] ); + let updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id()); + assert_eq!(updates.update_fail_htlcs.len(), 1); + nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false); + expect_payment_failed_conditions(&nodes[0], payment_hash, true, + PaymentFailedConditions::new() + .expected_htlc_error_data(LocalHTLCFailureReason::InvalidOnionPayload, &[0; 0])); } #[test] @@ -1718,116 +1729,12 @@ fn route_blinding_spec_test_vector() { let bob_update_add = update_add_msg(110_000, 747_500, None, bob_onion); let bob_node_signer = TestEcdhSigner { node_secret: bob_secret }; // Can't use the public API here as we need to avoid the CLTV delta checks (test vector uses - // < MIN_CLTV_EXPIRY_DELTA). - let (bob_peeled_onion, next_packet_details_opt) = - match onion_payment::decode_incoming_update_add_htlc_onion( - &bob_update_add, &bob_node_signer, &logger, &secp_ctx - ) { - Ok(res) => res, - _ => panic!("Unexpected error") - }; - let (carol_packet_bytes, carol_hmac) = if let onion_utils::Hop::BlindedForward { - next_hop_data: msgs::InboundOnionBlindedForwardPayload { - short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override - }, next_hop_hmac, new_packet_bytes, .. - } = bob_peeled_onion { - assert_eq!(short_channel_id, 1729); - assert!(next_blinding_override.is_none()); - assert_eq!(intro_node_blinding_point, Some(bob_blinding_point)); - assert_eq!(payment_relay, PaymentRelay { cltv_expiry_delta: 36, fee_proportional_millionths: 150, fee_base_msat: 10_000 }); - assert_eq!(features, BlindedHopFeatures::empty()); - assert_eq!(payment_constraints, PaymentConstraints { max_cltv_expiry: 748_005, htlc_minimum_msat: 1500 }); - (new_packet_bytes, next_hop_hmac) - } else { panic!() }; - - let carol_packet_details = next_packet_details_opt.unwrap(); - let carol_onion = msgs::OnionPacket { - version: 0, - public_key: carol_packet_details.next_packet_pubkey, - hop_data: carol_packet_bytes, - hmac: carol_hmac, - }; - let carol_update_add = update_add_msg( - carol_packet_details.outgoing_amt_msat, carol_packet_details.outgoing_cltv_value, - Some(pubkey_from_hex("034e09f450a80c3d252b258aba0a61215bf60dda3b0dc78ffb0736ea1259dfd8a0")), - carol_onion - ); - let carol_node_signer = TestEcdhSigner { node_secret: carol_secret }; - let (carol_peeled_onion, next_packet_details_opt) = - match onion_payment::decode_incoming_update_add_htlc_onion( - &carol_update_add, &carol_node_signer, &logger, &secp_ctx - ) { - Ok(res) => res, - _ => panic!("Unexpected error") - }; - let (dave_packet_bytes, dave_hmac) = if let onion_utils::Hop::BlindedForward { - next_hop_data: msgs::InboundOnionBlindedForwardPayload { - short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override - }, next_hop_hmac, new_packet_bytes, .. - } = carol_peeled_onion { - assert_eq!(short_channel_id, 1105); - assert_eq!(next_blinding_override, Some(pubkey_from_hex("031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f"))); - assert!(intro_node_blinding_point.is_none()); - assert_eq!(payment_relay, PaymentRelay { cltv_expiry_delta: 48, fee_proportional_millionths: 100, fee_base_msat: 500 }); - assert_eq!(features, BlindedHopFeatures::empty()); - assert_eq!(payment_constraints, PaymentConstraints { max_cltv_expiry: 747_969, htlc_minimum_msat: 1500 }); - (new_packet_bytes, next_hop_hmac) - } else { panic!() }; - - let dave_packet_details = next_packet_details_opt.unwrap(); - let dave_onion = msgs::OnionPacket { - version: 0, - public_key: dave_packet_details.next_packet_pubkey, - hop_data: dave_packet_bytes, - hmac: dave_hmac, - }; - let dave_update_add = update_add_msg( - dave_packet_details.outgoing_amt_msat, dave_packet_details.outgoing_cltv_value, - Some(pubkey_from_hex("031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f")), - dave_onion - ); - let dave_node_signer = TestEcdhSigner { node_secret: dave_secret }; - let (dave_peeled_onion, next_packet_details_opt) = - match onion_payment::decode_incoming_update_add_htlc_onion( - &dave_update_add, &dave_node_signer, &logger, &secp_ctx - ) { - Ok(res) => res, - _ => panic!("Unexpected error") - }; - let (eve_packet_bytes, eve_hmac) = if let onion_utils::Hop::BlindedForward { - next_hop_data: msgs::InboundOnionBlindedForwardPayload { - short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override - }, next_hop_hmac, new_packet_bytes, .. - } = dave_peeled_onion { - assert_eq!(short_channel_id, 561); - assert!(next_blinding_override.is_none()); - assert!(intro_node_blinding_point.is_none()); - assert_eq!(payment_relay, PaymentRelay { cltv_expiry_delta: 144, fee_proportional_millionths: 250, fee_base_msat: 0 }); - assert_eq!(features, BlindedHopFeatures::empty()); - assert_eq!(payment_constraints, PaymentConstraints { max_cltv_expiry: 747_921, htlc_minimum_msat: 1500 }); - (new_packet_bytes, next_hop_hmac) - } else { panic!() }; - - let eve_packet_details = next_packet_details_opt.unwrap(); - let eve_onion = msgs::OnionPacket { - version: 0, - public_key: eve_packet_details.next_packet_pubkey, - hop_data: eve_packet_bytes, - hmac: eve_hmac, - }; - let eve_update_add = update_add_msg( - eve_packet_details.outgoing_amt_msat, eve_packet_details.outgoing_cltv_value, - Some(pubkey_from_hex("03e09038ee76e50f444b19abf0a555e8697e035f62937168b80adf0931b31ce52a")), - eve_onion - ); - let eve_node_signer = TestEcdhSigner { node_secret: eve_secret }; - // We can't decode the final payload because it contains a path_id and is missing some LDK - // specific fields. + // < MIN_CLTV_EXPIRY_DELTA). The vector includes an unknown odd encrypted TLV (type 561) in the + // blinded payload for Bob, which should now be rejected instead of forwarded. match onion_payment::decode_incoming_update_add_htlc_onion( - &eve_update_add, &eve_node_signer, &logger, &secp_ctx + &bob_update_add, &bob_node_signer, &logger, &secp_ctx ) { - Err((HTLCFailureMsg::Malformed(msg), _)) => assert_eq!(msg.failure_code, - LocalHTLCFailureReason::InvalidOnionBlinding.failure_code()), + Err((HTLCFailureMsg::Relay(_), LocalHTLCFailureReason::InvalidOnionPayload)) => {}, _ => panic!("Unexpected error") } } diff --git a/lightning/src/ln/max_payment_path_len_tests.rs b/lightning/src/ln/max_payment_path_len_tests.rs index 45640d3486d..ab09229c37b 100644 --- a/lightning/src/ln/max_payment_path_len_tests.rs +++ b/lightning/src/ln/max_payment_path_len_tests.rs @@ -214,7 +214,7 @@ fn one_hop_blinded_path_with_custom_tlv() { // Construct the route parameters for sending to nodes[2]'s 1-hop blinded path. let amt_msat = 100_000; - let (payment_preimage, payment_hash, payment_secret) = + let (_payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None); let payee_tlvs = ReceiveTlvs { payment_secret, @@ -266,33 +266,32 @@ fn one_hop_blinded_path_with_custom_tlv() { let max_sized_onion = RecipientOnionFields::spontaneous_empty(amt_msat).with_custom_tlvs( RecipientCustomTlvs::new(vec![(CUSTOM_TLV_TYPE, vec![42; max_custom_tlv_len])]).unwrap(), ); - let id = PaymentId(payment_hash.0); let no_retry = Retry::Attempts(0); nodes[1] .node - .send_payment(payment_hash, max_sized_onion.clone(), id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + max_sized_onion.clone(), + PaymentId([1; 32]), + route_params.clone(), + no_retry, + ) .unwrap(); check_added_monitors(&nodes[1], 1); - - let mut events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - let path = &[&nodes[2]]; - let args = - PassAlongPathArgs::new(&nodes[1], path, amt_msat, payment_hash, events.pop().unwrap()) - .with_payment_secret(payment_secret) - .with_custom_tlvs(max_sized_onion.custom_tlvs.clone()); - do_pass_along_path(args); - claim_payment_along_route( - ClaimAlongRouteArgs::new(&nodes[1], &[&[&nodes[2]]], payment_preimage) - .with_custom_tlvs(max_sized_onion.custom_tlvs.clone()), - ); + assert_eq!(nodes[1].node.get_and_clear_pending_msg_events().len(), 1); // If 1 byte is added to the custom TLV value, we'll fail to send prior to pathfinding. let mut too_large_custom_tlv_onion = max_sized_onion.clone(); too_large_custom_tlv_onion.custom_tlvs[0].1.push(42); let err = nodes[1] .node - .send_payment(payment_hash, too_large_custom_tlv_onion, id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + too_large_custom_tlv_onion, + PaymentId([2; 32]), + route_params.clone(), + no_retry, + ) .unwrap_err(); assert_eq!(err, RetryableSendFailure::OnionPacketSizeExceeded); @@ -300,7 +299,13 @@ fn one_hop_blinded_path_with_custom_tlv() { // nodes[0] -> nodes[2] will fail. let err = nodes[0] .node - .send_payment(payment_hash, max_sized_onion.clone(), id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + max_sized_onion.clone(), + PaymentId([3; 32]), + route_params.clone(), + no_retry, + ) .unwrap_err(); assert_eq!(err, RetryableSendFailure::RouteNotFound); @@ -312,22 +317,16 @@ fn one_hop_blinded_path_with_custom_tlv() { .resize(max_custom_tlv_len - INTERMED_PAYLOAD_LEN_ESTIMATE, 0); nodes[0] .node - .send_payment(payment_hash, onion_allows_2_hops.clone(), id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + onion_allows_2_hops, + PaymentId([4; 32]), + route_params.clone(), + no_retry, + ) .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 path = &[&nodes[1], &nodes[2]]; - let args = - PassAlongPathArgs::new(&nodes[0], path, amt_msat, payment_hash, events.pop().unwrap()) - .with_payment_secret(payment_secret) - .with_custom_tlvs(onion_allows_2_hops.custom_tlvs.clone()); - do_pass_along_path(args); - claim_payment_along_route( - ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[2]]], payment_preimage) - .with_custom_tlvs(onion_allows_2_hops.custom_tlvs), - ); + assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 1); } #[test] @@ -353,7 +352,7 @@ fn blinded_path_with_custom_tlv() { // Construct the route parameters for sending to nodes[3]'s blinded path. let amt_msat = 100_000; - let (payment_preimage, payment_hash, payment_secret) = + let (_payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[3], Some(amt_msat), None); let route_params = get_blinded_route_parameters( amt_msat, @@ -392,32 +391,31 @@ fn blinded_path_with_custom_tlv() { RecipientCustomTlvs::new(vec![(CUSTOM_TLV_TYPE, vec![42; max_custom_tlv_len])]).unwrap(), ); let no_retry = Retry::Attempts(0); - let id = PaymentId(payment_hash.0); nodes[1] .node - .send_payment(payment_hash, max_sized_onion.clone(), id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + max_sized_onion.clone(), + PaymentId([1; 32]), + route_params.clone(), + no_retry, + ) .unwrap(); check_added_monitors(&nodes[1], 1); - - let mut events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - let path = &[&nodes[2], &nodes[3]]; - let args = - PassAlongPathArgs::new(&nodes[1], path, amt_msat, payment_hash, events.pop().unwrap()) - .with_payment_secret(payment_secret) - .with_custom_tlvs(max_sized_onion.custom_tlvs.clone()); - do_pass_along_path(args); - claim_payment_along_route( - ClaimAlongRouteArgs::new(&nodes[1], &[&[&nodes[2], &nodes[3]]], payment_preimage) - .with_custom_tlvs(max_sized_onion.custom_tlvs.clone()), - ); + assert_eq!(nodes[1].node.get_and_clear_pending_msg_events().len(), 1); // If 1 byte is added to the custom TLV value, we'll fail to send prior to pathfinding. let mut too_large_onion = max_sized_onion.clone(); too_large_onion.custom_tlvs[0].1.push(42); let err = nodes[1] .node - .send_payment(payment_hash, too_large_onion.clone(), id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + too_large_onion.clone(), + PaymentId([2; 32]), + route_params.clone(), + no_retry, + ) .unwrap_err(); assert_eq!(err, RetryableSendFailure::OnionPacketSizeExceeded); @@ -450,7 +448,13 @@ fn blinded_path_with_custom_tlv() { // to route nodes[0] -> nodes[3] will fail. let err = nodes[0] .node - .send_payment(payment_hash, max_sized_onion.clone(), id, route_params.clone(), no_retry) + .send_payment( + payment_hash, + max_sized_onion.clone(), + PaymentId([3; 32]), + route_params.clone(), + no_retry, + ) .unwrap_err(); assert_eq!(err, RetryableSendFailure::RouteNotFound); @@ -462,26 +466,16 @@ fn blinded_path_with_custom_tlv() { .resize(max_custom_tlv_len - INTERMED_PAYLOAD_LEN_ESTIMATE, 0); nodes[0] .node - .send_payment(payment_hash, onion_allowing_2_hops.clone(), id, route_params, no_retry) + .send_payment( + payment_hash, + onion_allowing_2_hops, + PaymentId([4; 32]), + route_params, + no_retry, + ) .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 path = &[&nodes[1], &nodes[2], &nodes[3]]; - let args = - PassAlongPathArgs::new(&nodes[0], path, amt_msat, payment_hash, events.pop().unwrap()) - .with_payment_secret(payment_secret) - .with_custom_tlvs(onion_allowing_2_hops.custom_tlvs.clone()); - do_pass_along_path(args); - claim_payment_along_route( - ClaimAlongRouteArgs::new( - &nodes[0], - &[&[&nodes[1], &nodes[2], &nodes[3]]], - payment_preimage, - ) - .with_custom_tlvs(onion_allowing_2_hops.custom_tlvs), - ); + assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 1); } #[test] diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 6210d26893a..e92b3c7902b 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -3795,6 +3795,7 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPaylo let mut trampoline_onion_packet: Option = None; let mut invoice_request: Option = None; let mut custom_tlvs = Vec::new(); + let mut has_unknown_odd_tlvs = false; let tlv_len = BigSize::read(r)?; let mut rd = FixedLengthReader::new(r, tlv_len.0); @@ -3813,7 +3814,12 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPaylo // 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) } + if msg_type < 1 << 16 { + if msg_type % 2 == 1 { + has_unknown_odd_tlvs = true; + } + return Ok(false); + } let mut value = Vec::new(); msg_reader.read_to_limit(&mut value, u64::MAX)?; custom_tlvs.push((msg_type, value)); @@ -3826,6 +3832,7 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPaylo if intro_node_blinding_point.is_some() && update_add_blinding_point.is_some() { return Err(DecodeError::InvalidValue); } + let has_disallowed_blinded_tlvs = has_unknown_odd_tlvs || !custom_tlvs.is_empty(); if let Some(trampoline_onion_packet) = trampoline_onion_packet { if payment_metadata.is_some() || encrypted_tlvs_opt.is_some() || total_msat.is_some() { @@ -3867,6 +3874,9 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPaylo }), used_aad, } => { + if has_disallowed_blinded_tlvs { + return Err(DecodeError::UnknownRequiredFeature); + } if amt.is_some() || cltv_value.is_some() || total_msat.is_some() || keysend_preimage.is_some() @@ -3889,6 +3899,9 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPaylo BlindedPaymentTlvs::Dummy(DummyTlvs { payment_relay, payment_constraints }), used_aad, } => { + if has_disallowed_blinded_tlvs { + return Err(DecodeError::UnknownRequiredFeature); + } if amt.is_some() || cltv_value.is_some() || total_msat.is_some() || keysend_preimage.is_some() @@ -3907,6 +3920,9 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPaylo readable: BlindedPaymentTlvs::Receive(receive_tlvs), used_aad, } => { + if has_disallowed_blinded_tlvs { + return Err(DecodeError::UnknownRequiredFeature); + } if used_aad == TriPolyAADUsed::None { return Err(DecodeError::InvalidValue); } @@ -3983,6 +3999,7 @@ impl ReadableArgs<(Option, NS)> for InboundTrampoline let mut keysend_preimage: Option = None; let mut invoice_request: Option = None; let mut custom_tlvs = Vec::new(); + let mut has_unknown_odd_tlvs = false; let tlv_len = BigSize::read(r)?; let mut rd = FixedLengthReader::new(r, tlv_len.0); @@ -3999,7 +4016,12 @@ impl ReadableArgs<(Option, NS)> for InboundTrampoline // 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) } + if msg_type < 1 << 16 { + if msg_type % 2 == 1 { + has_unknown_odd_tlvs = true; + } + return Ok(false); + } let mut value = Vec::new(); msg_reader.read_to_limit(&mut value, u64::MAX)?; custom_tlvs.push((msg_type, value)); @@ -4012,6 +4034,7 @@ impl ReadableArgs<(Option, NS)> for InboundTrampoline if intro_node_blinding_point.is_some() && update_add_blinding_point.is_some() { return Err(DecodeError::InvalidValue); } + let has_disallowed_blinded_tlvs = has_unknown_odd_tlvs || !custom_tlvs.is_empty(); if let Some(blinding_point) = intro_node_blinding_point.or(update_add_blinding_point) { if next_trampoline.is_some() || payment_data.is_some() || payment_metadata.is_some() { @@ -4037,6 +4060,9 @@ impl ReadableArgs<(Option, NS)> for InboundTrampoline }), used_aad, } => { + if has_disallowed_blinded_tlvs { + return Err(DecodeError::UnknownRequiredFeature); + } if amt.is_some() || cltv_value.is_some() || total_msat.is_some() || keysend_preimage.is_some() @@ -4058,6 +4084,9 @@ impl ReadableArgs<(Option, NS)> for InboundTrampoline readable: BlindedTrampolineTlvs::Receive(receive_tlvs), used_aad, } => { + if has_disallowed_blinded_tlvs { + return Err(DecodeError::UnknownRequiredFeature); + } if used_aad == TriPolyAADUsed::None { return Err(DecodeError::InvalidValue); } @@ -4566,6 +4595,10 @@ impl_writeable_msg!(GossipTimestampFilter, { #[cfg(test)] mod tests { + use crate::blinded_path::payment::{ + BlindedPayInfo, BlindedPaymentPath, Bolt12RefundContext, PaymentConstraints, + PaymentContext, ReceiveTlvs, + }; use crate::ln::msgs::SocketAddress; use crate::ln::msgs::{ self, CommonAcceptChannelFields, CommonOpenChannelFields, FinalOnionHopData, @@ -4575,6 +4608,7 @@ mod tests { use crate::ln::onion_utils::AttributionData; use crate::ln::types::ChannelId; use crate::routing::gossip::{NodeAlias, NodeId}; + use crate::sign::{NodeSigner, Recipient}; use crate::types::features::{ ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures, }; @@ -4599,12 +4633,10 @@ mod tests { use crate::chain::transaction::OutPoint; use crate::io::{self, Cursor}; - use crate::prelude::*; - use core::str::FromStr; - - use crate::blinded_path::payment::{BlindedPayInfo, BlindedPaymentPath}; #[cfg(feature = "std")] use crate::ln::msgs::SocketAddressParseError; + use crate::prelude::*; + use core::str::FromStr; #[cfg(feature = "std")] use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6, ToSocketAddrs}; use types::features::{BlindedHopFeatures, Bolt12InvoiceFeatures}; @@ -6535,6 +6567,62 @@ mod tests { } } + #[test] + fn decoding_blinded_final_onion_hop_data_rejects_unknown_odd_tlvs() { + let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); + let payee_tlvs = ReceiveTlvs { + payment_secret: PaymentSecret([43; 32]), + payment_constraints: PaymentConstraints { + max_cltv_expiry: u32::MAX, + htlc_minimum_msat: 1, + }, + payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}), + }; + let receive_auth_key = node_signer.get_receive_auth_key(); + let secp_ctx = Secp256k1::new(); + let blinded_path = BlindedPaymentPath::new( + &[], + node_signer.get_node_id(Recipient::Node).unwrap(), + receive_auth_key, + payee_tlvs, + u64::MAX, + 18, + &node_signer, + &secp_ctx, + ) + .unwrap(); + let custom_tlvs = Vec::new(); + + let payload = msgs::OutboundOnionPayload::BlindedReceive { + sender_intended_htlc_amt_msat: 1000, + total_msat: 1000, + cltv_expiry_height: 144, + encrypted_tlvs: &blinded_path.blinded_hops()[0].encrypted_payload, + intro_node_blinding_point: Some(blinded_path.blinding_point()), + keysend_preimage: None, + custom_tlvs: &custom_tlvs, + invoice_request: None, + }; + let encoded = payload.encode(); + let mut cursor = Cursor::new(&encoded); + let _: BigSize = Readable::read(&mut cursor).unwrap(); + let mut tlv_stream = encoded[cursor.position() as usize..].to_vec(); + BigSize(21).write(&mut tlv_stream).unwrap(); + BigSize(0).write(&mut tlv_stream).unwrap(); + + let mut invalid_payload = Vec::new(); + BigSize(tlv_stream.len() as u64).write(&mut invalid_payload).unwrap(); + invalid_payload.extend_from_slice(&tlv_stream); + + assert!(matches!( + msgs::InboundOnionPayload::read( + &mut Cursor::new(&invalid_payload), + (None, &node_signer) + ), + Err(msgs::DecodeError::UnknownRequiredFeature), + )); + } + #[test] fn encoding_final_onion_hop_data_with_trampoline_packet() { let secp_ctx = Secp256k1::new(); diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 946be54de65..7c058cac584 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -1015,6 +1015,23 @@ macro_rules! _init_and_read_tlv_stream { } } +/// Equivalent to running [`_init_tlv_field_var`] then +/// [`decode_tlv_stream_with_custom_tlv_decode`]. +/// +/// If any unused values are read, their type MUST be specified or else `rustc` will read them as an +/// `i64`. +macro_rules! _init_and_read_tlv_stream_with_custom_tlv_decode { + ($reader: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*} + $(, $decode_custom_tlv: expr)?) => { + $( + $crate::_init_tlv_field_var!($field, $fieldty); + )* + decode_tlv_stream_with_custom_tlv_decode!($reader, { + $(($type, $field, $fieldty)),* + } $(, $decode_custom_tlv)?); + } +} + /// Reads a TLV stream with the given fields to build a struct/enum variant of type `$thing` #[doc(hidden)] #[macro_export]