lightningnetwork / lightning-onion

Onion Routed Micropayments for the Lightning Network
MIT License
396 stars 125 forks source link

Strong Replay Protection and Batched Processing #16

Closed cfromknecht closed 6 years ago

cfromknecht commented 6 years ago

This PR extends the work of @Crypt-iQ in adding replay protection to the sphinx router by way of a persistent log. The log contains the hashed and truncated value of the shared secrets derived from previously processed onion packets, which is queried to ensure we do not reuse the same shared secret. The CLTV at which the shared secret expires is also persisted to the log, allowing it to be asynchronously garbage collected if provided with an appropriate chainntnfs.ChainNotifier instance.

In addition to the log, this PR also adds batched processing and replay detection of sphinx packets. This extends the router's API to include a transactional interface, which should offer better semantics for our use cases in lnd. The batch processing semantics ensure that attempts to process the same batch of sphinx packets will always result in an idempotent response, simplifying recovery logic within the link.

cfromknecht commented 6 years ago

Oops, forgot to respond to your q in comment! Yes, current plan is to use chanPoint || height as the identifier for batch ID's. Could do chanID || height, but then cross-chain stuff gets hairy.

cfromknecht commented 6 years ago

Rebased with latest optimizations and added ReconstructOnionPacket which decodes the next hop without applying replay protection.

cfromknecht commented 6 years ago

Thanks for reviewing this! I addressed the comments, should be gtg now 🔥

jimpo commented 6 years ago

Sorry, I'm kind of late to the party here, just saw this. I don't really like that this sphinx package now depends on chainntnfs from lnd and on bolt. What if instead this package just defined the ReplayLog interface and decayedlog was moved to somewhere in lnd? I think the dependency graph would be cleaner, and it also gels well with this comment suggesting a Bloom filter as an alternate data structure (that could implement the same interface).