Allow non strict mode

This commit is contained in:
riccardom
2026-08-04 18:04:10 +02:00
parent 6d0f6c4bb8
commit 915266ffff
5 changed files with 154 additions and 1 deletions

View File

@@ -0,0 +1,64 @@
package pqkem
import (
"testing"
"github.com/stretchr/testify/require"
)
// TestManager_NonCapablePeerNotOffered: a peer known not to run the KEM (it advertised
// no PQ port over signalling) is never offered an exchange, and no failure is raised —
// this is what stops the reoffer storm against non-PQ peers (e.g. Rosenpass peers).
func TestManager_NonCapablePeerNotOffered(t *testing.T) {
wg := newFakeWG()
d := NewManager("bbbb", wg, nil) // initiator vs "aaaa"
d.Start(&loopback{ep: epB, sw: newSwitch()})
defer d.Stop()
d.MarkNonCapable("aaaa")
offer, err := d.SignalOffer("aaaa")
require.NoError(t, err)
require.Nil(t, offer, "a non-capable peer must not be offered a KEM exchange")
require.Empty(t, wg.failed, "a non-capable peer must not raise a rekey failure")
}
// TestManager_MarkNonCapableCancelsInFlight: if we start an exchange with a peer whose
// capability is not yet known and then learn it does not run the KEM, the in-flight
// exchange is cancelled and no further offer is produced (no timeout -> no failure).
func TestManager_MarkNonCapableCancelsInFlight(t *testing.T) {
wg := newFakeWG()
d := NewManager("bbbb", wg, nil)
d.Start(&loopback{ep: epB, sw: newSwitch()})
defer d.Stop()
// Capability unknown -> the bootstrap offer goes out optimistically.
offer, err := d.SignalOffer("aaaa")
require.NoError(t, err)
require.NotNil(t, offer)
// Now we learn the peer is non-PQ: the exchange must be dropped.
d.MarkNonCapable("aaaa")
next, err := d.SignalOffer("aaaa")
require.NoError(t, err)
require.Nil(t, next, "after learning non-capability the peer is no longer offered")
require.Empty(t, wg.failed, "cancelling an in-flight exchange must not raise a failure")
}
// TestManager_EstablishedPeerNotDowngraded: a stray zero-port observation must not tear
// down a peer we already have a working PQ session with.
func TestManager_EstablishedPeerNotDowngraded(t *testing.T) {
dA, dB, _, wgB, _ := pair(t)
defer dA.Stop()
defer dB.Stop()
bootstrap(t, dA, dB)
require.NotEqual(t, PSK{}, wgB.psk("aaaa"), "established a PSK")
dB.MarkNonCapable("aaaa") // stray zero after establishment
offer, err := dB.SignalOffer("aaaa")
require.NoError(t, err)
require.NotNil(t, offer, "an established peer must keep running the KEM despite a stray zero")
}

View File

@@ -108,6 +108,7 @@ func (m *Manager) processOffer(remoteID RemoteID, o *OfferMsg) ([]byte, error) {
ex.lastSent = raw
ex.pendingPSK = psk
m.psks[remoteID] = psk
m.capable[remoteID] = true // a real KEM offer proves the peer runs the exchange
m.mu.Unlock()
m.trace("pqkem: new PSK derived", "peer", remoteID, "exchange", idHex(o.ExchangeID), "role", "responder", "psk_fp", pskFingerprint(psk))
@@ -153,6 +154,7 @@ func (m *Manager) processAnswer(remoteID RemoteID, a *AnswerMsg) error {
m.established[remoteID] = true
m.failures[remoteID] = 0
m.psks[remoteID] = psk
m.capable[remoteID] = true // a real KEM answer proves the peer runs the exchange
m.mu.Unlock()
m.trace("pqkem: new PSK derived", "peer", remoteID, "exchange", idHex(a.ExchangeID), "role", "initiator", "psk_fp", pskFingerprint(psk))

View File

@@ -107,6 +107,7 @@ type Manager struct {
established map[RemoteID]bool // peer has completed at least one exchange
failures map[RemoteID]int // consecutive rekey failures per peer
psks map[RemoteID]PSK // latest derived PSK per peer (pulled at WG peer-config time)
capable map[RemoteID]bool // peer runs the KEM (advertised a PQ port); false = known non-capable
peerAddrs map[RemoteID]netip.AddrPort // remoteID -> data-path endpoint (send routing)
peersByAddr map[netip.AddrPort]RemoteID // reverse: source endpoint -> remoteID (inbound)
wait sync.WaitGroup
@@ -133,6 +134,7 @@ func NewManager(localID LocalID, h CallbackHandler, logger *slog.Logger) *Manage
established: make(map[RemoteID]bool),
failures: make(map[RemoteID]int),
psks: make(map[RemoteID]PSK),
capable: make(map[RemoteID]bool),
peerAddrs: make(map[RemoteID]netip.AddrPort),
peersByAddr: make(map[netip.AddrPort]RemoteID),
}
@@ -184,7 +186,8 @@ func (m *Manager) trace(msg string, args ...any) {
}
// AddPeer registers where a peer's data-path messages are sent and received: its
// overlay endpoint (IP:port).
// overlay endpoint (IP:port). A peer that advertises a PQ endpoint is, by that fact,
// running the KEM, so it is marked capable.
func (m *Manager) AddPeer(remoteID RemoteID, endpoint netip.AddrPort) {
if !endpoint.IsValid() {
return
@@ -195,9 +198,35 @@ func (m *Manager) AddPeer(remoteID RemoteID, endpoint netip.AddrPort) {
}
m.peerAddrs[remoteID] = endpoint
m.peersByAddr[endpoint] = remoteID
m.capable[remoteID] = true
m.mu.Unlock()
}
// MarkNonCapable records that a peer does not run the KEM: it answered our offer with
// no KEM material over signalling (the capability signal is the peer's payload, not its
// optional data-path port). Any in-flight exchange is cancelled and further offers are
// suppressed (see SignalOffer), so a non-PQ peer never drives the rekey-recovery storm.
// An already-established peer is left untouched — a stray empty answer must not tear
// down a working PQ session.
func (m *Manager) MarkNonCapable(remoteID RemoteID) {
m.mu.Lock()
defer m.mu.Unlock()
if m.established[remoteID] {
return
}
if prev, ok := m.capable[remoteID]; ok && !prev {
return // already known non-capable, nothing to do
}
m.capable[remoteID] = false
if ex := m.exchanges[remoteID]; ex != nil {
if ex.cancel != nil {
ex.cancel()
}
delete(m.exchanges, remoteID)
}
m.trace("pqkem: peer advertises no PQ service — treating as non-capable, no KEM attempted", "peer", remoteID)
}
// RemovePeer stops any in-flight exchange for a peer and drops its state and routing.
func (m *Manager) RemovePeer(remoteID RemoteID) {
m.mu.Lock()
@@ -210,6 +239,7 @@ func (m *Manager) RemovePeer(remoteID RemoteID) {
delete(m.established, remoteID)
delete(m.failures, remoteID)
delete(m.psks, remoteID)
delete(m.capable, remoteID)
if ep, ok := m.peerAddrs[remoteID]; ok {
delete(m.peersByAddr, ep)
delete(m.peerAddrs, remoteID)
@@ -246,6 +276,10 @@ func (m *Manager) SignalOffer(remoteID RemoteID) ([]byte, error) {
return nil, nil
}
m.mu.Lock()
if capable, ok := m.capable[remoteID]; ok && !capable {
m.mu.Unlock()
return nil, nil // peer does not run the KEM; do not offer (avoids a failure/reoffer loop)
}
if ex := m.exchanges[remoteID]; ex != nil && ex.viaSignal && ex.state == stateAwaitingAnswer {
last := ex.lastSent
m.mu.Unlock()

View File

@@ -64,6 +64,14 @@ func (p pqHandshaker) OfferPayload(remoteKey string) ([]byte, int) {
func (p pqHandshaker) AnswerPayload(remoteKey string, recvOffer []byte) ([]byte, int) {
if len(recvOffer) == 0 {
// Capability signal (responder side): the KEM offer flows initiator->responder,
// so if we are the responder for this peer (it is the KEM initiator by role) an
// empty offer means it does not run the KEM. If we are the initiator, an empty
// offer is normal — the peer is the responder and puts its material in the
// answer — so we must not flag it.
if !p.mgr.IsInitiator(pqkem.RemoteID(remoteKey)) {
p.mgr.MarkNonCapable(pqkem.RemoteID(remoteKey))
}
return nil, p.mgr.LocalPort()
}
payload, err := p.mgr.SignalOnOffer(pqkem.RemoteID(remoteKey), recvOffer)
@@ -75,6 +83,14 @@ func (p pqHandshaker) AnswerPayload(remoteKey string, recvOffer []byte) ([]byte,
func (p pqHandshaker) OnAnswer(remoteKey string, recvAnswer []byte) {
if len(recvAnswer) == 0 {
// Capability signal (initiator side): the KEM answer flows responder->initiator,
// so an empty answer to our offer means the peer does not run the KEM — mark it
// non-capable to stop offering (no failure/reoffer storm). Only meaningful when
// we are the initiator: as the responder we also receive an (empty) answer to
// our own non-KEM offer from a perfectly capable peer, which must not be flagged.
if p.mgr.IsInitiator(pqkem.RemoteID(remoteKey)) {
p.mgr.MarkNonCapable(pqkem.RemoteID(remoteKey))
}
return
}
if err := p.mgr.SignalOnAnswer(pqkem.RemoteID(remoteKey), recvAnswer); err != nil {

View File

@@ -0,0 +1,37 @@
package internal
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal/pqkem"
)
type pqNoopHandler struct{}
func (pqNoopHandler) OnNewPSKReady(pqkem.RemoteID, pqkem.PSK) error { return nil }
func (pqNoopHandler) OnRekeyFailed(pqkem.RemoteID) error { return nil }
// TestPQAdapter_CapabilityRoleAware locks the role-aware capability signal: the KEM
// payload only flows initiator-offer -> responder-answer, so an empty message in the
// other direction comes from a perfectly capable peer and must NOT flag it. Only the
// message that should carry material (the answer we receive as initiator) marks a peer
// non-capable when empty.
func TestPQAdapter_CapabilityRoleAware(t *testing.T) {
// localID "zzzz" > "aaaa" => this manager is the KEM initiator for peer "aaaa".
mgr := pqkem.NewManager("zzzz", pqNoopHandler{}, nil)
defer mgr.Stop()
h := pqHandshaker{mgr: mgr}
// An empty OFFER from our peer is normal here: as the initiator's responder it puts
// its material in the answer, not the offer. It must not disable our offering.
h.AnswerPayload("aaaa", nil)
payload, _ := h.OfferPayload("aaaa")
require.NotNil(t, payload, "an empty offer from a responder-role peer must not mark it non-capable")
// An empty ANSWER to our offer means the peer does not run the KEM -> stop offering.
h.OnAnswer("aaaa", nil)
payload2, _ := h.OfferPayload("aaaa")
require.Nil(t, payload2, "an empty answer to our offer marks the peer non-capable, so we stop offering")
}