Add distributed locking for cross-node CA operations - #11
Merged
trevor-vaughan merged 7 commits intoMay 7, 2026
Conversation
Abstract the storage layer behind a Backend interface with a logical key namespace (KeyCACert, KeyCAKey, KeyCRL, KeySerial, KeyInventory, KeyInventoryHMAC, KeyHMACKey, KeyCAPubKey, and csr/cert prefixes). Add FilesystemBackend implementing the interface, preserving the existing Puppet CA layout and atomic-write / HMAC semantics. StorageService becomes a facade over Backend, exposing content-oriented methods (GetCACert/SaveCACert/HasCACert, GetCSR/SaveCSR/..., etc.) while keeping legacy *Path() methods for callers that still need filesystem paths via an optional PathProvider interface. Per-subject generated private keys continue to live on local disk through a separate localPrivateKeyDir, independent of the configured backend. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace direct os.ReadFile/os.WriteFile/os.Stat calls against Storage.*Path() with the new content-oriented methods (GetCACert/SaveCACert/HasCACert, GetCAKey/SaveCAKey, GetSerial, TouchInventory, CRLModTime, etc.) so callers no longer assume a filesystem-backed store. The *Path() accessors are still used for log messages and for resolving the optional CA-key passphrase path, which remain filesystem-scoped. Tests in the storage package retain a handful of filesystem-specific assertions (permissions, mtime) that validate the FilesystemBackend directly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Implement a Backend that stores CA cert/key, CRL, serial, inventory (and its HMAC), CSRs, and signed certificates in an etcd v3 cluster. Per-subject generated private keys continue to live on local disk regardless of the configured backend. Values are wrapped with an 8-byte big-endian unix-nano mtime prefix so ModTime is served from the same key without a second round-trip, keeping If-Modified-Since semantics on GET /crl working. AppendLine uses an etcd Txn guarded on the key's ModRevision with bounded retry, so inventory appends stay atomic across multiple puppet-ca processes sharing a cluster. A BackendSpec + NewServiceFromSpec helper selects the backend at startup; puppet-ca (frontend, signer, and single-process modes) builds the spec from new storage_backend / etcd_* config fields and matching PUPPET_CA_* env vars. Unit tests for the pure helpers (key translation, blob encoding) run by default. An etcd_integration build tag gates round-trip tests that spin up an in-process embedded etcd, including concurrent cross-client AppendLine to prove the Txn-based append holds under contention. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Introduce an OverlayBackend that wraps any Backend and redirects a fixed set of logical keys to explicit local filesystem paths. Surface it through BackendSpec as CACertFile / CAKeyFile, wired up via new ca_cert_file / ca_key_file config keys, matching PUPPET_CA_CA_*_FILE env vars, and --ca-cert-file / --ca-key-file CLI flags. Combined with the etcd backend this lets operators keep the CA material on a mounted secret volume while CSRs, signed certs, CRL, inventory and serial all live in the shared cluster. Also add --storage-backend, --etcd-endpoints, --etcd-key-prefix CLI flags so the backend choice doesn't require a config file, and document everything in docs/storage-backends.md (key layout, config examples for both backends, security considerations for the CA key in etcd, and pointers for adding further backends). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The key-isolation frontend read the CA cert with os.ReadFile(filepath.Join(absCADir, "ca_crt.pem")), bypassing the storage service and the overlay backend. When the CA cert is mounted via an overlay (e.g. a Kubernetes secret at /run/secrets/puppet-ca-ca/tls.crt), no file exists under absCADir and the frontend failed with ENOENT immediately after a successful PSK handshake with the signer. Defer the handshake and cert read until after the storage service is constructed, then read via store.GetCACert so the overlay is honoured. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Before this change, the etcd backend only had cross-node atomicity for
inventory AppendLine (via a ModRevision-guarded Txn). Everything else
that did a read-modify-write — CRL rotation during Revoke, CSR submission
with autosign, and first-run bootstrap — was protected only by a
process-local sync.RWMutex, so two replicas could race: losing a
revocation when both read the same CRL and wrote their own, issuing two
certs for the same subject, or generating two different CAs from a
simultaneous bootstrap.
Introduce an optional Locker capability on Backend, implemented on the
etcd backend via concurrency.Mutex over a lease-backed session, and
consumed through a new StorageService.WithLock helper that falls back to
a process-local named mutex when the backend does not (or cannot) provide
one. The filesystem backend keeps its single-process semantics
unchanged; OverlayBackend delegates to its base so CA-cert/key overrides
still participate in cluster-wide coordination.
Wire the CA layer through WithLock:
- bootstrap: Init loads without a lock on the fast path; on the slow
path it acquires a bootstrap lock, re-checks, and either loads the
winner's CA or generates a new one.
- crl: Revoke serialises the read-modify-write of the CRL so
concurrent revocations from different replicas don't clobber each
other.
- subject:<name>: Sign, SignWithTTL, and SaveRequest serialise on a
per-subject lock so two replicas can't both pass eviction and
produce duplicate certificates for the same identity.
concurrency.Mutex is not safe for re-entry by multiple goroutines
sharing one session, so the etcd Locker wraps every distributed lock in
a process-local per-name mutex first; unit tests cover the serialisation
invariants under both local fallback and cross-session contention.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bootc
force-pushed
the
etcd-backend-distributed-locks
branch
from
April 19, 2026 20:49
5ed55f4 to
7f4a761
Compare
Init's fast path runs finishLoadExisting() whenever loadCA() succeeds, which it does even when cert+key are served from a local-file overlay against a freshly-provisioned remote backend. In that setup the CRL, inventory, and serial counter live in the backend and don't exist yet, so loadCRLCache() failed with fs.ErrNotExist and startup aborted — bootstrapCA() never ran because loadCA() had already succeeded. Detect the missing CRL in finishLoadExisting(), take the bootstrap lock, re-check, and then generate an empty CRL signed by the loaded CA key, TouchInventory, and WriteSerial if absent. Skip seeding in frontend-only mode (ExternalSigner != nil) since the signer process owns bootstrapping. Covered by two new specs in internal/ca/ca_test.go: one asserts the supporting state is created on first Init with an overlay-style cert/key, the other asserts Init is idempotent across repeated calls. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
trevor-vaughan
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #10 (etcd backend). Adds a
Lockercapability onBackendand routes the CA's read-modify-write paths through it so two replicas sharing a cluster can't race on revocation, signing, or bootstrap.Summary
AppendLine(via a ModRevision-guardedTxn). Everything else that did a read-modify-write — CRL rotation duringRevoke, CSR submission with autosign, and first-run bootstrap — was protected only by a process-localsync.RWMutex. Two replicas could:Lockercapability onBackend. New optional interface implemented on the etcd backend viaconcurrency.Mutexover a lease-backed session. Consumed through a newStorageService.WithLockhelper that falls back to a process-local named mutex when the backend doesn't (or can't) provide one. The filesystem backend keeps its single-process semantics unchanged.OverlayBackenddelegates to its base so CA-cert/key overrides still participate in cluster-wide coordination.WithLock:bootstrap:Initloads without a lock on the fast path; on the slow path it acquires a bootstrap lock, re-checks, and either loads the winner's CA or generates a new one.crl:Revokeserialises the read-modify-write of the CRL so concurrent revocations from different replicas don't clobber each other.subject:<name>:Sign,SignWithTTL, andSaveRequestserialise on a per-subject lock so two replicas can't both pass eviction and produce duplicate certs for one identity.Init's fast path runsfinishLoadExisting()wheneverloadCA()succeeds — including when cert+key are served from a local-file overlay against a freshly-provisioned remote backend. In that setup the CRL, inventory, and serial counter live in the backend and don't yet exist, soloadCRLCache()failed withfs.ErrNotExistand startup aborted (bootstrapCA()never ran becauseloadCA()had already succeeded).finishLoadExistingnow detects the missing CRL, takes the bootstrap lock, re-checks, and generates an empty CRL signed by the loaded CA key,TouchInventorys, andWriteSerials if absent. Skipped in frontend-only mode (ExternalSigner != nil) since the signer process owns bootstrapping.Notes
concurrency.Mutexis not safe for re-entry by multiple goroutines sharing one session, so the etcdLockerwraps every distributed lock in a process-local per-name mutex first.WithLockfalls back to a local named mutex and never reaches across the network.