Reject out-of-order pairing messages

services/eap-noob/guards_test.go

@@ -0,0 +1,112 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package eapnoob
+
+import "testing"
+
+// expectStateMismatch asserts that a message was refused as out of order: a
+// terminal ErrStateMismatch, no success, and no transport error.
+func expectStateMismatch(t *testing.T, out Outcome, err error) {
+	t.Helper()
+	if err != nil {
+		t.Fatalf("Receive returned err %v; an out-of-order message must surface in out.Err", err)
+	}
+	if out.Err == nil || out.Err.Code != ErrStateMismatch {
+		t.Fatalf("out.Err = %v, want code %d (state mismatch)", out.Err, ErrStateMismatch)
+	}
+	if !out.Done || out.Success {
+		t.Fatalf("out Done=%v Success=%v, want a terminal failure", out.Done, out.Success)
+	}
+}
+
+// A completion message can arrive while the server is waiting for the PIN,
+// before its authentication keys exist. Before PIN verification the transcript
+// is public, so an empty-key MAC is computable without the PIN and must not
+// grant trust.
+func TestServerRejectsCompletionBeforeKeysAreDerived(t *testing.T) {
+	for _, suite := range []int{1, 2} {
+		srv := NewServer(ServerConfig{Cryptosuites: []int{suite}, Dirs: 2}, nil)
+		peer := NewPeer(PeerConfig{PreferDir: 2}, nil)
+		driveConversation(t, srv, peer)
+		if srv.State() != StateWaiting || len(srv.keyM.Kmp) != 0 {
+			t.Fatalf("suite %d: server state=%s Kmp=%d, want Waiting with no keys", suite, srv.State(), len(srv.keyM.Kmp))
+		}
+
+		macp := srv.suite.computeMAC(nil, 1, srv.in)
+		msg, _, err := encode(&wireMessage{
+			Type:   intp(typeCompletion),
+			PeerId: rawString(srv.peerId),
+			MACp:   rawString(b64Encode(macp)),
+		})
+		if err != nil {
+			t.Fatalf("encode: %v", err)
+		}
+
+		out, err := srv.Receive(msg)
+		expectStateMismatch(t, out, err)
+		if srv.State() == StateRegistered {
+			t.Fatalf("suite %d: server registered on an empty-key MAC", suite)
+		}
+	}
+}
+
+// A sender can answer discovery with a key-exchange response before
+// negotiation selects an algorithm and creates the server's private key.
+func TestServerRejectsKeyExchangeBeforeNegotiation(t *testing.T) {
+	srv := NewServer(ServerConfig{}, nil)
+	if _, err := srv.Start(); err != nil {
+		t.Fatalf("server Start: %v", err)
+	}
+	out, err := srv.Receive([]byte(`{"Type":3,"PKp":{},"Np":""}`))
+	expectStateMismatch(t, out, err)
+	if srv.State() != StateUnregistered {
+		t.Fatalf("server state %s, want Unregistered", srv.State())
+	}
+}
+
+// A sender can skip negotiation after discovery. The peer must reject its
+// key-exchange message before touching the algorithm negotiation selects.
+func TestPeerRejectsKeyExchangeBeforeNegotiation(t *testing.T) {
+	peer := NewPeer(PeerConfig{}, nil)
+	discovery, err := peer.Receive([]byte(`{"Type":1}`))
+	if err != nil || discovery.Err != nil {
+		t.Fatalf("discovery: err=%v out.Err=%v", err, discovery.Err)
+	}
+	out, err := peer.Receive([]byte(`{"Type":3,"PKs":{},"Ns":""}`))
+	expectStateMismatch(t, out, err)
+	if peer.State() != StateUnregistered {
+		t.Fatalf("peer state %s, want Unregistered", peer.State())
+	}
+}
+
+// EAP-Success only means something once the peer has verified the server's
+// Completion MAC. Earlier, it must not register the association.
+func TestPeerRejectsSuccessBeforeVerifiedCompletion(t *testing.T) {
+	srv := NewServer(ServerConfig{Dirs: 2}, nil)
+	peer := NewPeer(PeerConfig{PreferDir: 2}, nil)
+	driveConversation(t, srv, peer)
+
+	out, err := peer.Receive(resultBytes(eapSuccess))
+	expectStateMismatch(t, out, err)
+	if peer.State() == StateRegistered {
+		t.Fatal("peer registered on an unverified EAP-Success")
+	}
+	if _, ok, _ := peer.store.Load(peer.peerId); ok {
+		t.Fatal("peer stored an association on an unverified EAP-Success")
+	}
+}
+
+// Any message between the verified Completion and EAP-Success clears the
+// verification, a malformed one included.
+func TestPeerSuccessNeedsTheCompletionDirectlyBeforeIt(t *testing.T) {
+	srv := NewServer(ServerConfig{Dirs: 2}, nil)
+	peer := NewPeer(PeerConfig{PreferDir: 2}, nil)
+	driveConversation(t, srv, peer)
+	peer.completionVerified = true // as after a verified Completion request
+	if _, err := peer.Receive([]byte("not json")); err != nil {
+		t.Fatalf("malformed input: %v", err)
+	}
+	out, err := peer.Receive(resultBytes(eapSuccess))
+	expectStateMismatch(t, out, err)
+}

services/eap-noob/peer.go

@@ -35,6 +35,10 @@ type Peer struct {
 	cfg   PeerConfig
 	store Store
 	state State
+	// completionVerified is set only while the last message the peer handled
+	// was a Completion request whose MACs verified. EAP-Success is accepted
+	// only then: anything else would register on the server's word alone.
+	completionVerified bool
 
 	methodState
 
@@ -86,11 +90,13 @@ func (p *Peer) Export(label string, context []byte, length int) ([]byte, error)
 func (p *Peer) Receive(in []byte) (Outcome, error) {
 	wm, err := decode(in)
 	if err != nil {
+		p.completionVerified = false
 		return p.fail(ErrInvalidMessageStructure, "malformed JSON")
 	}
 	if wm.EAP != "" {
 		return p.onResult(wm)
 	}
+	p.completionVerified = false
 	if wm.ErrorCode != nil {
 		return Outcome{State: p.state, Done: true, Err: &ProtocolError{Code: *wm.ErrorCode, Info: wm.ErrorInfo}}, nil
 	}
@@ -116,7 +122,12 @@ func (p *Peer) Receive(in []byte) (Outcome, error) {
 }
 
 func (p *Peer) onResult(wm *wireMessage) (Outcome, error) {
+	verified := p.completionVerified
+	p.completionVerified = false
 	if wm.EAP == eapSuccess {
+		if !verified {
+			return p.fail(ErrStateMismatch, "success before verified Completion")
+		}
 		assoc := &Association{
 			PeerId:       p.peerId,
 			NAI:          p.nai,
@@ -230,6 +241,9 @@ func (p *Peer) onNegotiationRequest(wm *wireMessage) (Outcome, error) {
 }
 
 func (p *Peer) onKeyExchangeRequest(wm *wireMessage) (Outcome, error) {
+	if p.state != StateUnregistered || p.suite == nil {
+		return p.fail(ErrStateMismatch, "key exchange requires negotiation during the initial exchange")
+	}
 	jwk, err := rawToJWK(wm.PKs)
 	if err != nil {
 		return p.fail(ErrInvalidData, "bad PKs")
@@ -342,6 +356,7 @@ func (p *Peer) onCompletionRequest(wm *wireMessage) (Outcome, error) {
 	if err != nil {
 		return Outcome{}, err
 	}
+	p.completionVerified = true
 	return Outcome{Send: b, State: p.state}, nil
 }
 

services/eap-noob/server.go

@@ -264,6 +264,9 @@ func (s *Server) buildKeyExchangeRequest() (Outcome, error) {
 }
 
 func (s *Server) onKeyExchange(wm *wireMessage) (Outcome, error) {
+	if s.state != StateUnregistered || s.suite == nil || s.priv == nil {
+		return s.fail(ErrStateMismatch, "key exchange requires negotiation during the initial exchange")
+	}
 	if err := s.checkPeerId(wm); err != nil {
 		return s.fail(ErrUnexpectedPeerIdentier, err.Error())
 	}
@@ -365,6 +368,12 @@ func (s *Server) buildCompletionRequest() (Outcome, error) {
 }
 
 func (s *Server) onCompletion(wm *wireMessage) (Outcome, error) {
+	// Completion follows the OOB step: Waiting in the server-to-peer direction,
+	// OOBReceived in the peer-to-server direction. Either way the keys must
+	// already be derived, or MACp would be checked against an empty key.
+	if (s.state != StateWaiting && s.state != StateOOBReceived) || s.suite == nil || len(s.keyM.Kmp) == 0 {
+		return s.fail(ErrStateMismatch, "completion before authentication keys are ready")
+	}
 	if err := s.checkPeerId(wm); err != nil {
 		return s.fail(ErrUnexpectedPeerIdentier, err.Error())
 	}

services/nvpair-cluster-manager/httpserver.go

@@ -195,19 +195,28 @@ func (m *Manager) handlePairingInitial(w http.ResponseWriter, env *pairingEnvelo
 		return
 	}
 
-	sess.mu.Lock()
-	out, err := sess.peer.Receive(msg)
+	var completed *Invite
+	// Scope the lock so a panic releases it and notifications run after
+	// unlocking. A terminal protocol error releases the session too, or a
+	// sender repeating malformed exchanges would hold pending-session slots
+	// until they expire.
+	out, err := func() (eapnoob.Outcome, error) {
+		sess.mu.Lock()
+		defer sess.mu.Unlock()
+		out, err := sess.peer.Receive(msg)
+		if err != nil || out.Err != nil {
+			m.deleteSession(env.InviteID)
+			return out, err
+		}
+		if out.State == eapnoob.StateWaiting && !sess.invitedEmitted {
+			completed = m.prepareJoinerInitialComplete(env.InviteID, sess)
+		}
+		return out, nil
+	}()
 	if err != nil {
-		m.deleteSession(env.InviteID)
-		sess.mu.Unlock()
 		http.Error(w, "eap-noob: "+err.Error(), http.StatusBadRequest)
 		return
 	}
-	var completed *Invite
-	if out.State == eapnoob.StateWaiting && !sess.invitedEmitted {
-		completed = m.prepareJoinerInitialComplete(env.InviteID, sess)
-	}
-	sess.mu.Unlock()
 	if completed != nil {
 		m.onJoinerInitialComplete(completed, sess)
 	}

services/nvpair-cluster-manager/pairing_order_test.go

@@ -0,0 +1,57 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"encoding/base64"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"eapnoob"
+)
+
+// Skipping negotiation used to panic while holding the session lock, leaving
+// a session expiry could never reclaim. It must now fail as a protocol error
+// and release the pending-session slot.
+func TestPairingInitialRejectsKeyExchangeBeforeNegotiation(t *testing.T) {
+	m := testManagerAt(t, t.TempDir(), 15211)
+	env := &pairingEnvelope{InviteID: "skipped-negotiation", Phase: "initial"}
+
+	discovery := httptest.NewRecorder()
+	m.handlePairingInitial(discovery, env, []byte(`{"Type":1}`), "127.0.0.1")
+	if discovery.Code != http.StatusOK {
+		t.Fatalf("discovery status %d, want 200", discovery.Code)
+	}
+	if _, ok := m.getSession(env.InviteID); !ok {
+		t.Fatal("discovery did not register a joiner session")
+	}
+
+	response := httptest.NewRecorder()
+	m.handlePairingInitial(response, env, []byte(`{"Type":3,"PKs":{},"Ns":""}`), "127.0.0.1")
+	if response.Code != http.StatusOK {
+		t.Fatalf("key-exchange status %d, want 200 carrying an EAP-NOOB error", response.Code)
+	}
+	var reply pairingEnvelope
+	if err := json.Unmarshal(response.Body.Bytes(), &reply); err != nil {
+		t.Fatalf("decode envelope: %v", err)
+	}
+	message, err := base64.StdEncoding.DecodeString(reply.Msg)
+	if err != nil {
+		t.Fatalf("decode message: %v", err)
+	}
+	var failure struct {
+		ErrorCode int `json:"ErrorCode"`
+	}
+	if err := json.Unmarshal(message, &failure); err != nil {
+		t.Fatalf("decode failure: %v", err)
+	}
+	if failure.ErrorCode != eapnoob.ErrStateMismatch {
+		t.Fatalf("ErrorCode %d, want %d (state mismatch)", failure.ErrorCode, eapnoob.ErrStateMismatch)
+	}
+	if _, ok := m.getSession(env.InviteID); ok {
+		t.Fatal("a terminal protocol error must release the pending session slot")
+	}
+}

services/versions.json

@@ -13,7 +13,7 @@
     "nvpair-node-settings": "3.1.0",
     "nvpair-ui-broker": "2.4.1",
     "nvpair-engine-manager": "1.3.0",
-    "nvpair-cluster-manager": "2.1.0",
+    "nvpair-cluster-manager": "2.1.1",
     "nvpair-job-scheduler": "1.1.0",
     "nvpair-tui": "1.2.0",
     "omlx-proxy": "1.4.1"