Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move shared secret calculation into decode_next_payment_hop #3607

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Move shared secret calculation into decode_next_payment_hop #3607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1518e67
Clean up `Hop` match arms
arik-so Feb 19, 2025
5291445
Calculate shared secret within hop decode function
arik-so Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions lightning/src/ln/blinded_payment_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1561,7 +1561,7 @@ fn route_blinding_spec_test_vector() {
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) =
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
) {
Expand All @@ -1571,7 +1571,7 @@ fn route_blinding_spec_test_vector() {
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());
Expand All @@ -1595,7 +1595,7 @@ fn route_blinding_spec_test_vector() {
carol_onion
);
let carol_node_signer = TestEcdhSigner { node_secret: carol_secret };
let (carol_peeled_onion, _, next_packet_details_opt) =
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
) {
Expand All @@ -1605,7 +1605,7 @@ fn route_blinding_spec_test_vector() {
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")));
Expand All @@ -1629,7 +1629,7 @@ fn route_blinding_spec_test_vector() {
dave_onion
);
let dave_node_signer = TestEcdhSigner { node_secret: dave_secret };
let (dave_peeled_onion, _, next_packet_details_opt) =
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
) {
Expand All @@ -1639,7 +1639,7 @@ fn route_blinding_spec_test_vector() {
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());
Expand Down
49 changes: 20 additions & 29 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4427,27 +4427,16 @@ where
}
}
match decoded_hop {
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(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) => {
onion_utils::Hop::Receive { .. } | onion_utils::Hop::BlindedReceive { .. } => {
let inbound_onion_payload = match decoded_hop {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kinda thing could probably go in a separate commit since its just a cleanup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, done

onion_utils::Hop::Receive { hop_data, .. } => msgs::InboundOnionPayload::Receive(hop_data),
onion_utils::Hop::BlindedReceive { hop_data, .. } => msgs::InboundOnionPayload::BlindedReceive(hop_data),
_ => unreachable!()
};

// 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,
match create_recv_pending_htlc_info(inbound_onion_payload, shared_secret, msg.payment_hash,
msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat,
current_height)
{
Expand All @@ -4461,14 +4450,14 @@ where
Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
}
},
onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes, .. } => {
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 } => {
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),
Expand Down Expand Up @@ -5696,7 +5685,7 @@ where
let mut htlc_forwards = Vec::new();
let mut htlc_fails = Vec::new();
for update_add_htlc in &update_add_htlcs {
let (next_hop, shared_secret, next_packet_details_opt) = match decode_incoming_update_add_htlc_onion(
let (next_hop, next_packet_details_opt) = match decode_incoming_update_add_htlc_onion(
&update_add_htlc, &*self.node_signer, &*self.logger, &self.secp_ctx
) {
Ok(decoded_onion) => decoded_onion,
Expand All @@ -5708,6 +5697,7 @@ where

let is_intro_node_blinded_forward = next_hop.is_intro_node_blinded_forward();
let outgoing_scid_opt = next_packet_details_opt.as_ref().map(|d| d.outgoing_scid);
let shared_secret = next_hop.shared_secret().secret_bytes();

// Process the HTLC on the incoming channel.
match self.do_funded_channel_callback(incoming_scid, |chan: &mut FundedChannel<SP>| {
Expand Down Expand Up @@ -5866,10 +5856,9 @@ where
if let PendingHTLCRouting::Forward { ref onion_packet, .. } = routing {
let phantom_pubkey_res = self.node_signer.get_node_id(Recipient::PhantomNode);
if phantom_pubkey_res.is_ok() && fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, short_chan_id, &self.chain_hash) {
let phantom_shared_secret = self.node_signer.ecdh(Recipient::PhantomNode, &onion_packet.public_key.unwrap(), None).unwrap().secret_bytes();
let next_hop = match onion_utils::decode_next_payment_hop(
phantom_shared_secret, &onion_packet.hop_data, onion_packet.hmac,
payment_hash, None, &*self.node_signer
Recipient::PhantomNode, &onion_packet.public_key.unwrap(), &onion_packet.hop_data,
onion_packet.hmac, payment_hash, None, &*self.node_signer
) {
Ok(res) => res,
Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => {
Expand All @@ -5880,15 +5869,17 @@ where
// of the onion.
failed_payment!(err_msg, err_code, sha256_of_onion.to_vec(), None);
},
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => {
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code, shared_secret }) => {
let phantom_shared_secret = shared_secret.secret_bytes();
failed_payment!(err_msg, err_code, Vec::new(), Some(phantom_shared_secret));
},
};
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),
let (inbound_onion_payload, shared_secret) = match next_hop {
onion_utils::Hop::Receive { hop_data, shared_secret } => (msgs::InboundOnionPayload::Receive(hop_data), shared_secret),
onion_utils::Hop::BlindedReceive { hop_data, shared_secret } => (msgs::InboundOnionPayload::BlindedReceive(hop_data), shared_secret),
_ => panic!()
};
let phantom_shared_secret = shared_secret.secret_bytes();
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,
Expand Down
79 changes: 33 additions & 46 deletions lightning/src/ln/onion_payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
//! Primarily features [`peel_payment_onion`], which allows the decoding of an onion statelessly
//! and can be used to predict whether we'd accept a payment.

use bitcoin::hashes::{Hash, HashEngine};
use bitcoin::hashes::hmac::{Hmac, HmacEngine};
use bitcoin::hashes::Hash;
use bitcoin::hashes::sha256::Hash as Sha256;
use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1};
use bitcoin::secp256k1::{self, PublicKey, Secp256k1};

use crate::blinded_path;
use crate::blinded_path::payment::{PaymentConstraints, PaymentRelay};
Expand Down Expand Up @@ -285,7 +284,7 @@ where
NS::Target: NodeSigner,
L::Target: Logger,
{
let (hop, shared_secret, next_packet_details_opt) =
let (hop, next_packet_details_opt) =
decode_incoming_update_add_htlc_onion(msg, node_signer, logger, secp_ctx
).map_err(|e| {
let (err_code, err_data) = match e {
Expand All @@ -296,7 +295,8 @@ where
InboundHTLCErr { msg, err_code, err_data }
})?;
Ok(match hop {
onion_utils::Hop::Forward { next_hop_hmac, new_packet_bytes, .. } | onion_utils::Hop::BlindedForward { next_hop_hmac, new_packet_bytes, .. } => {
onion_utils::Hop::Forward { shared_secret, next_hop_hmac, new_packet_bytes, .. } |
onion_utils::Hop::BlindedForward { shared_secret, 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),
Expand Down Expand Up @@ -328,19 +328,19 @@ 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, inbound_onion_payload, next_hop_hmac, new_packet_bytes, shared_secret,
msg, inbound_onion_payload, next_hop_hmac, new_packet_bytes, shared_secret.secret_bytes(),
Some(next_packet_pubkey),
)?
},
onion_utils::Hop::Receive(received_data) => {
onion_utils::Hop::Receive { hop_data, shared_secret } => {
create_recv_pending_htlc_info(
msgs::InboundOnionPayload::Receive(received_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
msgs::InboundOnionPayload::Receive(hop_data), shared_secret.secret_bytes(), 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) => {
onion_utils::Hop::BlindedReceive { hop_data, shared_secret } => {
create_recv_pending_htlc_info(
msgs::InboundOnionPayload::BlindedReceive(received_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
msgs::InboundOnionPayload::BlindedReceive(hop_data), shared_secret.secret_bytes(), msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height,
)?
}
Expand All @@ -356,7 +356,7 @@ pub(super) struct NextPacketDetails {

pub(super) fn decode_incoming_update_add_htlc_onion<NS: Deref, L: Deref, T: secp256k1::Verification>(
msg: &msgs::UpdateAddHTLC, node_signer: NS, logger: L, secp_ctx: &Secp256k1<T>,
) -> Result<(onion_utils::Hop, [u8; 32], Option<NextPacketDetails>), HTLCFailureMsg>
) -> Result<(onion_utils::Hop, Option<NextPacketDetails>), HTLCFailureMsg>
where
NS::Target: NodeSigner,
L::Target: Logger,
Expand Down Expand Up @@ -384,16 +384,6 @@ where
return_malformed_err!("invalid ephemeral pubkey", 0x8000 | 0x4000 | 6);
}

let blinded_node_id_tweak = msg.blinding_point.map(|bp| {
let blinded_tlvs_ss = node_signer.ecdh(Recipient::Node, &bp, None).unwrap().secret_bytes();
let mut hmac = HmacEngine::<Sha256>::new(b"blinded_node_id");
hmac.input(blinded_tlvs_ss.as_ref());
Scalar::from_be_bytes(Hmac::from_engine(hmac).to_byte_array()).unwrap()
});
let shared_secret = node_signer.ecdh(
Recipient::Node, &msg.onion_routing_packet.public_key.unwrap(), blinded_node_id_tweak.as_ref()
).unwrap().secret_bytes();

if msg.onion_routing_packet.version != 0 {
//TODO: Spec doesn't indicate if we should only hash hop_data here (and in other
//sha256_of_onion error data packets), or the entire onion_routing_packet. Either way,
Expand All @@ -403,58 +393,55 @@ where
//node knows the HMAC matched, so they already know what is there...
return_malformed_err!("Unknown onion packet version", 0x8000 | 0x4000 | 4);
}
macro_rules! return_err {
($msg: expr, $err_code: expr, $data: expr) => {
{
if msg.blinding_point.is_some() {
return_malformed_err!($msg, INVALID_ONION_BLINDING)
}

log_info!(logger, "Failed to accept/forward incoming HTLC: {}", $msg);
return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: HTLCFailReason::reason($err_code, $data.to_vec())
.get_encrypted_failure_packet(&shared_secret, &None),
}));
}
let encode_relay_error = |message: &str, err_code: u16, shared_secret: [u8; 32], data: &[u8]| {
if msg.blinding_point.is_some() {
return_malformed_err!(message, INVALID_ONION_BLINDING)
}
}

log_info!(logger, "Failed to accept/forward incoming HTLC: {}", message);
return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: HTLCFailReason::reason(err_code, data.to_vec())
.get_encrypted_failure_packet(&shared_secret, &None),
}));
};

let next_hop = match onion_utils::decode_next_payment_hop(
shared_secret, &msg.onion_routing_packet.hop_data[..], msg.onion_routing_packet.hmac,
Recipient::Node, &msg.onion_routing_packet.public_key.unwrap(), &msg.onion_routing_packet.hop_data[..], msg.onion_routing_packet.hmac,
msg.payment_hash, msg.blinding_point, node_signer
) {
Ok(res) => res,
Err(onion_utils::OnionDecodeErr::Malformed { err_msg, err_code }) => {
return_malformed_err!(err_msg, err_code);
},
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code }) => {
return_err!(err_msg, err_code, &[0; 0]);
Err(onion_utils::OnionDecodeErr::Relay { err_msg, err_code, shared_secret }) => {
return encode_relay_error(err_msg, err_code, shared_secret.secret_bytes(), &[0; 0]);
},
};

let next_packet_details = match next_hop {
Hop::Forward { next_hop_data: 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 }, shared_secret, .. } => {
let next_packet_pubkey = onion_utils::next_hop_pubkey(secp_ctx,
msg.onion_routing_packet.public_key.unwrap(), &shared_secret);
msg.onion_routing_packet.public_key.unwrap(), &shared_secret.secret_bytes());
Some(NextPacketDetails {
next_packet_pubkey, outgoing_scid: short_channel_id,
outgoing_amt_msat: amt_to_forward, outgoing_cltv_value
})
}
Hop::BlindedForward { next_hop_data: 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, .. }, shared_secret, .. } => {
let (amt_to_forward, outgoing_cltv_value) = match check_blinded_forward(
msg.amount_msat, msg.cltv_expiry, &payment_relay, &payment_constraints, &features
) {
Ok((amt, cltv)) => (amt, cltv),
Err(()) => {
return_err!("Underflow calculating outbound amount or cltv value for blinded forward",
INVALID_ONION_BLINDING, &[0; 32]);
return encode_relay_error("Underflow calculating outbound amount or cltv value for blinded forward",
INVALID_ONION_BLINDING, shared_secret.secret_bytes(), &[0; 32]);
}
};
let next_packet_pubkey = onion_utils::next_hop_pubkey(&secp_ctx,
msg.onion_routing_packet.public_key.unwrap(), &shared_secret);
msg.onion_routing_packet.public_key.unwrap(), &shared_secret.secret_bytes());
Some(NextPacketDetails {
next_packet_pubkey, outgoing_scid: short_channel_id, outgoing_amt_msat: amt_to_forward,
outgoing_cltv_value
Expand All @@ -463,7 +450,7 @@ where
_ => None
};

Ok((next_hop, shared_secret, next_packet_details))
Ok((next_hop, next_packet_details))
}

pub(super) fn check_incoming_htlc_cltv(
Expand Down
Loading
Loading