Conversation
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
|
The These appear to be golang/go#75148, which should be fixable when golang/go#74630 is implemented. However, in order to upgrade to Go 1.25.1 now, we'll need to find a workaround. |
These errors are coming from Go downloading dependencies before executing the tests. The errors can be simulated like so: So we probably just need to download the dependencies explicitly, ensuring that |
I've implemented this approach in this PR and it has helped. However, now CI is failing with this odd error which seems unrelated to FIPS in any way. |
d8c500d to
0b69f33
Compare
Turns out this is a change in behavior in Go 1.25: https://tip.golang.org/doc/go1.25#change-to-unhandled-panic-output. Addressed in 46cc036. |
|
Looking at the latest build I see a couple of strange things (maybe some of those were already there and didn't notice until now)
|
|
Windows build steps are failing in CI on this PR. See a lot of |
4db5a4a to
1ceaae8
Compare
|
52d2f67 to
b6b1a81
Compare
|
This pull request is now in conflicts. Could you fix it? 🙏 |
b6b1a81 to
92a9716
Compare
a41a670 to
eaf3ce7
Compare
|
This pull request is now in conflicts. Could you fix it? 🙏 |
| // We also set GODEBUG=tlsmlkem=0 to disable the X25519MLKEM768 TLS key | ||
| // exchange mechanism; without this setting and with the GODEBUG=fips140=only | ||
| // setting, we get errors in tests like so: | ||
| // Failed to connect: crypto/ecdh: use of X25519 is not allowed in FIPS 140-only mode |
There was a problem hiding this comment.
Where are we using this? The FIPS mode is supposed to default you to only using FIPS compatible parts of TLS without us having to do anything. Are we explicitly testing a non-FIPS mechanism somewhere?
There was a problem hiding this comment.
That, I don't really know, Shaunak is the FIPS expert and he handled that part. I say we merge as-is and he can revisit this after he's back.
There was a problem hiding this comment.
=== FAIL: internal/pkg/remote TestClientWithCertificate/fips_invalid_key_fips140only (0.00s)
client_fips_test.go:186:
Error Trace: /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/internal/pkg/remote/client_fips_test.go:186
Error: "all hosts failed: requester 0/1 to host https://127.0.0.1:64269/ errored: Get \"https://127.0.0.1:64269/echo-hello?\": crypto/ecdh: use of X25519 is not allowed in FIPS 140-only mode" does not contain "use of keys smaller than 2048 bits is not allowed in FIPS 140-only mode"
Test: TestClientWithCertificate/fips_invalid_key_fips140only
=== FAIL: internal/pkg/remote TestClientWithCertificate/fips_valid_key_fips140only (0.00s)
client_fips_test.go:181:
Error Trace: /Users/cmackenzie/go/src/github.com/elastic/elastic-agent/internal/pkg/remote/client_fips_test.go:181
Error: Expected value not to be nil.
Test: TestClientWithCertificate/fips_valid_key_fips140only
=== FAIL: internal/pkg/remote TestClientWithCertificate (0.00s)
We are using this in a bunch of places, seeing it fail in fips specific tests is concerning. This particular test has a pre-generated certificate that appears to no longer be FIPS compliant.
It also comes up in the FakeInputSuite a lot for the gRPC connection which is more concerning.
There was a problem hiding this comment.
The FIPS mode is supposed to default you to only using FIPS compatible parts of TLS without us having to do anything.
tlsmlkem=0 is needed because of X25519MLKEM768, the hybrid post-quantum key exchange that Go 1.25 enables by default in TLS. It combines X25519 (not FIPS-approved) and ML-KEM-768. Even though Go's TLS layer lists X25519MLKEM768 in allowedCurvePreferencesFIPS (defaults_fips140.go:33), the underlying crypto/ecdh primitive unconditionally rejects X25519 under fips140=only (x25519.go:39), causing the handshake to fail before we ever get to certificate validation. The FIPS-safe post-quantum alternatives SecP256r1MLKEM768 and SecP384r1MLKEM1024 are also in allowedCurvePreferencesFIPS and would work fine — this is a bug in Go that I've filed at golang/go#78178.
In the short term, running these unit tests on the same FIPS-hardened VMs used by the integration FIPS tests (the IMAGE_UBUNTU_X86_64_FIPS images in bk.integration-fips.pipeline.yml) would avoid the need for tlsmlkem=0 — those VMs use the Microsoft Go SDK, which routes crypto through OpenSSL in FIPS mode. Since OpenSSL in FIPS mode doesn't support X25519, the Microsoft SDK automatically excludes X25519MLKEM768 from the default curve preferences before the ClientHello is even constructed. This is a workaround though, until the upstream Go issue is resolved.
There was a problem hiding this comment.
@ycombinator Thanks for the detailed explanation that is very helpful in understand what was going one. With this merged its to late to add this context as a comment, but that would be helpful in the future.
There was a problem hiding this comment.
I'm not sure we are blocked any more. Given that this is expected behavior (bug) in Go 1.25 and is fixed in Go 1.26, isn't supplying the tlsmlkem=0 flag in Go 1.25 so the tests pass an acceptable workaround until we upgrade to Go 1.26? This is already being done, in this PR and its backports.
Running these tests on the FIPS VMs would let us stay on Go 1.25 and no longer pass the tlsmlkem=0 flag. I'm not sure it's worth making this change, but happy to do it if that's the consensus.
Considering that we already have the first workaround in place, my vote would actually be to wait until we upgrade to Go 1.26. FWIW, we already have golang-crossbuild images available for Go 1.26.1 so we could put up a PR to make that bump instead of the VM change. As a precedent, when Go 1.24 was released on Feb. 11, 2025, we put up a PR to bump to that version about a week later.
There was a problem hiding this comment.
@ebeahan @swiatekm @blakerouse @cmacknz Just want to check if you all are in agreement that we are not blocked on the backports any more. I know there's discussion in Slack as well so I just wanted to get the discussion back to this comment thread since it has all the context. Thanks.
There was a problem hiding this comment.
Yes we have a path forward. Let's move forward with the backports to the release branches, and then we'll update to 1.26 with the bugfix in the near future.
There was a problem hiding this comment.
@ycombinator Yes all good from me as well, go ahead with the backports.
There was a problem hiding this comment.
@ycombinator yes, we should proceed with the backports.
blakerouse
left a comment
There was a problem hiding this comment.
This overall looks good. I am interested in the answers to @cmacknz questions as well. Holding off on approval until I read those.
This reverts commit 88ece11
ebeahan
left a comment
There was a problem hiding this comment.
Approving based on team discussion to merge to main and hold off to merge backports until we have more information about the FIPS related changes.
⏳ Build in-progress, with failures
Failed CI Steps
History
|
|
@Mergifyio backport 8.19 9.2 9.3 |
❌ No backport have been createdDetails
Git reported the following error:
Git reported the following error:
Git reported the following error: |
* Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
* Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
* Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
* Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
* Bump Go version to 1.25.8 (#10156) * Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * go mod tidy * fix NOTICE files --------- Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com> Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Eric Beahan <eric.beahan@elastic.co>
* Bump Go version to 1.25.8 (#10156) * Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * go mod tidy * fix NOTICE files --------- Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com> Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: ebeahan <eric.beahan@elastic.co>
* Bump Go version to 1.25.8 (#10156) * Bump Go version to 1.25.1 * Update CHANGELOG entry * Bump the version of golangci-lint * Update dev-tools/mage/otel/deps_test.go Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> * Remove references to the ms_tls13kdf build tag * Use mage target specifically intended for fips140=only unit testing * Download go module dependencies before GODEBUG=fips140=only is set * [Debugging] Print go test command environment right before command is run * Revert "[Debugging] Print go test command environment right before command is run" This reverts commit 079e74c. * Fix regular expression for matching panicking output * Run dependencies first * Append requirefips build tag for fips140=only unit tests * Fix policy change handler unit tests * Fix TestDownloadVersion test * Set GODEBUG=tlsmlkem=0 for test * No longer necessary * Formatting fixes * Remove pre-downloading of go modules * Revert unit test pipeline changes * Add GODEBUG=tlsmlkem=0 * s/1.25.1/1.25.2/g * Go version: s/1.25.2/1.25.4/g * Update go version in edot go.mod * s/1.24.4/1.24.5/g * Remove ms_tls13kdf from TestTagsWithFIPS * Running go mod tidy * s/1.25.5/1.25.8/g * Running go mod tidy and mage notice * Running mage otel:readme * Fixing go.mods * Bumping up beats * Running go mod tidy * Update unit tests * Use Go 1.24's RemoveAll in uninstall * Wait for fsnotify watcher to be released * Update internal/pkg/agent/application/upgrade/marker_watcher.go Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * Fix function signature in MarkerFileWatcher * Use the exact removaAll implementation from Go 1.24 * Add link to the new RemoveAll implementation * Revert "Use the exact removaAll implementation from Go 1.24" This reverts commit 88ece11 --------- Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> * try to manual bump to beats@c1cb121b7e6f to try unblocking crossbuild/npcap CI failures * Bump beats to c1cb121b7e6f812dd7857a1192648b39b13dae47 --------- Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com> Co-authored-by: Panos Koutsovasilis <koutsobill@hotmail.com> Co-authored-by: Mikołaj Świątek <mail@mikolajswiatek.com> Co-authored-by: Eric Beahan <eric.beahan@elastic.co>
This PR bumps up the Golang version to
1.25.8. It also:ms_tls13kdfGolang build tag when building in FIPS mode because this tag was only needed before Golang versions1.24.x.GODEBUG=tlsmlkem=0environment variable when running FIPS140-only unit tests. This prevents errors like so:Failed to connect: crypto/ecdh: use of X25519 is not allowed in FIPS 140-only mode.