Merge pull request #1503 from microsoft/dev/qmuntal/fips149deb

Prefer GODEBUG=fips140 over GOFIPS

Author: qmuntal

eng/_util/cmd/run-builder/run-builder.go

@@ -147,7 +147,7 @@ func main() {
 		}
 
 		if *fipsMode {
-			env("GOFIPS", "1")
+			envAppend("GODEBUG", "fips140=on")
 			// Enable system-wide FIPS if supported by the host platform.
 			restore, err := enableSystemWideFIPS()
 			if err != nil {
@@ -215,6 +215,15 @@ func env(key, value string) {
 	}
 }
 
+// envAppend appends a value to an env var and logs it.
+// Panics if it doesn't succeed.
+func envAppend(key, value string) {
+	if v, ok := os.LookupEnv(key); ok {
+		value = v + "," + value
+	}
+	env(key, value)
+}
+
 func run(cmdline ...string) error {
 	c := exec.Command(cmdline[0], cmdline[1:]...)
 	c.Stdout = os.Stdout

eng/_util/cmd/run-builder/systemfips_fallback.go

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-//go:build !windows && !linux
+//go:build !windows
 
 package main
 

eng/_util/cmd/run-builder/systemfips_linux.go

@@ -7,26 +7,26 @@ Subject: [PATCH] Implement crypto/internal/backend
  .gitignore                                    |   2 +
  src/crypto/internal/backend/backend_test.go   |  30 ++
  src/crypto/internal/backend/backendgen.go     |  20 +
- .../internal/backend/backendgen_test.go       | 284 ++++++++++++++
+ .../internal/backend/backendgen_test.go       | 284 +++++++++++++
  src/crypto/internal/backend/bbig/big.go       |  17 +
  .../internal/backend/bbig/big_boring.go       |  12 +
  src/crypto/internal/backend/bbig/big_cng.go   |  12 +
  .../internal/backend/bbig/big_darwin.go       |  12 +
  .../internal/backend/bbig/big_openssl.go      |  12 +
- src/crypto/internal/backend/boring_linux.go   | 279 ++++++++++++++
+ src/crypto/internal/backend/boring_linux.go   | 279 +++++++++++++
  src/crypto/internal/backend/cng_windows.go    | 336 ++++++++++++++++
  src/crypto/internal/backend/common.go         |  59 +++
  src/crypto/internal/backend/darwin_darwin.go  | 359 +++++++++++++++++
  src/crypto/internal/backend/fips140/boring.go |  11 +
  src/crypto/internal/backend/fips140/cng.go    |  33 ++
  src/crypto/internal/backend/fips140/darwin.go |  11 +
- .../internal/backend/fips140/fips140.go       |  55 +++
+ .../internal/backend/fips140/fips140.go       |  63 +++
  .../internal/backend/fips140/isrequirefips.go |   9 +
  .../internal/backend/fips140/norequirefips.go |   9 +
  .../backend/fips140/nosystemcrypto.go         |  11 +
  .../internal/backend/fips140/openssl.go       |  41 ++
- src/crypto/internal/backend/nobackend.go      | 240 ++++++++++++
- src/crypto/internal/backend/openssl_linux.go  | 360 ++++++++++++++++++
+ src/crypto/internal/backend/nobackend.go      | 240 +++++++++++
+ src/crypto/internal/backend/openssl_linux.go  | 377 ++++++++++++++++++
  src/crypto/internal/backend/stub.s            |  10 +
  src/go/build/deps_test.go                     |   7 +-
  .../exp_allowcryptofallback_off.go            |   9 +
@@ -45,7 +45,7 @@ Subject: [PATCH] Implement crypto/internal/backend
  ...ckenderr_gen_requirefips_nosystemcrypto.go |  17 +
  .../backenderr_gen_systemcrypto_nobackend.go  |  16 +
  src/runtime/runtime_boring.go                 |   5 +
- 41 files changed, 2492 insertions(+), 1 deletion(-)
+ 41 files changed, 2517 insertions(+), 1 deletion(-)
  create mode 100644 src/crypto/internal/backend/backend_test.go
  create mode 100644 src/crypto/internal/backend/backendgen.go
  create mode 100644 src/crypto/internal/backend/backendgen_test.go
@@ -1676,27 +1676,32 @@ index 00000000000000..ef5af5d956163e
 +}
 diff --git a/src/crypto/internal/backend/fips140/fips140.go b/src/crypto/internal/backend/fips140/fips140.go
 new file mode 100644
-index 00000000000000..f54d39970319af
+index 00000000000000..72f7a1644deedd
 --- /dev/null
 +++ b/src/crypto/internal/backend/fips140/fips140.go
-@@ -0,0 +1,55 @@
+@@ -0,0 +1,63 @@
 +// Copyright 2024 The Go Authors. All rights reserved.
 +// Use of this source code is governed by a BSD-style
 +// license that can be found in the LICENSE file.
 +
 +package fips140
 +
-+import "syscall"
++import (
++	"internal/godebug"
++	"syscall"
++)
 +
-+// Enabled reports whether FIPS 140 mode is enabled by using GOFIPS=1, GOLANG_FIPS=1,
++var fips140GODEBUG = godebug.New("#fips140")
++
++// Enabled reports whether FIPS 140 mode is enabled by using GODEBUG, GOFIPS, GOLANG_FIPS,
 +// the 'requirefips' build tag, or any other platform-specific mechanism.
 +func Enabled() bool {
 +	return enabled
 +}
 +
 +var enabled bool
 +
-+// Disabled reports whether FIPS 140 mode is disabled by using GOFIPS=0 or GOLANG_FIPS=0.
++// Disabled reports whether FIPS 140 mode is disabled by using GOFIPS or GOLANG_FIPS.
 +func Disabled() bool {
 +	return disabled
 +}
@@ -1709,9 +1714,12 @@ index 00000000000000..f54d39970319af
 +func init() {
 +	// TODO: Decide which environment variable to use.
 +	// See https://github.com/microsoft/go/issues/397.
-+	var value string
 +	var ok bool
-+	if value, ok = syscall.Getenv("GOFIPS"); ok {
++	value := fips140GODEBUG.Value()
++	if value == "on" || value == "only" || value == "debug" {
++		Message = "environment variable GODEBUG=fips140=" + value
++		value = "1"
++	} else if value, ok = syscall.Getenv("GOFIPS"); ok {
 +		Message = "environment variable GOFIPS"
 +	} else if value, ok = syscall.Getenv("GOLANG_FIPS"); ok {
 +		Message = "environment variable GOLANG_FIPS"
@@ -1735,7 +1743,6 @@ index 00000000000000..f54d39970319af
 +		return
 +	}
 +}
-\ No newline at end of file
 diff --git a/src/crypto/internal/backend/fips140/isrequirefips.go b/src/crypto/internal/backend/fips140/isrequirefips.go
 new file mode 100644
 index 00000000000000..b33d08c84e2dae
@@ -1787,7 +1794,7 @@ index 00000000000000..83691d7dd42d51
 +}
 diff --git a/src/crypto/internal/backend/fips140/openssl.go b/src/crypto/internal/backend/fips140/openssl.go
 new file mode 100644
-index 00000000000000..118efa3a492a7d
+index 00000000000000..2d126bcfc053de
 --- /dev/null
 +++ b/src/crypto/internal/backend/fips140/openssl.go
 @@ -0,0 +1,41 @@
@@ -1818,7 +1825,7 @@ index 00000000000000..118efa3a492a7d
 +			// If there is an error reading we could either panic or assume FIPS is not enabled.
 +			// Panicking would be too disruptive for apps that don't require FIPS.
 +			// If an app wants to be 100% sure that is running in FIPS mode
-+			// it should use boring.Enabled() or GOFIPS=1.
++			// it should use boring.Enabled() or GODEBUG=fips140=1.
 +			return false
 +		}
 +	}
@@ -2080,10 +2087,10 @@ index 00000000000000..7c3a95c2c64a2d
 +}
 diff --git a/src/crypto/internal/backend/openssl_linux.go b/src/crypto/internal/backend/openssl_linux.go
 new file mode 100644
-index 00000000000000..5ddcf98ea682a5
+index 00000000000000..0c4e0c9da6e1ce
 --- /dev/null
 +++ b/src/crypto/internal/backend/openssl_linux.go
-@@ -0,0 +1,360 @@
+@@ -0,0 +1,377 @@
 +// Copyright 2017 The Go Authors. All rights reserved.
 +// Use of this source code is governed by a BSD-style
 +// license that can be found in the LICENSE file.
@@ -2150,12 +2157,29 @@ index 00000000000000..5ddcf98ea682a5
 +		panic("opensslcrypto: can't initialize OpenSSL " + lcrypto + ": " + err.Error())
 +	}
 +	if fips140.Enabled() {
-+		// Use openssl.FIPSCapable instead of openssl.FIPS because some providers, e.g. SCOSSL, are FIPS compliant
-+		// even when FIPS mode is not enabled.
++		// Some distributions, e.g. Azure Linux 3, don't set the `fips=yes` property when running in FIPS mode,
++		// but they configure OpenSSL to use a FIPS-compliant provider (in the case of Azure Linux 3, the SCOSSL provider).
++		// In this cases, openssl.FIPS would return `false` and openssl.FIPSCapable would return `true`.
++		// We don't care about the `fips=yes` property as long as the provider is FIPS-compliant, so use
++		// openssl.FIPSCapable to determine whether FIPS mode is enabled.
 +		if !openssl.FIPSCapable() {
-+			panic("opensslcrypto: FIPS mode requested (" + fips140.Message + ") but not available in " + openssl.VersionText())
++			// This path can be reached for the following reasons:
++			// - In OpenSSL 1, the active engine doesn't support FIPS mode.
++			// - In OpenSSL 1, the active engine supports FIPS mode, but it is not enabled.
++			// - In OpenSSL 3, the provider used by default doesn't match the `fips=yes` query.
++			//
++			// A best-effort attempt is made to enable FIPS mode. It will only succeed if the following conditions are met:
++			// - In OpenSSL 1, the active engine supports FIPS mode and FIPS mode can be enabled.
++			// - In OpenSSL 3, there is an available provider that supports the `fips=yes` query.
++			//
++			// Note that this best effort is mainly to support test environments. FIPS-compliant production environments
++			// like Mariner 2 and Azure Linux 3 (when executed in kernel FIPS mode) will already be properly configured.
++			if err := openssl.SetFIPS(true); err != nil {
++				panic("opensslcrypto: FIPS mode requested (" + fips140.Message + ") but not available in " + openssl.VersionText() + ": " + err.Error())
++			}
 +		}
 +	} else if fips140.Disabled() {
++		// TODO: Remove this block when GOFIPS=0 is no longer supported.
 +		if openssl.FIPS() {
 +			panic("opensslcrypto: FIPS mode explicitly disabled (" + fips140.Message + ") but enabled in " + openssl.VersionText())
 +		}

patches/0003-Implement-crypto-internal-backend.patch

@@ -6,7 +6,7 @@ Subject: [PATCH] Use crypto backends
 ---
  src/cmd/api/boring_test.go                    |   2 +-
  src/cmd/dist/build.go                         |  13 ++
- src/cmd/dist/test.go                          |   8 +-
+ src/cmd/dist/test.go                          |  10 +-
  src/cmd/go/go_boring_test.go                  |  11 +-
  src/cmd/go/testdata/script/darwin_no_cgo.txt  |   2 +
  .../go/testdata/script/gopath_std_vendor.txt  |   9 +
@@ -38,6 +38,7 @@ Subject: [PATCH] Use crypto backends
  src/crypto/hmac/hmac_test.go                  |   2 +-
  src/crypto/internal/cryptotest/allocations.go |   2 +-
  .../internal/cryptotest/implementations.go    |   2 +-
+ src/crypto/internal/fips140test/check_test.go |   8 +-
  src/crypto/md5/md5.go                         |  10 +
  src/crypto/md5/md5_test.go                    |  16 ++
  src/crypto/pbkdf2/pbkdf2.go                   |   4 +
@@ -82,7 +83,7 @@ Subject: [PATCH] Use crypto backends
  src/net/smtp/smtp_test.go                     |  72 ++++---
  src/os/exec/exec_test.go                      |   9 +
  src/runtime/pprof/vminfo_darwin_test.go       |   6 +
- 78 files changed, 1110 insertions(+), 109 deletions(-)
+ 79 files changed, 1118 insertions(+), 111 deletions(-)
  create mode 100644 src/crypto/dsa/boring.go
  create mode 100644 src/crypto/dsa/notboring.go
  create mode 100644 src/crypto/ecdsa/badlinkname.go
@@ -132,9 +133,18 @@ index 1f467647f56143..4d770d7fc239e2 100644
  	// No need to enable PGO for toolchain2.
  	goInstall(toolenv(), goBootstrap, append([]string{"-pgo=off"}, toolchain...)...)
 diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
-index 0c992118f4287b..3316bb52a61ff1 100644
+index 0c992118f4287b..d224514552fd63 100644
 --- a/src/cmd/dist/test.go
 +++ b/src/cmd/dist/test.go
+@@ -714,7 +714,7 @@ func (t *tester) registerTests() {
+ 	})
+ 
+ 	// Check that all crypto packages compile (and test correctly, in longmode) with fips.
+-	if t.fipsSupported() {
++	if false { // Disable these tests, they don't work if CNG/OpenSSL FIPS mode is not enabled. We already have dedicated builders for this.
+ 		// Test standard crypto packages with fips140=on.
+ 		t.registerTest("GODEBUG=fips140=on go test crypto/...", &goTest{
+ 			variant: "gofips140",
 @@ -1161,6 +1161,11 @@ func (t *tester) internalLink() bool {
  	if goos == "windows" && goarch == "arm64" {
  		return false
@@ -1155,6 +1165,39 @@ index 3fa730459050f6..1f28f12a6e7b4f 100644
  	"crypto/internal/impl"
  	"internal/goos"
  	"internal/testenv"
+diff --git a/src/crypto/internal/fips140test/check_test.go b/src/crypto/internal/fips140test/check_test.go
+index 6b0cd3f39e1695..aa586ed30454a2 100644
+--- a/src/crypto/internal/fips140test/check_test.go
++++ b/src/crypto/internal/fips140test/check_test.go
+@@ -5,6 +5,8 @@
+ package fipstest
+ 
+ import (
++	boring "crypto/internal/backend"
++	bfips140 "crypto/internal/backend/fips140"
+ 	"crypto/internal/fips140"
+ 	. "crypto/internal/fips140/check"
+ 	"crypto/internal/fips140/check/checktest"
+@@ -18,7 +20,7 @@ import (
+ 	"unsafe"
+ )
+ 
+-const enableFIPSTest = true
++const enableFIPSTest = boring.Enabled
+ 
+ func TestFIPSCheckVerify(t *testing.T) {
+ 	if Verified {
+@@ -38,6 +40,10 @@ func TestFIPSCheckVerify(t *testing.T) {
+ 		t.Skipf("skipping: %v", err)
+ 	}
+ 
++	if !bfips140.Enabled() {
++		t.Skipf("skipping: FIPS is not enabled")
++	}
++
+ 	cmd := testenv.Command(t, os.Args[0], "-test.v", "-test.run=TestFIPSCheck")
+ 	cmd.Env = append(cmd.Environ(), "GODEBUG=fips140=on")
+ 	out, err := cmd.CombinedOutput()
 diff --git a/src/crypto/md5/md5.go b/src/crypto/md5/md5.go
 index a0384e175f31bd..f7aa6da36f02de 100644
 --- a/src/crypto/md5/md5.go