Address GOFIPS short-circuit bug#2244
Merged
michelle-clayton-work merged 1 commit intomicrosoft/mainfrom Apr 23, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a FIPS-mode detection short-circuit in the Microsoft Go build by tightening how environment variables are interpreted, ensuring platform-specific FIPS detection is only bypassed by explicit “disable” settings, and updates the FIPS documentation accordingly.
Changes:
- Update FIPS initialization logic so only explicit values (
GODEBUG=fips140=on|only|debug|off,GOFIPS=0|1,GOLANG_FIPS=0|1) affect behavior; unexpected values no longer bypass platform detection. - Update FIPS documentation and release notes to describe the revised env-var semantics and the prior bug.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| patches/0003-Implement-crypto-internal-backend.patch | Updates the patched Go source (notably fips140.go) to validate env var values and avoid accidental platform-detection bypass. |
| eng/doc/fips/README.md | Documents the new explicit-disable behavior and the “ignore unexpected values” rule; adds release notes for the change. |
Patches are happy!
Comment on lines
+2311
to
+2324
| + // Only "0" and "1" are meaningful values for GOFIPS and GOLANG_FIPS. | ||
| + // Any other value (including the empty string) is treated as if the | ||
| + // variable were unset, to avoid silently bypassing the platform FIPS | ||
| + // detection due to a typo or accidental setting. | ||
| + // "0" explicitly disables FIPS mode, including the platform-specific | ||
| + // FIPS detection. | ||
| + if v, ok := syscall.Getenv("GOFIPS"); ok && (v == "0" || v == "1") { | ||
| + message = "environment variable GOFIPS=" + v | ||
| + value = v | ||
| + } else if v, ok := syscall.Getenv("GOLANG_FIPS"); ok && (v == "0" || v == "1") { | ||
| + message = "environment variable GOLANG_FIPS=" + v | ||
| + value = v | ||
| + } else if systemFIPSMode() { | ||
| + message = "system FIPS mode" |
There was a problem hiding this comment.
This change introduces nuanced precedence/validation rules (e.g., ignore GOFIPS/GOLANG_FIPS unless exactly 0/1, and ensure invalid values don’t bypass systemFIPSMode() detection). There doesn’t appear to be any unit test coverage for these cases; adding focused tests around env var combinations/precedence would help prevent regressions.
dagood
reviewed
Apr 21, 2026
michelle-clayton-work
force-pushed
the
dev/mclayton/fix-gofips-2184
branch
3 times, most recently
from
3ceeeb2 to
4809d21
Compare
Fix GOFIPS handling to match the documented behavior: only GOFIPS=1 enables FIPS mode, and any other value (including 0 and the empty string) is treated as if GOFIPS were unset. The same applies to GOLANG_FIPS. Add GODEBUG=fips140=off as the supported way to explicitly disable FIPS mode and skip the platform-specific FIPS detection (e.g. the Linux kernel FIPS flag at /proc/sys/crypto/fips_enabled). Previously, due to a bug, setting GOFIPS or GOLANG_FIPS to any value silently bypassed the platform FIPS detection while only =1 actually enabled FIPS mode. That contradicted the docs and made it easy to accidentally disable FIPS detection through a typo or empty assignment. Programs that previously relied on GOFIPS=0 to skip platform FIPS detection should switch to GODEBUG=fips140=off. Fixes #2184
michelle-clayton-work
force-pushed
the
dev/mclayton/fix-gofips-2184
branch
from
4809d21 to
8f8ffd3
Compare
qmuntal
approved these changes
Apr 23, 2026
gdams
approved these changes
Apr 23, 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
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.
This pull request updates the FIPS mode detection logic and documentation in the Microsoft Go build to clarify and improve how environment variables control FIPS mode. The changes ensure that only explicit values (
on,off,0,1) affect FIPS mode, preventing accidental bypass of platform-specific detection, and update the documentation to match the new behavior.FIPS Mode Detection and Environment Variable Handling:
fips140.goto ensure that onlyGODEBUG=fips140=on|only|debugenables FIPS,GODEBUG=fips140=offexplicitly disables FIPS (and skips platform detection), and onlyGOFIPS=0|1andGOLANG_FIPS=0|1are meaningful—any other value is ignored and does not skip platform detection.README.md) thatGODEBUG=fips140=off,GOFIPS=0, andGOLANG_FIPS=0now explicitly disable FIPS mode and skip platform-specific detection, while other values are ignored. [1] [2]Documentation and Release Notes:
GODEBUG=fips140=off,GOFIPS=0, andGOLANG_FIPS=0, including the fix for a previous bug where settingGOFIPSorGOLANG_FIPSto any value would skip platform detection. [1] [2]Codebase Updates: