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.

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

Route Blinding (Feature 24/25) #765

Merged
merged 3 commits into from
Mar 28, 2023
Merged

Route Blinding (Feature 24/25) #765

merged 3 commits into from
Mar 28, 2023

Conversation

t-bast
Copy link
Collaborator

@t-bast t-bast commented Apr 6, 2020

Route blinding allows a recipient to provide a blinded route to potential payers. Each node_id in the route is tweaked, and dummy hops may be included.

This is an alternative to rendezvous to preserve recipient anonymity.
It has a different set of trade-offs: onions are re-usable, but the privacy guarantees are a bit weaker and require more work (e.g. when handling errors).

The main areas I'd like reviewers to focus on at first are:

  • the crypto itself, and whether we're missing an abstraction (it's really a kind of reverse sphinx) or a simpler way to do this
  • whether we can live with some update_add_htlc that have a different length than other update_add_htlc (which kind of gives away the blinded path if you're watching the network traffic)
  • error management: how do we ensure errors don't leak the blinded path? Obfuscation + delayed responses may do the trick, but is it enough (and is it necessary)?

The attack scenarios that reviewers should focus on include (but are not limited to):

  • can attackers figure out the identity of the recipient, and if so, with what kind of attack and resources?
  • note that attackers can do any kind of message tampering, delaying or dropping if they're intermediate nodes

proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Show resolved Hide resolved
proposals/route-blinding.md Show resolved Hide resolved
proposals/route-blinding.md Show resolved Hide resolved
@ariard
Copy link
Contributor

ariard commented Apr 27, 2020

@t-bast on E2E issues with a sender requiring multiples invoices you may bound through some utxo-pinning stuff, like if we adopt Poodle (or any confidential-utxo declaration scheme) for dual-funding we may reused it for this. "To require a new invoice, announce me a another channel utxo or you don't have more than X invoices for one channel utxo registered". Maybe not for a v1 but something to explore ?

@t-bast
Copy link
Collaborator Author

t-bast commented Apr 28, 2020

"To require a new invoice, announce me a another channel utxo or you don't have more than X invoices for one channel utxo registered"

That's an interesting idea, it would force payers to have some skin in the game (multiple channels). It's worth exploring!

@t-bast t-bast marked this pull request as ready for review April 28, 2020 13:49
@t-bast t-bast marked this pull request as draft April 28, 2020 13:49
Copy link
Collaborator

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Very nice proposal, just found some minor things that could be clarified and some markup :-)

proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ariard ariard left a comment

Choose a reason for hiding this comment

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

Achieved a new round, I think we should have a better model around recipient anonymity and attacker capabilities before to move forward.

I'm still skeptical about delegating relay parameters (cltv, fees) to an untrusted sender. We should assume such a a pair is a unique fingerprint across the network due to node relay policy heterogeneity. And sender sounds to be able to freely probe this "identifier" as you can screw up an onion path after this probed point.

proposals/route-blinding.md Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
proposals/route-blinding.md Outdated Show resolved Hide resolved
@t-bast
Copy link
Collaborator Author

t-bast commented Nov 16, 2020

@ariard @cdecker thanks for the early reviews, I marked many comments as resolved to make it more readable for newcomers, feel free to unresolve if you think it's useful.

@rustyrussell
Copy link
Collaborator

Needs (trivial?) rebase BTW, due to conflict.

Route blinding allows a recipient to provide a blinded route to
potential payers. Each node_id in the route is tweaked, and dummy
hops may be included.

This is an alternative to rendezvous to preserve recipient anonymity.
It has a different set of trade-offs: onions are re-usable, but the
privacy guarantees are a bit weaker and require more work (e.g. when
handling payment fees and errors).
Add specification requirements for creating and using blinded routes.
This commit contains the low-level details of the route blinding scheme,
decoupled from how it can be used by high-level components such as onion
messages or payments.
@t-bast
Copy link
Collaborator Author

t-bast commented Jan 9, 2023

Rebased.

@t-bast
Copy link
Collaborator Author

t-bast commented Jan 31, 2023

We have successfully completed interop tests between eclair and cln, so this PR is getting ready to be merged! I have rebased it and squashed the last commits, please have a (hopefully) last look at it @rustyrussell and we'll be able to integrate it. It's been a while since we last integrated such a big change to the specification, I'm excited 😄

Copy link
Collaborator

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Three final wording suggestions. Otherwise looks good!

04-onion-routing.md Outdated Show resolved Hide resolved
04-onion-routing.md Outdated Show resolved Hide resolved
04-onion-routing.md Outdated Show resolved Hide resolved
Add specification requirements for using route blinding to make payments
while preserving recipient anonymity. Implementers must ensure they
understand all those requirements, there are subtle attacks that could let
malicious senders deanonymize the route if incompletely implemented.
@t-bast
Copy link
Collaborator Author

t-bast commented Mar 28, 2023

Merging this pull request as discussed during yesterday's spec meeting.
Many thanks to everyone involved in the review and implementation!

@t-bast t-bast merged commit c4c5a8e into master Mar 28, 2023
@t-bast t-bast deleted the route-blinding branch March 28, 2023 06:44
ddustin pushed a commit to ddustin/lightning that referenced this pull request Apr 11, 2023
This is as of commit aed5518a80aade56218da87f92e0a39963b660cf

Signed-off-by: Rusty Russell <[email protected]>
- MUST create `encrypted_data_tlv` for each node in the blinded route (including itself).
- MUST include `encrypted_data_tlv.short_channel_id` and `encrypted_data_tlv.payment_relay` for each non-final node.
- MUST set `encrypted_data_tlv.payment_constraints` for each non-final node:
- `max_cltv_expiry` to the largest block height at which the route is allowed to be used, starting
Copy link
Contributor

@valentinewallace valentinewallace Apr 21, 2023

Choose a reason for hiding this comment

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

Is max_cltv_expiry currently missing in the offers spec?

Edit: merged #1069 to clarify this

ddustin pushed a commit to ddustin/lightning that referenced this pull request May 12, 2023
This is as of commit aed5518a80aade56218da87f92e0a39963b660cf

Signed-off-by: Rusty Russell <[email protected]>
* Aggregated route relay parameters and constraints:
* `fee_base_msat`: 201
* `fee_proportional_millionths`: 1001
* `htlc_minimum_msat`: 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

For offers, we also need to compute the aggregated htlc_maximum_msat for the BOLT 12 invoice BlindedPayInfo. ISTM this field be included in payment_constraints? I don't see why it'd be present in BlindedPayInfo if it can't be aggregated from the per-hop blinded relay parameters/constraints.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see what you mean, this is good feedback. I think it is unnecessary though to add htlc_maximum_msat to the payment_constraints (which would increase the size of encoded blinded paths). The recipient only needs to convey the smallest (most restrictive) value in the path once, in the invoice's BlindedPayInfo, that should be sufficient for the sender.

The recipient should use its liquidity estimation (especially for his local channels), combined with the channel_updates htlc_maximum_msat to set this value in BlindedPayInfo (and the size of the payment as well, if the payment is smaller than all intermediate htlc_maximum_msat, the invoice should simply set this to the payment amount). It is a good hint for senders to maximize payment success, but I don't think it should be a hard rule enforced by intermediate nodes: liquidity may shift before the payment is attempted, so the sender should be able to try sending more.

The only risk with that is if it can be exploited to unblind a path. If the BlindedPayInfo.htlc_maximum_msat is exactly set to the htlc_maximum_msat of one of the channel_updates from the blinded path, that can help the sender figure out who is inside the path. The recipient should fuzz BlindedPayInfo.htlc_maximum_msat to make sure it cannot be easily linked to existing channels.

Does that make sense? @thomash-acinq what are your thoughts on that as well?

Choose a reason for hiding this comment

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

I don't understand the original question (what is ISTM?) but payment_contraints and blinded_payinfo are completely different.
blinded_payinfo is an aggregate of the channel_updates along the path and as such contains the same fields that are found in a channel_update. And as @t-bast explained you may want to fuzz it a bit to make it harder to unblind the path.
payment_contraints do not correspond to a channel_update, that would be unnecessary as the relaying node is already checking the constraints from its channel_update. Instead payment_contraints are additional constraints chosen by the payment recipient. Actually payment_contraints is not really needed for intermediate nodes as the recipient will perform the check anyways, it just allows failing sooner without waiting to reach the recipient.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the responses, that makes sense to me. I had thought the payinfo was aggregated from the payment_constraints, but I see that's not the case.

ISTM = "it seems to me", sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet