Skip to content

Commit 5a9ba2c

Browse files
macdeweeclaude
andauthored
Follow up native Go FIPS migration; clean up missed leftovers (#14986)
* 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>
1 parent ec960fd commit 5a9ba2c

10 files changed

Lines changed: 96 additions & 14 deletions

File tree

.buildkite/bk.integration-fips.pipeline.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
env:
44
ASDF_MAGE_VERSION: 1.14.0
5-
MS_GOTOOLCHAIN_TELEMETRY_ENABLED: "0"
65

76
IMAGE_UBUNTU_2404_X86_64: "platform-ingest-elastic-agent-ubuntu-2404-1782867684"
87
IMAGE_UBUNTU_X86_64_FIPS: "platform-ingest-elastic-agent-ubuntu-2204-fips-1782867684"

.buildkite/scripts/buildkite-integration-tests.sh

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ if [ -z "$TEST_SUDO" ]; then
1717
exit 1
1818
fi
1919

20-
if [ "${FIPS:-false}" == "true" ]; then
21-
echo "~~~FIPS: Checking msft-go is installed"
22-
GOEXPERIMENT=systemcrypto go version
23-
fi
24-
2520
if [ "$TEST_SUDO" == "true" ]; then
2621
echo "Re-initializing ASDF. The user is changed to root..."
2722
export ASDF_DATA_DIR="/opt/buildkite-agent/.asdf"
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# REQUIRED
2+
# Kind can be one of:
3+
# - breaking-change: a change to previously-documented behavior
4+
# - deprecation: functionality that is being removed in a later release
5+
# - bug-fix: fixes a problem in a previous version
6+
# - enhancement: extends functionality but does not break or fix existing behavior
7+
# - feature: new functionality
8+
# - known-issue: problems that we are aware of in a given version
9+
# - security: impacts on the security of a product or a user’s deployment.
10+
# - upgrade: important information for someone upgrading from a prior version
11+
# - other: does not fit into any of the other categories
12+
kind: bug-fix
13+
14+
# REQUIRED for all kinds
15+
# Change summary; a 80ish characters long description of the change.
16+
summary: Fixed FIPS mode accepting non-compliant TLS certificates
17+
18+
# REQUIRED for breaking-change, deprecation, known-issue
19+
# Long description; in case the summary is not enough to describe the change
20+
# this field accommodate a description without length limits.
21+
# description:
22+
23+
# REQUIRED for breaking-change, deprecation, known-issue
24+
# impact:
25+
26+
# REQUIRED for breaking-change, deprecation, known-issue
27+
# action:
28+
29+
# REQUIRED for all kinds
30+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
31+
component: elastic-agent
32+
33+
# AUTOMATED
34+
# OPTIONAL to manually add other PR URLs
35+
# PR URL: A link the PR that added the changeset.
36+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
37+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
38+
# Please provide it if you are adding a fragment for a different PR.
39+
# pr: https://github.com/owner/repo/1234
40+
41+
# AUTOMATED
42+
# OPTIONAL to manually add other issue URLs
43+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
44+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
45+
# issue: https://github.com/owner/repo/1234

dev-tools/mage/build.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ func DefaultBuildArgs(cfg *Settings) BuildArgs {
106106
for _, tag := range fipsConfig.Compile.Tags {
107107
args.ExtraFlags = append(args.ExtraFlags, "-tags="+tag)
108108
}
109-
args.CGO = args.CGO || fipsConfig.Compile.CGO
110109
for varName, value := range fipsConfig.Compile.Env {
111110
args.Env[varName] = value
112111
}

dev-tools/mage/gotest.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/magefile/mage/sh"
2020

2121
"github.com/elastic/elastic-agent/dev-tools/mage/gotool"
22+
"github.com/elastic/elastic-agent/dev-tools/packaging"
2223
)
2324

2425
// GoTestArgs are the arguments used for the "go*Test" targets and they define
@@ -52,6 +53,9 @@ func makeGoTestArgs(cfg *Settings, name string) GoTestArgs {
5253
params.CoverageProfileFile = fileName + ".cov"
5354
}
5455
if cfg.Build.FIPSBuild {
56+
for k, v := range packaging.Settings().FIPS.Compile.Env {
57+
params.Env[k] = v
58+
}
5559
// fips140=only causes the Go runtime to reject non-FIPS algorithms.
5660
// tlsmlkem=0 disables X25519MLKEM768 (uses X25519, which is not FIPS-compliant).
5761
// Both are set here rather than in the shell environment so that they apply

dev-tools/mage/gotest_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,3 +357,45 @@ var wantTestWithWrongPanic = `(?sm:
357357
.*
358358
panic: Fail in goroutine after TestGoTest_Helper_WithWrongPanic/setup_failing_go-routine has completed.*
359359
)`
360+
361+
func TestMakeGoTestArgsFIPS(t *testing.T) {
362+
t.Run("FIPS disabled: no FIPS env vars injected", func(t *testing.T) {
363+
s := DefaultSettings()
364+
s.Build.FIPSBuild = false
365+
366+
args := makeGoTestArgs(s, "Unit")
367+
368+
assert.NotContains(t, args.Tags, "requirefips")
369+
assert.NotContains(t, args.Env, "GOFIPS140")
370+
assert.NotContains(t, args.Env, "GODEBUG")
371+
})
372+
373+
t.Run("FIPS enabled: requirefips tag added", func(t *testing.T) {
374+
s := DefaultSettings()
375+
s.Build.FIPSBuild = true
376+
377+
args := makeGoTestArgs(s, "Unit")
378+
379+
assert.Contains(t, args.Tags, "requirefips")
380+
})
381+
382+
t.Run("FIPS enabled: compile env from packages.yml propagated", func(t *testing.T) {
383+
s := DefaultSettings()
384+
s.Build.FIPSBuild = true
385+
386+
args := makeGoTestArgs(s, "Unit")
387+
388+
// packages.yml sets GOFIPS140=v1.0.0 under fips.compile.env;
389+
// makeGoTestArgs must forward every key from that map.
390+
assert.Equal(t, "v1.0.0", args.Env["GOFIPS140"])
391+
})
392+
393+
t.Run("FIPS enabled: GODEBUG enforces FIPS-only runtime", func(t *testing.T) {
394+
s := DefaultSettings()
395+
s.Build.FIPSBuild = true
396+
397+
args := makeGoTestArgs(s, "Unit")
398+
399+
assert.Equal(t, "fips140=only,tlsmlkem=0", args.Env["GODEBUG"])
400+
})
401+
}

dev-tools/packaging/settings.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,6 @@ func (p Platform) Platform() string {
197197

198198
type FIPSConfig struct {
199199
Compile struct {
200-
CGO bool `yaml:"cgo"`
201200
Env map[string]string `yaml:"env"`
202201
Tags []string `yaml:"tags"`
203202
Platforms []Platform `yaml:"platforms"`

dev-tools/packaging/testing/package_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,8 @@ func checkFIPS(t *testing.T, agentPackageRootDir string) {
877877
continue
878878
case "GOFIPS140":
879879
foundFIPS = true
880-
require.NotEmpty(t, setting.Value, "GOFIPS140 must be set in binary build info")
880+
// Go embeds a commit hash suffix (e.g. v1.0.0-c2097c7c), so check by prefix.
881+
require.True(t, strings.HasPrefix(setting.Value, "v1.0.0"), "GOFIPS140 must reference the certified module version v1.0.0, got: %s", setting.Value)
881882
continue
882883
case "DefaultGODEBUG":
883884
if strings.Contains(setting.Value, "fips140=on") {

internal/edot/receivers/verifierreceiver/internal/verifier/tls.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,9 @@ var fipsCurves = []tls.CurveID{tls.CurveP256, tls.CurveP384}
2525
// AWS STS, Azure Entra ID (login.microsoftonline.com), and all *.googleapis.com.
2626
//
2727
// Full binary-level FIPS compliance also requires building with
28-
// GOEXPERIMENT=systemcrypto (RHEL) or GOEXPERIMENT=boringcrypto, which
29-
// replaces the standard Go crypto library with a FIPS 140-validated
30-
// implementation. The TLS settings here provide defence-in-depth and are
31-
// effective regardless of build mode.
28+
// GOFIPS140=v1.0.0, which uses Go's native FIPS 140-3 certified crypto module.
29+
// The TLS settings here provide defence-in-depth and are effective regardless
30+
// of build mode.
3231
func newHTTPClient() *http.Client {
3332
var transport *http.Transport
3433
if t, ok := http.DefaultTransport.(*http.Transport); ok {

magefile.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,6 @@ func (Build) windowsArchiveRootBinaryForGoArch(ctx context.Context, goarch strin
299299
// there is no actual FIPS relevance for this particular binary
300300
// but better safe than sorry
301301
args.ExtraFlags = append(args.ExtraFlags, "-tags=requirefips")
302-
args.Env["MS_GOTOOLCHAIN_TELEMETRY_ENABLED"] = "0"
303302
args.CGO = true
304303
}
305304
}

0 commit comments

Comments
 (0)