Skip to content

Concurrency: add optimistic locking to rotation Phase 2 (finalize_rekey) #1006

Description

@Manuthor

Problem

finalize_rekey in crate/server/src/core/operations/rekey/common.rs reads dependant
objects (via find_wrapped_by) and then writes them back in a single atomic() call, but
there is no optimistic lock guarding those reads against concurrent writers.

Race scenario

If two rotation requests for the same key execute concurrently — two manual Re-Key
requests, or one manual + one auto-rotation tick:

  1. Both callers read the old key and its dependants at the same snapshot.
  2. Both callers generate a new key (different UIDs, so no Create collision).
  3. Both callers commit Phase 2: the second writer silently overwrites the first writer's
    ReplacementObjectLink on the old key and re-wraps dependants a second time.
    The first new key becomes a dangling orphan — correct key material that can never
    be found by keyset resolution.

Why it is not yet observable in production

The auto-rotation background task (run_auto_rotation in auto_rotate.rs) is currently
a no-op stub (// TODO: issue Re-Key / Re-Key Key Pair), so the manual + auto race
cannot be triggered today. The manual + manual race requires two requests that land within
the same database transaction window, which is unlikely but not impossible.

Required fix

SQL backends (SQLite, PostgreSQL, MySQL)

Add a version INTEGER NOT NULL DEFAULT 0 column to the objects table and a migration.
Replace the unconditional UPDATE objects SET … WHERE id = $uid in Phase 2 with:

UPDATE objects SET …, version = version + 1
WHERE id = $uid AND version = $read_version

If rowsAffected == 0, abort with a Conflict error so the caller can retry.

Redis-Findex backend

Use a Lua script for compare-and-swap on the object key (avoids the WATCH + shared
connection pool problem documented in objects_db.rs):

local cur = redis.call('GET', KEYS[1])
if cur == ARGV[1] then
  redis.call('SET', KEYS[1], ARGV[2])
  return 1
end
return 0

Interface changes required

  1. Add version: Option<u64> to ObjectWithMetadata (populated on retrieve).
  2. Add a new AtomicOperation::UpdateObjectIfVersion((uid, object, attrs, tags, version))
    variant (or a dedicated update_object_if_version trait method) to ObjectsStore.
  3. Implement in all backends: SQLite, PostgreSQL, MySQL, Redis-Findex, HsmStore (no-op or
    error — HSM keys are not concurrently rotated today).
  4. Update finalize_rekey to use the new variant for the old-key retirement step.

Acceptance criteria

  • Schema migration for all SQL backends.
  • AtomicOperation::UpdateObjectIfVersion (or equivalent) implemented in all stores.
  • finalize_rekey uses the conditional update; returns KmsError::Conflict on
    version mismatch.
  • Auto-rotation cron retries on Conflict with exponential back-off (max 3 attempts).
  • Unit test: two concurrent Re-Key calls on the same key — only one succeeds, the
    other returns Conflict.

Blocking dependency

This issue must be resolved before run_auto_rotation is wired up to issue actual
Re-Key requests. Until then the race window is manual-only and low probability.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or request

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions