Skip to content

[Security] Master-key rotation: re-encryption sweep race + no progress tracking #109

Description

@pathosDev

Severity / Size

  • Severity: HIGH — incorrect operator procedure can leave a bucket of objects undecryptable. Not a confidentiality bypass; an availability+recoverability problem. The damage is permanent if the retired key is gone.
  • Size: M (~2d).
  • Threat model: not an external attacker. This is an operator-error / partial-failure scenario: the operator follows a documented procedure that has a sharp edge. Production deployments doing key-rotation in earnest will hit this.

Affected files

  • src/persistence/object-storage/reEncryptionSweep.ts:123-220 — the sweep itself. Today: idempotent in the sense of "re-running skips already-rewritten objects", but no persistent progress state. After a crash, a resumed run has to re-list and re-check every object.
  • docs/operations/rolling-migration.md (Phase 3) — the recipe documents the sweep and is silent about the failure mode.

Background

In v0.8.0 (#70 / commit 80dc045) we shipped reEncryptObjectStorage() to walk an object-storage backend and re-encrypt every body under the active master key. It's the missing step in the documented master-key rotation procedure: add the new key as active, sweep the corpus, then drop the old key from retired.

The sweep is structurally idempotent — bodies already at the active version are skipped on the fast path. But "idempotent on success" is not the same as "robust against crash mid-run". The current crash story is:

  • Sweep starts; processes objects 0…499; crashes (process kill, OOM, network blip).
  • Operator inspects state. Sweep didn't complete. Operator may either:
    • (a) Re-run the sweep with the same keyring — works fine. Already-rewritten objects are skipped on the second pass. This is the happy path.
    • (b) Drop the retired key prematurely — because the operator believes the sweep "got far enough" or follows a hard schedule (e.g. compliance: "retired keys retired after T hours"). Then resume. Now the remaining 50% of objects encrypted under the old key are permanently unreadable — the keyring no longer has a key for their version byte.

Path (b) is operator error, but it's an avoidable one. A sweep that writes durable progress state to disk could let the keyring-config validate "all rotation versions are still resolvable" by cross-checking against the sweep's recorded resumability state.

Exploit walkthrough

Setup: production deployment with ~1M S3 objects encrypted under key v1. Operator rotates: v2 becomes active, v1 moves to retired.

Step 1 — sweep start: reEncryptObjectStorage(backend, { keyring: { active: v2, retired: [v1] } }). The script processes objects in backend.list() order.

Step 2 — crash at 50%: process OOMs after ~500k objects rewritten. Operator sees the script exited non-zero, fishes in logs, sees no further detail (no resume token).

Step 3 — operator drops v1: compliance dashboard says "v1 must be retired within 24h", or the operator simply believes "the sweep got most of it". New keyring config: { active: v2 } (no retired). Deploys.

Step 4 — application can't recover: any actor whose state was in the un-swept half of the bucket gets BodyCodec: no master key registered for version 1. Recovery fails. Data is lost (in the sense that the bytes still exist but can't be decrypted).

Reverse-engineering the key from the ciphertext is computationally infeasible. The retired key, if not backed up elsewhere, is gone for good.

How the 8 already-landed security fixes inform this

  • Path-traversal fix (a290b06): added a safeKey() validation helper. Pattern: validate at the entry point. → Here we validate the keyring against the corpus at entry: refuse to start a sweep whose retired[] doesn't cover every version byte we observe in the bucket.
  • Re-encryption sweep (Re-encryption sweep helper for master-key rotation #70 itself): established the basic sweep structure. Adding progress state is a layer on top.

This isn't a security finding in the strict sense (no adversary). But it's tracked as HIGH because the loss is permanent and the failure mode is non-obvious.

Fix design

Two-track defense.

Track 1 — durable resume token (primary).

The sweep writes a small JSON file to the same backend as the data, under a reserved prefix (e.g. .actor-ts/re-encrypt-progress/${runId}.json). Shape:

interface SweepProgress {
  readonly runId: string;             // uuid, generated at start
  readonly startedAt: number;          // wallclock ms
  readonly keyringFingerprint: string; // sha256 of (active.version + retired[].version sorted)
  readonly keyPrefix: string;
  readonly totalScanned: number;
  readonly lastCompletedKey: string;   // for resume
  readonly rewrittenCount: number;
}

On start, the sweep checks for an existing progress file with a matching keyringFingerprint:

  • If present and recent (< N days): resume from lastCompletedKey.
  • If present but stale: warn the operator, require an explicit --resume-stale flag (or force: true option) to continue.
  • If absent: start fresh.

After each batch of (say) 50 successful rewrites, atomically write the updated progress file.

On successful completion, delete (or mark completedAt) the progress file.

Track 2 — keyring drop-safety check (secondary, complementary).

reEncryptObjectStorage exposes a new helper verifyKeyringComplete() that the operator runs before dropping a retired key:

const result = await verifyKeyringComplete(backend, {
  keyPrefix: 'snapshots/',
  keyring: { active: v2 },     // proposed new keyring (after v1 drop)
});
// → { complete: false, missingVersions: [1], objectsAffected: 500_000 }

The check walks the corpus and tallies the version bytes; if any version is missing from the proposed keyring, return complete: false with the list of missing versions and an approximate count of affected objects. Operator sees this in their tooling before pulling the trigger.

CLI wrapper: actor-ts verify-keyring-complete --bucket ... --keyring-config ....

Track 3 — improve the docs.

docs/operations/rolling-migration.md Phase 3 currently shows reEncryptObjectStorage and stops there. Add:

  • Mandatory progress-state guidance (use the resume token).
  • Warning box about premature retired-key drop.
  • Recipe: "verify keyring completeness before dropping a retired key" using verifyKeyringComplete.

API surface

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

export interface ReEncryptOptions {
  // ... existing fields
  /**
   * Resume from a previous run that was interrupted.  Default: 'auto'
   * (resume if a matching progress file exists; warn on staleness).
   */
  readonly resume?: 'auto' | 'fresh' | 'force-stale';
  /** How often to flush progress, in records. Default: 50. */
  readonly progressFlushEvery?: number;
}

export async function verifyKeyringComplete(
  backend: ObjectStorageBackend,
  opts: { keyPrefix: string; keyring: MasterKeyRing },
): Promise<{
  complete: boolean;
  missingVersions: number[];
  objectsAffected: number;
}>;

Backward compatibility

Existing reEncryptObjectStorage calls without resume default to 'auto' and silently start writing progress files. Operators who don't want progress files can pass resume: 'fresh', progressFlushEvery: Infinity to disable. No wire-format change to the actual encrypted bodies.

Test plan

  1. Crash-resume test (tests/unit/persistence/object-storage/reEncryptionSweep-resume.test.ts): write 100 records under v1; start sweep; intercept the loop at idx 50 and throw; restart with the same keyring; verify the second run completes the remaining 50 without re-doing the first 50.

  2. Stale-progress test: write a progress file with an old keyringFingerprint; new sweep with a different keyring → defaults to fresh start, warns about the stale file.

  3. Keyring-completeness test: write 100 records under v1; verifyKeyringComplete({ keyring: { active: v2 } }){ complete: false, missingVersions: [1], objectsAffected: 100 }. After full sweep to v2: verifyKeyringComplete{ complete: true }.

  4. Defense test: simulate the exploit walkthrough — sweep at 50%, drop v1, retry remaining objects → fails on BodyCodec (current behaviour, expected). But: verifyKeyringComplete would have caught it pre-drop.

  5. Regression: all existing reEncryptionSweep tests pass.

Acceptance criteria

  • ReEncryptOptions accepts resume + progressFlushEvery.
  • Progress file written to a reserved prefix on the backend; atomic flush every N records.
  • Resume picks up at the recorded lastCompletedKey.
  • verifyKeyringComplete() helper exposed + tested.
  • docs/operations/rolling-migration.md Phase 3 rewritten with the two-step procedure (sweep → verify-complete → drop).
  • All existing tests pass; 4 new tests pass.
  • Plan-doc + README "Known security caveats" entry updated on land.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: highTop priority — high impact, plan nextsecuritySecurity-relevant — see severity label for impact tierseverity: highSignificant impact, exploitable in standard threat model

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions