From 34a716c4e31d01a30e91ce96ad03aef5a17d3073 Mon Sep 17 00:00:00 2001 From: sec-check Date: Wed, 2 Sep 2026 11:51:41 -0400 Subject: [PATCH 1/3] [architect] refactor: remove dead pkg/hub wrapkey envelope-encryption subsystem wrapkey.go and wrapkey_store.go (747 LOC, 19 functions) implement X25519/AEAD hub-spoke envelope encryption that is unreachable from every binary (hive, hivectl, apiproxy, bd, hive-backup) per 'deadcode ./cmd/...'. No production file outside the pair references any of its symbols, and no documentation mentions the feature. Only wrapkey_test.go exercised it. Refs #5694 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: sec-check --- src/pkg/hub/wrapkey.go | 423 ------------------ src/pkg/hub/wrapkey_store.go | 324 -------------- src/pkg/hub/wrapkey_test.go | 843 ----------------------------------- 3 files changed, 1590 deletions(-) delete mode 100644 src/pkg/hub/wrapkey.go delete mode 100644 src/pkg/hub/wrapkey_store.go delete mode 100644 src/pkg/hub/wrapkey_test.go diff --git a/src/pkg/hub/wrapkey.go b/src/pkg/hub/wrapkey.go deleted file mode 100644 index f9f6ec119..000000000 --- a/src/pkg/hub/wrapkey.go +++ /dev/null @@ -1,423 +0,0 @@ -package hub - -import ( - "crypto/aes" - "crypto/cipher" - "crypto/ecdh" - "crypto/hmac" - "crypto/rand" - "crypto/sha256" - "encoding/hex" - "errors" - "fmt" - "time" -) - -// Wrapped master delivery to pull-only spokes — the sealing primitive. -// -// WHY THIS EXISTS. The master (HIVE_HUB_SECRET) is the ONE value a spoke cannot -// self-derive; every other per-hive value is a pure function of it plus HIVE_ID -// (SpokeHeartbeatKey, SpokeInviteKey, spokeDomainKey, SpokeSSOPublicKey — see -// hub_keys.go). 44 of 66 spokes sit on pull_only clusters the hub cannot write -// to BY DESIGN (saas_provision.go KubectlReachable), so rotation cannot deliver -// a new master to them and RESIDUAL-1 (strip the plaintext master) is blocked -// behind the same boundary. -// -// Option D closes it without weakening pull_only: the spoke generates a -// keypair, keeps the private half on its PVC, publishes the public half over -// its own OUTBOUND heartbeat, and the hub seals each new master to that key. -// The delivery channel need not be confidential, because the payload is already -// sealed to a key only the recipient holds. Nothing here asks the hub to -// initiate a connection to a spoke. -// -// WHY X25519 AND NOT Ed25519. The repo's existing asymmetric material is -// Ed25519 (sso.go, hub_cookie.go, hub_pubkey_generations.go). Ed25519 is a -// SIGNING key and CANNOT encrypt. Reusing it here would require either the -// Ed25519->X25519 birational map — sharp-edged, easy to get wrong, and it -// creates one key used under two algorithms — or an ad-hoc scheme. Neither is -// acceptable for the one payload whose compromise is fleet-wide. This is a new -// key type, deliberately. See TestWrapKeyNeverAcceptsEd25519Material, which -// asserts no Ed25519-sized-or-shaped material can enter this path, because -// "unify the key types" is exactly the plausible future refactor and a -// behavioural test would not notice the crypto silently weakening. -// -// WHY NO HKDF. hub_keys.go:31-33 records the deliberate choice to avoid -// x/crypto/hkdf as a direct module dependency and to use single-block -// HMAC-SHA256 expansion instead. That reasoning holds identically here — fixed, -// unique context labels and one 32-byte output — and OQ-3 was decided to follow -// the precedent rather than reverse it. Adding a module to the one payload -// whose compromise is fleet-wide is the wrong place for new supply-chain -// surface. - -const ( - // wrapKeyLen is the X25519 key size in bytes, for both halves. Named rather - // than inlined so a reader does not have to know that 32 here means "an - // X25519 scalar" and 32 elsewhere in this file means "an AES-256 key". - wrapKeyLen = 32 - - // wrapAEADKeyLen is the AES-256 key size. AES-256-GCM has precedent in this - // repo (pkg/hubbackup) — precedent, not novelty. - wrapAEADKeyLen = 32 - - // wrapNonceLen is the GCM standard nonce size (96 bits). Every seal draws a - // fresh random nonce; nonces are never derived or counted, so there is no - // reuse hazard to reason about across restarts. - wrapNonceLen = 12 - - // infoWrapSharedSecret is the domain-separation label for the single-block - // HMAC-SHA256 expansion of the X25519 shared secret into the AEAD key. - // Versioned ("-v1") for the same reason every info string in hub_keys.go is: - // so the format can evolve without a verifier ever silently accepting a - // foreign-domain key. - infoWrapSharedSecret = "hive-wrap-master-v1" - - // infoWrapFingerprint is the domain-separation label for the public-key - // fingerprint. A fingerprint is a PUBLIC identifier that names a pinned key - // in logs, alerts and operator re-pin commands; it is separated from the - // AEAD-key label so a fingerprint can never collide with key material. - infoWrapFingerprint = "hive-wrap-fingerprint-v1" -) - -// wrapKeyMaxAge and wrapKeyOverlap are POLICY CHOICES, not derived values. -// -// There is no calculation behind either number and a future reader must not -// assume there is. They are recorded here with what they trade off so they can -// be changed DELIBERATELY rather than either treated as sacred or adjusted -// blindly. (OQ-4, decided by the operator 2026-08-14.) -// -// - wrapKeyMaxAge = 90 days bounds how long any single wrapping key is -// exposed. Shorter is more conservative but generates more republication -// churn; the constraint on the upper end of churn is the reconcile lane's -// perHiveEnvMaxPatchesPerCycle = 3 budget, since each patch rolls a tenant -// pod. 90 days is comfortably clear of making that a bottleneck. -// - wrapKeyOverlap = 24 hours is how long a spoke retains its PREVIOUS -// private key after rotating, so a master sealed to the old key and still -// in flight can be opened. It must exceed the time a full fleet -// convergence takes (~6 hours), with margin for a spoke that misses a -// cycle. 24 hours gives roughly 4x that. -// -// Safe to change deliberately. Not safe to change because a number "looked -// arbitrary". -const ( - wrapKeyMaxAge = 90 * 24 * time.Hour - wrapKeyOverlap = 24 * time.Hour -) - -var ( - // errWrapKeyMalformed is returned for any public or private key that is not - // exactly wrapKeyLen bytes of valid X25519 material. Callers must treat this - // as "no usable key", never as "accept something else" — an unparseable key - // must not silently unpin a good one (§8 row 1). - errWrapKeyMalformed = errors.New("wrap key: malformed X25519 key material") - - // errWrapOpenFailed is returned for every unseal failure — bad AAD, wrong - // recipient, tampered ciphertext, wrong generation. Deliberately ONE error - // with no detail: distinguishing "wrong hive" from "bad tag" would hand an - // attacker an oracle, and the caller's correct response is identical in - // every case (do not apply, do not ack, keep the current master). - errWrapOpenFailed = errors.New("wrap key: sealed payload did not open") -) - -// wrapPublicKey is a spoke's published X25519 public half. It is a PUBLIC -// value: it appears on the heartbeat, in the pin store, and in alerts. -type wrapPublicKey struct { - raw []byte -} - -// wrapPrivateKey is a spoke's X25519 private half. It NEVER leaves the pod and -// is never logged, never serialised into a heartbeat, and never returned by any -// hub API. Only the spoke holds one. -type wrapPrivateKey struct { - key *ecdh.PrivateKey -} - -// generateWrapKeypair produces a fresh X25519 keypair from crypto/rand. -// -// Spoke-side only. The hub never generates a wrapping keypair — if it did, it -// would hold the private half and the entire point of Option D (the hub cannot -// read what it seals) would be lost. -func generateWrapKeypair() (wrapPrivateKey, wrapPublicKey, error) { - priv, err := ecdh.X25519().GenerateKey(rand.Reader) - if err != nil { - return wrapPrivateKey{}, wrapPublicKey{}, fmt.Errorf("wrap key: generate: %w", err) - } - return wrapPrivateKey{key: priv}, wrapPublicKey{raw: priv.PublicKey().Bytes()}, nil -} - -// parseWrapPublicKey decodes a hex-encoded published public key. -// -// FAILS CLOSED. Anything that is not exactly wrapKeyLen bytes of valid X25519 -// material returns errWrapKeyMalformed and NO key. There is deliberately no -// lenient path, no truncation, and no padding: a caller that cannot parse a -// publication must count the hive as NOT having a usable key (which blocks -// retirement) rather than falling back to any other key. -func parseWrapPublicKey(hexEncoded string) (wrapPublicKey, error) { - raw, err := hex.DecodeString(hexEncoded) - if err != nil { - return wrapPublicKey{}, errWrapKeyMalformed - } - if len(raw) != wrapKeyLen { - return wrapPublicKey{}, errWrapKeyMalformed - } - // Round-trip through crypto/ecdh so the point is actually validated rather - // than merely length-checked. A 32-byte string is not necessarily a valid - // X25519 public key. - if _, err := ecdh.X25519().NewPublicKey(raw); err != nil { - return wrapPublicKey{}, errWrapKeyMalformed - } - return wrapPublicKey{raw: raw}, nil -} - -// parseWrapPrivateKey decodes a hex-encoded private key read from the spoke's -// PVC. Same fail-closed discipline as parseWrapPublicKey: a malformed key file -// yields no key, and the caller's correct response is to generate a fresh -// keypair (which then requires an operator re-pin — §8 row 7, deliberate). -func parseWrapPrivateKey(hexEncoded string) (wrapPrivateKey, error) { - raw, err := hex.DecodeString(hexEncoded) - if err != nil { - return wrapPrivateKey{}, errWrapKeyMalformed - } - if len(raw) != wrapKeyLen { - return wrapPrivateKey{}, errWrapKeyMalformed - } - k, err := ecdh.X25519().NewPrivateKey(raw) - if err != nil { - return wrapPrivateKey{}, errWrapKeyMalformed - } - return wrapPrivateKey{key: k}, nil -} - -// Hex renders the public key for publication on the heartbeat and for storage -// in the pin store. Hex for the same reason every derived key in hub_keys.go is -// hex: it is a safe ASCII value for an env var, a JSON field and a log line. -func (p wrapPublicKey) Hex() string { - if len(p.raw) != wrapKeyLen { - return "" - } - return hex.EncodeToString(p.raw) -} - -// hex renders the private key for persistence to the spoke PVC at 0600. -// Unexported and lowercase on purpose: this must be impossible to reach from -// any package that could serialise it outward. -func (p wrapPrivateKey) hex() string { - if p.key == nil { - return "" - } - return hex.EncodeToString(p.key.Bytes()) -} - -// publicKey returns the public half of a private key, so a spoke that loaded a -// key from disk can publish without storing the public half separately. -func (p wrapPrivateKey) publicKey() wrapPublicKey { - if p.key == nil { - return wrapPublicKey{} - } - return wrapPublicKey{raw: p.key.PublicKey().Bytes()} -} - -func (p wrapPublicKey) valid() bool { return len(p.raw) == wrapKeyLen } -func (p wrapPrivateKey) valid() bool { return p.key != nil } - -// wrapKeyFingerprint is the short PUBLIC identifier for a pinned key. It names -// a key in alerts, in the pin store, and in the operator's re-pin command — -// which is why it must be domain-separated from the AEAD key derivation: a -// value an operator reads off a dashboard must never be usable as key material. -func wrapKeyFingerprint(pub wrapPublicKey) string { - if !pub.valid() { - return "" - } - mac := hmac.New(sha256.New, []byte(infoWrapFingerprint)) - mac.Write(pub.raw) - return hex.EncodeToString(mac.Sum(nil)) -} - -// wrapAAD builds the additional authenticated data that binds a sealed payload -// to exactly one recipient and one generation. -// -// hiveID || 0x00 || generationID || 0x00 || fingerprint -// -// The 0x00 separators match derivePerHiveKey's convention (hub_keys.go:118-122) -// and exist for the same stated reason: hive IDs are attacker-influenced and -// plain concatenation is ambiguous, so ("ab","c") and ("a","bc") must not -// produce the same bytes. -// -// Each component closes a specific attack: -// - hiveID: a ciphertext sealed for X cannot be opened as Y even if misrouted -// or deliberately replayed cross-hive. -// - generationID: an attacker who captured a generation-N ciphertext cannot -// roll a spoke back to a retired master after N+1 has shipped. The channel -// is assumed NON-confidential, so replay resistance cannot come from it. -// - fingerprint: binds to the specific pinned key, so a payload sealed under -// a superseded wrapping key cannot be presented as current. -func wrapAAD(hiveID string, generationID int, fingerprint string) []byte { - aad := make([]byte, 0, len(hiveID)+len(fingerprint)+16) - aad = append(aad, []byte(hiveID)...) - aad = append(aad, 0) - aad = append(aad, []byte(fmt.Sprintf("%d", generationID))...) - aad = append(aad, 0) - aad = append(aad, []byte(fingerprint)...) - return aad -} - -// deriveWrapAEADKey expands an X25519 shared secret into the AES-256 key. -// -// Single-block HMAC-SHA256, following hub_keys.go's deliberate no-HKDF -// precedent (OQ-3). Both public keys are mixed in so the derived key is bound -// to BOTH parties — without that, a shared secret alone would not distinguish -// which ephemeral produced it. -func deriveWrapAEADKey(shared []byte, ephemeralPub, recipientPub wrapPublicKey) []byte { - mac := hmac.New(sha256.New, shared) - mac.Write([]byte(infoWrapSharedSecret)) - mac.Write([]byte{0}) - mac.Write(ephemeralPub.raw) - mac.Write([]byte{0}) - mac.Write(recipientPub.raw) - return mac.Sum(nil)[:wrapAEADKeyLen] -} - -// sealedPayload is the wire form of a wrapped master. -// -// Everything here is PUBLIC except by construction: the ephemeral public key is -// public, and the ciphertext is opaque to anyone without the recipient's -// private key. There is deliberately NO field carrying a plaintext secret — -// that would be Option B, which was rejected because it makes the heartbeat -// bearer a master-exfiltration primitive. -type sealedPayload struct { - // EphemeralPub is the hex X25519 public half of the fresh sender keypair - // generated for THIS seal. Fresh per wrap, which gives forward secrecy - // against later compromise of hub state and makes every ciphertext - // independently sealed. - EphemeralPub string `json:"ephemeral_pub"` - // Nonce is the hex 96-bit GCM nonce, fresh random per seal. - Nonce string `json:"nonce"` - // Ciphertext is the hex AES-256-GCM output (with tag). - Ciphertext string `json:"ciphertext"` - // GenerationID names the master generation sealed inside. It is REPEATED in - // the AAD; this copy exists only so the spoke can decide whether to attempt - // the open at all (refuse generation <= current) without first decrypting. - // The AAD copy is the authoritative one — a tampered GenerationID here fails - // the AEAD tag rather than being trusted. - GenerationID int `json:"generation_id"` - // Fingerprint names the wrapping key this was sealed to, so a spoke holding - // both a current and an overlap key knows which private half to try first. - // Also repeated in the AAD, and authoritative there for the same reason. - Fingerprint string `json:"fingerprint"` -} - -// sealForSpoke encrypts payload to a spoke's pinned wrapping key. -// -// HUB-SIDE. The hub holds no private half, generates a fresh ephemeral keypair -// per call, and discards it immediately — so the hub cannot re-open what it -// sealed, which is the property that makes a non-confidential delivery channel -// acceptable. -// -// payload is the caller's plaintext. Under OQ-2(c) it carries BOTH the new -// master AND the spoke's freshly derived heartbeat key for the incoming -// generation, sealed together — never as separate plaintext fields on the -// heartbeat response, which is what would turn Option D into the rejected -// Option B. -func sealForSpoke(recipient wrapPublicKey, hiveID string, generationID int, payload []byte) (sealedPayload, error) { - if !recipient.valid() { - return sealedPayload{}, errWrapKeyMalformed - } - if hiveID == "" { - // No identity means no AAD binding means a ciphertext openable by - // anyone holding any wrapping key. Refuse rather than seal something - // unbound. Mirrors derivePerHiveKey's refusal to derive without a hive - // ID (hub_keys.go:115-117), which the F2 banner names as the invariant - // that must never regain a bypass. - return sealedPayload{}, errors.New("wrap key: refusing to seal without a hive ID") - } - recipientPub, err := ecdh.X25519().NewPublicKey(recipient.raw) - if err != nil { - return sealedPayload{}, errWrapKeyMalformed - } - ephPriv, ephPub, err := generateWrapKeypair() - if err != nil { - return sealedPayload{}, err - } - shared, err := ephPriv.key.ECDH(recipientPub) - if err != nil { - return sealedPayload{}, fmt.Errorf("wrap key: ecdh: %w", err) - } - aeadKey := deriveWrapAEADKey(shared, ephPub, recipient) - block, err := aes.NewCipher(aeadKey) - if err != nil { - return sealedPayload{}, fmt.Errorf("wrap key: aes: %w", err) - } - gcm, err := cipher.NewGCM(block) - if err != nil { - return sealedPayload{}, fmt.Errorf("wrap key: gcm: %w", err) - } - nonce := make([]byte, wrapNonceLen) - if _, err := rand.Read(nonce); err != nil { - return sealedPayload{}, fmt.Errorf("wrap key: nonce: %w", err) - } - fp := wrapKeyFingerprint(recipient) - ct := gcm.Seal(nil, nonce, payload, wrapAAD(hiveID, generationID, fp)) - return sealedPayload{ - EphemeralPub: ephPub.Hex(), - Nonce: hex.EncodeToString(nonce), - Ciphertext: hex.EncodeToString(ct), - GenerationID: generationID, - Fingerprint: fp, - }, nil -} - -// openFromHub decrypts a sealed payload with the spoke's private wrapping key. -// -// SPOKE-SIDE. hiveID is supplied by the CALLER from its own local identity -// (/data/hive-id), never from the payload — taking it from the payload would -// let an attacker choose the AAD and defeat the cross-hive binding entirely. -// -// Every failure returns errWrapOpenFailed with no detail. On failure the spoke -// keeps its current master, does NOT apply, and does NOT ack, which is what -// makes the hub retry (§8 row 3). Applying a partially decoded master would -// brick the spoke. -func openFromHub(priv wrapPrivateKey, hiveID string, sp sealedPayload) ([]byte, error) { - if !priv.valid() || hiveID == "" { - return nil, errWrapOpenFailed - } - ephRaw, err := hex.DecodeString(sp.EphemeralPub) - if err != nil || len(ephRaw) != wrapKeyLen { - return nil, errWrapOpenFailed - } - ephPub, err := ecdh.X25519().NewPublicKey(ephRaw) - if err != nil { - return nil, errWrapOpenFailed - } - nonce, err := hex.DecodeString(sp.Nonce) - if err != nil || len(nonce) != wrapNonceLen { - return nil, errWrapOpenFailed - } - ct, err := hex.DecodeString(sp.Ciphertext) - if err != nil { - return nil, errWrapOpenFailed - } - shared, err := priv.key.ECDH(ephPub) - if err != nil { - return nil, errWrapOpenFailed - } - ownPub := priv.publicKey() - aeadKey := deriveWrapAEADKey(shared, wrapPublicKey{raw: ephRaw}, ownPub) - block, err := aes.NewCipher(aeadKey) - if err != nil { - return nil, errWrapOpenFailed - } - gcm, err := cipher.NewGCM(block) - if err != nil { - return nil, errWrapOpenFailed - } - // The AAD is rebuilt from the SPOKE's own identity and its OWN key's - // fingerprint — not from sp.Fingerprint. If the hub sealed to a different - // key, or to a different hive, the tag fails here. This is the line that - // makes cross-hive misdelivery and stale-key replay fail closed, so it must - // never be "fixed" to read the fingerprint off the payload. - aad := wrapAAD(hiveID, sp.GenerationID, wrapKeyFingerprint(ownPub)) - pt, err := gcm.Open(nil, nonce, ct, aad) - if err != nil { - return nil, errWrapOpenFailed - } - return pt, nil -} diff --git a/src/pkg/hub/wrapkey_store.go b/src/pkg/hub/wrapkey_store.go deleted file mode 100644 index 0ca01b8fc..000000000 --- a/src/pkg/hub/wrapkey_store.go +++ /dev/null @@ -1,324 +0,0 @@ -package hub - -import ( - "encoding/json" - "errors" - "fmt" - "os" - "path/filepath" - "strings" - "time" -) - -// Spoke-side wrapping-key lifecycle: generate, persist, load, rotate. -// -// STORAGE MIRRORS AN ESTABLISHED PATTERN rather than inventing one. The spoke -// already persists private key material on the same PVC at the same mode: -// spokeAppKeyPath = "/data/gh-app-key.pem" with spokeAppKeyFileMode = 0o600 and -// the comment "signing material must never be readable by anything else sharing -// the PVC or the pod" (src/cmd/hive/main.go:99,114-116). /data is the PVC mount -// in the spoke template, and /data/hive-id already establishes that -// identity-critical state persists there across restarts. -// -// PERSIST BEFORE USE. Step 2 of the lifecycle writes the key to disk BEFORE -// returning it to any caller, mirroring rotateMasterSecret's persist-before- -// install ordering and its stated rationale: never act on key material you -// might forget at the next roll. A key used but not persisted would be -// republished as a DIFFERENT key after the next pod roll, which the hub would -// correctly refuse as a pin mismatch — turning a write failure into an -// operator-intervention event on a cluster the hub cannot reach into. - -// spokeWrapKeyPath is where the spoke's X25519 private wrapping key lives on -// the PVC. A var rather than a const so tests can redirect it and exercise the -// real resolution order — the reason given at src/cmd/hive/main.go:95-97. -// Production never reassigns it. -// -// NOT YET READ BY ANY CALLER, deliberately kept. Every function in this file -// takes the path as an argument and the spoke-side lifecycle has no production -// entry point yet, so this declaration is the only record of where the key is -// supposed to land once it does. Deleting it to satisfy the linter would delete -// the answer, not the dead code — the first caller would then have to re-derive -// a path for private key material, which is exactly the kind of decision that -// should not be made twice. -// -//nolint:unused // staged lifecycle: documented production location, no caller yet (#4903) -var spokeWrapKeyPath = "/data/hive-wrap-key" - -// spokeWrapKeyFileMode is rw------- , identical to spokeAppKeyFileMode and for -// the identical reason: the wrapping private key is the ONLY thing standing -// between a sealed master and anything else sharing the PVC or the pod. -const spokeWrapKeyFileMode = 0o600 - -// spokeWrapKeyDirMode is rwx------ for the containing directory when it must be -// created. /data normally exists as the PVC mount; this covers the test and -// first-boot-before-mount cases without ever widening the directory. -const spokeWrapKeyDirMode = 0o700 - -// wrapKeyFile is the on-disk form. It is versioned and carries explicit -// timestamps because the overlap window is a SECOND dual-acceptance window and -// inherits the discipline the master generations carry: explicitly versioned -// (the previous key is a numbered entry with a fingerprint, not an unnamed `if` -// branch) and explicitly finite (it carries an expiry, and an expired key is -// EXCLUDED, not warned about). -type wrapKeyFile struct { - // Version guards the format. An unrecognised version is MALFORMED, not - // "probably fine" — see loadSpokeWrapKeys. - Version int `json:"version"` - // Current is the hex private key currently published and sealed to. - Current string `json:"current"` - // CurrentCreated is when Current was generated. Drives wrapKeyMaxAge. - CurrentCreated time.Time `json:"current_created"` - // Previous is the hex private key retained across a rotation so a master - // sealed to the old key and still in flight can be opened. Empty when there - // is no overlap in progress. - // - // NOTE THE ASYMMETRY THAT MAKES THIS SAFE, AND DO NOT REMOVE IT: retaining - // the old PRIVATE key spoke-side is what lets the spoke rotate WITHOUT - // asking the hub to accept a new public key. The hub-side pin is untouched - // by spoke-side rotation. If a future change instead makes the hub accept - // the new public key automatically, that is an auto-re-pin path and it - // silently degrades Option D into the rejected Option B. The adversarial - // review hunted specifically for such a path and found none; keep it that - // way. Spoke-side wrap-key rotation still requires a hub-side OPERATOR - // re-pin before the new key is sealed to. - Previous string `json:"previous,omitempty"` - // PreviousExpires is the hard expiry of Previous's acceptance. A ZERO value - // means ALREADY EXPIRED, never "never expires" — the same reading - // acceptableGenerations gives VerifyUntil (hub_generations.go:232-238). - // Reading it the other way would pin a superseded key live forever, which is - // the F1/F2 failure mode this project has spent five audits removing. - PreviousExpires time.Time `json:"previous_expires,omitempty"` -} - -const wrapKeyFileVersion = 1 - -// spokeWrapKeys is the in-memory view a spoke uses. -type spokeWrapKeys struct { - current wrapPrivateKey - currentCreated time.Time - previous wrapPrivateKey - previousExpires time.Time -} - -// errWrapKeyFileMalformed is returned when the on-disk key file cannot be -// trusted. Callers must treat it EXACTLY as they treat an absent file: generate -// fresh. There is deliberately no partial-recovery path — a half-readable key -// file is not evidence of anything. -var errWrapKeyFileMalformed = errors.New("wrap key file: malformed") - -// loadSpokeWrapKeys reads and validates the spoke's key file. -// -// Returns (keys, nil) on success, (zero, os.ErrNotExist) when absent, and -// (zero, errWrapKeyFileMalformed) for anything unparseable. The caller -// distinguishes absent from unreadable only for LOGGING — both lead to -// generate-fresh, because a spoke with no usable private key cannot open a -// sealed master either way. -// -// AN EXPIRED PREVIOUS KEY IS DROPPED, NOT WARNED ABOUT. That is the finiteness -// promise made real: the overlap window closes on its own without operator -// action, and a spoke cannot extend its own acceptance window. -func loadSpokeWrapKeys(path string, now time.Time) (spokeWrapKeys, error) { - raw, err := os.ReadFile(path) - if err != nil { - if os.IsNotExist(err) { - return spokeWrapKeys{}, os.ErrNotExist - } - // Present but unreadable. NOT the same as absent, and it must not be - // silently treated as "no key, all good" — this is the same - // absent-versus-unreadable distinction hub_generations_store.go makes. - // It still routes to generate-fresh, but the caller logs it loudly. - return spokeWrapKeys{}, fmt.Errorf("wrap key file: read: %w", err) - } - var f wrapKeyFile - if err := json.Unmarshal(raw, &f); err != nil { - return spokeWrapKeys{}, errWrapKeyFileMalformed - } - if f.Version != wrapKeyFileVersion { - return spokeWrapKeys{}, errWrapKeyFileMalformed - } - cur, err := parseWrapPrivateKey(strings.TrimSpace(f.Current)) - if err != nil { - return spokeWrapKeys{}, errWrapKeyFileMalformed - } - out := spokeWrapKeys{current: cur, currentCreated: f.CurrentCreated} - if strings.TrimSpace(f.Previous) != "" { - // A ZERO PreviousExpires means ALREADY EXPIRED. Same rule as - // acceptableGenerations. Dropping the key here rather than accepting it - // is what stops a hand-edited file with the field stripped from keeping - // a superseded wrapping key acceptable forever. - if !f.PreviousExpires.IsZero() && now.Before(f.PreviousExpires) { - prev, perr := parseWrapPrivateKey(strings.TrimSpace(f.Previous)) - if perr == nil { - out.previous = prev - out.previousExpires = f.PreviousExpires - } - // A malformed PREVIOUS key is dropped without failing the load: the - // current key is independently valid and the spoke can still - // publish and receive. Failing the whole load here would discard a - // good current key over a stale overlap entry. - } - } - return out, nil -} - -// persistSpokeWrapKeys writes the key file at 0600. -// -// Writes to a temp file in the same directory and renames, so a crash mid-write -// never leaves a truncated key file that the next boot would read as malformed -// and replace — which would produce a new public key, a pin mismatch, and an -// operator-intervention event on an unreachable cluster. -func persistSpokeWrapKeys(path string, keys spokeWrapKeys) error { - if !keys.current.valid() { - return errWrapKeyMalformed - } - f := wrapKeyFile{ - Version: wrapKeyFileVersion, - Current: keys.current.hex(), - CurrentCreated: keys.currentCreated.UTC(), - } - if keys.previous.valid() && !keys.previousExpires.IsZero() { - f.Previous = keys.previous.hex() - f.PreviousExpires = keys.previousExpires.UTC() - } - blob, err := json.Marshal(f) - if err != nil { - return fmt.Errorf("wrap key file: marshal: %w", err) - } - dir := filepath.Dir(path) - if err := os.MkdirAll(dir, spokeWrapKeyDirMode); err != nil { - return fmt.Errorf("wrap key file: mkdir: %w", err) - } - tmp, err := os.CreateTemp(dir, ".hive-wrap-key-*") - if err != nil { - return fmt.Errorf("wrap key file: temp: %w", err) - } - tmpName := tmp.Name() - // Chmod BEFORE writing: the key bytes must never exist on disk at a wider - // mode, not even for the microseconds between write and chmod. - if err := tmp.Chmod(spokeWrapKeyFileMode); err != nil { - // Best-effort cleanup: the chmod error above is what's returned, and a - // leftover temp file is a harmless artifact next run's os.CreateTemp - // won't collide with. - _ = tmp.Close() - _ = os.Remove(tmpName) - return fmt.Errorf("wrap key file: chmod: %w", err) - } - if _, err := tmp.Write(blob); err != nil { - // Best-effort cleanup: the write error above is what's returned. - _ = tmp.Close() - _ = os.Remove(tmpName) - return fmt.Errorf("wrap key file: write: %w", err) - } - if err := tmp.Close(); err != nil { - _ = os.Remove(tmpName) - return fmt.Errorf("wrap key file: close: %w", err) - } - if err := os.Rename(tmpName, path); err != nil { - // Best-effort cleanup: the rename error above is what's returned. - _ = os.Remove(tmpName) - return fmt.Errorf("wrap key file: rename: %w", err) - } - return nil -} - -// ensureSpokeWrapKeys is the ONE mechanism covering first boot, pod roll and -// PVC loss — they are one mechanism because they must be. -// -// 1. Read the file. If present and well-formed, use it. -// 2. If absent or malformed, generate a fresh keypair and PERSIST IT BEFORE -// returning it. -// -// A pod roll with the PVC intact is therefore a no-op: same key, same -// publication. PVC LOSS IS INDISTINGUISHABLE FROM FIRST BOOT by construction — -// the spoke generates a new keypair and publishes it. Whether the HUB should -// accept that new key is not decided here and must not be: it is refused as a -// pin mismatch and requires an operator re-pin. That availability cost is -// accepted DELIBERATELY, because the alternative is a design where losing a PVC -// and being attacked are indistinguishable and both silently succeed. -// -// Returns the keys and whether a fresh keypair was generated, so the caller can -// log the (operationally significant) difference. -func ensureSpokeWrapKeys(path string, now time.Time) (spokeWrapKeys, bool, error) { - keys, err := loadSpokeWrapKeys(path, now) - if err == nil && keys.current.valid() { - return keys, false, nil - } - priv, _, gerr := generateWrapKeypair() - if gerr != nil { - return spokeWrapKeys{}, false, gerr - } - fresh := spokeWrapKeys{current: priv, currentCreated: now} - if perr := persistSpokeWrapKeys(path, fresh); perr != nil { - // Persist-before-use: refuse to return a key we could not durably - // record. Returning it anyway would mean publishing a key that - // disappears at the next roll, producing a pin mismatch the operator - // then has to resolve on an unreachable cluster. - return spokeWrapKeys{}, false, perr - } - return fresh, true, nil -} - -// wrapKeyNeedsRotation reports whether the current key has exceeded -// wrapKeyMaxAge. -// -// A ZERO currentCreated reads as NEEDS ROTATION, not as "brand new". Same -// fail-closed direction as VerifyUntil.IsZero(): an unknown age must not read -// as a safe age. A hand-edited or migrated file missing the timestamp gets -// rotated rather than kept indefinitely. -func wrapKeyNeedsRotation(keys spokeWrapKeys, now time.Time) bool { - if !keys.current.valid() { - return true - } - if keys.currentCreated.IsZero() { - return true - } - return !now.Before(keys.currentCreated.Add(wrapKeyMaxAge)) -} - -// rotateSpokeWrapKeys generates a new current key and retains the OLD PRIVATE -// key for wrapKeyOverlap, so a master sealed to the old key and still in flight -// can be opened. -// -// SPOKE-LOCAL ONLY. This does not, and must not, cause the hub to accept the -// new public key. The hub keeps sealing to the PINNED key until an operator -// re-pins. That is why the overlap retains the old private half rather than -// asking for acceptance of a new public half — it is the construction that -// keeps the never-auto-re-pin guarantee intact through the one place the design -// itself introduces a legitimate key change. -func rotateSpokeWrapKeys(path string, keys spokeWrapKeys, now time.Time) (spokeWrapKeys, error) { - priv, _, err := generateWrapKeypair() - if err != nil { - return keys, err - } - next := spokeWrapKeys{ - current: priv, - currentCreated: now, - previous: keys.current, - previousExpires: now.Add(wrapKeyOverlap), - } - if perr := persistSpokeWrapKeys(path, next); perr != nil { - // Keep the existing keys on a persist failure. Rotating in memory only - // would publish a key that vanishes at the next roll. - return keys, perr - } - return next, nil -} - -// openWithSpokeKeys tries the current key, then the unexpired previous key. -// -// Order matters only for efficiency; correctness comes from the AEAD tag, which -// fails for the wrong key regardless of order. An EXPIRED previous key is not -// tried at all — loadSpokeWrapKeys never puts one in the struct. -func openWithSpokeKeys(keys spokeWrapKeys, hiveID string, sp sealedPayload) ([]byte, error) { - if keys.current.valid() { - if pt, err := openFromHub(keys.current, hiveID, sp); err == nil { - return pt, nil - } - } - if keys.previous.valid() { - if pt, err := openFromHub(keys.previous, hiveID, sp); err == nil { - return pt, nil - } - } - return nil, errWrapOpenFailed -} diff --git a/src/pkg/hub/wrapkey_test.go b/src/pkg/hub/wrapkey_test.go deleted file mode 100644 index 2093c904a..000000000 --- a/src/pkg/hub/wrapkey_test.go +++ /dev/null @@ -1,843 +0,0 @@ -package hub - -import ( - "bytes" - "crypto/ed25519" - "encoding/hex" - "encoding/json" - "os" - "path/filepath" - "reflect" - "regexp" - "strings" - "testing" - "time" -) - -// Tests for the Option D sealing primitive and the spoke-side wrapping-key -// lifecycle (src/docs/design/master-delivery-wrapped.md, PR #3833). -// -// THE HOUSE STANDARD THIS FILE IS WRITTEN TO. Fifteen tests in this repo have -// been found ENCODING vulnerabilities or passing for the wrong reason — -// heartbeatBearerOK's test literally asserting the hub "must accept raw master -// secret" (F24) is the canonical example, and F19's -// TestDesiredPerHiveEnvUsesCurrentNotPrevious passed while validating a code -// path production never takes. So: -// -// - Every negative assertion carries a POSITIVE CONTROL in the same test. A -// test asserting "decryption fails" passes trivially when decryption is -// broken outright; without the paired success case it asserts nothing. -// - Properties where a behavioural test would pass for the wrong reason get a -// SOURCE-ASSERTING test as well — with the subject chosen carefully, since -// the design's own §10 spec was found to assert a true property of the -// WRONG variable. - -// --- helpers ----------------------------------------------------------------- - -func mustWrapKeypair(t *testing.T) (wrapPrivateKey, wrapPublicKey) { - t.Helper() - priv, pub, err := generateWrapKeypair() - if err != nil { - t.Fatalf("generateWrapKeypair: %v", err) - } - return priv, pub -} - -// --- AAD BINDING: cross-hive, cross-generation, cross-key --------------------- - -// TestSealOpenRoundTrip is the POSITIVE CONTROL for every negative test below. -// Without it, an implementation whose Open always fails would pass the entire -// rest of this file. -func TestSealOpenRoundTrip(t *testing.T) { - priv, pub := mustWrapKeypair(t) - payload := []byte("master-generation-7-plus-heartbeat-key") - - sp, err := sealForSpoke(pub, "hive-alpha", 7, payload) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - got, err := openFromHub(priv, "hive-alpha", sp) - if err != nil { - t.Fatalf("openFromHub on the correctly-bound case failed: %v — every "+ - "negative assertion in this file is meaningless without this passing", err) - } - if !bytes.Equal(got, payload) { - t.Fatalf("round trip corrupted the payload: got %d bytes, want %d", len(got), len(payload)) - } - if sp.Ciphertext == hex.EncodeToString(payload) { - t.Fatal("ciphertext equals hex(plaintext) — the payload was not encrypted at all") - } - if strings.Contains(sp.Ciphertext, hex.EncodeToString(payload)) { - t.Fatal("plaintext appears inside the ciphertext") - } -} - -// TestAADBindsHiveID: a ciphertext sealed for hive X must not open as hive Y, -// even with the correct private key. This is what makes cross-hive -// misdelivery — accidental or deliberate — fail closed. -func TestAADBindsHiveID(t *testing.T) { - priv, pub := mustWrapKeypair(t) - sp, err := sealForSpoke(pub, "hive-alpha", 3, []byte("secret")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - - // Positive control FIRST, so a broken-open implementation cannot pass. - if _, err := openFromHub(priv, "hive-alpha", sp); err != nil { - t.Fatalf("positive control failed: correctly-bound open errored: %v", err) - } - - if _, err := openFromHub(priv, "hive-beta", sp); err == nil { - t.Error("a ciphertext sealed for hive-alpha opened as hive-beta — the AAD hiveID " + - "binding is not in force, so a misrouted or replayed payload crosses hives") - } -} - -// TestAADBindsGeneration: an attacker who captured a generation-N ciphertext -// must not be able to present it as any other generation. The channel is -// assumed NON-confidential, so replay resistance cannot come from the channel. -func TestAADBindsGeneration(t *testing.T) { - priv, pub := mustWrapKeypair(t) - sp, err := sealForSpoke(pub, "hive-alpha", 5, []byte("master-5")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - - if _, err := openFromHub(priv, "hive-alpha", sp); err != nil { - t.Fatalf("positive control failed: correctly-bound open errored: %v", err) - } - - // Rewrite the advertised generation, as a replaying attacker would. - replayed := sp - replayed.GenerationID = 4 - if _, err := openFromHub(priv, "hive-alpha", replayed); err == nil { - t.Error("a generation-5 ciphertext opened when presented as generation 4 — an attacker " + - "who captured an old ciphertext could roll a spoke back to a retired master") - } - forward := sp - forward.GenerationID = 6 - if _, err := openFromHub(priv, "hive-alpha", forward); err == nil { - t.Error("a generation-5 ciphertext opened when presented as generation 6") - } -} - -// TestAADBindsWrappingKey: a payload sealed to key A must not open with key B, -// and — critically — the AAD is rebuilt from the SPOKE's own key fingerprint, -// not from the fingerprint field on the payload. If openFromHub ever reads -// sp.Fingerprint into the AAD, an attacker chooses the AAD and the binding is -// gone. -func TestAADBindsWrappingKey(t *testing.T) { - privA, pubA := mustWrapKeypair(t) - privB, _ := mustWrapKeypair(t) - - sp, err := sealForSpoke(pubA, "hive-alpha", 2, []byte("secret")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - if _, err := openFromHub(privA, "hive-alpha", sp); err != nil { - t.Fatalf("positive control failed: open with the sealed-to key errored: %v", err) - } - if _, err := openFromHub(privB, "hive-alpha", sp); err == nil { - t.Error("a payload sealed to key A opened with key B's private half") - } - - // THE FINGERPRINT-TRUST CASE, AND WHY IT IS SHAPED LIKE THIS. - // - // The obvious construction — rewrite sp.Fingerprint to key B's and open with - // B — does NOT isolate the property. It fails whether or not openFromHub - // trusts sp.Fingerprint, because opening with a different key also changes - // the ECDH shared secret and therefore the AEAD key. The tag fails for an - // unrelated reason and the test passes for the wrong reason. (Verified: the - // regression replay for this property PASSED while neutered when written - // that way.) - // - // The isolating construction keeps the recipient key FIXED, so the ECDH - // shared secret is unchanged and the ONLY varying input is the fingerprint - // that goes into the AAD. - spoofed := sp - spoofed.Fingerprint = wrapKeyFingerprint(privB.publicKey()) - if _, err := openFromHub(privA, "hive-alpha", spoofed); err != nil { - t.Error("rewriting sp.Fingerprint broke the open for the CORRECT recipient — openFromHub " + - "is reading the fingerprint off the payload instead of rebuilding it from the " + - "spoke's own key, so an attacker who controls the payload controls the AAD") - } -} - -// TestOpenRebuildsAADFromOwnKeyNotPayload is the source-asserting half of the -// fingerprint-trust property, because the behavioural test above can only prove -// the negative direction (a rewritten fingerprint does not BREAK a good open). -// It cannot prove the positive one — that a payload whose fingerprint names a -// key the spoke does not hold is still rejected on the fingerprint's account — -// since any such payload also fails on the ECDH secret. -// -// The subject is chosen deliberately, per §10's worked example that a -// source-assertion is only as good as its choice of subject: assert that the -// AAD construction inside openFromHub passes wrapKeyFingerprint(ownPub) and NOT -// sp.Fingerprint. That is the property actually needed. -func TestOpenRebuildsAADFromOwnKeyNotPayload(t *testing.T) { - raw, err := os.ReadFile("wrapkey.go") - if err != nil { - t.Fatalf("read wrapkey.go: %v", err) - } - src := stripGoComments(string(raw)) - i := strings.Index(src, "func openFromHub(") - if i < 0 { - t.Fatal("openFromHub not found — this test is not reading the implementation it thinks it is") - } - body := src[i:] - if j := strings.Index(body[1:], "\nfunc "); j >= 0 { - body = body[:j+1] - } - if !strings.Contains(body, "wrapAAD(hiveID, sp.GenerationID, wrapKeyFingerprint(ownPub))") { - t.Error("openFromHub does not rebuild the AAD from the spoke's OWN key fingerprint. " + - "The AAD must be wrapAAD(hiveID, sp.GenerationID, wrapKeyFingerprint(ownPub)) — " + - "reading sp.Fingerprint instead lets an attacker who controls the payload choose " + - "the AAD, and cross-key replay stops failing closed") - } - if regexp.MustCompile(`wrapAAD\([^)]*sp\.Fingerprint`).MatchString(body) { - t.Error("openFromHub passes sp.Fingerprint into wrapAAD — the payload's own copy is " + - "attacker-controlled and must never be an AAD input") - } - // Positive control on the subject: hiveID IS supposed to come from the - // caller, so confirm the matcher can see real content and is not matching an - // empty body. - if !strings.Contains(body, "priv.key.ECDH(ephPub)") { - t.Fatal("the extracted openFromHub body does not contain its ECDH call — the extraction " + - "is wrong and every assertion above is vacuous") - } -} - -// TestSealRefusesWithoutHiveID mirrors derivePerHiveKey's refusal to derive -// without a hive ID (hub_keys.go:115-117). An unbound ciphertext is openable by -// anyone holding any wrapping key, so sealing one must be impossible. -func TestSealRefusesWithoutHiveID(t *testing.T) { - _, pub := mustWrapKeypair(t) - if _, err := sealForSpoke(pub, "", 1, []byte("secret")); err == nil { - t.Error("sealForSpoke produced a ciphertext with no hive ID in the AAD — F2's " + - "identity-binding invariant is re-opened through a new door") - } - // Positive control: with an ID it succeeds. - if _, err := sealForSpoke(pub, "hive-alpha", 1, []byte("secret")); err != nil { - t.Fatalf("positive control failed: sealForSpoke with a hive ID errored: %v", err) - } -} - -// TestOpenRefusesWithoutHiveID: the spoke supplies the hive ID from its own -// local identity, never from the payload. An empty one must fail closed. -func TestOpenRefusesWithoutHiveID(t *testing.T) { - priv, pub := mustWrapKeypair(t) - sp, err := sealForSpoke(pub, "hive-alpha", 1, []byte("secret")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - if _, err := openFromHub(priv, "", sp); err == nil { - t.Error("openFromHub opened a payload with an empty hive ID") - } - if _, err := openFromHub(priv, "hive-alpha", sp); err != nil { - t.Fatalf("positive control failed: %v", err) - } -} - -// TestCiphertextTamperingFailsClosed covers the ordinary AEAD property, with a -// positive control, so "GCM is wired up at all" is actually asserted. -func TestCiphertextTamperingFailsClosed(t *testing.T) { - priv, pub := mustWrapKeypair(t) - sp, err := sealForSpoke(pub, "hive-alpha", 1, []byte("a-master-secret-value")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - if _, err := openFromHub(priv, "hive-alpha", sp); err != nil { - t.Fatalf("positive control failed: %v", err) - } - - raw, err := hex.DecodeString(sp.Ciphertext) - if err != nil || len(raw) == 0 { - t.Fatalf("ciphertext not hex-decodable: %v", err) - } - raw[0] ^= 0xFF - tampered := sp - tampered.Ciphertext = hex.EncodeToString(raw) - if _, err := openFromHub(priv, "hive-alpha", tampered); err == nil { - t.Error("a tampered ciphertext opened — the GCM tag is not being verified") - } -} - -// TestEphemeralIsFreshPerSeal: sealing the same payload to the same key twice -// must produce different ciphertexts. Otherwise the "fresh sender keypair per -// wrap" forward-secrecy claim is false and an observer can correlate deliveries. -func TestEphemeralIsFreshPerSeal(t *testing.T) { - _, pub := mustWrapKeypair(t) - a, err := sealForSpoke(pub, "hive-alpha", 1, []byte("same")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - b, err := sealForSpoke(pub, "hive-alpha", 1, []byte("same")) - if err != nil { - t.Fatalf("sealForSpoke: %v", err) - } - if a.EphemeralPub == b.EphemeralPub { - t.Error("two seals reused the same ephemeral public key — forward secrecy claim is false") - } - if a.Ciphertext == b.Ciphertext { - t.Error("two seals of the same payload produced identical ciphertext") - } - if a.Nonce == b.Nonce { - t.Error("two seals reused the same GCM nonce") - } -} - -// --- MALFORMED KEY HANDLING: fail closed, never fall back --------------------- - -func TestParseWrapPublicKeyFailsClosed(t *testing.T) { - _, pub := mustWrapKeypair(t) - // Positive control first. - if _, err := parseWrapPublicKey(pub.Hex()); err != nil { - t.Fatalf("positive control failed: a valid key did not parse: %v", err) - } - for _, bad := range []struct{ name, in string }{ - {"empty", ""}, - {"not hex", "zzzz"}, - {"too short", hex.EncodeToString(make([]byte, wrapKeyLen-1))}, - {"too long", hex.EncodeToString(make([]byte, wrapKeyLen+1))}, - {"ed25519 public key length", hex.EncodeToString(make([]byte, ed25519.PublicKeySize+1))}, - } { - if _, err := parseWrapPublicKey(bad.in); err == nil { - t.Errorf("parseWrapPublicKey accepted %s — a malformed publication must count as "+ - "NO usable key, never as something to fall back on", bad.name) - } - } -} - -// TestWrapKeyNeverAcceptsEd25519Material is the source+behaviour assertion for -// the Ed25519-is-not-an-encryption-key invariant. -// -// WHY THIS IS SOURCE-ASSERTING. "Unify the key types" is exactly the plausible -// future refactor, and a behavioural test would not notice the crypto silently -// weakening: an Ed25519 seed is 32 bytes, the same as an X25519 scalar, so a -// refactor feeding Ed25519 material into this path would still produce -// working-looking round trips. The subject is chosen deliberately: assert that -// wrapkey.go imports no ed25519 and names no ed25519 symbol, because that is -// the property actually needed — not the weaker "the happy path still works". -func TestWrapKeyNeverAcceptsEd25519Material(t *testing.T) { - for _, file := range []string{"wrapkey.go", "wrapkey_store.go"} { - raw, err := os.ReadFile(file) - if err != nil { - t.Fatalf("read %s: %v", file, err) - } - src := string(raw) - // Strip comments before matching, so the explanatory prose in these - // files (which necessarily says "Ed25519") does not trip the assertion. - src = stripGoComments(src) - if strings.Contains(src, "crypto/ed25519") { - t.Errorf("%s imports crypto/ed25519 — Ed25519 is a SIGNING key and cannot encrypt; "+ - "routing it into the wrapping path is the refactor this test exists to block", file) - } - if regexp.MustCompile(`\bed25519\.`).MatchString(src) { - t.Errorf("%s references an ed25519 symbol in code", file) - } - } - // Positive control on the subject choice: the file DOES use crypto/ecdh, so - // this test is reading the file it thinks it is reading and a typo'd - // filename would not silently pass. - raw, err := os.ReadFile("wrapkey.go") - if err != nil { - t.Fatalf("read wrapkey.go: %v", err) - } - if !strings.Contains(string(raw), "crypto/ecdh") { - t.Fatal("wrapkey.go does not import crypto/ecdh — this test is not reading the wrapping " + - "implementation, so its negative assertions above prove nothing") - } -} - -// TestNoHKDFModuleDependency asserts OQ-3: single-block HMAC-SHA256 expansion, -// following hub_keys.go:31-33's deliberate choice, not x/crypto/hkdf. -func TestNoHKDFModuleDependency(t *testing.T) { - raw, err := os.ReadFile("wrapkey.go") - if err != nil { - t.Fatalf("read wrapkey.go: %v", err) - } - src := stripGoComments(string(raw)) - if strings.Contains(src, "hkdf") { - t.Error("wrapkey.go references hkdf — OQ-3 decided to follow hub_keys.go's no-HKDF " + - "precedent; adding a module to the one payload whose compromise is fleet-wide " + - "is a deliberate operator decision, not a side effect") - } - if !strings.Contains(src, "hmac.New(sha256.New") { - t.Error("wrapkey.go does not use hmac.New(sha256.New) — the expansion is not the " + - "single-block HMAC-SHA256 shape OQ-3 selected") - } -} - -// --- POLICY CONSTANTS: recorded as policy, not derivation -------------------- - -// TestWrapKeyPolicyConstants asserts OQ-4's decided values AND that they carry -// the rationale comment the operator required. "A bare 90 is how the next -// reader assumes it was computed." -func TestWrapKeyPolicyConstants(t *testing.T) { - if wrapKeyMaxAge != 90*24*time.Hour { - t.Errorf("wrapKeyMaxAge = %v, want 90 days (OQ-4)", wrapKeyMaxAge) - } - if wrapKeyOverlap != 24*time.Hour { - t.Errorf("wrapKeyOverlap = %v, want 24h (OQ-4)", wrapKeyOverlap) - } - if wrapKeyOverlap >= wrapKeyMaxAge { - t.Error("the overlap window is not shorter than the key max age — a key would be " + - "retained past the life of its successor") - } - raw, err := os.ReadFile("wrapkey.go") - if err != nil { - t.Fatalf("read wrapkey.go: %v", err) - } - src := string(raw) - idx := strings.Index(src, "wrapKeyMaxAge = 90") - if idx < 0 { - t.Fatal("could not locate the wrapKeyMaxAge declaration — this test is not reading " + - "what it thinks it is") - } - preamble := src[:idx] - for _, want := range []string{"POLICY CHOICES", "not derived", "changed"} { - if !strings.Contains(preamble, want) { - t.Errorf("the comment above wrapKeyMaxAge does not record %q — OQ-4 requires these "+ - "constants carry a comment saying they are policy choices with no derivation "+ - "behind them, what they trade off, and that they are safe to change deliberately", want) - } - } -} - -// --- SPOKE KEY LIFECYCLE ------------------------------------------------------ - -// TestEnsureWrapKeysPersistsBeforeUse: the key returned on first boot must -// already be on disk. A key used but not persisted republishes as a DIFFERENT -// key after the next pod roll, which the hub correctly refuses as a pin -// mismatch — turning a write failure into an operator-intervention event on a -// cluster the hub cannot reach into. -func TestEnsureWrapKeysPersistsBeforeUse(t *testing.T) { - path := filepath.Join(t.TempDir(), "hive-wrap-key") - now := time.Now() - - keys, fresh, err := ensureSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("ensureSpokeWrapKeys: %v", err) - } - if !fresh { - t.Error("first boot did not report generating a fresh keypair") - } - if !keys.current.valid() { - t.Fatal("no current key returned") - } - - reloaded, err := loadSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("the key ensure() returned was not on disk: %v", err) - } - if reloaded.current.publicKey().Hex() != keys.current.publicKey().Hex() { - t.Error("the persisted key differs from the one returned — the spoke would publish one " + - "key and republish a different one after a roll") - } -} - -// TestWrapKeyFileMode: the private key must be 0600, matching spokeAppKeyPath's -// stated rationale. -func TestWrapKeyFileMode(t *testing.T) { - path := filepath.Join(t.TempDir(), "hive-wrap-key") - if _, _, err := ensureSpokeWrapKeys(path, time.Now()); err != nil { - t.Fatalf("ensureSpokeWrapKeys: %v", err) - } - fi, err := os.Stat(path) - if err != nil { - t.Fatalf("stat: %v", err) - } - // ASSERT THE LITERAL, NOT THE CONSTANT. Comparing fi.Mode() against - // spokeWrapKeyFileMode is a tautology: widening the constant to 0644 moves - // both sides of the comparison and the test passes. (Verified: the - // regression replay for this property PASSED while neutered when written - // that way.) The invariant is 0600 specifically, so 0600 is what is written - // here. - const wantMode os.FileMode = 0o600 - if got := fi.Mode().Perm(); got != wantMode { - t.Errorf("wrap key file mode = %04o, want %04o — the wrapping private key must never be "+ - "readable by anything else sharing the PVC or the pod", got, wantMode) - } - if spokeWrapKeyFileMode != wantMode { - t.Errorf("spokeWrapKeyFileMode = %04o, want %04o — the constant itself was widened", - spokeWrapKeyFileMode, wantMode) - } - // NOT ASSERTED HERE: the mode of the containing directory. In production - // that is /data, the PVC mount, whose mode the spoke does not own — and in - // this test it is t.TempDir(), which pre-exists at 0755 so MkdirAll's mode - // never applies. Asserting on it would test the harness, not the code. The - // 0600 file mode is what actually protects the key from anything else - // sharing the PVC or the pod, exactly as for spokeAppKeyPath. -} - -// TestPodRollWithIntactPVCIsNoOp: same key, same publication. This is the -// property that makes ordinary restarts invisible to the hub-side pin. -func TestPodRollWithIntactPVCIsNoOp(t *testing.T) { - path := filepath.Join(t.TempDir(), "hive-wrap-key") - now := time.Now() - first, _, err := ensureSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("first boot: %v", err) - } - second, fresh, err := ensureSpokeWrapKeys(path, now.Add(time.Hour)) - if err != nil { - t.Fatalf("after roll: %v", err) - } - if fresh { - t.Error("a pod roll with the PVC intact regenerated the keypair — every restart would " + - "then require an operator re-pin") - } - if first.current.publicKey().Hex() != second.current.publicKey().Hex() { - t.Error("the published public key changed across a pod roll") - } -} - -// TestPVCLossGeneratesFreshKey: PVC loss is indistinguishable from first boot BY -// CONSTRUCTION. The spoke generates and publishes a new key; whether the hub -// accepts it is emphatically not decided here. -func TestPVCLossGeneratesFreshKey(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "hive-wrap-key") - now := time.Now() - first, _, err := ensureSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("first boot: %v", err) - } - if err := os.Remove(path); err != nil { - t.Fatalf("simulating PVC loss: %v", err) - } - second, fresh, err := ensureSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("after PVC loss: %v", err) - } - if !fresh { - t.Error("PVC loss did not produce a fresh keypair") - } - if first.current.publicKey().Hex() == second.current.publicKey().Hex() { - t.Error("PVC loss reproduced the same key — impossible unless generation is deterministic, " + - "which would mean the keypair is derived rather than random") - } -} - -// TestMalformedKeyFileIsReplacedNotTrusted: a malformed file must route to -// generate-fresh, never to a partial-recovery path. A half-readable key file is -// not evidence of anything. -func TestMalformedKeyFileIsReplacedNotTrusted(t *testing.T) { - for _, tc := range []struct{ name, body string }{ - {"not json", "definitely not json"}, - {"wrong version", `{"version":99,"current":"` + hex.EncodeToString(make([]byte, wrapKeyLen)) + `"}`}, - {"truncated key", `{"version":1,"current":"abcd"}`}, - {"empty object", `{}`}, - } { - t.Run(tc.name, func(t *testing.T) { - path := filepath.Join(t.TempDir(), "hive-wrap-key") - if err := os.WriteFile(path, []byte(tc.body), spokeWrapKeyFileMode); err != nil { - t.Fatalf("write: %v", err) - } - if _, err := loadSpokeWrapKeys(path, time.Now()); err == nil { - t.Fatal("loadSpokeWrapKeys accepted a malformed file") - } - keys, fresh, err := ensureSpokeWrapKeys(path, time.Now()) - if err != nil { - t.Fatalf("ensureSpokeWrapKeys: %v", err) - } - if !fresh || !keys.current.valid() { - t.Error("a malformed key file did not route to generate-fresh") - } - }) - } -} - -// --- ROTATION OVERLAP: versioned, finite, and NOT an auto-re-pin ------------- - -// TestRotationRetainsOldPrivateKeyForOverlap is the behavioural half of -// non-weakenable property 1. The wrap-key rotation path retains the old PRIVATE -// key spoke-side rather than asking the hub to accept a new PUBLIC key — that -// construction is what keeps "a different key is never accepted on bearer -// authority" intact through the one place the design itself introduces a -// legitimate key change. -func TestRotationRetainsOldPrivateKeyForOverlap(t *testing.T) { - path := filepath.Join(t.TempDir(), "hive-wrap-key") - now := time.Now() - keys, _, err := ensureSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("ensure: %v", err) - } - oldPub := keys.current.publicKey() - - // The hub sealed to the OLD (still pinned) key while this was in flight. - inFlight, err := sealForSpoke(oldPub, "hive-alpha", 4, []byte("master-4")) - if err != nil { - t.Fatalf("seal: %v", err) - } - - rotated, err := rotateSpokeWrapKeys(path, keys, now) - if err != nil { - t.Fatalf("rotate: %v", err) - } - if rotated.current.publicKey().Hex() == oldPub.Hex() { - t.Fatal("rotation did not produce a new current key") - } - if !rotated.previous.valid() { - t.Fatal("rotation discarded the old private key — a master sealed to the pinned key and " + - "still in flight becomes unopenable, stranding the spoke") - } - if got, err := openWithSpokeKeys(rotated, "hive-alpha", inFlight); err != nil { - t.Errorf("an in-flight payload sealed to the pinned key did not open after rotation: %v", err) - } else if string(got) != "master-4" { - t.Errorf("overlap open returned %q", got) - } -} - -// TestOverlapKeyIsFiniteAndZeroMeansExpired: a ZERO PreviousExpires reads as -// ALREADY EXPIRED, never "never expires" — the same rule acceptableGenerations -// gives VerifyUntil. Reading it the other way would keep a superseded wrapping -// key acceptable forever, which is the F1/F2 failure mode. -func TestOverlapKeyIsFiniteAndZeroMeansExpired(t *testing.T) { - dir := t.TempDir() - now := time.Now() - privCur, _ := mustWrapKeypair(t) - privPrev, prevPub := mustWrapKeypair(t) - - write := func(t *testing.T, expires time.Time) string { - t.Helper() - path := filepath.Join(dir, "k-"+strings.ReplaceAll(t.Name(), "/", "_")) - f := wrapKeyFile{ - Version: wrapKeyFileVersion, - Current: privCur.hex(), - CurrentCreated: now, - Previous: privPrev.hex(), - PreviousExpires: expires, - } - blob, err := json.Marshal(f) - if err != nil { - t.Fatalf("marshal: %v", err) - } - if err := os.WriteFile(path, blob, spokeWrapKeyFileMode); err != nil { - t.Fatalf("write: %v", err) - } - return path - } - - sealed, err := sealForSpoke(prevPub, "hive-alpha", 1, []byte("old-gen")) - if err != nil { - t.Fatalf("seal: %v", err) - } - - // POSITIVE CONTROL: an unexpired overlap key IS retained and DOES open. - t.Run("unexpired overlap opens", func(t *testing.T) { - path := write(t, now.Add(time.Hour)) - keys, err := loadSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("load: %v", err) - } - if !keys.previous.valid() { - t.Fatal("an unexpired previous key was dropped") - } - if _, err := openWithSpokeKeys(keys, "hive-alpha", sealed); err != nil { - t.Errorf("unexpired overlap key did not open an in-flight payload: %v", err) - } - }) - - t.Run("zero expiry means already expired", func(t *testing.T) { - path := write(t, time.Time{}) - keys, err := loadSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("load: %v", err) - } - if keys.previous.valid() { - t.Error("a previous key with a ZERO expiry was retained — a hand-edited file with " + - "the field stripped would keep a superseded wrapping key acceptable forever") - } - if _, err := openWithSpokeKeys(keys, "hive-alpha", sealed); err == nil { - t.Error("a payload sealed to a zero-expiry previous key still opened") - } - }) - - t.Run("past expiry is excluded not warned about", func(t *testing.T) { - path := write(t, now.Add(-time.Minute)) - keys, err := loadSpokeWrapKeys(path, now) - if err != nil { - t.Fatalf("load: %v", err) - } - if keys.previous.valid() { - t.Error("an EXPIRED previous key was retained") - } - if _, err := openWithSpokeKeys(keys, "hive-alpha", sealed); err == nil { - t.Error("a payload sealed to an expired previous key still opened") - } - }) -} - -// TestWrapKeyRotationDueOnUnknownAge: a ZERO currentCreated reads as NEEDS -// ROTATION, not "brand new" — the same fail-closed direction as -// VerifyUntil.IsZero(). -func TestWrapKeyRotationDueOnUnknownAge(t *testing.T) { - priv, _ := mustWrapKeypair(t) - now := time.Now() - - if wrapKeyNeedsRotation(spokeWrapKeys{current: priv, currentCreated: now}, now) { - t.Error("positive control failed: a brand-new key was reported as needing rotation") - } - if !wrapKeyNeedsRotation(spokeWrapKeys{current: priv}, now) { - t.Error("a key with a ZERO creation time was NOT reported as needing rotation — an " + - "unknown age must not read as a safe age") - } - old := now.Add(-wrapKeyMaxAge - time.Minute) - if !wrapKeyNeedsRotation(spokeWrapKeys{current: priv, currentCreated: old}, now) { - t.Error("a key past wrapKeyMaxAge was not reported as needing rotation") - } - if !wrapKeyNeedsRotation(spokeWrapKeys{}, now) { - t.Error("an absent key was not reported as needing rotation") - } -} - -// --- OPTION B REPLAY ---------------------------------------------------------- - -// TestSealedPayloadCarriesNoPlaintextSecret is the Option B replay for this -// stage. The invariant separating D from the rejected B is that no plaintext -// secret ever rides the delivery structure. sealedPayload must have no field -// that could carry one. -// -// This is checked against the STRUCT rather than an instance, because an -// instance test would pass simply by not populating the field. The failure mode -// being blocked is a future field being ADDED — e.g. a heartbeat key delivered -// alongside the master as a separate plaintext field, which OQ-2's -// implementation note calls out by name as the way (c) becomes Option B by the -// back door. -func TestSealedPayloadCarriesNoPlaintextSecret(t *testing.T) { - // REFLECT OVER THE STRUCT, DO NOT MARSHAL AN INSTANCE. Marshalling a - // zero-valued instance hides any field tagged `omitempty` — and a new - // plaintext secret field would almost certainly be tagged that way, since - // every optional field on HeartbeatResponse is. (Verified: the regression - // replay for this property PASSED while neutered when written as a marshal - // of an instance.) reflect.Type sees the field regardless of its tag. - rt := reflect.TypeOf(sealedPayload{}) - allowed := map[string]bool{ - "EphemeralPub": true, "Nonce": true, "Ciphertext": true, - "GenerationID": true, "Fingerprint": true, - } - seen := map[string]bool{} - for i := 0; i < rt.NumField(); i++ { - name := rt.Field(i).Name - seen[name] = true - if !allowed[name] { - t.Errorf("sealedPayload gained an unreviewed field %q (json tag %q). Under OQ-2(c) "+ - "the new heartbeat key ships SEALED INSIDE the ciphertext, never as a separate "+ - "plaintext field on the delivery structure — a plaintext secret field here is "+ - "Option B by the back door, and Option B was rejected because it makes the "+ - "heartbeat bearer a master-exfiltration primitive", - name, rt.Field(i).Tag.Get("json")) - } - } - // Positive control on the subject: the fields this test knows about are - // actually present, so a renamed or emptied struct would not silently pass. - for k := range allowed { - if !seen[k] { - t.Errorf("expected field %q missing — this test is not inspecting the payload it "+ - "thinks it is, so its negative assertions above prove nothing", k) - } - } - // And the marshalled form must carry no unexpected key either, which catches - // a field renamed only at the json-tag level. - blob, err := json.Marshal(sealedPayload{ - EphemeralPub: "aa", Nonce: "bb", Ciphertext: "cc", GenerationID: 1, Fingerprint: "dd", - }) - if err != nil { - t.Fatalf("marshal: %v", err) - } - var fields map[string]any - if err := json.Unmarshal(blob, &fields); err != nil { - t.Fatalf("unmarshal: %v", err) - } - allowedJSON := map[string]bool{ - "ephemeral_pub": true, "nonce": true, "ciphertext": true, - "generation_id": true, "fingerprint": true, - } - for k := range fields { - if !allowedJSON[k] { - t.Errorf("sealedPayload marshals an unreviewed json key %q", k) - } - } -} - -// TestHubCannotReopenWhatItSealed: the hub generates a fresh ephemeral per seal -// and discards it. If the hub retained any means of reopening, the entire -// premise (a non-confidential delivery channel is acceptable because only the -// recipient can open) would be false. -func TestHubCannotReopenWhatItSealed(t *testing.T) { - priv, pub := mustWrapKeypair(t) - sp, err := sealForSpoke(pub, "hive-alpha", 1, []byte("master")) - if err != nil { - t.Fatalf("seal: %v", err) - } - // The only public material the hub retains is the ephemeral public key and - // the recipient public key. Neither yields the shared secret. - ephPub, err := parseWrapPublicKey(sp.EphemeralPub) - if err != nil { - t.Fatalf("ephemeral pub did not parse: %v", err) - } - if ephPub.Hex() == pub.Hex() { - t.Error("the ephemeral public key equals the recipient's — no ephemeral was generated") - } - // Positive control: the recipient CAN open. - if _, err := openFromHub(priv, "hive-alpha", sp); err != nil { - t.Fatalf("positive control failed: the recipient could not open: %v", err) - } -} - -// --- fingerprint -------------------------------------------------------------- - -// TestFingerprintIsStableDistinctAndNotKeyMaterial. A fingerprint is a PUBLIC -// identifier that appears on dashboards and in operator re-pin commands, so it -// must be domain-separated from the AEAD key derivation: a value an operator -// reads off a screen must never be usable as key material. -func TestFingerprintIsStableDistinctAndNotKeyMaterial(t *testing.T) { - privA, pubA := mustWrapKeypair(t) - _, pubB := mustWrapKeypair(t) - - if wrapKeyFingerprint(pubA) == "" { - t.Fatal("fingerprint of a valid key was empty") - } - fpA1 := wrapKeyFingerprint(pubA) - fpA2 := wrapKeyFingerprint(pubA) - if fpA1 != fpA2 { - t.Error("fingerprint is not stable") - } - if wrapKeyFingerprint(pubA) == wrapKeyFingerprint(pubB) { - t.Error("two distinct keys share a fingerprint") - } - if wrapKeyFingerprint(wrapPublicKey{}) != "" { - t.Error("an invalid key produced a non-empty fingerprint") - } - if strings.Contains(wrapKeyFingerprint(pubA), pubA.Hex()) { - t.Error("the fingerprint contains the raw public key") - } - // It must not equal the AEAD key derived from any shared secret involving - // this key. Different info labels guarantee this; assert it so a future - // "simplify the labels" change is caught. - _, ephPub := mustWrapKeypair(t) - shared := make([]byte, wrapKeyLen) - aeadKey := hex.EncodeToString(deriveWrapAEADKey(shared, ephPub, pubA)) - if wrapKeyFingerprint(pubA) == aeadKey { - t.Error("the fingerprint collides with a derived AEAD key — the domain separation " + - "between a public identifier and key material has been lost") - } - _ = privA -} - -// stripGoComments removes // and /* */ comments so source assertions match on -// CODE rather than on the explanatory prose that necessarily names the very -// things being forbidden. -func stripGoComments(src string) string { - src = regexp.MustCompile(`(?s)/\*.*?\*/`).ReplaceAllString(src, "") - var b strings.Builder - for _, line := range strings.Split(src, "\n") { - if i := strings.Index(line, "//"); i >= 0 { - line = line[:i] - } - b.WriteString(line) - b.WriteString("\n") - } - return b.String() -} From 9040c21a4f2f8fa5f98af10724bf9239fe6c1646 Mon Sep 17 00:00:00 2001 From: Andy Anderson Date: Wed, 2 Sep 2026 21:43:43 -0400 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=8C=B1=20chore:=20add=20wrapkey=20rem?= =?UTF-8?q?oval=20changelog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Andy Anderson --- changelog.d/remove-dead-wrapkey.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/remove-dead-wrapkey.md diff --git a/changelog.d/remove-dead-wrapkey.md b/changelog.d/remove-dead-wrapkey.md new file mode 100644 index 000000000..05b71a1c2 --- /dev/null +++ b/changelog.d/remove-dead-wrapkey.md @@ -0,0 +1 @@ +- Removed the unused hub wrap-key prototype code; no runtime path read or wrote the sealed-master records. From 5bfae63ecb1f78a5bf1c30f0999dfcdc799cf013 Mon Sep 17 00:00:00 2001 From: Andy Anderson Date: Wed, 2 Sep 2026 21:45:06 -0400 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=8C=B1=20chore:=20use=20valid=20chang?= =?UTF-8?q?elog=20fragment=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Andy Anderson --- .../{remove-dead-wrapkey.md => changed-remove-dead-wrapkey.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{remove-dead-wrapkey.md => changed-remove-dead-wrapkey.md} (100%) diff --git a/changelog.d/remove-dead-wrapkey.md b/changelog.d/changed-remove-dead-wrapkey.md similarity index 100% rename from changelog.d/remove-dead-wrapkey.md rename to changelog.d/changed-remove-dead-wrapkey.md