From d3981e11a4bd4839e44a302087eb91a7f36e9683 Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Wed, 12 Feb 2025 13:37:26 -0800 Subject: [PATCH 1/2] Create structs for InboundOnionPayload variants In an upcoming commit, we will eliminate various invalid state combinations between `Hop` and `InboundOnionPayload` enums. To do so, rather than nesting one within the other, we will instead have them both referring to the same structs, with variant-dependent supplemental data. This requires pulling each variant's data into its own type. --- lightning/src/ln/blinded_payment_tests.rs | 12 +-- lightning/src/ln/channelmanager.rs | 21 +++-- lightning/src/ln/msgs.rs | 110 ++++++++++++---------- lightning/src/ln/onion_payment.rs | 22 ++--- lightning/src/ln/onion_utils.rs | 7 +- 5 files changed, 91 insertions(+), 81 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 32ae334e51f..ba83a4e9a53 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -1561,9 +1561,9 @@ fn route_blinding_spec_test_vector() { _ => panic!("Unexpected error") }; let (carol_packet_bytes, carol_hmac) = if let onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward { + next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override - }, next_hop_hmac, new_packet_bytes + }), next_hop_hmac, new_packet_bytes } = bob_peeled_onion { assert_eq!(short_channel_id, 1729); assert!(next_blinding_override.is_none()); @@ -1595,9 +1595,9 @@ fn route_blinding_spec_test_vector() { _ => panic!("Unexpected error") }; let (dave_packet_bytes, dave_hmac) = if let onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward { + next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override - }, next_hop_hmac, new_packet_bytes + }), 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"))); @@ -1629,9 +1629,9 @@ fn route_blinding_spec_test_vector() { _ => panic!("Unexpected error") }; let (eve_packet_bytes, eve_hmac) = if let onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward { + next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override - }, next_hop_hmac, new_packet_bytes + }), next_hop_hmac, new_packet_bytes } = dave_peeled_onion { assert_eq!(short_channel_id, 561); assert!(next_blinding_override.is_none()); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 57ed9898e5d..89b596e3595 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -15570,16 +15570,17 @@ mod tests { let node = create_network(1, &node_cfg, &node_chanmgr); let sender_intended_amt_msat = 100; let extra_fee_msat = 10; - let hop_data = msgs::InboundOnionPayload::Receive { + let hop_data = msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload { sender_intended_htlc_amt_msat: 100, cltv_expiry_height: 42, payment_metadata: None, keysend_preimage: None, payment_data: Some(msgs::FinalOnionHopData { - payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat, + 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. let current_height: u32 = node[0].node.best_block.read().unwrap().height; @@ -15592,16 +15593,17 @@ mod tests { } else { panic!(); } // If amt_received + extra_fee is equal to the sender intended amount, we're fine. - let hop_data = msgs::InboundOnionPayload::Receive { // This is the same payload as above, InboundOnionPayload doesn't implement Clone + let hop_data = msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload { // This is the same payload as above, InboundOnionPayload doesn't implement Clone sender_intended_htlc_amt_msat: 100, cltv_expiry_height: 42, payment_metadata: None, keysend_preimage: None, payment_data: Some(msgs::FinalOnionHopData { - payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat, + payment_secret: PaymentSecret([0; 32]), + total_msat: sender_intended_amt_msat, }), custom_tlvs: Vec::new(), - }; + }); let current_height: u32 = node[0].node.best_block.read().unwrap().height; assert!(create_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), @@ -15616,16 +15618,17 @@ mod tests { let node = create_network(1, &node_cfg, &node_chanmgr); let current_height: u32 = node[0].node.best_block.read().unwrap().height; - let result = create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive { + let result = create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload { sender_intended_htlc_amt_msat: 100, cltv_expiry_height: 22, payment_metadata: None, keysend_preimage: None, payment_data: Some(msgs::FinalOnionHopData { - payment_secret: PaymentSecret([0; 32]), total_msat: 100, + payment_secret: PaymentSecret([0; 32]), + total_msat: 100, }), custom_tlvs: Vec::new(), - }, [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height); + }), [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height); // Should not return an error as this condition: // https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334 diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 8204d90076a..3601445f4a5 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1768,41 +1768,47 @@ mod fuzzy_internal_msgs { // These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize // them from untrusted input): + pub struct InboundOnionForwardPayload { + pub short_channel_id: u64, + /// The value, in msat, of the payment after this hop's fee is deducted. + pub amt_to_forward: u64, + pub outgoing_cltv_value: u32, + } + + pub struct InboundOnionReceivePayload { + pub payment_data: Option, + pub payment_metadata: Option>, + pub keysend_preimage: Option, + pub custom_tlvs: Vec<(u64, Vec)>, + pub sender_intended_htlc_amt_msat: u64, + pub cltv_expiry_height: u32, + } + pub struct InboundOnionBlindedForwardPayload { + pub short_channel_id: u64, + pub payment_relay: PaymentRelay, + pub payment_constraints: PaymentConstraints, + pub features: BlindedHopFeatures, + pub intro_node_blinding_point: Option, + pub next_blinding_override: Option, + } + pub struct InboundOnionBlindedReceivePayload { + pub sender_intended_htlc_amt_msat: u64, + pub total_msat: u64, + pub cltv_expiry_height: u32, + pub payment_secret: PaymentSecret, + pub payment_constraints: PaymentConstraints, + pub payment_context: PaymentContext, + pub intro_node_blinding_point: Option, + pub keysend_preimage: Option, + pub invoice_request: Option, + pub custom_tlvs: Vec<(u64, Vec)>, + } + pub enum InboundOnionPayload { - Forward { - short_channel_id: u64, - /// The value, in msat, of the payment after this hop's fee is deducted. - amt_to_forward: u64, - outgoing_cltv_value: u32, - }, - Receive { - payment_data: Option, - payment_metadata: Option>, - keysend_preimage: Option, - custom_tlvs: Vec<(u64, Vec)>, - sender_intended_htlc_amt_msat: u64, - cltv_expiry_height: u32, - }, - BlindedForward { - short_channel_id: u64, - payment_relay: PaymentRelay, - payment_constraints: PaymentConstraints, - features: BlindedHopFeatures, - intro_node_blinding_point: Option, - next_blinding_override: Option, - }, - BlindedReceive { - sender_intended_htlc_amt_msat: u64, - total_msat: u64, - cltv_expiry_height: u32, - payment_secret: PaymentSecret, - payment_constraints: PaymentConstraints, - payment_context: PaymentContext, - intro_node_blinding_point: Option, - keysend_preimage: Option, - invoice_request: Option, - custom_tlvs: Vec<(u64, Vec)>, - } + Forward(InboundOnionForwardPayload), + Receive(InboundOnionReceivePayload), + BlindedForward(InboundOnionBlindedForwardPayload), + BlindedReceive(InboundOnionBlindedReceivePayload), } pub(crate) enum OutboundOnionPayload<'a> { @@ -2907,14 +2913,14 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPayload wh { return Err(DecodeError::InvalidValue) } - Ok(Self::BlindedForward { + Ok(Self::BlindedForward(InboundOnionBlindedForwardPayload { short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override, - }) + })) }, ChaChaPolyReadAdapter { readable: BlindedPaymentTlvs::Receive(receive_tlvs) } => { let ReceiveTlvs { tlvs, authentication: (hmac, nonce) } = receive_tlvs; @@ -2927,7 +2933,7 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPayload wh payment_secret, payment_constraints, payment_context, } = tlvs; if total_msat.unwrap_or(0) > MAX_VALUE_MSAT { return Err(DecodeError::InvalidValue) } - Ok(Self::BlindedReceive { + Ok(Self::BlindedReceive(InboundOnionBlindedReceivePayload { sender_intended_htlc_amt_msat: amt.ok_or(DecodeError::InvalidValue)?, total_msat: total_msat.ok_or(DecodeError::InvalidValue)?, cltv_expiry_height: cltv_value.ok_or(DecodeError::InvalidValue)?, @@ -2938,18 +2944,18 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPayload wh keysend_preimage, invoice_request, custom_tlvs, - }) + })) }, } } else if let Some(short_channel_id) = short_id { if payment_data.is_some() || payment_metadata.is_some() || encrypted_tlvs_opt.is_some() || total_msat.is_some() || invoice_request.is_some() { return Err(DecodeError::InvalidValue) } - Ok(Self::Forward { + Ok(Self::Forward(InboundOnionForwardPayload { short_channel_id, amt_to_forward: amt.ok_or(DecodeError::InvalidValue)?, outgoing_cltv_value: cltv_value.ok_or(DecodeError::InvalidValue)?, - }) + })) } else { if encrypted_tlvs_opt.is_some() || total_msat.is_some() || invoice_request.is_some() { return Err(DecodeError::InvalidValue) @@ -2959,14 +2965,14 @@ impl ReadableArgs<(Option, NS)> for InboundOnionPayload wh return Err(DecodeError::InvalidValue); } } - Ok(Self::Receive { + Ok(Self::Receive(InboundOnionReceivePayload { payment_data, payment_metadata: payment_metadata.map(|w| w.0), keysend_preimage, sender_intended_htlc_amt_msat: amt.ok_or(DecodeError::InvalidValue)?, cltv_expiry_height: cltv_value.ok_or(DecodeError::InvalidValue)?, custom_tlvs, - }) + })) } } } @@ -3387,7 +3393,7 @@ mod tests { use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret}; use crate::types::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures}; - use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket}; + use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload}; use crate::ln::msgs::SocketAddress; use crate::routing::gossip::{NodeAlias, NodeId}; use crate::util::ser::{BigSize, FixedLengthReader, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable}; @@ -4559,9 +4565,9 @@ mod tests { let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &node_signer)).unwrap(); - if let msgs::InboundOnionPayload::Forward { + if let msgs::InboundOnionPayload::Forward(InboundOnionForwardPayload { short_channel_id, amt_to_forward, outgoing_cltv_value - } = inbound_msg { + }) = inbound_msg { assert_eq!(short_channel_id, 0xdeadbeef1bad1dea); assert_eq!(amt_to_forward, 0x0badf00d01020304); assert_eq!(outgoing_cltv_value, 0xffffffff); @@ -4584,9 +4590,9 @@ mod tests { let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &node_signer)).unwrap(); - if let msgs::InboundOnionPayload::Receive { + if let msgs::InboundOnionPayload::Receive(InboundOnionReceivePayload { payment_data: None, sender_intended_htlc_amt_msat, cltv_expiry_height, .. - } = inbound_msg { + }) = inbound_msg { assert_eq!(sender_intended_htlc_amt_msat, 0x0badf00d01020304); assert_eq!(cltv_expiry_height, 0xffffffff); } else { panic!(); } @@ -4612,7 +4618,7 @@ mod tests { let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &node_signer)).unwrap(); - if let msgs::InboundOnionPayload::Receive { + if let msgs::InboundOnionPayload::Receive(InboundOnionReceivePayload { payment_data: Some(FinalOnionHopData { payment_secret, total_msat: 0x1badca1f @@ -4621,7 +4627,7 @@ mod tests { payment_metadata: None, keysend_preimage: None, custom_tlvs, - } = inbound_msg { + }) = inbound_msg { assert_eq!(payment_secret, expected_payment_secret); assert_eq!(sender_intended_htlc_amt_msat, 0x0badf00d01020304); assert_eq!(cltv_expiry_height, 0xffffffff); @@ -4658,7 +4664,7 @@ mod tests { let encoded_value = msg.encode(); let inbound_msg = ReadableArgs::read(&mut Cursor::new(&encoded_value[..]), (None, &node_signer)).unwrap(); match inbound_msg { - msgs::InboundOnionPayload::Receive { custom_tlvs, .. } => assert!(custom_tlvs.is_empty()), + msgs::InboundOnionPayload::Receive(InboundOnionReceivePayload { custom_tlvs, .. }) => assert!(custom_tlvs.is_empty()), _ => panic!(), } } @@ -4682,7 +4688,7 @@ mod tests { assert_eq!(encoded_value, target_value); let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet); let inbound_msg: msgs::InboundOnionPayload = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &node_signer)).unwrap(); - if let msgs::InboundOnionPayload::Receive { + if let msgs::InboundOnionPayload::Receive(InboundOnionReceivePayload { payment_data: None, payment_metadata: None, keysend_preimage: None, @@ -4690,7 +4696,7 @@ mod tests { sender_intended_htlc_amt_msat, cltv_expiry_height: outgoing_cltv_value, .. - } = inbound_msg { + }) = inbound_msg { assert_eq!(custom_tlvs, expected_custom_tlvs); assert_eq!(sender_intended_htlc_amt_msat, 0x0badf00d01020304); assert_eq!(outgoing_cltv_value, 0xffffffff); diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 8b560f72624..dbbfd637906 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -78,12 +78,12 @@ pub(super) fn create_fwd_pending_htlc_info( short_channel_id, amt_to_forward, outgoing_cltv_value, intro_node_blinding_point, next_blinding_override ) = match hop_data { - msgs::InboundOnionPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } => + msgs::InboundOnionPayload::Forward(msgs::InboundOnionForwardPayload { short_channel_id, amt_to_forward, outgoing_cltv_value }) => (short_channel_id, amt_to_forward, outgoing_cltv_value, None, None), - msgs::InboundOnionPayload::BlindedForward { + msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { short_channel_id, payment_relay, payment_constraints, intro_node_blinding_point, features, next_blinding_override, - } => { + }) => { let (amt_to_forward, outgoing_cltv_value) = check_blinded_forward( msg.amount_msat, msg.cltv_expiry, &payment_relay, &payment_constraints, &features ).map_err(|()| { @@ -139,17 +139,17 @@ pub(super) fn create_recv_pending_htlc_info( payment_metadata, payment_context, requires_blinded_error, has_recipient_created_payment_secret, invoice_request ) = match hop_data { - msgs::InboundOnionPayload::Receive { + msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload { payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat, cltv_expiry_height, payment_metadata, .. - } => + }) => (payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat, cltv_expiry_height, payment_metadata, None, false, keysend_preimage.is_none(), None), - msgs::InboundOnionPayload::BlindedReceive { + msgs::InboundOnionPayload::BlindedReceive(msgs::InboundOnionBlindedReceivePayload { sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, payment_secret, intro_node_blinding_point, payment_constraints, payment_context, keysend_preimage, custom_tlvs, invoice_request - } => { + }) => { check_blinded_payment_constraints( sender_intended_htlc_amt_msat, cltv_expiry, &payment_constraints ) @@ -425,9 +425,9 @@ where let next_packet_details = match next_hop { onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::Forward { + next_hop_data: msgs::InboundOnionPayload::Forward(msgs::InboundOnionForwardPayload { short_channel_id, amt_to_forward, outgoing_cltv_value - }, .. + }), .. } => { let next_packet_pubkey = onion_utils::next_hop_pubkey(secp_ctx, msg.onion_routing_packet.public_key.unwrap(), &shared_secret); @@ -437,9 +437,9 @@ where } }, onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward { + next_hop_data: msgs::InboundOnionPayload::BlindedForward (msgs::InboundOnionBlindedForwardPayload{ short_channel_id, ref payment_relay, ref payment_constraints, ref features, .. - }, .. + }), .. } => { let (amt_to_forward, outgoing_cltv_value) = match check_blinded_forward( msg.amount_msat, msg.cltv_expiry, &payment_relay, &payment_constraints, &features diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 6e0a162406c..4d3483f8e10 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -1434,9 +1434,10 @@ impl Hop { match self { Self::Forward { next_hop_data: - msgs::InboundOnionPayload::BlindedForward { - intro_node_blinding_point: Some(_), .. - }, + msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { + intro_node_blinding_point: Some(_), + .. + }), .. } => true, _ => false, From 38284a0d5de9f244fc97d42ed2753648bf77d075 Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Wed, 12 Feb 2025 15:28:36 -0800 Subject: [PATCH 2/2] Remove invalid state options from Hop Now that each `InboundOnionPayload` variant corresponds to its own struct, we can reference these same types inside `Hop` and thereby avoid nesting that allowed invalid combinations, and instead store supplemental data as each variant calls for. --- lightning/src/ln/blinded_payment_tests.rs | 32 +++++++----- lightning/src/ln/channelmanager.rs | 60 ++++++++++++++++------- lightning/src/ln/onion_payment.rs | 58 +++++++++++----------- lightning/src/ln/onion_utils.rs | 56 ++++++++++++++++----- 4 files changed, 133 insertions(+), 73 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index ba83a4e9a53..0662d5f9395 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -411,15 +411,23 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) { do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false); let failed_destination = match check { ForwardCheckFail::InboundOnionCheck => HTLCDestination::InvalidOnion, - ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::FailedPayment { payment_hash }, + ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::InvalidOnion, ForwardCheckFail::OutboundChannelCheck => HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1_2.2 }, }; expect_htlc_handling_failed_destinations!( nodes[1].node.get_and_clear_pending_events(), &[failed_destination.clone()] ); - expect_payment_failed_conditions(&nodes[0], payment_hash, false, - PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); + match check { + ForwardCheckFail::ForwardPayloadEncodedAsReceive => { + expect_payment_failed_conditions(&nodes[0], payment_hash, false, + PaymentFailedConditions::new().expected_htlc_error_data(0x4000 | 22, &[0; 0])); + } + _ => { + expect_payment_failed_conditions(&nodes[0], payment_hash, false, + PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); + } + }; return } @@ -1560,10 +1568,10 @@ fn route_blinding_spec_test_vector() { Ok(res) => res, _ => panic!("Unexpected error") }; - let (carol_packet_bytes, carol_hmac) = if let onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { + 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 + }, next_hop_hmac, new_packet_bytes } = bob_peeled_onion { assert_eq!(short_channel_id, 1729); assert!(next_blinding_override.is_none()); @@ -1594,10 +1602,10 @@ fn route_blinding_spec_test_vector() { Ok(res) => res, _ => panic!("Unexpected error") }; - let (dave_packet_bytes, dave_hmac) = if let onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { + 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 + }, 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"))); @@ -1628,10 +1636,10 @@ fn route_blinding_spec_test_vector() { Ok(res) => res, _ => panic!("Unexpected error") }; - let (eve_packet_bytes, eve_hmac) = if let onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { + 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 + }, next_hop_hmac, new_packet_bytes } = dave_peeled_onion { assert_eq!(short_channel_id, 561); assert!(next_blinding_override.is_none()); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 89b596e3595..8be60fa9e1f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4430,7 +4430,24 @@ where onion_utils::Hop::Receive(next_hop_data) => { // OUR PAYMENT! let current_height: u32 = self.best_block.read().unwrap().height; - match create_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash, + match create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive(next_hop_data), shared_secret, msg.payment_hash, + msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat, + current_height) + { + Ok(info) => { + // Note that we could obviously respond immediately with an update_fulfill_htlc + // message, however that would leak that we are the recipient of this payment, so + // instead we stay symmetric with the forwarding case, only responding (after a + // delay) once they've send us a commitment_signed! + PendingHTLCStatus::Forward(info) + }, + Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data) + } + }, + onion_utils::Hop::BlindedReceive(next_hop_data) => { + // OUR PAYMENT! + let current_height: u32 = self.best_block.read().unwrap().height; + match create_recv_pending_htlc_info(msgs::InboundOnionPayload::BlindedReceive(next_hop_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat, current_height) { @@ -4445,7 +4462,14 @@ where } }, onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => { - match create_fwd_pending_htlc_info(msg, next_hop_data, next_hop_hmac, + match create_fwd_pending_htlc_info(msg, msgs::InboundOnionPayload::Forward(next_hop_data), next_hop_hmac, + new_packet_bytes, shared_secret, next_packet_pubkey_opt) { + Ok(info) => PendingHTLCStatus::Forward(info), + Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data) + } + }, + onion_utils::Hop::BlindedForward { next_hop_data, next_hop_hmac, new_packet_bytes } => { + match create_fwd_pending_htlc_info(msg, msgs::InboundOnionPayload::BlindedForward(next_hop_data), next_hop_hmac, new_packet_bytes, shared_secret, next_packet_pubkey_opt) { Ok(info) => PendingHTLCStatus::Forward(info), Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data) @@ -5860,22 +5884,22 @@ where failed_payment!(err_msg, err_code, Vec::new(), Some(phantom_shared_secret)); }, }; - match next_hop { - onion_utils::Hop::Receive(hop_data) => { - let current_height: u32 = self.best_block.read().unwrap().height; - match create_recv_pending_htlc_info(hop_data, - incoming_shared_secret, payment_hash, outgoing_amt_msat, - outgoing_cltv_value, Some(phantom_shared_secret), false, None, - current_height) - { - Ok(info) => phantom_receives.push(( - prev_short_channel_id, prev_counterparty_node_id, prev_funding_outpoint, - prev_channel_id, prev_user_channel_id, vec![(info, prev_htlc_id)] - )), - Err(InboundHTLCErr { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret)) - } - }, - _ => panic!(), + let inbound_onion_payload = match next_hop { + onion_utils::Hop::Receive(hop_data) => msgs::InboundOnionPayload::Receive(hop_data), + onion_utils::Hop::BlindedReceive(hop_data) => msgs::InboundOnionPayload::BlindedReceive(hop_data), + _ => panic!() + }; + let current_height: u32 = self.best_block.read().unwrap().height; + match create_recv_pending_htlc_info(inbound_onion_payload, + incoming_shared_secret, payment_hash, outgoing_amt_msat, + outgoing_cltv_value, Some(phantom_shared_secret), false, None, + current_height) + { + Ok(info) => phantom_receives.push(( + prev_short_channel_id, prev_counterparty_node_id, prev_funding_outpoint, + prev_channel_id, prev_user_channel_id, vec![(info, prev_htlc_id)] + )), + Err(InboundHTLCErr { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret)) } } else { fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None); diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index dbbfd637906..e3505e536de 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -16,7 +16,7 @@ use crate::ln::channelmanager::{BlindedFailure, BlindedForward, CLTV_FAR_FAR_AWA use crate::types::features::BlindedHopFeatures; use crate::ln::msgs; use crate::ln::onion_utils; -use crate::ln::onion_utils::{HTLCFailReason, INVALID_ONION_BLINDING}; +use crate::ln::onion_utils::{Hop, HTLCFailReason, INVALID_ONION_BLINDING}; use crate::sign::{NodeSigner, Recipient}; use crate::util::logger::Logger; @@ -296,7 +296,13 @@ where InboundHTLCErr { msg, err_code, err_data } })?; Ok(match hop { - onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => { + onion_utils::Hop::Forward { next_hop_hmac, new_packet_bytes, .. } | onion_utils::Hop::BlindedForward { next_hop_hmac, new_packet_bytes, .. } => { + let inbound_onion_payload = match hop { + onion_utils::Hop::Forward { next_hop_data, .. } => msgs::InboundOnionPayload::Forward(next_hop_data), + onion_utils::Hop::BlindedForward { next_hop_data, .. } => msgs::InboundOnionPayload::BlindedForward(next_hop_data), + _ => unreachable!() + }; + let NextPacketDetails { next_packet_pubkey, outgoing_amt_msat: _, outgoing_scid: _, outgoing_cltv_value } = match next_packet_details_opt { @@ -310,7 +316,7 @@ where }; if let Err((err_msg, code)) = check_incoming_htlc_cltv( - cur_height, outgoing_cltv_value, msg.cltv_expiry + cur_height, outgoing_cltv_value, msg.cltv_expiry, ) { return Err(InboundHTLCErr { msg: err_msg, @@ -321,16 +327,21 @@ where // TODO: If this is potentially a phantom payment we should decode the phantom payment // onion here and check it. - create_fwd_pending_htlc_info( - msg, next_hop_data, next_hop_hmac, new_packet_bytes, shared_secret, - Some(next_packet_pubkey) + msg, inbound_onion_payload, next_hop_hmac, new_packet_bytes, shared_secret, + Some(next_packet_pubkey), )? }, onion_utils::Hop::Receive(received_data) => { create_recv_pending_htlc_info( - received_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, - None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height + msgs::InboundOnionPayload::Receive(received_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, + None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height, + )? + }, + onion_utils::Hop::BlindedReceive(received_data) => { + create_recv_pending_htlc_info( + msgs::InboundOnionPayload::BlindedReceive(received_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry, + None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height, )? } }) @@ -424,23 +435,15 @@ where }; let next_packet_details = match next_hop { - onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::Forward(msgs::InboundOnionForwardPayload { - short_channel_id, amt_to_forward, outgoing_cltv_value - }), .. - } => { + Hop::Forward { next_hop_data: msgs::InboundOnionForwardPayload { short_channel_id, amt_to_forward, outgoing_cltv_value }, .. } => { let next_packet_pubkey = onion_utils::next_hop_pubkey(secp_ctx, msg.onion_routing_packet.public_key.unwrap(), &shared_secret); - NextPacketDetails { + Some(NextPacketDetails { next_packet_pubkey, outgoing_scid: short_channel_id, outgoing_amt_msat: amt_to_forward, outgoing_cltv_value - } - }, - onion_utils::Hop::Forward { - next_hop_data: msgs::InboundOnionPayload::BlindedForward (msgs::InboundOnionBlindedForwardPayload{ - short_channel_id, ref payment_relay, ref payment_constraints, ref features, .. - }), .. - } => { + }) + } + Hop::BlindedForward { next_hop_data: msgs::InboundOnionBlindedForwardPayload { short_channel_id, ref payment_relay, ref payment_constraints, ref features, .. }, .. } => { let (amt_to_forward, outgoing_cltv_value) = match check_blinded_forward( msg.amount_msat, msg.cltv_expiry, &payment_relay, &payment_constraints, &features ) { @@ -452,20 +455,15 @@ where }; let next_packet_pubkey = onion_utils::next_hop_pubkey(&secp_ctx, msg.onion_routing_packet.public_key.unwrap(), &shared_secret); - NextPacketDetails { + Some(NextPacketDetails { next_packet_pubkey, outgoing_scid: short_channel_id, outgoing_amt_msat: amt_to_forward, outgoing_cltv_value - } - }, - onion_utils::Hop::Receive { .. } => return Ok((next_hop, shared_secret, None)), - onion_utils::Hop::Forward { next_hop_data: msgs::InboundOnionPayload::Receive { .. }, .. } | - onion_utils::Hop::Forward { next_hop_data: msgs::InboundOnionPayload::BlindedReceive { .. }, .. } => - { - return_err!("Final Node OnionHopData provided for us as an intermediary node", 0x4000 | 22, &[0; 0]); + }) } + _ => None }; - Ok((next_hop, shared_secret, Some(next_packet_details))) + Ok((next_hop, shared_secret, next_packet_details)) } pub(super) fn check_incoming_htlc_cltv( diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 4d3483f8e10..35de2403441 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -1415,29 +1415,40 @@ impl NextPacketBytes for Vec { /// Data decrypted from a payment's onion payload. pub(crate) enum Hop { - /// This onion payload was for us, not for forwarding to a next-hop. Contains information for - /// verifying the incoming payment. - Receive(msgs::InboundOnionPayload), /// This onion payload needs to be forwarded to a next-hop. Forward { /// Onion payload data used in forwarding the payment. - next_hop_data: msgs::InboundOnionPayload, + next_hop_data: msgs::InboundOnionForwardPayload, /// HMAC of the next hop's onion packet. next_hop_hmac: [u8; 32], /// Bytes of the onion packet we're forwarding. new_packet_bytes: [u8; ONION_DATA_LEN], }, + /// This onion payload needs to be forwarded to a next-hop. + BlindedForward { + /// Onion payload data used in forwarding the payment. + next_hop_data: msgs::InboundOnionBlindedForwardPayload, + /// HMAC of the next hop's onion packet. + next_hop_hmac: [u8; 32], + /// Bytes of the onion packet we're forwarding. + new_packet_bytes: [u8; ONION_DATA_LEN], + }, + /// This onion payload was for us, not for forwarding to a next-hop. Contains information for + /// verifying the incoming payment. + Receive(msgs::InboundOnionReceivePayload), + /// This onion payload was for us, not for forwarding to a next-hop. Contains information for + /// verifying the incoming payment. + BlindedReceive(msgs::InboundOnionBlindedReceivePayload), } impl Hop { pub(crate) fn is_intro_node_blinded_forward(&self) -> bool { match self { - Self::Forward { + Self::BlindedForward { next_hop_data: - msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload { - intro_node_blinding_point: Some(_), - .. - }), + msgs::InboundOnionBlindedForwardPayload { + intro_node_blinding_point: Some(_), .. + }, .. } => true, _ => false, @@ -1461,16 +1472,35 @@ pub(crate) fn decode_next_payment_hop( where NS::Target: NodeSigner, { - match decode_next_hop( + let decoded_hop: Result<(msgs::InboundOnionPayload, Option<_>), _> = decode_next_hop( shared_secret, hop_data, hmac_bytes, Some(payment_hash), (blinding_point, node_signer), - ) { - Ok((next_hop_data, None)) => Ok(Hop::Receive(next_hop_data)), + ); + match decoded_hop { Ok((next_hop_data, Some((next_hop_hmac, FixedSizeOnionPacket(new_packet_bytes))))) => { - Ok(Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes }) + match next_hop_data { + msgs::InboundOnionPayload::Forward(next_hop_data) => { + Ok(Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes }) + }, + msgs::InboundOnionPayload::BlindedForward(next_hop_data) => { + Ok(Hop::BlindedForward { next_hop_data, next_hop_hmac, new_packet_bytes }) + }, + _ => Err(OnionDecodeErr::Relay { + err_msg: "Final Node OnionHopData provided for us as an intermediary node", + err_code: 0x4000 | 22, + }), + } + }, + Ok((next_hop_data, None)) => match next_hop_data { + msgs::InboundOnionPayload::Receive(payload) => Ok(Hop::Receive(payload)), + msgs::InboundOnionPayload::BlindedReceive(payload) => Ok(Hop::BlindedReceive(payload)), + _ => Err(OnionDecodeErr::Relay { + err_msg: "Intermediate Node OnionHopData provided for us as a final node", + err_code: 0x4000 | 22, + }), }, Err(e) => Err(e), }