PR Draft: membership: fix SHA-1 panic under GODEBUG=fips140=only#21804
PR Draft: membership: fix SHA-1 panic under GODEBUG=fips140=only#21804herosql wants to merge 1 commit into
Conversation
…ompatibility Fixes etcd-io#21673 - panic when etcd starts with GODEBUG=fips140=only Use crypto/fips140.Enabled() to select hash at runtime: - Non-FIPS mode: SHA-1 path unchanged (backward compatible) - FIPS mode: SHA-256 only, no panic Both computeMemberID() and genID() avoid calling sha1.Sum when FIPS is enabled. Sentinel tests (TestMemberTime, TestSnapshotStatus) skip under FIPS mode since their expected values are SHA-1-derived. New reproduction tests in fips_repro_test.go for both panic sites.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: herosql The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @herosql. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
66b4299 to
8c12112
Compare
8c12112 to
66b4299
Compare
PR Draft: membership: fix SHA-1 panic under GODEBUG=fips140=only
Title
Description
Problem
etcd panics on startup when launched with
GODEBUG=fips140=onlyon Go 1.24+. Go's strictFIPS 140-3 mode prohibits any call to
crypto/sha1, causing an immediate panic during clustermembership initialization — before a single Raft log entry is written.
Two call sites are affected:
server/etcdserver/api/membership/member.go:74—computeMemberID()hashes peer URLs andcluster token to derive a deterministic
uint64member IDserver/etcdserver/api/membership/cluster.go:237—genID()hashes the sorted set ofmember IDs to derive the cluster ID
Both functions use SHA-1 purely as a deterministic hash function — not for any cryptographic
or integrity purpose — but Go's FIPS runtime does not distinguish by intent.
Root Cause
The panic fires unconditionally at process startup for any cluster that does not yet have a
WAL — new bootstraps, member additions, and snapshot restores all reach
computeMemberIDbefore any other initialization proceeds.
Why a naive SHA-256 replacement is wrong
Simply substituting
sha256.Sum256would change the ID values produced during bootstrap.Member IDs and cluster IDs are written into the WAL metadata header and persisted in the
membersBoltDB bucket on first boot. Two existing sentinel tests encode the exact expectedoutputs:
TestMemberTimeasserts specificuint64values derived from SHA-1 inputsTestSnapshotStatusasserts a CRC32 over the full BoltDB content (0xe7a6e44b), whichincludes the
membersbucket keyed by SHA-1-derived member IDsBoth tests carry the explicit comment "must not be changed — if it changes, there must be
some backwards incompatible change introduced." A global algorithm swap violates this
invariant for non-FIPS deployments even though they have no FIPS requirement.
Solution
Use
crypto/fips140.Enabled()(public API since Go 1.24,crypto/fips140package) toselect the hash at runtime:
In non-FIPS mode the code path is identical to before — same algorithm, same output, same
truncation to 8 bytes. Neither sentinel test is affected. In FIPS mode SHA-256 is used and
the panic is eliminated.
The two sentinel tests themselves are updated to skip under
fips140.Enabled(), since theirhardcoded expected values are inherently SHA-1-derived and have no meaning in a context where
SHA-1 is prohibited. The
t.Skippreserves the tests' protective value for all non-FIPSconfigurations.
Backward Compatibility
bootstrap.go:658readsNodeID/ClusterIDdirectly from WAL metadata;computeMemberIDandgenIDare never calledMemberAddat runtimeuint64etcdutl snapshot restoreThe storage format (
uint64persisted as a 16-character hex key in BoltDB) is unchanged.No WAL format changes. No protocol changes.
Testing
Reproduction tests (
server/etcdserver/api/membership/fips_repro_test.go):Full unit regression — 36 packages, both modes:
Files Changed
server/etcdserver/api/membership/member.gocrypto/fips140,crypto/sha256; runtime branch incomputeMemberIDserver/etcdserver/api/membership/cluster.gogenIDserver/etcdserver/api/membership/member_test.goTestMemberTimeunderfips140.Enabled()etcdutl/snapshot/v3_snapshot_test.goTestSnapshotStatusunderfips140.Enabled()server/etcdserver/api/membership/fips_repro_test.go