Skip to content

Data race in KeyRegistry - mutex only held by getCert() #1904

@smuda

Description

@smuda

Which component:
Controller (sealed-secrets-controller), all versions

Describe the bug
KeyRegistry embeds a sync.Mutex but only getCert() actually holds it. Several other methods read or write the shared keys map and mostRecentKey pointer without synchronization:

Method Lock held? Access
getCert() Yes read mostRecentKey
registerNewKey() No write keys map + mostRecentKey │
generateKey() No calls registerNewKey() │
latestPrivateKey() No readMostRecentKey
initKeyRenewal No. read keys + mostRecentKey

Because the background key-rotation goroutine calls generateKey() → registerNewKey() concurrently with HTTP handlers calling getCert() and latestPrivateKey(), this is a data race on the keys map and the mostRecentKey pointer.

To Reproduce

  1. Deploy sealed-secrets-controller with a short key renewal interval (e.g. --key-renew-period=1m)
  2. Run go test -race ./pkg/controller/... or send concurrent requests to /v1/cert.pem and /v1/rotate while a key rotation is in progress
  3. The Go race detector reports a data race on KeyRegistry.keys and KeyRegistry.mostRecentKey

Expected behavior
All reads and writes to KeyRegistry.keys and KeyRegistry.mostRecentKey should be protected by the mutex. registerNewKey(), generateKey(), latestPrivateKey(), and any direct field access in initKeyRenewal() should hold the lock (or be provably single-threaded) when touching shared state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    backlogIssues/PRs that will be included in the project roadmap

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions