lightning / bolts

BOLT: Basis of Lightning Technology (Lightning Network Specifications)
2.11k stars 491 forks source link

Encrypt-then-MAC for onion replies #250

Closed cdecker closed 7 years ago

cdecker commented 7 years ago

As mentioned by @Roasbeef during the last meeting, we currently MAC-then-encrypt, going against established wisdom of ensuring the integrity of the ciphertext before decrypting. The problem with that is that we have a single MAC, but encrypt on every hop. We could switch the order on the origin to encrypt the payload and then compute the MAC, before forwarding and adding additional layers, but we'd still be decrypting n-1 times before being able to check the ciphertext integrity and then decrypt one last time.

So we are decrypting a number of times without the ability to check the integrity anyway, and the current treatment is similar to a n+1 path with the last decryption dropped. There are a number of ways to fix this:

  1. Simply move the MAC computation after the first encryption, leading to that decrypt n-1 times, then check ciphertext MAC, then decrypt last time pattern described above
  2. Add a MAC to every hop, by adding a fixed size field of 20*32 bytes and shifting down when forwarding. Notice that we can't reuse the filler construction to include the MACs in the new MAC since the packet creator does not know the shared secrets, so the MAC will only include the ciphertext and cannot be encrypted (also means packet creator needs to fill the MAC array field with random data to avoid identifying the path length)
  3. Go against crowd wisdom and do nothing. After all the only thing that may leak is a temporary shared secret between the origin of the transfer and the node that produced the failure.

I like options 2 and 3 simply because they keep the same pattern for the sender, forwarder and recipient the same. Option 1 is the simplest that maintains encrypt-then-MAC, but as mentioned I don't think it is less risky than the current scheme since we perform a number of decryptions anyway before we get to check the encrypted ciphertext.

What do you guys think?

cdecker commented 7 years ago

Ping @Roasbeef, @rustyrussell, @pm47 and @sstone :-)

cdecker commented 7 years ago

During the last meeting we decided that more research for this particular issue is needed. The fact that it is not just the recipient of a route that may need to create an error message makes this issue particularly hard. In lieu of a permanent fix we decided to add a suggestion to simulate unwrapping the error message for the maximum path length independently of the actual route length in order to make timing attacks to determine the route length harder. This is implemented in #255