registry: constant-time admin compare, WAL doc fix, opt-in heartbeat freshness and expiry binding - #112
Merged
Merged
Conversation
Live mutex/block profiles from the production registry under a fleet reconverge showed two single global locks dominating: handshakeMu 50.6% of all block time (HandlePollHandshakes) trust st.mu 32.1% of all mutex time (HandleReportTrust) Every agent polls its handshake inbox on a timer, so at fleet scale all polls serialised on one mutex despite each touching only its own node id; report_trust did the same for trust pairs. Both are now sharded 256 ways by node id / pair key. Every mutating critical section already touched exactly one key, so no path needs two shards and there is no lock order to observe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…doc, opt-in heartbeat freshness + set_key_expiry binding L3 — trust.checkAdminToken compared the supplied admin token with the string operator, so a rejection returned as soon as a byte differed and the time it took leaked how much of the token was right. Switched to subtle.ConstantTimeCompare, matching every sibling admin-token check in the repo (authz, membership, dashboard, accept). This was the only divergence. L4 — wal.Append's doc claimed the entry is fsync'd. The write(2) is synchronous, the fsync is batched (walSyncInterval / walSyncBatch), so host power loss can lose a bounded tail. Corrected the comment on Append, the package doc, and the constants rather than fsyncing every append, which would change throughput on the write path. L2 and M12 both change what a client has to sign, so both are behind default-off flags mirroring -strict-registration-auth. Default behaviour is unchanged. L2 — the heartbeat challenge covered only the node id, so a captured heartbeat stayed valid indefinitely and could hold a node that is gone in the "seen recently" state. -strict-heartbeat-freshness / RENDEZVOUS_STRICT_HEARTBEAT_FRESHNESS=1 requires a "ts" field within heartbeatMaxSkew and binds it into the challenge. Enabling it also bypasses the 120s signature-verification cache, which would otherwise wave through anything sent after one accepted heartbeat, and refuses the binary heartbeat encoding: its payload is a fixed node id + signature with no field for a timestamp, so leaving it open would make the gate decorative. Carrying a timestamp there needs a registry wire-format revision and is not attempted here. M12 — the set_key_expiry challenge covered only the node id, so one signature authorized any expiry for that node, including substituting a distant one. -strict-expiry-binding / RENDEZVOUS_STRICT_EXPIRY_BINDING=1 appends the requested expires_at to the challenge, the way HandleRotateKey's challenge appends the new public key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Security findings L3, L4, L2, M12. Recreated against
main(the original PR auto-closed when its base branch was merged and deleted).checkAdminTokenused!=— nowsubtle.ConstantTimeCompare, matching the sibling checks.Appenddoc corrected: the write is synchronous, the fsync is batched. Deliberately not switched to per-append fsync (throughput change, not a doc fix).-strict-heartbeat-freshness. Note: enabling it refuses the binary heartbeat encoding, which has no timestamp field — fully closing that needs a wire format revision.expires_atbound into theset_key_expirychallenge behind default-off-strict-expiry-binding.🤖 Generated with Claude Code