fix(crypto): per-ciphertext PBKDF2 salt + v2 versioned format with v1 fallback (M-8)

2026-06-07 13:51:36 +00:00 · 2026-04-17 05:36:29 +00:00
parent b1df6dab27
commit 5abeeb882b
16 changed files with 580 additions and 158 deletions
@@ -1,4 +1,31 @@
 // Package crypto provides AES-256-GCM encryption for sensitive configuration data.
+//
+// The on-disk format for blobs produced by [EncryptIfKeySet] is versioned. Two
+// versions coexist and both can be read by [DecryptIfKeySet]:
+//
+//	v2 (current, M-8)
+//	    magic(0x02) || salt(16) || nonce(12) || ciphertext+tag
+//	    — 32-byte AES-256 key derived via PBKDF2-SHA256 from the operator
+//	      passphrase and the per-ciphertext random salt.
+//
+//	v1 (legacy, pre-M-8)
+//	    nonce(12) || ciphertext+tag
+//	    — 32-byte AES-256 key derived via PBKDF2-SHA256 from the operator
+//	      passphrase and the package-level fixed salt
+//	      "certctl-config-encryption-v1".
+//
+// v1 blobs are accepted by the read path for backward compatibility with rows
+// persisted before the M-8 remediation. They are never produced by the write
+// path. Any row that is updated after M-8 is re-sealed as v2 in-place via the
+// normal UPDATE flow.
+//
+// Rationale for the per-ciphertext salt (see M-8 / CWE-916 / CWE-329): the
+// pre-M-8 design reused a single 28-byte fixed salt for every ciphertext, which
+// (a) removes one defense-in-depth layer against passphrase-space brute force
+// and (b) makes every encrypted column across every row share the exact same
+// derived key. v2 replaces the fixed salt with 16 fresh random bytes per write
+// and stores the salt alongside the ciphertext. Derived keys now differ per
+// row and per re-encryption.
 package crypto

 import (
@@ -14,7 +41,8 @@ import (
 )

 // ErrEncryptionKeyRequired is returned by EncryptIfKeySet and DecryptIfKeySet when
-// the caller provides an empty key but the data on the wire requires protection.
+// the caller provides an empty passphrase but the data on the wire requires
+// protection.
 //
 // Historically these helpers silently returned plaintext when no key was configured,
 // which produced a data-at-rest confidentiality bypass (CWE-311): sensitive fields
@@ -24,16 +52,58 @@ import (
 // and plaintext branches at runtime, so the only visible signal was a warning
 // line emitted once at startup.
 //
-// The fix is to fail closed: EncryptIfKeySet/DecryptIfKeySet now require a key
-// whenever they are invoked on sensitive material, and the server refuses to
-// start if any source='database' rows already exist without a configured key.
+// The fix (C-2, commit fb4ce1a) is to fail closed: EncryptIfKeySet/DecryptIfKeySet
+// now require a passphrase whenever they are invoked on sensitive material, and
+// the server refuses to start if any source='database' rows already exist without
+// a configured passphrase.
 var ErrEncryptionKeyRequired = errors.New("crypto: CERTCTL_CONFIG_ENCRYPTION_KEY is required to encrypt or decrypt sensitive config")

+// v2Magic is the first byte of every v2-format ciphertext blob. It distinguishes
+// v2 blobs (per-ciphertext random salt, embedded in the blob) from v1 legacy
+// blobs (no magic byte, fixed package-level salt).
+//
+// The choice of 0x02 is deliberate: v1 blobs begin with a random 12-byte AES-GCM
+// nonce. A v1 nonce can coincidentally start with 0x02 with probability 1/256,
+// which makes a pure magic-byte dispatch ambiguous. [DecryptIfKeySet] resolves
+// the ambiguity by falling back to the v1 path when v2 AEAD verification fails.
+const v2Magic byte = 0x02
+
+// v2SaltSize is the length in bytes of the per-ciphertext salt embedded in a
+// v2 blob. 16 bytes (128 bits) matches the lower bound recommended in NIST
+// SP 800-132 §5.1 for PBKDF2 salts and is sufficient given the one-shot-per-row
+// nature of the derivation.
+const v2SaltSize = 16
+
+// pbkdf2Iterations is the PBKDF2-SHA256 work factor applied uniformly to both
+// v1 and v2 key derivations. The value is preserved from the pre-M-8 design so
+// that v1 fallback reads stay bit-identical.
+const pbkdf2Iterations = 100000
+
+// aes256KeySize is the output length in bytes of both [DeriveKey] and
+// [deriveKeyWithSalt]. It is also the only AES key length accepted by [Encrypt]
+// and [Decrypt].
+const aes256KeySize = 32
+
+// legacyV1Salt is the fixed salt used by pre-M-8 config encryption. It is
+// retained exclusively to preserve the v1 read path — any v1 blob that pre-dates
+// M-8 remediation must be decryptable with a key derived from (passphrase,
+// legacyV1Salt). The write path never uses this salt.
+//
+// Exposed as a package-level var rather than a local so that tests can reason
+// about v1 fixture bytes symbolically.
+var legacyV1Salt = []byte("certctl-config-encryption-v1")
+
 // Encrypt encrypts plaintext using AES-256-GCM with a random 12-byte nonce prepended to the output.
 // The key must be exactly 32 bytes (AES-256). Returns [12-byte nonce][ciphertext+tag].
+//
+// Encrypt is a low-level primitive. It is intentionally kept byte-identical to
+// the pre-M-8 implementation so that existing v1 blobs on disk remain
+// decryptable via [Decrypt] when paired with a [DeriveKey]-derived key. New
+// callers should prefer [EncryptIfKeySet], which handles key derivation and
+// emits the v2 wire format.
 func Encrypt(plaintext []byte, key []byte) ([]byte, error) {
-	if len(key) != 32 {
-		return nil, fmt.Errorf("encryption key must be exactly 32 bytes, got %d", len(key))
+	if len(key) != aes256KeySize {
+		return nil, fmt.Errorf("encryption key must be exactly %d bytes, got %d", aes256KeySize, len(key))
 	}

 	block, err := aes.NewCipher(key)
@@ -57,9 +127,14 @@ func Encrypt(plaintext []byte, key []byte) ([]byte, error) {

 // Decrypt decrypts ciphertext that was encrypted with Encrypt.
 // Expects format: [12-byte nonce][ciphertext+tag]. Key must be exactly 32 bytes.
+//
+// Decrypt is a low-level primitive. It is intentionally kept byte-identical to
+// the pre-M-8 implementation so that [DecryptIfKeySet] can delegate to it for
+// both the v2 inner blob (after stripping the magic byte + embedded salt) and
+// the v1 legacy blob (unmodified).
 func Decrypt(ciphertext []byte, key []byte) ([]byte, error) {
-	if len(key) != 32 {
-		return nil, fmt.Errorf("encryption key must be exactly 32 bytes, got %d", len(key))
+	if len(key) != aes256KeySize {
+		return nil, fmt.Errorf("encryption key must be exactly %d bytes, got %d", aes256KeySize, len(key))
 	}

 	block, err := aes.NewCipher(key)
@@ -86,48 +161,133 @@ func Decrypt(ciphertext []byte, key []byte) ([]byte, error) {
 	return plaintext, nil
 }

-// DeriveKey derives a 32-byte AES-256 key from a passphrase using PBKDF2-SHA256.
-// Uses a fixed application-specific salt and 100,000 iterations for resistance
-// to brute-force attacks on weak passphrases.
+// DeriveKey derives a 32-byte AES-256 key from a passphrase using PBKDF2-SHA256
+// with the legacy v1 fixed salt.
+//
+// This helper is preserved byte-identical to the pre-M-8 implementation so that
+// v1 ciphertexts persisted before the M-8 remediation remain decryptable
+// unchanged. New code paths should prefer [EncryptIfKeySet] and
+// [DecryptIfKeySet], which use a per-ciphertext random salt.
 func DeriveKey(passphrase string) []byte {
-	// Fixed salt is acceptable here because:
-	// 1. Each certctl instance has its own passphrase
-	// 2. The salt prevents generic rainbow table attacks
-	// 3. Per-user salts are unnecessary (single server key, not user passwords)
-	salt := []byte("certctl-config-encryption-v1")
-	return pbkdf2.Key([]byte(passphrase), salt, 100000, 32, sha256.New)
+	return deriveKeyWithSalt(passphrase, legacyV1Salt)
 }

-// EncryptIfKeySet encrypts plaintext with the supplied 32-byte AES-256 key.
+// deriveKeyWithSalt derives a 32-byte AES-256 key from a passphrase and an
+// explicit salt using PBKDF2-SHA256 with [pbkdf2Iterations] rounds.
+//
+// The per-ciphertext random salt path (v2) calls this directly with a fresh
+// 16-byte random salt embedded in the ciphertext blob. The legacy path
+// ([DeriveKey]) calls it with the package-level fixed salt [legacyV1Salt].
+func deriveKeyWithSalt(passphrase string, salt []byte) []byte {
+	return pbkdf2.Key([]byte(passphrase), salt, pbkdf2Iterations, aes256KeySize, sha256.New)
+}
+
+// IsLegacyFormat reports whether blob is in the v1 legacy wire format (no magic
+// byte, fixed-salt derivation) as opposed to the v2 wire format
+// (magic(0x02) || salt(16) || nonce(12) || ciphertext+tag).
+//
+// A return value of false is a necessary but not sufficient condition for a
+// blob to be a valid v2 ciphertext: the shortest possible v2 blob is
+// 1 + v2SaltSize + 12 = 29 bytes, and even a 29+ byte blob that starts with
+// 0x02 may turn out to be a v1 ciphertext whose random nonce happens to begin
+// with 0x02 (probability 1/256). [DecryptIfKeySet] resolves this ambiguity at
+// decrypt time by falling back to v1 when v2 AEAD verification fails; callers
+// of IsLegacyFormat should use it only as a heuristic (e.g. migration
+// tooling, log annotation).
+func IsLegacyFormat(blob []byte) bool {
+	if len(blob) == 0 {
+		return false
+	}
+	return blob[0] != v2Magic
+}
+
+// EncryptIfKeySet encrypts plaintext with the supplied passphrase and emits a
+// v2 wire-format blob: magic(0x02) || salt(16) || nonce(12) || ciphertext+tag.
+//
+// Key derivation is performed internally per invocation with a fresh 16-byte
+// random salt, producing a distinct AES-256 key for every ciphertext. The
+// operator-supplied passphrase is the only cross-ciphertext shared secret.
 //
 // The second return value is always true when err == nil — the "wasEncrypted"
-// flag is retained for source-compatibility with callers that previously used it
-// to log provenance. Callers MUST handle err: passing an empty key now returns
-// ErrEncryptionKeyRequired rather than silently emitting plaintext. See the
-// package-level ErrEncryptionKeyRequired documentation for the history behind
-// this behavior change.
-func EncryptIfKeySet(plaintext []byte, key []byte) ([]byte, bool, error) {
-	if len(key) == 0 {
+// flag is retained for source-compatibility with callers that previously used
+// it to log provenance. Callers MUST handle err: passing an empty passphrase
+// returns [ErrEncryptionKeyRequired] rather than silently emitting plaintext.
+// See the package-level [ErrEncryptionKeyRequired] documentation for the
+// history behind this behavior change (C-2).
+//
+// The write path never produces a v1 blob. v1 blobs are read-only legacy
+// state — see [DecryptIfKeySet] for the compatibility fallback.
+func EncryptIfKeySet(plaintext []byte, passphrase string) ([]byte, bool, error) {
+	if passphrase == "" {
 		return nil, false, ErrEncryptionKeyRequired
 	}
-	encrypted, err := Encrypt(plaintext, key)
+
+	salt := make([]byte, v2SaltSize)
+	if _, err := io.ReadFull(rand.Reader, salt); err != nil {
+		return nil, false, fmt.Errorf("failed to generate v2 salt: %w", err)
+	}
+
+	key := deriveKeyWithSalt(passphrase, salt)
+	inner, err := Encrypt(plaintext, key)
 	if err != nil {
 		return nil, false, err
 	}
-	return encrypted, true, nil
+
+	// v2 blob layout: magic(1) || salt(v2SaltSize) || inner
+	blob := make([]byte, 0, 1+v2SaltSize+len(inner))
+	blob = append(blob, v2Magic)
+	blob = append(blob, salt...)
+	blob = append(blob, inner...)
+	return blob, true, nil
 }

-// DecryptIfKeySet decrypts ciphertext with the supplied 32-byte AES-256 key.
+// DecryptIfKeySet decrypts blob with the supplied passphrase, supporting both
+// v2 (M-8 and later) and v1 (legacy) on-disk formats.
 //
-// Passing an empty key now returns ErrEncryptionKeyRequired. Callers that
-// legitimately store plaintext (e.g. env-seeded source='env' rows that keep
-// the raw JSON in the unencrypted `config` column) must branch on the presence
-// of the ciphertext themselves rather than relying on this helper to silently
-// pass bytes through. See the package-level ErrEncryptionKeyRequired
-// documentation for the history behind this behavior change.
-func DecryptIfKeySet(ciphertext []byte, key []byte) ([]byte, error) {
-	if len(key) == 0 {
+// Dispatch is first-byte magic + AEAD fallback. If blob starts with
+// [v2Magic] and is long enough to contain a v2 header plus an AEAD-authenticated
+// inner ciphertext, a v2 decrypt is attempted using a key derived from the
+// embedded salt. If that succeeds, its plaintext is returned. If v2 AEAD
+// verification fails — which covers both the "wrong passphrase" case and the
+// 1/256 case where a v1 blob's first byte happens to be 0x02 — the function
+// falls through to the v1 path and attempts decryption using a key derived
+// from the package-level fixed salt [legacyV1Salt].
+//
+// Passing an empty passphrase returns [ErrEncryptionKeyRequired]. Callers that
+// legitimately store plaintext (e.g. env-seeded source='env' rows that keep the
+// raw JSON in the unencrypted `config` column) must branch on the presence of
+// the ciphertext themselves rather than relying on this helper to silently
+// pass bytes through. See the package-level [ErrEncryptionKeyRequired]
+// documentation for the history behind this behavior change (C-2).
+//
+// The function never re-encrypts in place. A v1 blob that is successfully
+// decrypted is returned to the caller as plaintext; re-sealing as v2 happens
+// naturally on the next UPDATE via [EncryptIfKeySet].
+func DecryptIfKeySet(blob []byte, passphrase string) ([]byte, error) {
+	if passphrase == "" {
 		return nil, ErrEncryptionKeyRequired
 	}
-	return Decrypt(ciphertext, key)
+	if len(blob) == 0 {
+		return nil, fmt.Errorf("ciphertext is empty")
+	}
+
+	// v2 path: magic || salt(16) || nonce(12) || ciphertext+tag (min 29 bytes
+	// ignoring the GCM tag; the AEAD verify inside Decrypt enforces the tag).
+	if blob[0] == v2Magic && len(blob) >= 1+v2SaltSize+12 {
+		salt := blob[1 : 1+v2SaltSize]
+		sealed := blob[1+v2SaltSize:]
+		key := deriveKeyWithSalt(passphrase, salt)
+		if plaintext, err := Decrypt(sealed, key); err == nil {
+			return plaintext, nil
+		}
+		// v2 AEAD verification failed. Fall through to v1 so that a v1 blob
+		// whose first byte happens to be 0x02 (1/256 probability) is still
+		// decryptable. If this is truly a v2 blob with the wrong passphrase,
+		// the v1 attempt below will also fail and the v1 error is returned.
+	}
+
+	// v1 legacy path: blob is the full ciphertext with no header and was
+	// sealed with a key derived from (passphrase, legacyV1Salt).
+	key := DeriveKey(passphrase)
+	return Decrypt(blob, key)
 }
@@ -2,6 +2,8 @@ package crypto

 import (
 	"bytes"
+	"crypto/aes"
+	"crypto/cipher"
 	"errors"
 	"testing"
 )
@@ -126,21 +128,20 @@ func TestDeriveKeyDifferentPassphrases(t *testing.T) {
 }

 func TestEncryptIfKeySet_WithKey(t *testing.T) {
-	key := DeriveKey("test-key")
 	plaintext := []byte("config data")

-	result, wasEncrypted, err := EncryptIfKeySet(plaintext, key)
+	result, wasEncrypted, err := EncryptIfKeySet(plaintext, "test-passphrase")
 	if err != nil {
 		t.Fatalf("EncryptIfKeySet failed: %v", err)
 	}
 	if !wasEncrypted {
-		t.Fatal("expected wasEncrypted=true when key provided")
+		t.Fatal("expected wasEncrypted=true when passphrase provided")
 	}
 	if bytes.Equal(result, plaintext) {
 		t.Fatal("result should be encrypted")
 	}

-	decrypted, err := DecryptIfKeySet(result, key)
+	decrypted, err := DecryptIfKeySet(result, "test-passphrase")
 	if err != nil {
 		t.Fatalf("DecryptIfKeySet failed: %v", err)
 	}
@@ -150,67 +151,43 @@ func TestEncryptIfKeySet_WithKey(t *testing.T) {
 }

 // TestEncryptIfKeySet_EmptyKeyFailsClosed asserts the C-2 regression guard:
-// EncryptIfKeySet must refuse to silently emit plaintext when no key is configured.
-// The pre-fix behavior was to return plaintext with wasEncrypted=false, which
-// produced a data-at-rest confidentiality bypass (CWE-311) for GUI-created
-// issuer and target configs.
+// EncryptIfKeySet must refuse to silently emit plaintext when no passphrase is
+// configured. The pre-fix behavior was to return plaintext with
+// wasEncrypted=false, which produced a data-at-rest confidentiality bypass
+// (CWE-311) for GUI-created issuer and target configs.
 func TestEncryptIfKeySet_EmptyKeyFailsClosed(t *testing.T) {
 	plaintext := []byte("config data")

-	cases := []struct {
-		name string
-		key  []byte
-	}{
-		{"nil_key", nil},
-		{"empty_key", []byte{}},
+	result, wasEncrypted, err := EncryptIfKeySet(plaintext, "")
+	if err == nil {
+		t.Fatal("expected ErrEncryptionKeyRequired, got nil")
 	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			result, wasEncrypted, err := EncryptIfKeySet(plaintext, tc.key)
-			if err == nil {
-				t.Fatal("expected ErrEncryptionKeyRequired, got nil")
-			}
-			if !errors.Is(err, ErrEncryptionKeyRequired) {
-				t.Fatalf("expected ErrEncryptionKeyRequired, got %v", err)
-			}
-			if wasEncrypted {
-				t.Fatal("wasEncrypted must be false on error")
-			}
-			if result != nil {
-				t.Fatalf("expected nil result on error, got %q", result)
-			}
-		})
+	if !errors.Is(err, ErrEncryptionKeyRequired) {
+		t.Fatalf("expected ErrEncryptionKeyRequired, got %v", err)
+	}
+	if wasEncrypted {
+		t.Fatal("wasEncrypted must be false on error")
+	}
+	if result != nil {
+		t.Fatalf("expected nil result on error, got %q", result)
 	}
 }

 // TestDecryptIfKeySet_EmptyKeyFailsClosed asserts the matching C-2 regression
 // guard on the read path: DecryptIfKeySet must refuse to pass ciphertext
-// through as plaintext when no key is configured.
+// through as plaintext when no passphrase is configured.
 func TestDecryptIfKeySet_EmptyKeyFailsClosed(t *testing.T) {
 	data := []byte("plaintext config data")

-	cases := []struct {
-		name string
-		key  []byte
-	}{
-		{"nil_key", nil},
-		{"empty_key", []byte{}},
+	result, err := DecryptIfKeySet(data, "")
+	if err == nil {
+		t.Fatal("expected ErrEncryptionKeyRequired, got nil")
 	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			result, err := DecryptIfKeySet(data, tc.key)
-			if err == nil {
-				t.Fatal("expected ErrEncryptionKeyRequired, got nil")
-			}
-			if !errors.Is(err, ErrEncryptionKeyRequired) {
-				t.Fatalf("expected ErrEncryptionKeyRequired, got %v", err)
-			}
-			if result != nil {
-				t.Fatalf("expected nil result on error, got %q", result)
-			}
-		})
+	if !errors.Is(err, ErrEncryptionKeyRequired) {
+		t.Fatalf("expected ErrEncryptionKeyRequired, got %v", err)
+	}
+	if result != nil {
+		t.Fatalf("expected nil result on error, got %q", result)
 	}
 }

@@ -218,21 +195,20 @@ func TestDecryptIfKeySet_EmptyKeyFailsClosed(t *testing.T) {
 // "if set" helpers produce real AES-GCM output (not plaintext) and that a full
 // round-trip through both helpers recovers the original bytes.
 func TestEncryptDecryptIfKeySet_RoundTripProducesDifferentCiphertext(t *testing.T) {
-	key := DeriveKey("round-trip-key")
 	plaintext := []byte(`{"api_key":"s3cr3t","token":"abc"}`)

-	encrypted, wasEncrypted, err := EncryptIfKeySet(plaintext, key)
+	encrypted, wasEncrypted, err := EncryptIfKeySet(plaintext, "round-trip-key")
 	if err != nil {
 		t.Fatalf("EncryptIfKeySet failed: %v", err)
 	}
 	if !wasEncrypted {
-		t.Fatal("wasEncrypted must be true when key is present")
+		t.Fatal("wasEncrypted must be true when passphrase is present")
 	}
 	if bytes.Equal(encrypted, plaintext) {
 		t.Fatal("EncryptIfKeySet returned plaintext — would regress C-2")
 	}

-	decrypted, err := DecryptIfKeySet(encrypted, key)
+	decrypted, err := DecryptIfKeySet(encrypted, "round-trip-key")
 	if err != nil {
 		t.Fatalf("DecryptIfKeySet failed: %v", err)
 	}
@@ -242,22 +218,24 @@ func TestEncryptDecryptIfKeySet_RoundTripProducesDifferentCiphertext(t *testing.
 }

 // TestDecryptIfKeySet_RejectsTamperedCiphertext confirms the AEAD auth tag
-// still rejects modified ciphertext when routed through the helper.
+// still rejects modified ciphertext when routed through the helper. The v2
+// wire format is magic(1) || salt(16) || nonce(12) || ciphertext+tag, so
+// flipping a byte anywhere past offset 29 lands squarely inside the AEAD body.
 func TestDecryptIfKeySet_RejectsTamperedCiphertext(t *testing.T) {
-	key := DeriveKey("tamper-test-key")
 	plaintext := []byte("authenticated data")

-	encrypted, _, err := EncryptIfKeySet(plaintext, key)
+	encrypted, _, err := EncryptIfKeySet(plaintext, "tamper-test-key")
 	if err != nil {
 		t.Fatalf("EncryptIfKeySet failed: %v", err)
 	}
-	// Flip a byte inside the GCM body (past the 12-byte nonce) to invalidate the tag.
-	if len(encrypted) <= 13 {
+	// Flip a byte past the v2 header (1 + 16 + 12 = 29) to invalidate the tag.
+	const minV2HeaderLen = 1 + v2SaltSize + 12
+	if len(encrypted) <= minV2HeaderLen {
 		t.Fatalf("ciphertext too short to tamper: %d bytes", len(encrypted))
 	}
-	encrypted[13] ^= 0xFF
+	encrypted[minV2HeaderLen] ^= 0xFF

-	if _, err := DecryptIfKeySet(encrypted, key); err == nil {
+	if _, err := DecryptIfKeySet(encrypted, "tamper-test-key"); err == nil {
 		t.Fatal("DecryptIfKeySet accepted tampered ciphertext — AEAD tag check bypassed")
 	}
 }
@@ -296,3 +274,217 @@ func TestEncryptProducesDifferentCiphertexts(t *testing.T) {
 		t.Fatal("encrypting same plaintext twice should produce different ciphertexts (random nonce)")
 	}
 }
+
+// ---------------------------------------------------------------------------
+// M-8 additions: per-ciphertext salt + v2 wire format + v1 backward compat.
+// ---------------------------------------------------------------------------
+
+// TestDeriveKey_DifferentSaltsProduceDifferentKeys asserts that
+// deriveKeyWithSalt fans out distinct 32-byte keys for the same passphrase
+// across different salts. This is the core M-8 defense-in-depth property: even
+// if an attacker obtains two v2 ciphertexts encrypted with the same master
+// passphrase, the derived AES keys differ, and a brute-force attempt on one
+// blob cannot be amortized across the other.
+func TestDeriveKey_DifferentSaltsProduceDifferentKeys(t *testing.T) {
+	passphrase := "master-passphrase"
+	saltA := bytes.Repeat([]byte{0xAA}, v2SaltSize)
+	saltB := bytes.Repeat([]byte{0xBB}, v2SaltSize)
+
+	keyA := deriveKeyWithSalt(passphrase, saltA)
+	keyB := deriveKeyWithSalt(passphrase, saltB)
+
+	if len(keyA) != aes256KeySize || len(keyB) != aes256KeySize {
+		t.Fatalf("derived key length wrong: %d / %d", len(keyA), len(keyB))
+	}
+	if bytes.Equal(keyA, keyB) {
+		t.Fatal("deriveKeyWithSalt must produce different keys for different salts")
+	}
+
+	// Sanity-check that deterministic behaviour is preserved under a fixed salt.
+	keyA2 := deriveKeyWithSalt(passphrase, saltA)
+	if !bytes.Equal(keyA, keyA2) {
+		t.Fatal("deriveKeyWithSalt must be deterministic for a fixed (passphrase, salt)")
+	}
+}
+
+// TestEncryptIfKeySet_ProducesV2Format asserts the exact v2 wire-format bytes:
+// magic(0x02) || salt(16) || nonce(12) || ciphertext+tag.
+func TestEncryptIfKeySet_ProducesV2Format(t *testing.T) {
+	blob, _, err := EncryptIfKeySet([]byte("hello"), "any-passphrase")
+	if err != nil {
+		t.Fatalf("EncryptIfKeySet failed: %v", err)
+	}
+
+	const minLen = 1 + v2SaltSize + 12 + 16 // magic + salt + nonce + GCM tag (16)
+	if len(blob) < minLen {
+		t.Fatalf("v2 blob too short: got %d, want >= %d", len(blob), minLen)
+	}
+	if blob[0] != v2Magic {
+		t.Fatalf("v2 blob must start with magic byte 0x%02x, got 0x%02x", v2Magic, blob[0])
+	}
+	if IsLegacyFormat(blob) {
+		t.Fatal("IsLegacyFormat must return false for a freshly produced v2 blob")
+	}
+}
+
+// TestEncryptIfKeySet_SaltIsRandom asserts that two calls with the same
+// passphrase and plaintext produce distinct embedded salts.
+func TestEncryptIfKeySet_SaltIsRandom(t *testing.T) {
+	plaintext := []byte("same plaintext")
+	passphrase := "same-passphrase"
+
+	blob1, _, err := EncryptIfKeySet(plaintext, passphrase)
+	if err != nil {
+		t.Fatalf("EncryptIfKeySet #1 failed: %v", err)
+	}
+	blob2, _, err := EncryptIfKeySet(plaintext, passphrase)
+	if err != nil {
+		t.Fatalf("EncryptIfKeySet #2 failed: %v", err)
+	}
+
+	salt1 := blob1[1 : 1+v2SaltSize]
+	salt2 := blob2[1 : 1+v2SaltSize]
+	if bytes.Equal(salt1, salt2) {
+		t.Fatal("two EncryptIfKeySet invocations must produce distinct per-ciphertext salts")
+	}
+	if bytes.Equal(blob1, blob2) {
+		t.Fatal("two v2 blobs with same (passphrase, plaintext) must differ end-to-end")
+	}
+}
+
+// TestDecryptIfKeySet_V1BackwardCompat builds a deterministic v1-format
+// ciphertext using the pre-M-8 recipe (DeriveKey with the fixed salt, then
+// Encrypt with an all-zero nonce for reproducibility) and asserts that
+// DecryptIfKeySet still decrypts it correctly. This is the migration guarantee:
+// v1 blobs persisted before M-8 must remain decryptable.
+func TestDecryptIfKeySet_V1BackwardCompat(t *testing.T) {
+	passphrase := "legacy-passphrase"
+	plaintext := []byte(`{"api_key":"legacy","org_id":"789"}`)
+
+	// Build a deterministic v1 blob directly: nonce(12 zero bytes) || ct+tag.
+	// This matches the exact wire shape that Encrypt produces, minus the random
+	// nonce, so the test is stable rather than 1/256 flaky.
+	key := DeriveKey(passphrase) // fixed-salt derivation (pre-M-8 behavior)
+	block, err := aes.NewCipher(key)
+	if err != nil {
+		t.Fatalf("aes.NewCipher: %v", err)
+	}
+	gcm, err := cipher.NewGCM(block)
+	if err != nil {
+		t.Fatalf("cipher.NewGCM: %v", err)
+	}
+	nonce := make([]byte, gcm.NonceSize()) // all zeros → first byte != v2Magic
+	v1Blob := gcm.Seal(nonce, nonce, plaintext, nil)
+	if v1Blob[0] == v2Magic {
+		t.Fatalf("fixture nonce collided with v2 magic byte — test design error")
+	}
+
+	decrypted, err := DecryptIfKeySet(v1Blob, passphrase)
+	if err != nil {
+		t.Fatalf("DecryptIfKeySet(v1) failed: %v", err)
+	}
+	if !bytes.Equal(decrypted, plaintext) {
+		t.Fatalf("v1 decrypt mismatch: got %q, want %q", decrypted, plaintext)
+	}
+
+	// Cross-check: IsLegacyFormat should flag this as legacy.
+	if !IsLegacyFormat(v1Blob) {
+		t.Fatal("IsLegacyFormat must return true for a v1 blob whose first byte != v2Magic")
+	}
+}
+
+// TestDecryptIfKeySet_V1MagicByteCollisionFallsThrough covers the 1/256 edge
+// case where a v1 ciphertext's random 12-byte nonce happens to begin with
+// 0x02. The dispatch must attempt v2, see AEAD failure, and fall through to
+// v1 — never return a decrypt error when the passphrase is correct.
+func TestDecryptIfKeySet_V1MagicByteCollisionFallsThrough(t *testing.T) {
+	passphrase := "collision-passphrase"
+	plaintext := []byte("colliding v1 blob")
+
+	// Craft a v1 blob whose first byte equals v2Magic by choosing a nonce
+	// starting with 0x02 and sealing manually.
+	key := DeriveKey(passphrase)
+	block, err := aes.NewCipher(key)
+	if err != nil {
+		t.Fatalf("aes.NewCipher: %v", err)
+	}
+	gcm, err := cipher.NewGCM(block)
+	if err != nil {
+		t.Fatalf("cipher.NewGCM: %v", err)
+	}
+	nonce := make([]byte, gcm.NonceSize())
+	nonce[0] = v2Magic // force collision
+	v1Blob := gcm.Seal(nonce, nonce, plaintext, nil)
+	if v1Blob[0] != v2Magic {
+		t.Fatal("fixture construction bug: first byte must equal v2Magic")
+	}
+
+	decrypted, err := DecryptIfKeySet(v1Blob, passphrase)
+	if err != nil {
+		t.Fatalf("DecryptIfKeySet must fall through to v1 on AEAD failure, got err: %v", err)
+	}
+	if !bytes.Equal(decrypted, plaintext) {
+		t.Fatalf("v1-via-fallback decrypt mismatch: got %q, want %q", decrypted, plaintext)
+	}
+}
+
+// TestDecryptIfKeySet_V2WithWrongPassphraseFails asserts that a v2 blob
+// sealed under passphrase A cannot be decrypted under passphrase B. Both the
+// v2 AEAD verify (with salt from the blob + passphrase B) and the v1 fallback
+// (with fixed salt + passphrase B) must fail, and an error must be returned
+// rather than silently-corrupt plaintext.
+func TestDecryptIfKeySet_V2WithWrongPassphraseFails(t *testing.T) {
+	blob, _, err := EncryptIfKeySet([]byte("secret"), "passphrase-A")
+	if err != nil {
+		t.Fatalf("EncryptIfKeySet failed: %v", err)
+	}
+
+	got, err := DecryptIfKeySet(blob, "passphrase-B")
+	if err == nil {
+		t.Fatalf("DecryptIfKeySet must return error for wrong passphrase, got plaintext %q", got)
+	}
+	if got != nil {
+		t.Fatalf("result must be nil on decrypt error, got %q", got)
+	}
+}
+
+// TestDecryptIfKeySet_TruncatedV2Blob asserts that a blob starting with the v2
+// magic byte but too short to contain a full v2 header does not trip an
+// out-of-bounds slice and does not succeed. It either returns an error (v1
+// fallback on the short bytes fails with "ciphertext too short") or at minimum
+// never returns plaintext.
+func TestDecryptIfKeySet_TruncatedV2Blob(t *testing.T) {
+	truncated := []byte{v2Magic, 0x00, 0x01, 0x02, 0x03} // 5 bytes — well below the 29-byte v2 minimum
+	got, err := DecryptIfKeySet(truncated, "any-passphrase")
+	if err == nil {
+		t.Fatalf("DecryptIfKeySet must reject a truncated v2 blob, got plaintext %q", got)
+	}
+	if got != nil {
+		t.Fatalf("result must be nil on decrypt error, got %q", got)
+	}
+}
+
+// TestIsLegacyFormat covers the three branches of the public magic-byte
+// heuristic: v2 blob → false, v1 blob → true, empty blob → false.
+func TestIsLegacyFormat(t *testing.T) {
+	v2Blob, _, err := EncryptIfKeySet([]byte("data"), "p")
+	if err != nil {
+		t.Fatalf("EncryptIfKeySet failed: %v", err)
+	}
+	if IsLegacyFormat(v2Blob) {
+		t.Fatal("v2 blob must not be flagged as legacy")
+	}
+
+	// Any blob whose first byte isn't v2Magic should be reported as legacy.
+	v1Shape := []byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0xFF}
+	if !IsLegacyFormat(v1Shape) {
+		t.Fatal("non-v2-magic blob must be flagged as legacy")
+	}
+
+	if IsLegacyFormat(nil) {
+		t.Fatal("nil blob must not be flagged as legacy (undefined)")
+	}
+	if IsLegacyFormat([]byte{}) {
+		t.Fatal("empty blob must not be flagged as legacy (undefined)")
+	}
+}