Skip to content

[Security] Key-version byte overflow after >256 rotations #111

Description

@pathosDev

Severity / Size

  • Severity: LOW — purely a longevity concern. Requires >256 master-key rotations over the lifetime of a single bucket. Realistic timeframe: decades at typical rotation cadences (quarterly = ~64 years to wrap). Documenting the limit is enough; the structural fix is for future-proofing.
  • Size: S (~1d).
  • Threat model: not external. Operator-error: deployment rotates master keys often enough that the key-version byte wraps; old bodies labelled v42 now appear to be from after-the-wrap and decrypt with the wrong (newer) key. Decryption fails loudly (auth tag mismatch), so it's an availability bug, not a confidentiality one.

Affected files

  • src/persistence/object-storage/BodyCodec.ts:36-37 — flag bits + key-version byte definition. Single byte gives values 0–255.
  • src/persistence/object-storage/BodyCodec.ts:102-117 — encode-path that stamps keyVersion into byte 5.
  • src/persistence/object-storage/BodyCodec.ts:148-156 — decode-path reads keyVersion = framed[5].
  • src/persistence/object-storage/Encryption.ts:152-167activeEncryptKey() reads active.version.
  • src/persistence/PersistenceOptions.ts:43-58MasterKeyRingEntry.version typed as number with no overflow guard.

Background

When client-side AES encryption is enabled (#8 / v0.6.0), each body's manifest carries a 1-byte key-version stamp. The keyring config supplies the mapping version → masterKey; the decode-path uses the stamp to pick the right master key.

The byte size (0–255) was chosen for compactness — at 1 byte, the overhead is negligible. But it caps the lifetime of the bucket at 256 distinct master keys (with no key-version reuse), or ~64 years at quarterly rotation, ~21 years at monthly rotation.

If a deployment ever wraps the version byte:

  • Body written at v=0 (original) collides with body written at v=256 (after wrap).
  • Decode picks the active v=256-mod-256=0 key for the wrapped body → wrong key → auth-tag mismatch → BodyCodec: no master key registered for version 0 (or a decrypt failure depending on what's in retired).

Discoverable at decode-time but recovery requires manually intervening with the keyring config to disambiguate.

Exploit walkthrough

Setup: deployment has been running for 21 years at monthly key rotation. At rotation #257, the operator promotes a new key to active with version = 1 (re-using a version number from year 1).

Step 1 — encode: bodies from the past month are now stamped v=1. Bodies from year 1, also stamped v=1, still live in the bucket. Both look identical on the wire.

Step 2 — decode-time confusion: an old body from year 1 arrives at decode. The keyring has active: {v=1, key=newKey-2026} and retired: [...{v=1, key=oldKey-2005}...]. But retired[] is indexed by version, and both entries claim v=1 — which one wins?

Looking at resolveDecryptSubkey (lines 180-192): the active version check happens first. If ring.active.version === version, the active key is used. So the year-1 body decrypts under the year-2026 key → auth-tag mismatch → throws.

Damage: year-1 bodies become undecryptable. Operator has to either:

  • Re-encrypt the entire corpus before the wrap (using reEncryptObjectStorage).
  • Manage a separate keyring per "epoch" outside the framework's abstraction.

How the 8 already-landed security fixes inform this

  • Re-encryption sweep (Re-encryption sweep helper for master-key rotation #70): established the rotation-tooling pattern. Avoiding the wrap is one extra reason to keep reEncryptObjectStorage running on a schedule.
  • Frame-size DoS (d454079): pattern "fail fast at the boundary". Same here: refuse to register a keyring whose active.version would create a wrap collision with any existing retired[] entry.

Fix design

Three small additions, all defensive.

Track 1 — runtime guard at keyring-load time.

In the plugin-init path, validate that no version appears twice in (active, ...retired). Today the code accepts a malformed keyring without complaint:

function validateKeyring(ring: MasterKeyRing): void {
  const versions = new Set<number>();
  for (const entry of [ring.active, ...(ring.retired ?? [])]) {
    if (entry.version < 0 || entry.version > 255) {
      throw new Error(`MasterKeyRing version must be in [0, 255], got ${entry.version}`);
    }
    if (versions.has(entry.version)) {
      throw new Error(`MasterKeyRing has duplicate version ${entry.version} — would cause decode ambiguity`);
    }
    versions.add(entry.version);
  }
}

Called from registerObjectStoragePlugins() and any other entry-point that builds a keyring.

Track 2 — approaching-wrap warning.

When active.version >= 240 (15 left before wrap), log a warn-level message at plugin-init recommending a wider version space. Threshold is configurable.

Track 3 — opt-in 2-byte version (future expansion).

Reserve a flag bit FLAG_KEY_VERSIONED_WIDE (bit 4) for a future wire-format upgrade where the version is 2 bytes (0–65535). Spec the wire layout but don't implement until needed:

ATS1 | flags | [keyVersion-byte | OR | keyVersion-u16-LE] | [iv] | ciphertext

Decode: if FLAG_KEY_VERSIONED is set, read 1 byte; if FLAG_KEY_VERSIONED_WIDE is set, read 2 bytes (LE). Both bits set is illegal. Without FLAG_KEY_VERSIONED_WIDE the encode path stays byte-only, so this is a forward-compat reservation only.

Not landing 2-byte today — the documentation + Track 1+2 are enough. Reserve the flag bit to keep the door open without changing wire format right now.

API surface

// src/persistence/PersistenceOptions.ts — no public-API change
// src/persistence/object-storage/Encryption.ts — internal validateKeyring()

// src/persistence/object-storage/BodyCodec.ts — reserve flag bit
export const FLAG_KEY_VERSIONED_WIDE = 0b10000;  // reserved; not yet used in encode

Backward compatibility

  • Validation at plugin-init is new; any existing user with a broken keyring (duplicate versions, out-of-range) will now fail at startup. Failing fast is better than silent corruption at decode-time, but it is a behavior change.
  • 2-byte version is a forward-compat reservation; no observable behaviour change yet.

Test plan

  1. Duplicate-version validation: register an encrypting plugin with keyring: { active: {v: 1}, retired: [{v: 1, ...}] } → throws at registration.

  2. Wrap-warning test: register with active.version = 250 → warn-level log.

  3. Wrap-collision exploit (negative): a malicious keyring with two entries both claiming v=1 — without the validation, the bucket decodes ambiguously. With Track 1, the keyring is rejected.

  4. Documentation: ROADMAP entry mentions "FLAG_KEY_VERSIONED_WIDE reserved for future 2-byte version" if anyone tries to use bit 4.

  5. Regression: existing KeyRotation.test.ts + ObjectStorageSnapshotStore.test.ts pass.

Acceptance criteria

  • validateKeyring() enforced at plugin-init; out-of-range or duplicate versions throw.
  • Warn-on-approaching-wrap (default threshold active.version >= 240).
  • FLAG_KEY_VERSIONED_WIDE = 0b10000 reserved but unused.
  • Three new tests pass; existing crypto + plugin tests still green.
  • Plan-doc + README "Known security caveats" updated on land.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: lowNice-to-have / niche / demand-drivensecuritySecurity-relevant — see severity label for impact tierseverity: lowMinor / informational / mitigated-by-design

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions