Skip to content

[Security] AES-GCM IV reuse not prevented by wrapper API #110

Description

@pathosDev

Severity / Size

  • Severity: MEDIUM — defence-in-depth for a low-probability bug class. Real exploitation requires a code path that reuses an IV with the same subkey, which would also be a discrete bug. The fix removes the foot-gun structurally.
  • Size: S (~1d).
  • Threat model: not external. A future code change (or a third-party integration) that calls aesGcmEncrypt directly with a caller-supplied IV. If the caller reuses the IV with the same subkey, AES-GCM's confidentiality and integrity both collapse — two ciphertexts under the same (key, IV) leak the XOR of their plaintexts and allow forging auth tags.

Affected files

  • src/persistence/object-storage/Encryption.ts:86-107aesGcmEncrypt(subKey, iv, plaintext) accepts the IV as a parameter. No structural guarantee that the same IV isn't passed twice for the same key.
  • src/persistence/object-storage/BodyCodec.ts:107 — only call site inside the framework. Generates a fresh IV via randomIv() per encrypt; safe today.
  • src/persistence/object-storage/Encryption.ts:131-136randomIv() itself sources from crypto.getRandomValues.

Background

AES-GCM is a counter-mode + GMAC AEAD. The 12-byte IV (nonce) must be unique per (key, plaintext) pair — never reused with the same key. Reuse breaks:

  • Confidentiality: two ciphertexts produced with the same (key, IV) leak the XOR of their plaintexts. Attacker who captures both and knows one plaintext recovers the other.
  • Integrity (auth tag): with two ciphertexts under the same (key, IV), the attacker can derive the GHASH authentication key and forge tags on arbitrary new ciphertexts.

randomIv() uses 96 bits from crypto.getRandomValues. The birthday bound for collision is ~2⁴⁸ encryptions under the same key. That's a lot — we won't run into it in practice for snapshots. BUT: the aesGcmEncrypt wrapper accepts the IV as a parameter, leaving the door open to a future bug where the caller reuses the same IV (off a fixed buffer, a constant for testing, accidental closure-capture, etc.).

Exploit walkthrough (hypothetical, not exploitable today)

A hypothetical buggy caller:

// BAD: same IV reused across two encrypts for the same key
const iv = randomIv();
const ct1 = await aesGcmEncrypt(subkey, iv, plaintext1);
const ct2 = await aesGcmEncrypt(subkey, iv, plaintext2);

Both ct1 and ct2 are under (subkey, iv). Attacker who observes both:

  1. Computes ct1 ⊕ ct2 → recovers plaintext1 ⊕ plaintext2. Known-plaintext on one side reveals the other.
  2. With the leaked GHASH key, forges tags on attacker-chosen new ciphertexts that decrypt under the same key.

The framework today doesn't have such a caller — BodyCodec always uses a fresh randomIv(). But the signature of aesGcmEncrypt permits the bug. This issue is about closing that hole at the API level.

How the 8 already-landed security fixes inform this

  • FrameDecoder size cap (d454079): pattern "validate at the API boundary, reject before damage". Same idea: don't let the caller pass an IV that could be unsafe — generate it internally.
  • Path-traversal guard (a290b06): closed a foot-gun in FilesystemObjectStorageBackend by validating keys at the entry-point. This is the crypto-layer equivalent.

Fix design

Two complementary changes.

Track 1 — wrapper API that generates IV internally.

Add a new function that doesn't accept an IV:

// src/persistence/object-storage/Encryption.ts

export async function aesGcmEncryptSafe(
  subkey: Uint8Array,
  plaintext: Uint8Array,
  aad?: Uint8Array,
): Promise<{ iv: Uint8Array; ciphertext: Uint8Array }> {
  const iv = randomIv();
  const ciphertext = await aesGcmEncrypt(subkey, iv, plaintext, aad);
  return { iv, ciphertext };
}

BodyCodec calls aesGcmEncryptSafe() instead of the IV-accepting form. The old aesGcmEncrypt(subkey, iv, plaintext) is kept for low-level callers but marked @internal and the JSDoc explicitly warns about IV-reuse danger.

Track 2 — per-key IV-counter (defence-in-depth, optional).

For deployments that want stronger-than-random guarantees, expose a counter-based IV generator:

export function createIvCounter(): IvCounter;

export interface IvCounter {
  next(): Uint8Array;  // 96 bits: 32-bit prefix (random fixed) + 64-bit counter
}

The counter starts at a random 32-bit prefix at process start; the 64-bit counter is incremented monotonically per call. Reset-on-process-restart is fine because the random prefix changes. Mathematically: 2⁶⁴ encryptions before counter wraps, with a fresh 32-bit prefix per restart.

This is opt-in via a new IvStrategy setting; default stays random.

Track 3 — assert single-use at the test layer.

A new test helper assertUniqueIvs(N) runs the encrypt path N times, collects every IV from the resulting framed bodies, and asserts pairwise uniqueness. Used as a regression test against future refactors that might accidentally reintroduce IV reuse.

API surface

// src/persistence/object-storage/Encryption.ts

/**
 * Encrypt with a freshly-generated IV.  Preferred over `aesGcmEncrypt`
 * for new code — eliminates the IV-reuse foot-gun.
 */
export async function aesGcmEncryptSafe(
  subkey: Uint8Array,
  plaintext: Uint8Array,
  aad?: Uint8Array,
): Promise<{ iv: Uint8Array; ciphertext: Uint8Array }>;

/**
 * Counter-based IV strategy for very-high-throughput deployments.
 * Optional; default IV strategy stays `random`.
 */
export interface IvStrategy {
  next(): Uint8Array;
}
export function randomIvStrategy(): IvStrategy;
export function counterIvStrategy(): IvStrategy;

// src/persistence/PersistenceOptions.ts — optional new field
export interface EncryptionConfig {
  // ... existing
  readonly ivStrategy?: IvStrategy;  // default: randomIvStrategy()
}

Backward compatibility

Old aesGcmEncrypt(subkey, iv, plaintext) stays exported, marked @internal and documented with the IV-reuse warning. Existing call sites (only BodyCodec) migrate to aesGcmEncryptSafe(). No wire-format change.

IvStrategy is opt-in; default behaviour unchanged.

Test plan

  1. Defence test: 10K encrypt calls via aesGcmEncryptSafe; assert pairwise IV uniqueness in the resulting bodies.

  2. Hypothetical-exploit test: build two ciphertexts via the unsafe aesGcmEncrypt(subkey, sameIv, ...) directly; verify XOR-leak property exists — documents the danger that the safe API prevents. This test stays as a negative example, not a regression target.

  3. Counter strategy test: with counterIvStrategy(), encrypt 100 messages; verify monotonic 64-bit counter component + stable 32-bit prefix.

  4. Regression: existing BodyCodec.test.ts + Encryption.test.ts + ObjectStorageSnapshotStore.test.ts all pass.

Acceptance criteria

  • aesGcmEncryptSafe exported and used by BodyCodec.
  • aesGcmEncrypt marked @internal with IV-reuse warning in JSDoc.
  • Optional counterIvStrategy() exported; opt-in via EncryptionConfig.ivStrategy.
  • Four new tests pass; existing crypto tests still green.
  • Plan-doc + README "Known security caveats" updated on land.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: mediumUseful, not urgentsecuritySecurity-relevant — see severity label for impact tierseverity: mediumModerate impact or requires specific conditions

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions