Uh oh!
There was an error while loading. Please reload this page.
Build tests against the FIPS 140-3 validated Go crypto module - #6193
Conversation
Integration test reportCommit: 3b0ca32
10 interesting tests: 4 SKIP, 3 KNOWN, 2 flaky, 1 RECOVERED
Top 6 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac
4493134 to
8b08765Compare| cwd, err := os.Getwd() | ||
| require.NoError(t, err) | ||
| execPath := BuildCLI(t, getBuildDir(t, cwd, runtime.GOOS, runtime.GOARCH), "", runtime.GOOS, runtime.GOARCH) |
There was a problem hiding this comment.
We already build CLI in acceptance test runner, we can add this check there as a parallel test.
There was a problem hiding this comment.
Yes, regular go version -m in acceptance test should be good enough check I think, I'll update this
| func TestCLIBuiltWithFIPSModule(t *testing.T) { | ||
| // Required: the integration suite runs this package too (task integration passes | ||
| // ./acceptance), and it runs from eng-dev-ecosystem, which does not set GOFIPS140. | ||
| if os.Getenv("GOFIPS140") == "" { |
There was a problem hiding this comment.
I'd not add this skip if it's important that we test it. We can just enable env var here and in eng-dev-ecosystem and then add a test if needed.
There was a problem hiding this comment.
I wanted to keep it as skipped to have more info. With this we can verify that env is correctly/incorrectly set in other pipelines correctly, but we don't actually block anything as it's not failing whole suite
I'll drop it once all other places are merged
| execPath := BuildCLI(t, getBuildDir(t, cwd, runtime.GOOS, runtime.GOARCH), "", runtime.GOOS, runtime.GOARCH) | ||
| out, err := exec.Command("go", "version", "-m", execPath).Output() |
There was a problem hiding this comment.
this could probably be regular acceptance test? go version -m $CLI
| ENVFILTER: DATABRICKS_BUNDLE_ENGINE=${{ matrix.deployment }} | ||
| # Pinned to a frozen version: only v1.0.0 has a CMVP certificate, and | ||
| # "latest" tracks the in-tree source. | ||
| GOFIPS140: v1.0.0 |
There was a problem hiding this comment.
Why not add this env var to Taskfile.yml? Then everyone gets it.
| @@ -0,0 +1 @@ | |||
| trace go version -m $CLI | contains.py 'GOFIPS140=v1.0.0' 'DefaultGODEBUG=fips140=on' > LOG.buildinfo | |||
There was a problem hiding this comment.
Nit: can you write the grep -i fips matches to output.txt as well (e.g. via tee)
Nice to see the actual values committed as well.
| @@ -0,0 +1 @@ | |||
| trace go version -m $CLI | grep -i fips140 | |||
There was a problem hiding this comment.
Btw the "contains" assert is still good because it fails the test if the match doesn't happen.
Output changes are visible but can be glossed over.
There was a problem hiding this comment.
Fixed
Failed test looks like this:
--- Expected
+++ Actual
@@ -2,5 +2,6 @@
>>> go version -m [CLI]
- build -tags=fips140v1.0
- build DefaultGODEBUG=fips140=on
- build GOFIPS140=v1.0.0-c2097c7c
+contains error: 'DefaultGODEBUG=fips140=on' not found in the output.
+contains error: 'GOFIPS140=v1.0.0' not found in the output.
+Exit code: 1
Uh oh!
There was an error while loading. Please reload this page.
…abricks#6262) ## Changes Build released binaries with `GOFIPS140=v1.0.0`. This links the FIPS 140-3 validated Go Cryptographic Module into the binary and defaults FIPS 140-3 mode on, so no runtime flag is needed. ## Why Regulated environments (FedRAMP, IL5) require cryptography to come from a validated module. This has to be set at build time: the runtime `GODEBUG=fips140=on` only toggles the mode on whatever module was already linked, so without the build variable, there is no validated module in the binary and no certificate to cite. Pinned to a frozen version rather than an alias. `v1.0.0` is the version covered by CMVP certificate `databricks#5247` This changes behavior for everyone, not just regulated users: the TLS client offers only FIPS-approved suites (6 instead of 13, dropping ChaCha20 and CBC) CI coverage for the FIPS build is added separately in databricks#6193 ## Tests Ran the whole unit and acceptance suite against a FIPS build — no failures. Also confirmed all six release targets (linux/darwin/windows x amd64/arm64) cross-compile with the variable set; none of the platforms Go excludes from FIPS mode are in our matrix. _This PR was written by Claude Code._
Changes
Sets
GOFIPS140=v1.0.0inTaskfile.yml, so everytaskinvocation builds the CLI against the FIPS 140-3 validated Go Cryptographic Module. Adds a test that reads the built binary's build info to confirm it really was built with it.Why
Releases are moving to FIPS builds (#6262). Go picks its cryptographic module at build time, so testing without it exercises a configuration we no longer ship.
In the Taskfile rather than a workflow env because every consumer goes through
task— local./task test, CI'stask test, and the integration suite, which runstask integrationfrom eng-dev-ecosystem. One line covers all three, with no cross-repo change. Barego testoutsidetaskis not covered, so the check fails there rather than silently passing.Worth flagging: this makes eng-dev-ecosystem integration runs FIPS without a change in that repo.
Tests
Full unit + acceptance suite under FIPS, locally and in CI (all six cells, macOS and Windows included) — no failures. Integration against real workspaces under FIPS: ~11,000 tests across six cells (three clouds x linux/windows), and the FIPS check passed in every cell. The four unrelated failures were a leftover-state 409 on AWS and three cluster timing flakes; baseline nightlies show the same rate without FIPS.
Perf: CLI startup 12ms -> 13ms, RSA-4096 key generation 653ms -> 1.35s. The new build tag invalidates the Go build cache once, so the first build after this merges is a full rebuild.