rust-lightning Improve privacy for Blinded Message Paths using Dummy Hops

trafficstars

Resolves #3252

This PR improves privacy in blinded path construction by introducing support for dummy hops.

While blinded paths obscure node identities, they still might reveal the number of hops—potentially leaking information about the distance between sender and receiver.

To mitigate this, we now prepend dummy hops for BlindedMessagePaths, that serve no routing purpose but act as decoys. This makes it significantly harder to estimate the true position of the destination node based on path length.

Apr 09 '25 14:04 shaavan

👋 I see @joostjager was un-assigned. If you'd like another reviewer assignment, please click here.

Apr 09 '25 14:04 ldk-reviews-bot

Updated from pr3728.01 to pr3728.02 (diff): Addressed @joostjager comments

Changes:

DRYed up the code.
Update logic so that we bring randomness to the number of blinded hops we add to a blinded path.
Updated test to make sure dummy hops are properly added and padded.

Apr 11 '25 17:04 shaavan

🔔 1st Reminder

Hey @valentinewallace! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Apr 12 '25 00:04 ldk-reviews-bot

🔔 2nd Reminder

Hey @valentinewallace! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Apr 14 '25 00:04 ldk-reviews-bot

Updated from pr3728.02 to pr3728.03 (diff): Addressed @valentinewallace comments

Changes:

Remove the rand dependency, and instead using entropy source for random dummy hop count.
Updated code to not add dummy hops by default for compact paths.

Apr 15 '25 12:04 shaavan

Codecov Report

:x: Patch coverage is 91.03774% with 19 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 88.76%. Comparing base (1c36624) to head (fdf352b). :warning: Report is 42 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/onion_message/messenger.rs	86.30%	8 Missing and 2 partials :warning:
lightning/src/onion_message/packet.rs	72.72%	3 Missing :warning:
lightning/src/ln/blinded_payment_tests.rs	75.00%	2 Missing :warning:
lightning/src/blinded_path/message.rs	96.29%	0 Missing and 1 partial :warning:
lightning/src/ln/offers_tests.rs	97.67%	1 Missing :warning:
lightning/src/sign/mod.rs	50.00%	1 Missing :warning:
lightning/src/util/test_utils.rs	66.66%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3726      +/-   ##
==========================================
- Coverage   88.78%   88.76%   -0.03%     
==========================================
  Files         176      176              
  Lines      128139   128938     +799     
  Branches   128139   128938     +799     
==========================================
+ Hits       113768   114447     +679     
- Misses      11803    11895      +92     
- Partials     2568     2596      +28

Flag	Coverage Δ
fuzzing	`21.91% <25.00%> (-0.01%)`	:arrow_down:
tests	`88.59% <91.03%> (-0.03%)`	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

:snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Apr 15 '25 12:04 codecov[bot]

Just re-request review when this PR is ready again

Apr 22 '25 06:04 joostjager

Updated from pr3728.03 to pr3728.04 (diff): Addressed @valentinewallace, @joostjager comments

Introduce new Dummy ControlTlvs that allows add authentication to each dummy hop, so that we can verify the hop before decoding.

May 02 '25 13:05 shaavan

Updated from pr3728.04 to pr3728.05 (diff): Addressed @valentinewallace, @joostjager comments

Changes:

Rebase on main to resolve merge conflicts.

May 02 '25 13:05 shaavan

🔔 1st Reminder

Hey @joostjager! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

May 07 '25 14:05 ldk-reviews-bot

🔔 2nd Reminder

Hey @joostjager! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

May 10 '25 00:05 ldk-reviews-bot

🔔 3rd Reminder

Hey @joostjager! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

May 12 '25 00:05 ldk-reviews-bot

Updated from pr3728.05 to pr3728.06 (diff): Addressed @joostjager comments

Changes:

Renamed dummy_tlvs -> dummy_tlv.
Expanded Documentation.
Fix number of added dummy hops (from 1-5, to 0-5), to prevent guessing the actual final node.
Cleaned up code

May 12 '25 16:05 shaavan

@joostjager

Main thing I try to understand is how the hmac authentication of the dummy hops helps with a time attack. Isn't this accomplishing the exact opposite? Peeling is stopped early when an hmac doesn't check out, but I thought you wanted to always process all the hops to make it look real?

Yeah, you're absolutely right — authenticating dummy hops does reveal how many hops are real based on timing, since we stop peeling once the HMAC fails. That can open up timing-based attacks.

That said, I believe @valentinewallace was pointing to a different risk: if the dummy TLVs are left unauthenticated, an attacker could craft a fake blinded path with a large number of bogus hops and arbitrary ReceiveTlvs, which could slow down parsing — something closer to a DoS vector.

So we’re kind of stuck between two concerns:

If we authenticate early, we risk leaking the real hop count via timing.
If we don’t authenticate, we risk being vulnerable to slow-path or DoS-style attacks.

Open to any suggestions you might have on how to walk this line — happy to explore if there's a middle ground that avoids both leakage and performance abuse.

May 12 '25 16:05 shaavan

@joostjager not sure how authenticating dummy hops creates a timing attack? We'll fail on the first hop if an attacker creates a bogus path that fails authentication, which seems correct because in this case the attacker already knows who we are, otherwise they wouldn't have been able to create the path 🤔

There may still be a timing attack here in the sense that dummy hops take a different amount of processing time to "regular" forward hops, though. IMO it's okay to punt on that though.

May 12 '25 18:05 valentinewallace

Okay, indeed, I see that an attacker crafting dummy hops with a bad authentication code can't learn anything from that. It will fail immediately, and they already knew it would. Do you also see it this way @shaavan, or is there still something there?

Regarding the DoS-style attack, it isn't immediately clear to me that that hop processing time is significant. But I guess it is not worth spending the cycles if not necessary.

Definitely appreciate the docs that you added. In my opinion, we should do that a lot in pull requests. Explain it extensively when everything is still fresh.

May 12 '25 19:05 joostjager

Okay, indeed, I see that an attacker crafting dummy hops with a bad authentication code can't learn anything from that. It will fail immediately, and they already knew it would. Do you also see it this way @shaavan, or is there still something there? Regarding the DoS-style attack, it isn't immediately clear to me that that hop processing time is significant. But I guess it is not worth spending the cycles if not necessary. Definitely appreciate the docs that you added. In my opinion, we should do that a lot in pull requests. Explain it extensively when everything is still fresh.

I think that makes perfect sense! I completely missed Val’s point—that someone can only construct a blinded path to us if they already know who we are. And if they already know us, there’s no additional privacy risk in how quickly we fail bogus paths, since there’s nothing left to “dox.”

And on the authentication front, as you said, it’s good that we don’t end up spending cycles on invalid paths. So it’s a win-win!

Thank you so much for the kind words on the docs! Will make sure to keep things tidy and detailed in future PRs as well.

May 14 '25 11:05 shaavan

Updated from pr3728.06 to pr3728.07 (diff): Addressed @jkczyz comments

Changes:

Rename get_inbound_payment_key -> get_expanded_key and expanded documentation.

May 16 '25 11:05 shaavan

Updated from pr3728.07 to pr3728.08 (diff):

Changes:

Rebase on main

May 16 '25 18:05 shaavan

🔔 1st Reminder