fix(fips): drop empty SSH algorithm names leaked by x/crypto on GOFIPS140 builds

Problem

fips.SupportedAlgorithms() returns a trailing empty string in Ciphers on any GOFIPS140 build.

Cause

An upstream bug in golang.org/x/crypto/ssh, present v0.53.0 through v0.54.0 (latest):

# What happens Where
1 defaultCiphers = supportedCiphers — the two variables share one backing array common.go
2 Under fips140.Enabled(), slices.DeleteFunc drops ChaCha20-Poly1305 in place, zeroing the array tail cipher.go
3 supportedCiphers keeps its original length, so its last element is now ""
4 SupportedAlgorithms() returns slices.Clone(supportedCiphers), trailing "" included common.go

LabKit passed it straight through to callers.

Impact is bounded. The empty name never reaches the wire — ssh.Config.SetDefaults, called unconditionally by NewServerConn and NewClientConn, drops it because cipherModes[""] is nil. It is still a malformed value for any caller that logs, validates or persists the list, and it already fails this package's own tests.

Fix

filterFIPSCompliant becomes a shared dropAlgorithms in a new untagged fips/filter.go, so the guard applies in both build configurations. That matters because the trigger is GOFIPS140, not the fips build tag — and the existing exclusion lists don't contain "", so the empty entry survives the filter under -tags fips too.

dropAlgorithms filters in place. Safe for every caller here: ssh.SupportedAlgorithms() returns slices.Clone'd slices, and SetDefaults() rebuilds Ciphers/KeyExchanges/MACs into freshly appended slices rather than aliasing package globals.

Why CI missed it

No job runs GOFIPS140 without -tags fips: test-fips-native sets both, test-fips uses GOEXPERIMENT=boringcrypto. The new fips/filter_test.go is untagged so it runs in both configurations.

Verification

BeforeGOFIPS140=v1.0.0 go test ./fips/... fails TestSupportedAlgorithmsReturnsValidAlgorithms_NonFips ("Cipher algorithm should not be empty").

After — all six combinations of {none, GOEXPERIMENT=boringcrypto, GOFIPS140=v1.0.0} × {no tag, -tags fips} build and pass with -race. Plus scripts/test.sh (both modules), both FIPS job equivalents, gofmt, golangci-lint (0 issues), go-mod-tidy (no changes).

Out of scope

Two further pre-existing defects in this package surfaced while investigating. Both are now fixed for v1 in !598 (merged), stacked on this MR, and in the v2 port (!595 (merged)):

  1. DefaultAlgorithms() under -tags fips filters only KeyExchanges, so it still returns ChaCha20-Poly1305, HMAC-SHA1 and HMAC-SHA1-96.
  2. It strips ML-KEM768/X25519, which Go approves in native FIPS mode — costing post-quantum key exchange in FIPS deployments.

Related: #129 (closed) · Epic: &22761

🤖 Generated with Claude Code

Edited by Jason Plum

Merge request reports

Loading
Loading