Repository navigation
Conversation
5a52503 to
e430f79
Compare
When enabled for a peer, the handshake uses the Noise IKhfs+psk2 pattern instead of the usual Noise IKpsk2. The hfs extension gives the handshake hybrid forward secrecy, protecting past traffic from a future quantum computer able to break X25519.
e430f79 to
86ba1cc
Compare
| NoiseKEMEncapsulationKeySize = 1184 | ||
| NoiseKEMCiphertextSize = 1088 |
There was a problem hiding this comment.
optional: we can point at https://pkg.go.dev/crypto/mlkem#pkg-constants, which is already a dependency
| peer.handshake.mutex.Lock() | ||
| peer.SetHybridHandshake(value == "true") |
There was a problem hiding this comment.
Peer.SetHybridHandshake also acquires Peer.handshake.mutex.
| ) | ||
| setZero(ss[:]) | ||
| aead, _ := chacha20poly1305New(key[:]) | ||
| ss, ciphertext := handshake.remoteKEM.Encapsulate() |
There was a problem hiding this comment.
If we consume a classical initiation, but then hybrid is enabled before we enter CreateMessageResponse, we'll panic on a nil remoteKem field here.
We probably want to bail out in such a case? Or just invalidate the state when mode switches. Or both.
There was a problem hiding this comment.
The simplest solution I see is to use localKEM and remoteKEM nilness more intentionally, e.g. only set/gen localKEM when hybridHandshake is set, check remoteKEM nilness here instead of hybridHandshake, etc.
Relates to #98 (comment)
| mixHash(&InitialHash, &InitialChainKey, []byte(WGIdentifier)) | ||
| HybridInitialChainKey = blake2s.Sum256([]byte(HybridNoiseConstruction)) | ||
| mixHash(&HybridInitialHash, &HybridInitialChainKey, []byte(WGIdentifier)) | ||
| OneNonce[0] = 1 |
There was a problem hiding this comment.
With 12-byte nonce noise specifies a 4-byte all zeroes prefix: https://noiseprotocol.org/noise.html#the-chachapoly-cipher-functions
So OneNonce[4] = 1
| handshake := &peer.handshake | ||
|
|
||
| // verify that handshake type matches expectation | ||
| if msg.HasKEM != handshake.hybridHandshake { |
There was a problem hiding this comment.
handshake.mutex guards this field, but isn't acquired here.
There's a handshake.mutex.RLock() a few lines down.
| MessageEncapsulatingTransportSize = 8 // size of optional, free (for use by conn.Bind.Send()) space preceding the transport header | ||
| MessageTransportSize = MessageTransportHeaderSize + poly1305.TagSize // size of empty transport | ||
| MessageKeepaliveSize = MessageTransportSize // size of keepalive | ||
| MessageHandshakeSize = MessageInitiationSize // size of largest handshake related message |
There was a problem hiding this comment.
This constant is unused within wireguard-go and tailscaled, but its comment is now outdated/misleading. We should probably just delete it, or update to point at ml-kem initiation message size constant.
| device.log.Verbosef("%v - UAPI: Updating hybrid", peer.Peer) | ||
|
|
||
| peer.handshake.mutex.Lock() | ||
| peer.SetHybridHandshake(value == "true") |
There was a problem hiding this comment.
we should validate value is true or false, otherwise we can mislead the operator into thinking they changed something when they didn't
replace_allowed_ips is an example bool line that validates
| if err != nil { | ||
| return nil, err | ||
| } | ||
| handshake.localKEM, err = mlkem.GenerateKey768() |
There was a problem hiding this comment.
this runs for classical init, so we pay resource taxes even though classical will never use the field in a useful way
one could argue it's acceptable in the interest of simplicity, if the intent is to move most/all handshakes to hfs over time
3 participants
When enabled for a peer, the handshake uses the Noise IKhfs+psk2 pattern instead of the usual Noise IKpsk2. The hfs extension gives the handshake hybrid forward secrecy, protecting past traffic from a future quantum computer able to break X25519.