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

BOLT04: Add rationale for constant error decryption. #1154

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ziggie1984
Copy link
Contributor

@ziggie1984 ziggie1984 commented Apr 16, 2024

Fixes #1125

@ziggie1984 ziggie1984 marked this pull request as ready for review April 16, 2024 15:15
04-onion-routing.md Outdated Show resolved Hide resolved
To avoid timing analysis when decrypting failed payments the sender
should act as if the failure in the route came for the 27th hop.
Also changed the maximum number of hops in the route from 20 (legacy)
to 27 (tlv onion).
04-onion-routing.md Outdated Show resolved Hide resolved
@t-bast
Copy link
Collaborator

t-bast commented Apr 17, 2024

Shouldn't we instead just get rid of this section entirely? This is completely useless, isn't it? I don't see how timing the decryption steps of the error message can be useful to any attacker, as there is a lot of random delay before a retry that is orders of magnitude larger (path-finding and network latency).

@ziggie1984
Copy link
Contributor Author

ziggie1984 commented Apr 17, 2024

Agree, either we get rid of the whole section or we introduce the rationale because when I was reading this part of the spec I had a hard time understanding the reasoning behind it. LND has implemented it, it actually does the 27 decryption cycle so maybe keep it? But open for both approaches, wdyt @morehouse

@t-bast
Copy link
Collaborator

t-bast commented Apr 17, 2024

LND has implemented it, it actually does the 27 decryption cycle

Then it's probably a good idea to simplify lnd by removing that extra code if it's useless 😉
This timing information seems completely unusable by an attacker to me, but maybe I'm missing something.

@ProofOfKeags
Copy link
Contributor

I would like to second the opinion that it seems entirely useless. Two reasons for this.

  1. Like y'all have already stated, network delay middle 50% variance probably dwarfs the entirety of the decryption process.
  2. Learning your relative position in the route is pretty useless beyond 3 hops. It is important to make it hard to distinguish whether your upstream is the sender or a forwarder, but let's say you know you're hop number 6... what then?! I'm supposedly 6 degrees of separation from everyone on the planet, so you've successfully ascertained it could be....anyone...

All that said, this PR aims to clarify and for as long as we continue to have the normative requirement to do that extra decryption. I support merging this as the rationale to clarify the purpose of this (seemingly ridiculous) practice.

@t-bast
Copy link
Collaborator

t-bast commented May 7, 2024

This was discussed during the last two spec meetings, and it was agreed that this specific decryption step isn't really a fingerprinting vector. But there are other fingerprinting vectors when retrying payments, so this section should be re-worked to explain that generally, as a payment sender, you should ensure you're protecting against fingerprinting by adding random delays before retries and trying as much as possible to be constant time in most operations.

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.

Error decryption - adopt requirement also for new tlv package format.
3 participants