Skip to content

Commit

Permalink
Revert "Prevent unexpected Go crypto fallback: fail instead (#965)" w…
Browse files Browse the repository at this point in the history 
…ith patch revert performed manually

This reverts commit 2eb34d1.
  • Loading branch information
@dagood
dagood committed Jul 14, 2023
1 parent 22980fc commit ca7d4bc
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 252 deletions.
15 changes: 4 additions & 11 deletions eng/_core/cmd/build/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ func build(o *options) error {
return err
}

if o.Experiment != "" {
buildutil.AppendExperimentEnv(o.Experiment)
}

if !o.SkipBuild {
// If we have a stage 0 copy of Go in an env variable (as set by run.ps1), use it in the
// build command by setting GOROOT_BOOTSTRAP. The upstream build script "make.bash" uses
Expand Down Expand Up @@ -225,17 +229,6 @@ func build(o *options) error {
// redirect doesn't cause an issue where tests succeed that should have failed.)
testCmd.Stderr = os.Stdout

if o.Experiment != "" {
buildutil.AppendExperimentEnv(o.Experiment)
// We aren't able to rebuild Go standard library packages under a crypto experiment,
// but "cmd/dist/test.go" will normally do this for local test runs to make sure the
// build is clean. The problem is that the build doesn't include cgo, which is
// required for some crypto backends. Set this variable specific to Microsoft Go to
// indicate that our build is fresh because it's running within our scripts and
// doesn't need a rebuild. A patch in the test runner detects this.
os.Setenv("GO_MSFT_SCRIPTED_BUILD", "1")
}

return runCmd(testCmd)
}

Expand Down
247 changes: 6 additions & 241 deletions patches/0007-Add-backend-code-gen.patch
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,162 +25,24 @@ harder to update the generators and to deal with conflicts.
Use "go/bin/go generate crypto/internal/backend" after recently building
the repository to run the generators.
---
src/cmd/dist/build.go | 2 +-
src/cmd/dist/test.go | 9 +-
src/cmd/internal/obj/x86/pcrelative_test.go | 4 +
src/cmd/internal/testdir/testdir_test.go | 4 +
src/cmd/link/internal/ld/stackcheck_test.go | 2 +-
src/cmd/link/link_test.go | 4 +-
src/cmd/vet/vet_test.go | 2 +-
src/crypto/internal/backend/backendgen.go | 20 ++
.../internal/backend/backendgen_test.go | 279 ++++++++++++++++++
.../internal/backend/backendgen_test.go | 257 ++++++++++++++++++
src/crypto/internal/backend/nobackend.go | 2 +-
.../backenderr_gen_conflict_boring_cng.go | 17 ++
.../backenderr_gen_conflict_boring_openssl.go | 17 ++
.../backenderr_gen_conflict_cng_openssl.go | 17 ++
.../backenderr_gen_nofallback_boring.go | 19 ++
src/runtime/backenderr_gen_nofallback_cng.go | 19 ++
.../backenderr_gen_nofallback_openssl.go | 19 ++
...ckenderr_gen_requirefips_nosystemcrypto.go | 17 ++
.../backenderr_gen_systemcrypto_nobackend.go | 16 +
.../backenderr_gen_systemcrypto_nobackend.go | 16 ++
src/syscall/exec_linux_test.go | 2 +-
19 files changed, 463 insertions(+), 8 deletions(-)
9 files changed, 363 insertions(+), 2 deletions(-)
create mode 100644 src/crypto/internal/backend/backendgen.go
create mode 100644 src/crypto/internal/backend/backendgen_test.go
create mode 100644 src/runtime/backenderr_gen_conflict_boring_cng.go
create mode 100644 src/runtime/backenderr_gen_conflict_boring_openssl.go
create mode 100644 src/runtime/backenderr_gen_conflict_cng_openssl.go
create mode 100644 src/runtime/backenderr_gen_nofallback_boring.go
create mode 100644 src/runtime/backenderr_gen_nofallback_cng.go
create mode 100644 src/runtime/backenderr_gen_nofallback_openssl.go
create mode 100644 src/runtime/backenderr_gen_requirefips_nosystemcrypto.go
create mode 100644 src/runtime/backenderr_gen_systemcrypto_nobackend.go

diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go
index 8973a871682816..61a2f52a462f4c 100644
--- a/src/cmd/dist/build.go
+++ b/src/cmd/dist/build.go
@@ -1472,7 +1472,7 @@ func cmdbootstrap() {
// Now that cmd/go is in charge of the build process, enable GOEXPERIMENT.
os.Setenv("GOEXPERIMENT", goexperiment)
// No need to enable PGO for toolchain2.
- goInstall(toolenv(), goBootstrap, append([]string{"-pgo=off"}, toolchain...)...)
+ goInstall(toolenv(), goBootstrap, append([]string{"-pgo=off", "-tags=allow_missing_crypto_backend_fallback"}, toolchain...)...)
if debug {
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
copyfile(pathf("%s/compile2", tooldir), pathf("%s/compile", tooldir), writeExec)
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index a0525bf42e3c18..d123e25725eea2 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -153,7 +153,7 @@ func (t *tester) run() {
}

if !t.listMode {
- if builder := os.Getenv("GO_BUILDER_NAME"); builder == "" {
+ if builder, scripted := os.Getenv("GO_BUILDER_NAME"), os.Getenv("GO_MSFT_SCRIPTED_BUILD"); builder == "" && scripted != "1" {
// Ensure that installed commands are up to date, even with -no-rebuild,
// so that tests that run commands end up testing what's actually on disk.
// If everything is up-to-date, this is a no-op.
@@ -166,6 +166,12 @@ func (t *tester) run() {
// and virtualization we usually start with a clean GOCACHE, so we would
// end up rebuilding large parts of the standard library that aren't
// otherwise relevant to the actual set of packages under test.
+ //
+ // Also skip this step if GO_MSFT_SCRIPTED_BUILD is 1. This is
+ // similar to running in a builder, but it works locally. However,
+ // the skip isn't for performance reasons: rebuilding the toolchain
+ // may not work. For example, testing the OpenSSLCrypto GOEXPERIMENT
+ // requires cgo, but cgo is disabled by toolenv().
goInstall(toolenv(), gorootBinGo, toolchain...)
goInstall(toolenv(), gorootBinGo, toolchain...)
goInstall(toolenv(), gorootBinGo, "cmd")
@@ -761,6 +767,7 @@ func (t *tester) registerTests() {
ldflags: "-linkmode=internal",
env: []string{"CGO_ENABLED=0"},
pkg: "reflect",
+ tags: []string{"allow_missing_crypto_backend_fallback"},
})
// Also test a cgo package.
if t.cgoEnabled && t.internalLink() && !disablePIE {
diff --git a/src/cmd/internal/obj/x86/pcrelative_test.go b/src/cmd/internal/obj/x86/pcrelative_test.go
index 3827100123f26d..92344e8a681114 100644
--- a/src/cmd/internal/obj/x86/pcrelative_test.go
+++ b/src/cmd/internal/obj/x86/pcrelative_test.go
@@ -63,6 +63,10 @@ func objdumpOutput(t *testing.T, mname, source string) []byte {
testenv.GoToolPath(t), "build", "-o",
filepath.Join(tmpdir, "output"))

+ // Crypto backends are not available on all platforms (CNG is not available
+ // on Linux), but it's ok to fall back to pure Go for this test.
+ cmd.Args = append(cmd.Args, "-tags=allow_missing_crypto_backend_fallback")
+
cmd.Env = append(os.Environ(),
"GOARCH=amd64", "GOOS=linux", "GOPATH="+filepath.Join(tmpdir, "_gopath"))
cmd.Dir = tmpdir
diff --git a/src/cmd/internal/testdir/testdir_test.go b/src/cmd/internal/testdir/testdir_test.go
index bd7785900c637a..0657ee09dc04ca 100644
--- a/src/cmd/internal/testdir/testdir_test.go
+++ b/src/cmd/internal/testdir/testdir_test.go
@@ -694,6 +694,10 @@ func (t test) run() error {
// -S=2 forces outermost line numbers when disassembling inlined code.
cmdline := []string{"build", "-gcflags", "-S=2"}

+ // Crypto backends are not available on all platforms (386), but
+ // it's ok to fall back to pure Go for this test.
+ cmdline = append(cmdline, "-tags=allow_missing_crypto_backend_fallback")
+
// Append flags, but don't override -gcflags=-S=2; add to it instead.
for i := 0; i < len(flags); i++ {
flag := flags[i]
diff --git a/src/cmd/link/internal/ld/stackcheck_test.go b/src/cmd/link/internal/ld/stackcheck_test.go
index dd7e20528f0959..6b0d28d14f4935 100644
--- a/src/cmd/link/internal/ld/stackcheck_test.go
+++ b/src/cmd/link/internal/ld/stackcheck_test.go
@@ -19,7 +19,7 @@ func TestStackCheckOutput(t *testing.T) {
testenv.MustHaveGoBuild(t)
t.Parallel()

- cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-o", os.DevNull, "./testdata/stackcheck")
+ cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-tags=allow_missing_crypto_backend_fallback", "-o", os.DevNull, "./testdata/stackcheck")
// The rules for computing frame sizes on all of the
// architectures are complicated, so just do this on amd64.
cmd.Env = append(os.Environ(), "GOARCH=amd64", "GOOS=linux")
diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index c37d6e57bc0920..a4fd751dcd7c3b 100644
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -162,7 +162,7 @@ TEXT ·x(SB),0,$0
MOVD ·zero(SB), AX
RET
`)
- cmd := testenv.Command(t, testenv.GoToolPath(t), "build")
+ cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-tags=allow_missing_crypto_backend_fallback")
cmd.Dir = tmpdir
cmd.Env = append(os.Environ(),
"GOARCH=amd64", "GOOS=linux", "GOPATH="+filepath.Join(tmpdir, "_gopath"))
@@ -362,7 +362,7 @@ func TestMachOBuildVersion(t *testing.T) {
}

exe := filepath.Join(tmpdir, "main")
- cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-ldflags=-linkmode=internal", "-o", exe, src)
+ cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-ldflags=-linkmode=internal", "-tags=allow_missing_crypto_backend_fallback", "-o", exe, src)
cmd.Env = append(os.Environ(),
"CGO_ENABLED=0",
"GOOS=darwin",
diff --git a/src/cmd/vet/vet_test.go b/src/cmd/vet/vet_test.go
index 8b29907e818c9d..f29a76796de71f 100644
--- a/src/cmd/vet/vet_test.go
+++ b/src/cmd/vet/vet_test.go
@@ -54,7 +54,7 @@ var (
)

func vetCmd(t *testing.T, arg, pkg string) *exec.Cmd {
- cmd := testenv.Command(t, testenv.GoToolPath(t), "vet", "-vettool="+vetPath(t), arg, path.Join("cmd/vet/testdata", pkg))
+ cmd := testenv.Command(t, testenv.GoToolPath(t), "vet", "-tags=allow_missing_crypto_backend_fallback", "-vettool="+vetPath(t), arg, path.Join("cmd/vet/testdata", pkg))
cmd.Env = os.Environ()
return cmd
}
diff --git a/src/crypto/internal/backend/backendgen.go b/src/crypto/internal/backend/backendgen.go
new file mode 100644
index 00000000000000..acf0113bbefb6c
Expand Down Expand Up @@ -209,10 +71,10 @@ index 00000000000000..acf0113bbefb6c
+//go:generate go test -run TestGenerated -fix
diff --git a/src/crypto/internal/backend/backendgen_test.go b/src/crypto/internal/backend/backendgen_test.go
new file mode 100644
index 00000000000000..1cc4952e7ca9c2
index 00000000000000..5755840f735a21
--- /dev/null
+++ b/src/crypto/internal/backend/backendgen_test.go
@@ -0,0 +1,279 @@
@@ -0,0 +1,257 @@
+// Copyright 2023 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.
Expand Down Expand Up @@ -342,8 +204,6 @@ index 00000000000000..1cc4952e7ca9c2
+ f := testConflict(t, backends[i].name, backends[j].name)
+ delete(existingFiles, f)
+ }
+ f := testPreventUnintendedFallback(t, backends[i])
+ delete(existingFiles, f)
+ }
+ f := testUnsatisfied(t, backends)
+ delete(existingFiles, f)
Expand Down Expand Up @@ -375,26 +235,6 @@ index 00000000000000..1cc4952e7ca9c2
+ "Please make sure only one crypto backend experiment is enabled by GOEXPERIMENT or '-tags'.")
+}
+
+func testPreventUnintendedFallback(t *testing.T, backend *backend) string {
+ expTag := &constraint.TagExpr{Tag: "goexperiment." + backend.name + "crypto"}
+ optOutTag := &constraint.TagExpr{Tag: "allow_missing_crypto_backend_fallback"}
+ c := constraint.AndExpr{
+ X: &constraint.AndExpr{
+ X: expTag,
+ Y: &constraint.NotExpr{X: backend.constraint},
+ },
+ Y: &constraint.NotExpr{X: optOutTag},
+ }
+ return testErrorFile(
+ t,
+ filepath.Join(runtimePackageDir, backendErrPrefix+"nofallback_"+backend.name+".go"),
+ "//go:build "+c.String(),
+ "The "+expTag.String()+" tag is specified, but other tags required to enable that backend were not met.",
+ "Required build tags:",
+ " "+backend.constraint.String(),
+ "Please check your build environment and build command for a reason one or more of these tags weren't specified.")
+}
+
+// testUnsatisfied checks/generates a file that fails if systemcrypto is enabled
+// on an OS with no suitable backend.
+func testUnsatisfied(t *testing.T, backends []*backend) string {
Expand Down Expand Up @@ -493,7 +333,7 @@ index 00000000000000..1cc4952e7ca9c2
+ return bs
+}
diff --git a/src/crypto/internal/backend/nobackend.go b/src/crypto/internal/backend/nobackend.go
index ad6081552af15d..64d2330186e795 100644
index ad6081552af15d..d5948dbc5f8a2a 100644
--- a/src/crypto/internal/backend/nobackend.go
+++ b/src/crypto/internal/backend/nobackend.go
@@ -4,7 +4,7 @@
Expand Down Expand Up @@ -574,81 +414,6 @@ index 00000000000000..bf44084570bbbc
+ For more information, visit https://github.com/microsoft/go/tree/microsoft/main/eng/doc/fips
+ `
+}
diff --git a/src/runtime/backenderr_gen_nofallback_boring.go b/src/runtime/backenderr_gen_nofallback_boring.go
new file mode 100644
index 00000000000000..764172d1159e17
--- /dev/null
+++ b/src/runtime/backenderr_gen_nofallback_boring.go
@@ -0,0 +1,19 @@
+// Copyright 2023 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.
+
+// This file is generated by crypto/internal/backend. DO NOT EDIT. DO NOT manually create files with the prefix "backenderr_gen_".
+
+//go:build goexperiment.boringcrypto && !(goexperiment.boringcrypto && linux && cgo && (amd64 || arm64) && !android && !msan) && !allow_missing_crypto_backend_fallback
+
+package runtime
+
+func init() {
+ `
+ The goexperiment.boringcrypto tag is specified, but other tags required to enable that backend were not met.
+ Required build tags:
+ goexperiment.boringcrypto && linux && cgo && (amd64 || arm64) && !android && !msan
+ Please check your build environment and build command for a reason one or more of these tags weren't specified.
+ For more information, visit https://github.com/microsoft/go/tree/microsoft/main/eng/doc/fips
+ `
+}
diff --git a/src/runtime/backenderr_gen_nofallback_cng.go b/src/runtime/backenderr_gen_nofallback_cng.go
new file mode 100644
index 00000000000000..3187f794dd49a4
--- /dev/null
+++ b/src/runtime/backenderr_gen_nofallback_cng.go
@@ -0,0 +1,19 @@
+// Copyright 2023 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.
+
+// This file is generated by crypto/internal/backend. DO NOT EDIT. DO NOT manually create files with the prefix "backenderr_gen_".
+
+//go:build goexperiment.cngcrypto && !(goexperiment.cngcrypto && windows) && !allow_missing_crypto_backend_fallback
+
+package runtime
+
+func init() {
+ `
+ The goexperiment.cngcrypto tag is specified, but other tags required to enable that backend were not met.
+ Required build tags:
+ goexperiment.cngcrypto && windows
+ Please check your build environment and build command for a reason one or more of these tags weren't specified.
+ For more information, visit https://github.com/microsoft/go/tree/microsoft/main/eng/doc/fips
+ `
+}
diff --git a/src/runtime/backenderr_gen_nofallback_openssl.go b/src/runtime/backenderr_gen_nofallback_openssl.go
new file mode 100644
index 00000000000000..36ea1ea1884d8c
--- /dev/null
+++ b/src/runtime/backenderr_gen_nofallback_openssl.go
@@ -0,0 +1,19 @@
+// Copyright 2023 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.
+
+// This file is generated by crypto/internal/backend. DO NOT EDIT. DO NOT manually create files with the prefix "backenderr_gen_".
+
+//go:build goexperiment.opensslcrypto && !(goexperiment.opensslcrypto && linux && cgo && !android) && !allow_missing_crypto_backend_fallback
+
+package runtime
+
+func init() {
+ `
+ The goexperiment.opensslcrypto tag is specified, but other tags required to enable that backend were not met.
+ Required build tags:
+ goexperiment.opensslcrypto && linux && cgo && !android
+ Please check your build environment and build command for a reason one or more of these tags weren't specified.
+ For more information, visit https://github.com/microsoft/go/tree/microsoft/main/eng/doc/fips
+ `
+}
diff --git a/src/runtime/backenderr_gen_requirefips_nosystemcrypto.go b/src/runtime/backenderr_gen_requirefips_nosystemcrypto.go
new file mode 100644
index 00000000000000..1c015dd2b08972
Expand Down

0 comments on commit ca7d4bc

Please sign in to comment.