Skip to content

Improve privacy for Blinded Message Paths using Dummy Hops #3726

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 37 additions & 2 deletions lightning/src/blinded_path/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,37 @@ impl BlindedMessagePath {
/// pubkey in `node_pks` will be the destination node.
///
/// Errors if no hops are provided or if `node_pk`(s) are invalid.
// TODO: make all payloads the same size with padding + add dummy hops
pub fn new<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
context: MessageContext, entropy_source: ES, secp_ctx: &Secp256k1<T>,
) -> Result<Self, ()>
where
ES::Target: EntropySource,
{
BlindedMessagePath::new_with_dummy_hops(
intermediate_nodes,
0,
recipient_node_id,
context,
entropy_source,
secp_ctx,
)
}

/// Create a path for an onion message, to be forwarded along `node_pks`.
///
/// Additionally allows appending a number of dummy hops before the final hop,
/// increasing the total path length and enhancing privacy by obscuring the true
/// distance between sender and recipient.
///
/// The last node pubkey in `node_pks` will be the destination node.
///
/// Errors if no hops are provided or if `node_pk`(s) are invalid.
pub fn new_with_dummy_hops<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
intermediate_nodes: &[MessageForwardNode], dummy_hops_count: u8,
recipient_node_id: PublicKey, context: MessageContext, entropy_source: ES,
secp_ctx: &Secp256k1<T>,
) -> Result<Self, ()>
where
ES::Target: EntropySource,
{
Expand All @@ -91,6 +117,7 @@ impl BlindedMessagePath {
intermediate_nodes,
recipient_node_id,
context,
dummy_hops_count,
&blinding_secret,
)
.map_err(|_| ())?,
Expand Down Expand Up @@ -507,11 +534,13 @@ pub(crate) const MESSAGE_PADDING_ROUND_OFF: usize = 100;
/// Construct blinded onion message hops for the given `intermediate_nodes` and `recipient_node_id`.
pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
secp_ctx: &Secp256k1<T>, intermediate_nodes: &[MessageForwardNode],
recipient_node_id: PublicKey, context: MessageContext, session_priv: &SecretKey,
recipient_node_id: PublicKey, context: MessageContext, dummy_hops_count: u8,
session_priv: &SecretKey,
) -> Result<Vec<BlindedHop>, secp256k1::Error> {
let pks = intermediate_nodes
.iter()
.map(|node| node.node_id)
.chain((0..dummy_hops_count).map(|_| recipient_node_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

I looked up the bolt spec and read

"MAY add additional "dummy" hops at the end of the path (which it will ignore on receipt) to obscure the path length."

What does ignore mean exactly? It seems in the next commit that it means to keep peeling? Using the recipient node id for all the dummy hops isn't really described in the bolt I think. Maybe mistaking.

Also a mention of padding is made:

"The padding field can be used to ensure that all encrypted_recipient_data have the same length. It's particularly useful when adding dummy hops at the end of a blinded route, to prevent the sender from figuring out which node is the final recipient"

Not sure if that is done now too?

Copy link
Member Author

Choose a reason for hiding this comment

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

What does “ignore” mean exactly? It seems in the next commit that it means to keep peeling? Using the recipient node id for all the dummy hops isn't really described in the bolt I think. Maybe mistaking.

The thinking behind this approach was that if dummy hops were added after the ReceiveTlvs, it could open up timing-based attacks—where an attacker might estimate the position of the actual recipient based on how quickly a response is returned.

To avoid that, I added the dummy hops just before the final node. This way, even after receiving a dummy hop (with ForwardTlvs directed to self), the node still has to keep peeling until it reaches the actual ReceiveTlvs. This helps make response timing more uniform and avoids leaking information about path length.

Also a mention of padding is made

Yes! In PR #3177, we added support for padding in both BlindedMessagePaths and BlindedPaymentPaths, ensuring all payloads are a multiple of PADDING_ROUND_OFF.

Since the MESSAGE_PADDING_ROUND_OFF buffer is large enough, every payload—whether it's a ForwardTlvs, dummy hop, or ReceiveTlvs—ends up with the same total length. This helps prevent the sender from inferring the number of hops based on packet size.

I've also updated the padding tests to use new_with_dummy_hops, so we make sure even dummy hops are padded the same way as real ones.

Thanks so much again for the super helpful feedback!

Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid that, I added the dummy hops just before the final node.

If this is strictly better, it could be worth a PR to the bolt spec? At the minimum it might get you some feedback on this line of thinking.

I am not sure if the timing attack is avoided though, and worth the extra complexity. Peeling seems to be so much faster than an actual hop with network latency etc. Some random delay might be more effective?

Copy link
Contributor

Choose a reason for hiding this comment

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

The code also still works for blinded paths where dummy hops are added after the ReceiveTlvs right? Just making sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

The code also still works for blinded paths where dummy hops are added after the ReceiveTlvs right? Just making sure.

There shouldn't be a need to support that because we only support receiving to blinded paths that we create.

Copy link
Contributor

Choose a reason for hiding this comment

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

The timing attack does seem like a potential issue though. Not sure how to address that without adding some kind of ProcessPendingHtlcsForwardable event for onion messages, which seems like overkill. I think we can maybe document it on the issue and push to follow-up? @TheBlueMatt do you have any thoughts on how to simulate a fake onion message forward when processing dummy hops?

.chain(core::iter::once(recipient_node_id));
let is_compact = intermediate_nodes.iter().any(|node| node.short_channel_id.is_some());

Expand All @@ -526,6 +555,12 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
.map(|next_hop| {
ControlTlvs::Forward(ForwardTlvs { next_hop, next_blinding_override: None })
})
.chain((0..dummy_hops_count).map(|_| {
ControlTlvs::Forward(ForwardTlvs {
next_hop: NextMessageHop::NodeId(recipient_node_id),
next_blinding_override: None,
})
}))
.chain(core::iter::once(ControlTlvs::Receive(ReceiveTlvs { context: Some(context) })));

if is_compact {
Expand Down
32 changes: 30 additions & 2 deletions lightning/src/onion_message/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,32 @@ fn one_blinded_hop() {
pass_along_path(&nodes);
}

#[test]
fn blinded_path_with_dummy() {
let nodes = create_nodes(2);
let test_msg = TestCustomMessage::Pong;

let secp_ctx = Secp256k1::new();
let context = MessageContext::Custom(Vec::new());
let entropy = &*nodes[1].entropy_source;
let blinded_path = BlindedMessagePath::new_with_dummy_hops(
&[],
5,
nodes[1].node_id,
context,
entropy,
&secp_ctx,
)
.unwrap();
// Make sure that dummy hops are do added to the blinded path.
assert_eq!(blinded_path.blinded_hops().len(), 6);
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap();
nodes[1].custom_message_handler.expect_message(TestCustomMessage::Pong);
pass_along_path(&nodes);
}

#[test]
fn two_unblinded_two_blinded() {
let nodes = create_nodes(5);
Expand Down Expand Up @@ -611,8 +637,9 @@ fn test_blinded_path_padding_for_full_length_path() {
// Update the context to create a larger final receive TLVs, ensuring that
// the hop sizes vary before padding.
let context = MessageContext::Custom(vec![0u8; 42]);
let blinded_path = BlindedMessagePath::new(
let blinded_path = BlindedMessagePath::new_with_dummy_hops(
&intermediate_nodes,
5,
nodes[3].node_id,
context,
&*nodes[3].entropy_source,
Expand Down Expand Up @@ -644,8 +671,9 @@ fn test_blinded_path_no_padding_for_compact_path() {
// Update the context to create a larger final receive TLVs, ensuring that
// the hop sizes vary before padding.
let context = MessageContext::Custom(vec![0u8; 42]);
let blinded_path = BlindedMessagePath::new(
let blinded_path = BlindedMessagePath::new_with_dummy_hops(
&intermediate_nodes,
5,
nodes[3].node_id,
context,
&*nodes[3].entropy_source,
Expand Down
36 changes: 28 additions & 8 deletions lightning/src/onion_message/messenger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,13 @@ where
// recipient's node_id.
const MIN_PEER_CHANNELS: usize = 3;

// Add a random number (1 to 5) of dummy hops to each non-compact blinded path
// to make it harder to infer the recipient's position.
let dummy_hops_count = compact_paths.then_some(0).unwrap_or_else(|| {
let random_byte = entropy_source.get_secure_random_bytes()[0];
(random_byte % 5) + 1
});

let network_graph = network_graph.deref().read_only();
let is_recipient_announced =
network_graph.nodes().contains_key(&NodeId::from_pubkey(&recipient));
Expand Down Expand Up @@ -602,8 +609,15 @@ where
Ok(paths) if !paths.is_empty() => Ok(paths),
_ => {
if is_recipient_announced {
BlindedMessagePath::new(&[], recipient, context, &**entropy_source, secp_ctx)
.map(|path| vec![path])
BlindedMessagePath::new_with_dummy_hops(
&[],
dummy_hops_count,
recipient,
context,
&**entropy_source,
secp_ctx,
)
.map(|path| vec![path])
} else {
Err(())
}
Expand Down Expand Up @@ -1104,11 +1118,6 @@ where
})),
Some((next_hop_hmac, new_packet_bytes)),
)) => {
// TODO: we need to check whether `next_hop` is our node, in which case this is a dummy
// blinded hop and this onion message is destined for us. In this situation, we should keep
// unwrapping the onion layers to get to the final payload. Since we don't have the option
// of creating blinded paths with dummy hops currently, we should be ok to not handle this
// for now.
let packet_pubkey = msg.onion_routing_packet.public_key;
let new_pubkey_opt =
onion_utils::next_hop_pubkey(&secp_ctx, packet_pubkey, &onion_decode_ss);
Expand Down Expand Up @@ -1145,7 +1154,18 @@ where
onion_routing_packet: outgoing_packet,
};

Ok(PeeledOnion::Forward(next_hop, onion_message))
let our_node_id = node_signer.get_node_id(Recipient::Node).unwrap();

match next_hop {
NextMessageHop::NodeId(ref id) if id == &our_node_id => peel_onion_message(
&onion_message,
secp_ctx,
node_signer,
logger,
custom_handler,
),
_ => Ok(PeeledOnion::Forward(next_hop, onion_message)),
}
},
Err(e) => {
log_trace!(logger, "Errored decoding onion message packet: {:?}", e);
Expand Down
Loading