sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-10-21 12:59:08 +02:00
Code Issues Packages Projects Releases Wiki Activity

[client] pqkem: bind the configured pre-shared key into the PQ PSK

Browse Source 
--pre-shared-key promises that only peers holding the same key can
communicate. With ML-KEM enabled the derived PQ PSK replaces it in
WireGuard, but the derivation never saw it: two peers with different
configured keys, or only one with a key, still agreed on the same PQ PSK
and talked. A permitted peer identity, or a stolen identity key, was enough
to bypass the extra secret (security review of #7098, finding F-001).
Rosenpass does not have this gap because it seeds its exchange with the
configured key.

Mix the configured key into the HKDF keying material, so a peer without it
derives a different PSK and WireGuard fails closed. It goes through the
binding shared by every derivation, so bootstrap and rotation both enforce
it. The library takes it as a generic option, keeping it free of WireGuard
concepts.
This commit is contained in:
riccardom
2026-10-19 16:37:25 +02:00
parent 2a2f7a9bbc
commit a73ce11912
4 changed files with 110 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/internal/engine.go
+7 -1
View File
@@ -619,7 +619,13 @@ func (e *Engine) startPQKEMManager(publicKey wgtypes.Key) error {
}
},
}
e.pqkemManager = pqkem.NewManager(pqkem.LocalID(publicKey.String()), cbHandler, pqkem.NewLogger())
var opts []pqkem.Option
if e.config.PreSharedKey != nil {
// The configured key promises that only peers holding it can communicate. The PQ
// PSK replaces it in WireGuard, so it has to carry that guarantee itself.
opts = append(opts, pqkem.WithPresharedSecret(e.config.PreSharedKey[:]))
}
e.pqkemManager = pqkem.NewManager(pqkem.LocalID(publicKey.String()), cbHandler, pqkem.NewLogger(), opts...)
e.pqkemManager.Start(tr)
log.Infof("pqkem: enabled (udp port %d on overlay %s)", e.pqkemManager.LocalPort(), e.config.WgAddr.IP)
return nil
client/internal/pqkem/kem.go
+16 -8
View File
@@ -15,9 +15,9 @@
// Combiner note: this follows draft-ietf-tls-ecdhe-mlkem for X25519MLKEM768 — on
// the wire ML-KEM ‖ X25519 (the draft deliberately reversed the share order for
// this group), and ML-KEM_ss ‖ X25519_ss as the KDF input. The PSK is derived with
// HKDF-SHA256 over that hybrid secret, salted with a domain-separation label and
// bound (via the HKDF info) to the full transcript and the canonicalised peer
// identities.
// HKDF-SHA256 over that hybrid secret (plus an optional pre-shared secret both peers
// hold), salted with a domain-separation label and bound (via the HKDF info) to the
// full transcript and the canonicalised peer identities.
package pqkem
import (
@@ -47,6 +47,10 @@ type PSK [32]byte
type Binding struct {
LocalID []byte
RemoteID []byte
// Secret is an optional pre-shared secret both peers must hold. It is mixed into the
// derivation, so peers holding different secrets (or only one holding one) derive
// different PSKs and the consumer's channel fails closed.
Secret []byte
}
// Initiator holds the ephemeral secrets between Offer and Finish.
@@ -144,10 +148,10 @@ func Respond(offer []byte, b Binding) (answer []byte, psk PSK, err error) {
}
// derivePSK runs HKDF-SHA256 over the hybrid shared secret (ML-KEM_ss ‖ X25519_ss,
// per draft-ietf-tls-ecdhe-mlkem), salted with the domain-separation label, and binds
// the result — via the HKDF info — to the full transcript (offer ‖ answer) and the
// canonicalised peer identities, so the PSK cannot be transplanted to another peer
// pair or a different exchange.
// per draft-ietf-tls-ecdhe-mlkem) plus the binding's optional pre-shared secret, salted
// with the domain-separation label, and binds the result — via the HKDF info — to the
// full transcript (offer ‖ answer) and the canonicalised peer identities, so the PSK
// cannot be transplanted to another peer pair or a different exchange.
func derivePSK(ssMLKEM, ssX, offer, answer []byte, b Binding) (PSK, error) {
// A PSK not bound to both peer identities could be transplanted to a different peer
// pair, so refuse to derive one from an empty binding.
@@ -156,9 +160,13 @@ func derivePSK(ssMLKEM, ssX, offer, answer []byte, b Binding) (PSK, error) {
}
lo, hi := canonicalPair(b.LocalID, b.RemoteID)
ikm := make([]byte, 0, len(ssMLKEM)+len(ssX))
// The shared secrets have fixed lengths, so appending the optional pre-shared secret
// after them keeps the input unambiguous. It sits in the keying material rather than
// the info, so a peer without it cannot derive the PSK at all.
ikm := make([]byte, 0, len(ssMLKEM)+len(ssX)+len(b.Secret))
ikm = append(ikm, ssMLKEM...)
ikm = append(ikm, ssX...)
ikm = append(ikm, b.Secret...)
info := make([]byte, 0, len(offer)+len(answer)+len(lo)+len(hi))
info = append(info, offer...)
client/internal/pqkem/manager.go
+21 -3
View File
@@ -99,6 +99,7 @@ type Manager struct {
localID LocalID
cbHandler CallbackHandler
logger *slog.Logger
secret []byte // optional pre-shared secret bound into every derivation; immutable
retryInterval time.Duration
maxRetries int
@@ -129,15 +130,28 @@ func (m *Manager) nextGenLocked(remoteID RemoteID) uint64 {
return m.genCounter[remoteID]
}
// Option configures a Manager at construction.
type Option func(*Manager)
// WithPresharedSecret binds every derived PSK, bootstrap and rotation alike, to a secret
// both peers must already share. A peer with a different secret, or none, derives a
// different PSK, so the consumer's channel fails closed instead of bypassing the secret.
// An empty secret is the same as no option.
func WithPresharedSecret(secret []byte) Option {
return func(m *Manager) {
m.secret = append([]byte(nil), secret...)
}
}
// NewManager builds a manager for the local peer identified by its peer identity key
// (used for the deterministic initiator role and the identity binding). A nil logger
// falls back to slog.Default(). Install the data-path transport with Start.
func NewManager(localID LocalID, h CallbackHandler, logger *slog.Logger) *Manager {
func NewManager(localID LocalID, h CallbackHandler, logger *slog.Logger, opts ...Option) *Manager {
if logger == nil {
logger = slog.Default()
}
ctx, cancel := context.WithCancel(context.Background())
return &Manager{
m := &Manager{
localID: localID,
cbHandler: h,
logger: logger,
@@ -155,6 +169,10 @@ func NewManager(localID LocalID, h CallbackHandler, logger *slog.Logger) *Manage
peersByAddr: make(map[netip.AddrPort]RemoteID),
genCounter: make(map[RemoteID]uint64),
}
for _, opt := range opts {
opt(m)
}
return m
}
// Start installs the data-path transport and begins its inbound delivery. The Manager
@@ -566,7 +584,7 @@ func (m *Manager) pushDataPath(remoteID RemoteID, msg []byte) error {
}
func (m *Manager) binding(remoteID RemoteID) Binding {
return Binding{LocalID: []byte(m.localID), RemoteID: []byte(remoteID)}
return Binding{LocalID: []byte(m.localID), RemoteID: []byte(remoteID), Secret: m.secret}
}
func newExchangeID() (ExchangeID, error) {
client/internal/pqkem/secret_test.go
+66
View File
@@ -0,0 +1,66 @@
package pqkem
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// pairWithSecrets builds a wired initiator ("bbbb") / responder ("aaaa") pair whose
// managers hold the given pre-shared secrets (nil = no secret).
func pairWithSecrets(t *testing.T, secretA, secretB []byte) (dA, dB *Manager, wgA, wgB *fakeWG) {
t.Helper()
sw := newSwitch()
wgA, wgB = newFakeWG(), newFakeWG()
dA = NewManager("aaaa", wgA, nil, WithPresharedSecret(secretA))
dB = NewManager("bbbb", wgB, nil, WithPresharedSecret(secretB))
dA.Start(&loopback{ep: epA, sw: sw})
dB.Start(&loopback{ep: epB, sw: sw})
dA.AddPeer("bbbb", epB)
dB.AddPeer("aaaa", epA)
t.Cleanup(func() {
dA.Stop()
dB.Stop()
})
return dA, dB, wgA, wgB
}
// TestManager_PresharedSecretBindsPSK covers the configured-secret contract (a host
// option like WireGuard's --pre-shared-key: only peers holding the same secret may
// communicate). Without binding the secret into the derivation, two peers with
// different secrets derived the same PSK and the secret was silently bypassed. The PSK
// must match only when the secrets match, for the bootstrap and for a rotation alike.
func TestManager_PresharedSecretBindsPSK(t *testing.T) {
secret1 := []byte("configured-secret-one-0123456789")
secret2 := []byte("configured-secret-two-0123456789")
cases := map[string]struct {
a, b []byte
match bool
}{
"same secret": {a: secret1, b: secret1, match: true},
"different secrets": {a: secret1, b: secret2, match: false},
"only the responder has one": {a: secret1, b: nil, match: false},
"only the initiator has one": {a: nil, b: secret1, match: false},
"no secret": {a: nil, b: nil, match: true},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
dA, dB, wgA, wgB := pairWithSecrets(t, tc.a, tc.b)
bootstrap(t, dA, dB)
bootA, bootB := wgA.psk("bbbb"), wgB.psk("aaaa")
require.NotEqual(t, PSK{}, bootA, "responder must derive a bootstrap PSK")
require.NotEqual(t, PSK{}, bootB, "initiator must derive a bootstrap PSK")
assert.Equal(t, tc.match, bootA == bootB, "bootstrap PSKs must match only when the secrets match")
// Rotation over the data path re-derives the PSK; it must honour the secret too.
dB.OnDataPathRekeyed("aaaa", 0)
require.Eventually(t, func() bool {
return wgA.psk("bbbb") != bootA && wgB.psk("aaaa") != bootB
}, time.Second, 5*time.Millisecond, "the rotation must re-key both peers")
assert.Equal(t, tc.match, wgA.psk("bbbb") == wgB.psk("aaaa"), "rotated PSKs must match only when the secrets match")
})
}
}