Use native Go for Linux FIPS builds#51345
Conversation
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR migrates Linux FIPS builds from OpenSSL-based Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This pull request is now in conflicts. Could you fix it? 🙏 |
This comment has been minimized.
This comment has been minimized.
28dc67a to
a504c40
Compare
TL;DR
Remediation
Investigation detailsRoot CauseThe PR changes Evidence
Validation
Follow-up
What is this? | From workflow: PR Actions Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Switch Linux FIPS builds from Microsoft Go (GOEXPERIMENT=systemcrypto) to the upstream Go native FIPS module (GOFIPS140=v1.0.0, CMVP cert #5247). Drops CGO requirement and the Microsoft Go cross-build Docker image. Updates package tests to verify GOFIPS140 and DefaultGODEBUG buildinfo instead of GOEXPERIMENT and OpenSSL ELF symbols. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove GOEXPERIMENT=systemcrypto from CI pipelines and ECH test script - Switch FIPS unit/ECH test steps from AWS FIPS image to standard GCP image; unskip - Update VerifyFIPSBinary to check GOFIPS140+DefaultGODEBUG instead of GOEXPERIMENT+OpenSSL symbols - Strengthen GOFIPS140 assertion to require v1.0.0 version; improve error messages - Remove dead CGO field and OR from FIPSConfig/DefaultBuildArgs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co>
…or crossbuild - Inject FIPS compile env vars (GOFIPS140=v1.0.0) into go test args so that test binaries actually enforce FIPS crypto at runtime, not just compile with the requirefips tag. - Pre-populate golang.org/fips140 in the host module cache before FIPS crossbuild containers mount it read-only, avoiding a write failure when the toolchain tries to unpack the module inside the container. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add verification_mode:strict so Go's native TLS stack performs chain verification (InsecureSkipVerify=false) and the FIPS key-size check triggers. Update the expected error string to match native Go's message. Simplify the test comment to remove implementation details. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Picks up the unreleased FIPS 140-3 peer-cert key-type enforcement (elastic/elastic-agent-libs#426), which TestConnectionTLS depends on. Update the test's expected error string to match the actual message that check produces, rather than Go's own (unreachable, since a custom TLS callback is always installed) internal FIPS error. This is a temporary pin for CI validation ahead of a tagged elastic-agent-libs release containing the fix.
Swaps the temporary pseudo-version pin (unreleased main commit) for the now-tagged release containing the FIPS 140-3 peer-cert key-type enforcement fix (elastic/elastic-agent-libs#426). Same commit content (d72ff2012311), now backed by a real release.
- gotest.go: wire fipsTestEnv() into makeGoTestArgsForPackage too, so per-module integration test builds (FIPS=true mage goIntegTest on a single module) get GOFIPS140 set, not just the requirefips tag. - crossbuild.go: drop the bogus "G122" gosec rule code from the nolint comment on os.Chown (gosec has no G122; nolint:gosec suppresses by linter name, so the code was misleading, not functional). - connection_test.go: cover both elastic-agent-libs FIPS callback paths (verification_mode strict and none use different callbacks) and assert the RSA-1024-specific error message instead of the more generic FIPS-rejection substring.
Contains would tolerate v1.0.0 appearing anywhere in the buildinfo value; HasPrefix asserts it's actually the version identifier itself.
The comment named elastic-agent-libs' unexported checkAllChainsFIPS and fipsVerifyNoneCallback, leaking implementation details of a dependency with no compatibility guarantee. Describe the observable strict-vs-none distinction instead.
df4e775 to
53abbd3
Compare
|
@macdewee let's make sure someone from @elastic/elastic-agent-data-plane also gives a proper review since my membership dismissed 😄 |
fr4nc1sc0-r4m0n
left a comment
There was a problem hiding this comment.
The CI changes LGTM
|
Queued — the merge queue status continues in this comment ↓. |
Merge Queue Status
This pull request spent 17 minutes 46 seconds in the queue, including 17 minutes 17 seconds running CI. Waiting for
All conditions
ReasonThe merge conditions cannot be satisfied due to failing checks
Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. Tick the box to put this pull request back in the merge queue (same as
|
TL;DRMetricbeat Remediation
Investigation detailsRoot Cause
func NewWrapper(config *conf.C, r *mb.Register, info *beat.Info, monitoring beatmonitoring.Monitoring, p *paths.Path, options ...Option) (*Wrapper, error)Most calls in Evidence
I also reproduced the package compile failure locally: Verification
Follow-upSince the failing line comes from What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
…eGoTestArgs Picks up elastic/elastic-agent-libs#426 (enforce FIPS 140-3 peer cert key types across all TLS verification modes), mirroring the native-Go FIPS migration landing in elastic/beats#51345. Replaces a local filesystem replace directive used to validate against that PR before it merged. Also adds TestMakeGoTestArgsFIPS covering the FIPS env propagation added in the prior follow-up commit.
* Follow up native Go FIPS migration: remove dead msft-go artefacts and tighten test assertion - Remove MS_GOTOOLCHAIN_TELEMETRY_ENABLED from the FIPS integration pipeline and magefile.go Windows archive build; both Linux and Windows FIPS now use GOFIPS140 (native Go), so the Microsoft toolchain env var is dead code. - Remove the msft-go toolchain smoke-check from buildkite-integration-tests.sh; the check was specific to GOEXPERIMENT=systemcrypto and is no longer meaningful. - Remove CGO field from FIPSConfig.Compile and its use in DefaultBuildArgs; native Go FIPS does not require CGO. - Propagate FIPS compile env vars (e.g. GOFIPS140) into Go test args so unit tests run with the same crypto constraints as packaged binaries. - Change GOFIPS140 test assertion from require.Contains(..., "v1.0.0") to require.Equal so it fails loudly on any version mismatch. - Update tls.go comment to reference GOFIPS140 instead of the removed GOEXPERIMENT paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: assert prefix rather than exact match for GOFIPS140 env Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> * Bump elastic-agent-libs to v0.46.0 and add FIPS test coverage for makeGoTestArgs Picks up elastic/elastic-agent-libs#426 (enforce FIPS 140-3 peer cert key types across all TLS verification modes), mirroring the native-Go FIPS migration landing in elastic/beats#51345. Replaces a local filesystem replace directive used to validate against that PR before it merged. Also adds TestMakeGoTestArgsFIPS covering the FIPS env propagation added in the prior follow-up commit. * docs: add changelog fragment for FIPS TLS cert key-type fix Documents the customer-facing fix (elastic-agent-libs v0.46.0 bump) where FIPS mode was silently accepting TLS peer certificates with non-approved key types due to a native Go FIPS gap with custom verify callbacks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Apply suggestion from @macdewee --------- Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@Mergifyio backport 9.5 9.4 9.3 |
✅ Backports have been createdDetails
Cherry-pick of a829d56 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Cherry-pick of a829d56 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Mergify left unresolved conflict markers in three Buildkite pipeline files. Resolve by keeping 9.3's image versions (1782878510) and dropping IMAGE_UBUNTU_X86_64_FIPS, which no step references after the switch to native Go FIPS (GOFIPS140=v1.0.0). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Use native Go for Linux FIPS builds (#51345) * feat: use native Go for Linux FIPS builds Switch Linux FIPS builds from Microsoft Go (GOEXPERIMENT=systemcrypto) to the upstream Go native FIPS module (GOFIPS140=v1.0.0, CMVP cert #5247). Drops CGO requirement and the Microsoft Go cross-build Docker image. Updates package tests to verify GOFIPS140 and DefaultGODEBUG buildinfo instead of GOEXPERIMENT and OpenSSL ELF symbols. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: clean up MS Go FIPS artifacts - Remove GOEXPERIMENT=systemcrypto from CI pipelines and ECH test script - Switch FIPS unit/ECH test steps from AWS FIPS image to standard GCP image; unskip - Update VerifyFIPSBinary to check GOFIPS140+DefaultGODEBUG instead of GOEXPERIMENT+OpenSSL symbols - Strengthen GOFIPS140 assertion to require v1.0.0 version; improve error messages - Remove dead CGO field and OR from FIPSConfig/DefaultBuildArgs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: golangci-lint fixes Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> * fix: propagate FIPS env to unit tests and pre-populate module cache for crossbuild - Inject FIPS compile env vars (GOFIPS140=v1.0.0) into go test args so that test binaries actually enforce FIPS crypto at runtime, not just compile with the requirefips tag. - Pre-populate golang.org/fips140 in the host module cache before FIPS crossbuild containers mount it read-only, avoiding a write failure when the toolchain tries to unpack the module inside the container. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update TestConnectionTLS for native Go FIPS Add verification_mode:strict so Go's native TLS stack performs chain verification (InsecureSkipVerify=false) and the FIPS key-size check triggers. Update the expected error string to match native Go's message. Simplify the test comment to remove implementation details. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * mod: pin elastic-agent-libs to main for native Go FIPS validation Picks up the unreleased FIPS 140-3 peer-cert key-type enforcement (elastic/elastic-agent-libs#426), which TestConnectionTLS depends on. Update the test's expected error string to match the actual message that check produces, rather than Go's own (unreachable, since a custom TLS callback is always installed) internal FIPS error. This is a temporary pin for CI validation ahead of a tagged elastic-agent-libs release containing the fix. * notice: regenerate for elastic-agent-libs pin bump * mod: bump elastic-agent-libs to released v0.45.0 Swaps the temporary pseudo-version pin (unreleased main commit) for the now-tagged release containing the FIPS 140-3 peer-cert key-type enforcement fix (elastic/elastic-agent-libs#426). Same commit content (d72ff2012311), now backed by a real release. * notice: regenerate for elastic-agent-libs v0.45.0 * fix: address reviewer findings on FIPS migration - gotest.go: wire fipsTestEnv() into makeGoTestArgsForPackage too, so per-module integration test builds (FIPS=true mage goIntegTest on a single module) get GOFIPS140 set, not just the requirefips tag. - crossbuild.go: drop the bogus "G122" gosec rule code from the nolint comment on os.Chown (gosec has no G122; nolint:gosec suppresses by linter name, so the code was misleading, not functional). - connection_test.go: cover both elastic-agent-libs FIPS callback paths (verification_mode strict and none use different callbacks) and assert the RSA-1024-specific error message instead of the more generic FIPS-rejection substring. * test: require GOFIPS140 value to start with v1.0.0, not just contain it Contains would tolerate v1.0.0 appearing anywhere in the buildinfo value; HasPrefix asserts it's actually the version identifier itself. * test: describe TestConnectionTLS modes by behavior, not libs internals The comment named elastic-agent-libs' unexported checkAllChainsFIPS and fipsVerifyNoneCallback, leaking implementation details of a dependency with no compatibility guarantee. Describe the observable strict-vs-none distinction instead. --------- Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit a829d56) * fix: suppress gosec G706 on dev-tool log line DEV_OS/DEV_ARCH are trusted build-time env vars set by CI, not user input. Add nolint:gosec to match the fix already present on main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> Co-authored-by: Dominik Rosiek <6997361+macdewee@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Dominik Rosiek <dominik.rosiek@elastic.co>
* Use native Go for Linux FIPS builds (#51345) * feat: use native Go for Linux FIPS builds Switch Linux FIPS builds from Microsoft Go (GOEXPERIMENT=systemcrypto) to the upstream Go native FIPS module (GOFIPS140=v1.0.0, CMVP cert #5247). Drops CGO requirement and the Microsoft Go cross-build Docker image. Updates package tests to verify GOFIPS140 and DefaultGODEBUG buildinfo instead of GOEXPERIMENT and OpenSSL ELF symbols. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: clean up MS Go FIPS artifacts - Remove GOEXPERIMENT=systemcrypto from CI pipelines and ECH test script - Switch FIPS unit/ECH test steps from AWS FIPS image to standard GCP image; unskip - Update VerifyFIPSBinary to check GOFIPS140+DefaultGODEBUG instead of GOEXPERIMENT+OpenSSL symbols - Strengthen GOFIPS140 assertion to require v1.0.0 version; improve error messages - Remove dead CGO field and OR from FIPSConfig/DefaultBuildArgs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: golangci-lint fixes Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> * fix: propagate FIPS env to unit tests and pre-populate module cache for crossbuild - Inject FIPS compile env vars (GOFIPS140=v1.0.0) into go test args so that test binaries actually enforce FIPS crypto at runtime, not just compile with the requirefips tag. - Pre-populate golang.org/fips140 in the host module cache before FIPS crossbuild containers mount it read-only, avoiding a write failure when the toolchain tries to unpack the module inside the container. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update TestConnectionTLS for native Go FIPS Add verification_mode:strict so Go's native TLS stack performs chain verification (InsecureSkipVerify=false) and the FIPS key-size check triggers. Update the expected error string to match native Go's message. Simplify the test comment to remove implementation details. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * mod: pin elastic-agent-libs to main for native Go FIPS validation Picks up the unreleased FIPS 140-3 peer-cert key-type enforcement (elastic/elastic-agent-libs#426), which TestConnectionTLS depends on. Update the test's expected error string to match the actual message that check produces, rather than Go's own (unreachable, since a custom TLS callback is always installed) internal FIPS error. This is a temporary pin for CI validation ahead of a tagged elastic-agent-libs release containing the fix. * notice: regenerate for elastic-agent-libs pin bump * mod: bump elastic-agent-libs to released v0.45.0 Swaps the temporary pseudo-version pin (unreleased main commit) for the now-tagged release containing the FIPS 140-3 peer-cert key-type enforcement fix (elastic/elastic-agent-libs#426). Same commit content (d72ff2012311), now backed by a real release. * notice: regenerate for elastic-agent-libs v0.45.0 * fix: address reviewer findings on FIPS migration - gotest.go: wire fipsTestEnv() into makeGoTestArgsForPackage too, so per-module integration test builds (FIPS=true mage goIntegTest on a single module) get GOFIPS140 set, not just the requirefips tag. - crossbuild.go: drop the bogus "G122" gosec rule code from the nolint comment on os.Chown (gosec has no G122; nolint:gosec suppresses by linter name, so the code was misleading, not functional). - connection_test.go: cover both elastic-agent-libs FIPS callback paths (verification_mode strict and none use different callbacks) and assert the RSA-1024-specific error message instead of the more generic FIPS-rejection substring. * test: require GOFIPS140 value to start with v1.0.0, not just contain it Contains would tolerate v1.0.0 appearing anywhere in the buildinfo value; HasPrefix asserts it's actually the version identifier itself. * test: describe TestConnectionTLS modes by behavior, not libs internals The comment named elastic-agent-libs' unexported checkAllChainsFIPS and fipsVerifyNoneCallback, leaking implementation details of a dependency with no compatibility guarantee. Describe the observable strict-vs-none distinction instead. --------- Signed-off-by: Dominik Rosiek <dominik.rosiek@elastic.co> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit a829d56) # Conflicts: # .buildkite/libbeat/pipeline.libbeat.yml # .buildkite/x-pack/pipeline.xpack.filebeat.yml # .buildkite/x-pack/pipeline.xpack.metricbeat.yml * fix: resolve merge conflicts in backport of #51345 Mergify left unresolved conflict markers in three Buildkite pipeline files. Resolve by keeping 9.3's image versions (1782878510) and dropping IMAGE_UBUNTU_X86_64_FIPS, which no step references after the switch to native Go FIPS (GOFIPS140=v1.0.0). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: suppress gosec G706 on dev-tool log line DEV_OS/DEV_ARCH are trusted build-time env vars set by CI, not user input. Add nolint:gosec to match the fix already present on main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Dominik Rosiek <6997361+macdewee@users.noreply.github.com> Co-authored-by: Dominik Rosiek <dominik.rosiek@elastic.co> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@Mergifyio backport 9.5 9.4 9.3 8.19 |
|
@Mergifyio backport 9.5 9.4 9.3 8.19 |
Proposed commit message
Go 1.24+ ships its own FIPS 140-3 certified crypto module (
GOFIPS140), so we no longer need the Microsoft Go fork (GOEXPERIMENT=systemcrypto) which required CGO and a linked OpenSSL. This switchesauditbeat,filebeat, andmetricbeatLinux FIPS builds to use the native module instead.Changes:
fips-settings.yaml: replaceGOEXPERIMENT=systemcrypto+cgo: truewithGOFIPS140: v1.0.0crossbuild.go: stop selecting the MS Go-fipscrossbuild Docker imagebuild.go/settings.go: remove dead CGO field and ORcheckFIPS()/VerifyFIPSBinary(): checkGOFIPS140+DefaultGODEBUG=fips140=oninstead ofGOEXPERIMENT+ OpenSSL ELF symbolsMirrors elastic/elastic-agent@07f98a8.
Checklist
I have commented my code, particularly in hard-to-understand areasI have made corresponding changes to the documentationI have made corresponding change to the default configuration files./changelog/fragmentsusing the changelog tool.Disruptive User Impact
FIPS binaries no longer link against system OpenSSL — no runtime OpenSSL dependency.
How to test this PR locally
Related issues