use a stateless reset key derived from the private key #122
Conversation
transport.go
Outdated
| @@ -2,9 +2,13 @@ package libp2pquic | |||
|
|
|||
| import ( | |||
| "context" | |||
| "crypto/sha256" | |||
There was a problem hiding this comment.
We usually use https://github.com/minio/sha256-simd for performance.
There was a problem hiding this comment.
I don't think performance is critical here, since the only time we calculate a hash is when NewTransport is called. I'm ok with changing it to keep things consistent though.
This will be obsoleted with libp2p/go-libp2p-core#131 anyway.
transport.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| keyReader := hkdf.Expand(sha256.New, keyBytes, []byte("libp2p quic stateless reset key")) |
There was a problem hiding this comment.
Let's define the info as a constant.
There was a problem hiding this comment.
Also, I don't think we can do this as the key may not be uniformly random. I think we need the "extract" step (or need to call New)
I wonder if we should just define some form of "DeriveSecretKey(info)` function on our secret key interface. That way, we have a standard way to do this.
There was a problem hiding this comment.
Sounds good. Let me prepare a PR.
There was a problem hiding this comment.
@Stebalien Can we move forward with this PR without resolving libp2p/go-libp2p-core#131 first? For the stateless reset token, the only thing that matter is that the peer has any was of deriving a key. No interoperability required.
There was a problem hiding this comment.
Note: if we do that, you'll need to update this patch to call hkdf.New instead of hkdf.Expand.
There was a problem hiding this comment.
@Stebalien Done.
marten-seemann
force-pushed
the
set-stateless-reset-key
branch
from
f05ab19
to
add6b69
Compare
transport.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| keyReader := hkdf.New(sha256.New, keyBytes, nil, []byte("libp2p quic stateless reset key")) |
There was a problem hiding this comment.
- Let's make the "info" a variable/constant (private).
- Let's add a test to make sure we don't accidentally change it. That will cause silent breakage that won't be caught except in interop tests (which we need anyways but still).
There was a problem hiding this comment.
- Done.
- I'm not convinced that's necessary. Stateless resets are an optimization, and the worst case behavior in the absence of one is a connection timeout.
There was a problem hiding this comment.
That's even worse. That's a performance bug that we won't catch in any tests because it'll "just work" even if we break it.
Fixes #121.
When a QUIC endpoint loses state (e.g. due to a reboot or a crash), it won't be able to decrypt packets that arrive for connection that it previously handled. QUIC uses a stateless reset to allow the peer to quickly detect this condition (and not having to rely on a lengthy connection timeout).
From the outside, a QUIC stateless reset packet looks like any other (Short Header) QUIC packet. An endpoint recognizes a stateless reset by comparing the last 16 bytes of the encrypted packet to a stateless reset token communicated earlier during the connection.
This token can't be tied to any connection state. Instead, it needs to be derived from values that survive a crash of the node. It therefore makes sense to derive it from the node's private key.