Skip to content

Commit fc66a56

Browse files
FiloSottilegopherbot
FiloSottile
authored and
gopherbot
committed
crypto: clean up subprocess-spawning tests
Consistently use testenv.Command and testenv.Executable, avoid redundant testenv.Must, use testenv.CleanCmdEnv where the output is parsed, always log the output with a preceding newline, invoke tests with -v, and always use cmd.Environ() to preserve existing env. Change-Id: I647ff1a8b7d162e5e8df9424030fac446a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/728641 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
1 parent b130dab commit fc66a56

File tree

20 files changed

+121
-184
lines changed

20 files changed

+121
-184
lines changed

‎src/crypto/cipher/gcm_fips140v2.0_test.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,9 @@ func TestGCMNoncesFIPSV2(t *testing.T) {
2222
cryptotest.MustSupportFIPS140(t)
2323
if !fips140.Enabled {
2424
cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestGCMNoncesFIPSV2$", "-test.v")
25-
cmd = testenv.CleanCmdEnv(cmd)
26-
cmd.Env = append(cmd.Env, "GODEBUG=fips140=on")
25+
cmd.Env = append(cmd.Environ(), "GODEBUG=fips140=on")
2726
out, err := cmd.CombinedOutput()
28-
if len(out) != 0 {
29-
t.Logf("\n%s", out)
30-
}
27+
t.Logf("running with GODEBUG=fips140=on:\n%s", out)
3128
if err != nil {
3229
t.Errorf("fips140=on subprocess failed: %v", err)
3330
}

‎src/crypto/cipher/gcm_test.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -766,12 +766,9 @@ func TestGCMNoncesFIPSV1(t *testing.T) {
766766
cryptotest.MustSupportFIPS140(t)
767767
if !fips140.Enabled {
768768
cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestGCMNoncesFIPSV1$", "-test.v")
769-
cmd = testenv.CleanCmdEnv(cmd)
770-
cmd.Env = append(cmd.Env, "GODEBUG=fips140=on")
769+
cmd.Env = append(cmd.Environ(), "GODEBUG=fips140=on")
771770
out, err := cmd.CombinedOutput()
772-
if len(out) != 0 {
773-
t.Logf("\n%s", out)
774-
}
771+
t.Logf("running with GODEBUG=fips140=on:\n%s", out)
775772
if err != nil {
776773
t.Errorf("fips140=on subprocess failed: %v", err)
777774
}

‎src/crypto/ecdh/ecdh_test.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"internal/testenv"
1717
"io"
1818
"os"
19-
"os/exec"
2019
"path/filepath"
2120
"regexp"
2221
"strings"
@@ -450,7 +449,6 @@ func TestLinker(t *testing.T) {
450449
if testing.Short() {
451450
t.Skip("test requires running 'go build'")
452451
}
453-
testenv.MustHaveGoBuild(t)
454452

455453
dir := t.TempDir()
456454
hello := filepath.Join(dir, "hello.go")
@@ -460,25 +458,24 @@ func TestLinker(t *testing.T) {
460458
}
461459

462460
run := func(args ...string) string {
463-
cmd := exec.Command(args[0], args[1:]...)
461+
cmd := testenv.Command(t, args[0], args[1:]...)
464462
cmd.Dir = dir
465-
out, err := cmd.CombinedOutput()
463+
out, err := testenv.CleanCmdEnv(cmd).CombinedOutput()
466464
if err != nil {
467465
t.Fatalf("%v: %v\n%s", args, err, string(out))
468466
}
469467
return string(out)
470468
}
471469

472-
goBin := testenv.GoToolPath(t)
473-
run(goBin, "build", "-o", "hello.exe", "hello.go")
470+
run(testenv.GoToolPath(t), "build", "-o", "hello.exe", "hello.go")
474471
if out := run("./hello.exe"); out != "OK\n" {
475472
t.Error("unexpected output:", out)
476473
}
477474

478475
// List all text symbols under crypto/... and make sure there are some for
479476
// P256, but none for the other curves.
480477
var consistent bool
481-
nm := run(goBin, "tool", "nm", "hello.exe")
478+
nm := run(testenv.GoToolPath(t), "tool", "nm", "hello.exe")
482479
for _, match := range regexp.MustCompile(`(?m)T (crypto/.*)$`).FindAllStringSubmatch(nm, -1) {
483480
symbol := strings.ToLower(match[1])
484481
if strings.Contains(symbol, "p256") {

‎src/crypto/fips140/enforcement_test.go‎

Lines changed: 64 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,42 +5,77 @@
55
package fips140_test
66

77
import (
8+
"crypto/des"
9+
"crypto/fips140"
810
"crypto/internal/cryptotest"
911
"internal/testenv"
10-
"path/filepath"
11-
"strings"
1212
"testing"
1313
)
1414

15+
func expectAllowed(t *testing.T, why string, expected bool) {
16+
t.Helper()
17+
result := isAllowed()
18+
if result != expected {
19+
t.Fatalf("%v: expected: %v, got: %v", why, expected, result)
20+
}
21+
}
22+
23+
func isAllowed() bool {
24+
_, err := des.NewCipher(make([]byte, 8))
25+
return err == nil
26+
}
27+
1528
func TestWithoutEnforcement(t *testing.T) {
16-
testenv.MustHaveExec(t)
17-
testenv.MustHaveGoBuild(t)
1829
cryptotest.MustSupportFIPS140(t)
19-
20-
tool, _ := testenv.GoTool()
21-
tmpdir := t.TempDir()
22-
binFile := filepath.Join(tmpdir, "fips140.test")
23-
cmd := testenv.Command(t, tool, "test", "-c", "-o", binFile, "./testdata")
24-
out, err := cmd.CombinedOutput()
25-
if err != nil {
26-
t.Log(string(out))
27-
t.Errorf("Could not build enforcement tests")
28-
}
29-
cmd = testenv.Command(t, binFile, "-test.list", ".")
30-
list, err := cmd.CombinedOutput()
31-
if err != nil {
32-
t.Log(string(out))
33-
t.Errorf("Could not get enforcement test list")
30+
if !fips140.Enforced() {
31+
cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestWithoutEnforcement$", "-test.v")
32+
cmd.Env = append(cmd.Environ(), "GODEBUG=fips140=only")
33+
out, err := cmd.CombinedOutput()
34+
t.Logf("running with GODEBUG=fips140=only:\n%s", out)
35+
if err != nil {
36+
t.Errorf("fips140=only subprocess failed: %v", err)
37+
}
38+
return
3439
}
35-
for test := range strings.Lines(string(list)) {
36-
test = strings.TrimSpace(test)
37-
t.Run(test, func(t *testing.T) {
38-
cmd = testenv.Command(t, binFile, "-test.run", "^"+test+"$")
39-
cmd.Env = append(cmd.Env, "GODEBUG=fips140=only")
40-
out, err := cmd.CombinedOutput()
41-
if err != nil {
42-
t.Error(string(out))
43-
}
40+
41+
t.Run("Disabled", func(t *testing.T) {
42+
expectAllowed(t, "before enforcement disabled", false)
43+
fips140.WithoutEnforcement(func() {
44+
expectAllowed(t, "inside WithoutEnforcement", true)
4445
})
45-
}
46+
// make sure that bypass doesn't live on after returning
47+
expectAllowed(t, "after WithoutEnforcement", false)
48+
})
49+
50+
t.Run("Nested", func(t *testing.T) {
51+
expectAllowed(t, "before enforcement bypass", false)
52+
fips140.WithoutEnforcement(func() {
53+
fips140.WithoutEnforcement(func() {
54+
expectAllowed(t, "inside nested WithoutEnforcement", true)
55+
})
56+
expectAllowed(t, "inside nested WithoutEnforcement", true)
57+
})
58+
expectAllowed(t, "after enforcement bypass", false)
59+
})
60+
61+
t.Run("GoroutineInherit", func(t *testing.T) {
62+
ch := make(chan bool, 2)
63+
expectAllowed(t, "before enforcement bypass", false)
64+
fips140.WithoutEnforcement(func() {
65+
go func() {
66+
ch <- isAllowed()
67+
}()
68+
})
69+
allowed := <-ch
70+
if !allowed {
71+
t.Fatal("goroutine didn't inherit enforcement bypass")
72+
}
73+
go func() {
74+
ch <- isAllowed()
75+
}()
76+
allowed = <-ch
77+
if allowed {
78+
t.Fatal("goroutine inherited bypass after WithoutEnforcement return")
79+
}
80+
})
4681
}

‎src/crypto/fips140/testdata/enforcement_test.go‎

Lines changed: 0 additions & 65 deletions
This file was deleted.

‎src/crypto/internal/cryptotest/fetchmodule.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@ import (
1818
// possible in this environment.
1919
func FetchModule(t *testing.T, module, version string) string {
2020
testenv.MustHaveExternalNetwork(t)
21-
goTool := testenv.GoToolPath(t)
2221

2322
// If the default GOMODCACHE doesn't exist, use a temporary directory
2423
// instead. (For example, run.bash sets GOPATH=/nonexist-gopath.)
25-
out, err := testenv.Command(t, goTool, "env", "GOMODCACHE").Output()
24+
out, err := testenv.CleanCmdEnv(testenv.Command(t, testenv.GoToolPath(t), "env", "GOMODCACHE")).Output()
2625
if err != nil {
27-
t.Errorf("%s env GOMODCACHE: %v\n%s", goTool, err, out)
26+
t.Errorf("%s env GOMODCACHE: %v\n%s", testenv.GoToolPath(t), err, out)
2827
if ee, ok := err.(*exec.ExitError); ok {
2928
t.Logf("%s", ee.Stderr)
3029
}
@@ -44,7 +43,7 @@ func FetchModule(t *testing.T, module, version string) string {
4443

4544
t.Logf("fetching %s@%s\n", module, version)
4645

47-
output, err := testenv.Command(t, goTool, "mod", "download", "-json", module+"@"+version).CombinedOutput()
46+
output, err := testenv.Command(t, testenv.GoToolPath(t), "mod", "download", "-json", module+"@"+version).CombinedOutput()
4847
if err != nil {
4948
t.Fatalf("failed to download %s@%s: %s\n%s\n", module, version, err, output)
5049
}

‎src/crypto/internal/fips140deps/fipsdeps_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestImports(t *testing.T) {
4444
{{range .XTestImports -}}
4545
{{$path}} {{.}}
4646
{{end -}}`, "crypto/internal/fips140/...")
47-
bout, err := cmd.CombinedOutput()
47+
bout, err := testenv.CleanCmdEnv(cmd).CombinedOutput()
4848
if err != nil {
4949
t.Fatalf("go list: %v\n%s", err, bout)
5050
}

‎src/crypto/internal/fips140test/acvp_test.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"bytes"
2424
"crypto/elliptic"
2525
"crypto/internal/cryptotest"
26-
"crypto/internal/entropy/v1.0.0"
26+
entropy "crypto/internal/entropy/v1.0.0"
2727
"crypto/internal/fips140"
2828
"crypto/internal/fips140/aes"
2929
"crypto/internal/fips140/aes/gcm"
@@ -2128,16 +2128,13 @@ func TestACVP(t *testing.T) {
21282128

21292129
// Build the acvptool binary.
21302130
toolPath := filepath.Join(t.TempDir(), "acvptool.exe")
2131-
goTool := testenv.GoToolPath(t)
2132-
cmd := testenv.Command(t, goTool,
2131+
cmd := testenv.Command(t, testenv.GoToolPath(t),
21332132
"build",
21342133
"-o", toolPath,
21352134
"./util/fipstools/acvp/acvptool")
21362135
cmd.Dir = bsslDir
2137-
out := &strings.Builder{}
2138-
cmd.Stderr = out
2139-
if err := cmd.Run(); err != nil {
2140-
t.Fatalf("failed to build acvptool: %s\n%s", err, out.String())
2136+
if out, err := cmd.CombinedOutput(); err != nil {
2137+
t.Fatalf("failed to build acvptool: %s\n%s", err, out)
21412138
}
21422139

21432140
// Similarly, fetch the ACVP data module that has vectors/expected answers.
@@ -2163,17 +2160,17 @@ func TestACVP(t *testing.T) {
21632160
"-module-wrappers", "go:" + os.Args[0],
21642161
"-tests", configPath,
21652162
}
2166-
cmd = testenv.Command(t, goTool, args...)
2163+
cmd = testenv.Command(t, testenv.GoToolPath(t), args...)
21672164
cmd.Dir = dataDir
21682165
cmd.Env = append(os.Environ(),
21692166
"ACVP_WRAPPER=1",
21702167
"GODEBUG=fips140=on",
21712168
)
2172-
output, err := cmd.CombinedOutput()
2169+
out, err := cmd.CombinedOutput()
2170+
t.Logf("\n%s", out)
21732171
if err != nil {
2174-
t.Fatalf("failed to run acvp tests: %s\n%s", err, string(output))
2172+
t.Fatalf("failed to run acvp tests: %s", err)
21752173
}
2176-
t.Log(string(output))
21772174
}
21782175

21792176
func TestTooFewArgs(t *testing.T) {

‎src/crypto/internal/fips140test/cast_test.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func TestAllCASTs(t *testing.T) {
7878
// Ask "go list" for the location of the crypto/internal/fips140 tree, as it
7979
// might be the unpacked frozen tree selected with GOFIPS140.
8080
cmd := testenv.Command(t, testenv.GoToolPath(t), "list", "-f", `{{.Dir}}`, "crypto/internal/fips140")
81-
out, err := cmd.CombinedOutput()
81+
out, err := testenv.CleanCmdEnv(cmd).CombinedOutput()
8282
if err != nil {
8383
t.Fatalf("go list: %v\n%s", err, out)
8484
}
@@ -161,13 +161,12 @@ func TestConditionals(t *testing.T) {
161161

162162
func TestCASTPasses(t *testing.T) {
163163
moduleStatus(t)
164-
testenv.MustHaveExec(t)
165164
cryptotest.MustSupportFIPS140(t)
166165

167166
cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestConditionals$", "-test.v")
168-
cmd.Env = append(cmd.Env, "GODEBUG=fips140=debug")
167+
cmd.Env = append(cmd.Environ(), "GODEBUG=fips140=debug")
169168
out, err := cmd.CombinedOutput()
170-
t.Logf("%s", out)
169+
t.Logf("running with GODEBUG=fips140=debug:\n%s", out)
171170
if err != nil || !strings.Contains(string(out), "completed successfully") {
172171
t.Errorf("TestConditionals did not complete successfully")
173172
}
@@ -185,7 +184,6 @@ func TestCASTPasses(t *testing.T) {
185184

186185
func TestCASTFailures(t *testing.T) {
187186
moduleStatus(t)
188-
testenv.MustHaveExec(t)
189187
cryptotest.MustSupportFIPS140(t)
190188

191189
for _, name := range allCASTs {
@@ -197,11 +195,12 @@ func TestCASTFailures(t *testing.T) {
197195
}
198196
t.Logf("Testing CAST/PCT failure...")
199197
cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestConditionals$", "-test.v")
200-
cmd.Env = append(cmd.Env, fmt.Sprintf("GODEBUG=failfipscast=%s,fips140=on", name))
198+
GODEBUG := fmt.Sprintf("GODEBUG=failfipscast=%s,fips140=on", name)
199+
cmd.Env = append(cmd.Environ(), GODEBUG)
201200
out, err := cmd.CombinedOutput()
202-
t.Logf("%s", out)
201+
t.Logf("running with %s:\n%s", GODEBUG, out)
203202
if err == nil {
204-
t.Fatal("Test did not fail as expected")
203+
t.Fatal("test did not fail as expected")
205204
}
206205
if strings.Contains(string(out), "completed successfully") {
207206
t.Errorf("CAST/PCT %s failure did not stop the program", name)

‎src/crypto/internal/fips140test/check_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ func TestIntegrityCheck(t *testing.T) {
4444

4545
func TestIntegrityCheckFailure(t *testing.T) {
4646
moduleStatus(t)
47-
testenv.MustHaveExec(t)
4847
cryptotest.MustSupportFIPS140(t)
4948

5049
bin, err := os.ReadFile(os.Args[0])
@@ -73,7 +72,7 @@ func TestIntegrityCheckFailure(t *testing.T) {
7372
cmd := testenv.Command(t, binPath, "-test.v", "-test.run=^TestIntegrityCheck$")
7473
cmd.Env = append(cmd.Environ(), "GODEBUG=fips140=on")
7574
out, err := cmd.CombinedOutput()
76-
t.Logf("%s", out)
75+
t.Logf("running with GODEBUG=fips140=on:\n%s", out)
7776
if err == nil {
7877
t.Errorf("modified binary did not fail as expected")
7978
}

0 commit comments

Comments
 (0)