Skip to content

Simplify pairing code and reject out-of-order messages - #184

Open
kjlubick wants to merge 6 commits into
NVIDIA:developfrom
kjlubick:simplify-pairing-crypto
Open

kjlubick wants to merge 6 commits into
NVIDIA:developfrom
kjlubick:simplify-pairing-crypto

Conversation

@kjlubick

Copy link
Copy Markdown
Collaborator

Changelog title

Simplify pairing code and reject out-of-order messages

Changelog body

Pairing rejects premature completion and key-exchange messages, preventing authentication bypasses, crashes, and stuck pairing sessions. Unused cryptography functionality has been removed.

Bumps

  • services: patch
  • nvpair-cluster-manager: patch
  • nvpair-engine-manager: none
  • nvpair-errors: none
  • nvpair-job-scheduler: none
  • nvpair-manual-nodes: none
  • nvpair-node-info: none
  • nvpair-node-scanner: none
  • nvpair-node-settings: none
  • nvpair-proxy: none
  • nvpair-tui: none
  • nvpair-ui-broker: none
  • nvpair-workload-manager: none

Summary

The in-tree EAP-NOOB component exposed a general-purpose authentication and key-export API, while PAIR only needs PIN-authenticated certificate pairing. The extra configuration, persistence, and cryptographic code increased the amount of code we had to maintain and review.

Trim the component to PAIR's requirements:

  • Remove unused application-secret export, its handwritten HKDF, association storage, and unused key fields.
  • Replace public configuration knobs and map conversions with typed PairingInfo inputs and fixed protocol settings.
  • Avoid generating discarded fingerprints and OOB envelopes in the PIN flow.
  • Require verified completion before accepting success, and require authentication keys before verifying the peer's completion MAC.
  • Reject key exchange before negotiation. Release session locks reliably and remove failed sessions on terminal protocol errors.
  • Update documentation and examples, and organize regression tests beside the code they exercise.

Compatibility and scope

Retain both supported cryptosuites, the PIN encoding, authenticated certificate exchange, certificate pinning, mTLS, and durable membership formats. The full server-to-peer OOB-message path remains available for planned high-entropy pairing codes.

The internal Go pairing API changes. The small handwritten completion KDF remains, using standard-library primitives. Replacing the low-entropy six-digit PIN is outside this change.

Validation

Ran tests using go test

Suggested review order: go commit by commit

Retain only Completion authentication keys and make PIN NOOB setup avoid discarded OOB material. Require verified Completion before peer success, update PAIR callers and PIN checks, and remove unused application-key persistence and export APIs.

Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
Accept typed PairingInfo values directly, remove map conversions and forwarding constructors, and name the supported cryptosuite identifiers. Fix PAIR protocol policy while retaining both algorithms and authenticated transcript inputs. Update callers and colocate behavior tests with their implementations.

Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
Share negotiation setup and message drivers without removing test cases. Explain the typed pairing API and protocol terms, document required X25519 support and the retained standard-library-backed pairing KDF, and keep the usage example concise.

Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
An out-of-order Completion response could reach the inviter after the Initial Exchange but before the PIN was supplied and authentication keys were derived. onCompletion checked MACp using the still-empty Kmp and could mark pairing Registered when given an empty-key MAC over the public transcript.

Require the server to be Waiting with a selected cryptosuite and a derived peer-authentication key before processing Completion. Reject premature responses with ErrStateMismatch.

Add regression coverage for X25519 and P-256. Both cases fail without the guard and pass with it. The eap-noob suite, affected cluster-manager tests, and successful-pairing and wrong-PIN integration tests pass.

Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
A sender could skip negotiation and send a key-exchange message before an
algorithm was selected. Both pairing roles could dereference a nil algorithm.
On the joiner, this happened while holding the pairing session mutex: HTTP
panic recovery left it locked, so expiry could never reclaim the session.
Repeating the sequence could exhaust the 64 pending-session slots and block
pairing until restart. On the inviter, a malformed response was processed
outside HTTP panic recovery and could crash the cluster manager.

Require negotiation before either role processes key exchange, and require
the inviter's private key and the initial-exchange state. Scope joiner message
processing with a deferred session unlock, remove sessions on terminal
protocol errors as well as processing errors, and notify after unlocking.

Add focused regressions for both roles and HTTP session cleanup. The new
inviter regression reproduces the panic before the guard and passes with it.
The eap-noob and cluster-manager suites and successful-pairing and wrong-PIN
integration tests pass.

Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
Use suiteByID as the single place that validates and constructs a cryptosuite.
Keep the first supported suite from the server's offer instead of checking
support and then looking up the same suite again. Preserve both algorithms,
their identifiers, and the server's preference order.

Remove the unused SleepTime wire field and reuse the transcript's encoded
NOOB instead of storing a duplicate. Replace the marshal-then-unmarshal
encoding helper with direct json.Marshal calls.

Merge the duplicate wrong-PIN scenarios into the test that verifies the
terminal protocol error reaches the peer. Retain assertions that neither
side registers, and shorten the explanation of the incorrect-pin contract.

The eap-noob and cluster-manager suites, successful-pairing and wrong-PIN
integration tests, service-contract check, and SPDX header check pass.

Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant