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
- Deploy sealed-secrets-controller with a short key renewal interval (e.g. --key-renew-period=1m)
- Run go test -race ./pkg/controller/... or send concurrent requests to /v1/cert.pem and /v1/rotate while a key rotation is in progress
- 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.
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:
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
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.