Allow non-approved MD5 use under GODEBUG=fips140=only - #1133
Open
sameerforge wants to merge 1 commit into
Open
Conversation
kapp's own resource-labeling and diff code calls crypto/md5.Sum() to build short, stable keys for a k8s label (AssociationLabel.v1Value) and to key/dedup diffs (OpsDiff.MinimalMD5, TextDiff.MinimalMD5). None of these are security-relevant: they aren't used for authentication or integrity verification, just to keep values short/stable and to detect duplicate diffs. Building with GOFIPS140 and running with GODEBUG=fips140=only (Go's native FIPS 140-3 mode) panics on any non-approved primitive regardless of how it's used, so these calls panic on the very first resource kapp tries to label - i.e. on every real `kapp deploy`. Wrap each call in crypto/fips140.WithoutEnforcement, which disables strict enforcement only for the scoped callback (a documented no-op when fips140=only isn't set, so this is a no-behavior-change for every other build). WithoutEnforcement requires Go >= 1.26, so go.mod is bumped from 1.25.10 to 1.26.0 to match. Verified: the previously-panicking TestChangeSet_WithoutNew_And_WithoutUnexpectedChanges_And_IgnoredFields now passes under GOFIPS140=v1.0.0 GODEBUG=fips140=only, and a full `go test ./...` baseline vs GODEBUG=fips140=only diff shows zero new failures and zero panics. Signed-off-by: Sameer <sameer.khan@broadcom.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Sameer <sameer.khan@broadcom.com>
sameerforge
force-pushed
the
topic/sameerkh/fips140-md5-without-enforcement
branch
from
August 11, 2026 09:01
0610cae to
fca9391
Compare
Author
|
Wait for carvel-dev/vendir#458 to get merged. |
Contributor
|
If the hashing itself is not relevant, why not changing to something like sha? Just a thought, i'm not sure if this would require any migration |
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
3 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.
Problem
kapp uses
crypto/md5.Sum()in three places, purely as a non-security convenience hash — never for authentication or integrity verification:AssociationLabel.v1Value()— builds a short, stable Kubernetes label valueOpsDiff.MinimalMD5()/TextDiff.MinimalMD5()— key/dedup diffsWhen built with
GOFIPS140and run underGODEBUG=fips140=only(Go's native FIPS 140-3 enforcement mode, Go 1.24+), any call to a non-approved cryptographic primitive panics at runtime regardless of how the result is used. BecauseAssociationLabelis applied to essentially every resource kapp touches, this panics on the first resource of any realkapp deploy— making kapp unusable under strict FIPS 140-3 enforcement.Fix
Wrap each
md5.Sum()call incrypto/fips140.WithoutEnforcement(func() { ... })(Go 1.26+). Per its documented contract, this disables strict enforcement only for the duration of the scoped callback, and is a no-op whenfips140=onlyisn't active — so behavior under any other build/run configuration (no FIPS,fips140=on, etc.) is unchanged.go.modis bumped from Go 1.25.10 to 1.26.5 sinceWithoutEnforcementrequires Go 1.26+.Testing
TestChangeSet_WithoutNew_And_WithoutUnexpectedChanges_And_IgnoredFields, which exercisesAssociationLabel) now passes underGOFIPS140=v1.0.0 GODEBUG=fips140=only.md5.Sum()output with and without theWithoutEnforcementwrapper produces byte-identical hashes under both a plain build andGODEBUG=fips140=on.go test ./...suite underGOFIPS140=v1.0.0 GODEBUG=fips140=onlyand again underGODEBUG=fips140=on: zero panics in both, and no new test failures relative to an unmodified baseline run.Note
kapp's embedded ytt library also exposes an
md5.sum()Starlark function to user-authoredoverlayContractV1rebase rules and wait rules, which has the same panic underfips140=only. That's already being addressed upstream in ytt itself (carvel-dev/ytt#1001) rather than in this PR, since the fix belongs in the dependency that owns the exposure. This PR only addresses kapp's own direct MD5 use.