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

BOLT-04: modify Sphinx packet construction to use starting random bytes #697

Open
wants to merge 1 commit into
base: master
from

Conversation

@Roasbeef
Copy link
Member

Roasbeef commented Nov 6, 2019

In this commit, we modify the existing instructions to create the Sphinx
packet to no longer start out with a zero initialize set of 1366 bytes.
Instead, we now instruct the sender to use random bytes derived from a
CSPRG. This fixes a recently discovered privacy leak that allows an
adversarial exit hop to ascertain a lower bound on the true path length.

Note that this doesn't affect packet processing, so this is a backwards
compatible change. Only clients need to update in order to avoid this
privacy leak.

After this change is applied, the test vectors as is don't match the
spec, as they're created using the original all zero starting bytes. We
can either update these with our specified set of random bytes, or leave
them as is, as they're fully deterministic as is.

An alternative path would be to generate more random bytes from the
session key as we do elsewhere (the chacha based CSPRNG).

In this commit, we modify the existing instructions to create the Sphinx
packet to no longer start out with a zero initialize set of 1366 bytes.
Instead, we now instruct the sender to use _random_ bytes derived from a
CSPRG. This fixes a recently discovered privacy leak that allows an
adversarial exit hop to ascertain a lower bound on the true path length.

Note that this doesn't affect packet processing, so this is a backwards
compatible change. Only clients need to update in order to avoid this
privacy leak.

After this change is applied, the test vectors as is don't match the
spec, as they're created using the original all zero starting bytes. We
can either update these with our specified set of random bytes, or leave
them as is, as they're fully deterministic as is.

An alternative path would be to generate more random bytes from the
shared secret as we do elsewhere (the chacha based CSPRNG).
Copy link
Collaborator

rustyrussell left a comment

Ack e116441

rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Nov 6, 2019
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Nov 6, 2019
@@ -470,6 +470,15 @@ func NewOnionPacket(paymentPath []*btcec.PublicKey, sessionKey *btcec.PrivateKey
var mixHeader [routingInfoSize]byte
var nextHmac [hmacSize]byte
// Our starting packet needs to be padded with random bytes, so we read

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Nov 6, 2019

Collaborator

indentation for these first three lines is off

@@ -402,7 +402,7 @@ The construction returns a single 1366-byte packet along with the first receivin
The packet construction is performed in the reverse order of the route, i.e.
the last hop's operations are applied first.

The packet is initialized with 1366 `0x00`-bytes.
The packet is initialized with 1366 _random_ bytes derived from a CSPRNG.

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Nov 6, 2019

Collaborator

this doesn't match the code below. the mix header is only 1300 bytes, of which we initialize 1300-firstHopSize bytes via the CSPRNG. since we copy the hop payloads directly into the mix header, we can specify that all 1300 should be random, and leave it as a minor optimization for implementations to not randomize the firstHopSize bytes.

the overall size is 1366 bytes including the session key, next mac, and version, but this is suggesting that those should be initialized via the CSPRNG as well.

This comment has been minimized.

Copy link
@rustyrussell

rustyrussell Nov 13, 2019

Collaborator

Yes, in our code we generated the 1300 byte stream using the session key and the 3-byte string "pad" (I know, not a greek letter, but meh).

// some here.
firstHopSize := hopData[numHops-1].NumBytes()
randBytes := make([]byte, routingInfoSize-firstHopSize)
if _, err := rand.Read(randBytes[:]); err != nil {

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Nov 6, 2019

Collaborator

can be made simpler/more efficient: lightningnetwork/lightning-onion#40 (comment)

@cfromknecht

This comment has been minimized.

Copy link
Collaborator

cfromknecht commented Nov 6, 2019

Misspelled words in 04-onion-routing.md: CSPRNG
cdecker added a commit to rustyrussell/lightning that referenced this pull request Nov 7, 2019
cdecker added a commit to rustyrussell/lightning that referenced this pull request Nov 7, 2019
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Nov 8, 2019
lightningnetwork/lightning-rfc#697
https://lists.linuxfoundation.org/pipermail/lightning-dev/2019-November/002288.html

We generate it from an hmac using the session secret.  It's not
clear that this will be useful for reproducing test vectors though,
since we don't generate the first 66 bytes, which is what the
spec says to do.

Reported-by: @Roasbeef
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker added a commit to ElementsProject/lightning that referenced this pull request Nov 8, 2019
lightningnetwork/lightning-rfc#697
https://lists.linuxfoundation.org/pipermail/lightning-dev/2019-November/002288.html

We generate it from an hmac using the session secret.  It's not
clear that this will be useful for reproducing test vectors though,
since we don't generate the first 66 bytes, which is what the
spec says to do.

Reported-by: @Roasbeef
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker

This comment has been minimized.

Copy link
Collaborator

cdecker commented Nov 8, 2019

fwiw we have decided to generate the initial content of the routing packet using ChaCha20 keyed with a key derived from the session_key using the pad type:

https://github.com/rustyrussell/lightning/blob/d8c386a141beccc67fc9d30de70cf027331de089/common/sphinx.c#L534-L542

This would allow us to maintain reproducibility, at the cost of slightly more computation to generate the stream (then again CSPRGs are likely also not free).

One notable difference to this PR is that we just fill the entire routing packet instead of offsetting from the end of the last hop's payload.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.