From: Filippo Valsorda Date: Wed, 23 Oct 2024 18:21:50 +0000 (+0200) Subject: Revert "crypto/rand: add randcrash=0 GODEBUG" X-Git-Tag: go1.24rc1~581 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0138c1abef3871b72e47d5909ce08c9218f61b16;p=gostls13.git Revert "crypto/rand: add randcrash=0 GODEBUG" A GODEBUG is actually a security risk here: most programs will start to ignore errors from Read because they can't happen (which is the intended behavior), but then if a program is run with GODEBUG=randcrash=0 it will use a partial buffer in case an error occurs, which may be catastrophic. Note that the proposal was accepted without the GODEBUG, which was only added later. This (partially) reverts CL 608435. I kept the tests. Updates #66821 Change-Id: I3fd20f9cae0d34115133fe935f0cfc7a741a2662 Reviewed-on: https://go-review.googlesource.com/c/go/+/622115 LUCI-TryBot-Result: Go LUCI Reviewed-by: Carlos Amedee Reviewed-by: Roland Shoemaker Auto-Submit: Filippo Valsorda Reviewed-by: Daniel McCarney --- diff --git a/doc/godebug.md b/doc/godebug.md index a97d8234fe..7b5fd3e48b 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -168,11 +168,6 @@ For Go 1.24, it now defaults to multipathtcp="2", thus enabled by default on listerners. Using multipathtcp="0" reverts to the pre-Go 1.24 behavior. -Go 1.24 changed [`crypto/rand.Read`](/pkg/crypto/rand/#Read) to crash the -program on any error. This setting is controlled by the `randcrash` setting. -For Go 1.24 it defaults to `randcrash=1`. -Using `randcrash=0` reverts to the pre-Go 1.24 behavior. - ### Go 1.23 Go 1.23 changed the channels created by package time to be unbuffered diff --git a/src/crypto/rand/rand.go b/src/crypto/rand/rand.go index 7c18d595c2..b3d0a7368f 100644 --- a/src/crypto/rand/rand.go +++ b/src/crypto/rand/rand.go @@ -8,7 +8,6 @@ package rand import ( "crypto/internal/boring" - "internal/godebug" "io" "os" "sync" @@ -65,8 +64,6 @@ func (r *reader) Read(b []byte) (n int, err error) { //go:linkname fatal func fatal(string) -var randcrash = godebug.New("randcrash") - // Read fills b with cryptographically secure random bytes. It never returns an // error, and always fills b entirely. // @@ -86,10 +83,6 @@ func Read(b []byte) (n int, err error) { copy(b, bb) } if err != nil { - if randcrash.Value() == "0" { - randcrash.IncNonDefault() - return 0, err - } fatal("crypto/rand: failed to read random data (see https://go.dev/issue/66821): " + err.Error()) panic("unreachable") // To be sure. } diff --git a/src/crypto/rand/rand_test.go b/src/crypto/rand/rand_test.go index 2372413279..63581b75fd 100644 --- a/src/crypto/rand/rand_test.go +++ b/src/crypto/rand/rand_test.go @@ -198,8 +198,7 @@ func TestReadError(t *testing.T) { } testenv.MustHaveExec(t) - // We run this test in a subprocess because it's expected to crash the - // program unless the GODEBUG is set. + // We run this test in a subprocess because it's expected to crash. if os.Getenv("GO_TEST_READ_ERROR") == "1" { defer func(r io.Reader) { Reader = r }(Reader) Reader = readerFunc(func([]byte) (int, error) { @@ -221,13 +220,6 @@ func TestReadError(t *testing.T) { if !bytes.Contains(out, []byte(exp)) { t.Errorf("subprocess output does not contain %q: %s", exp, out) } - - cmd = testenv.Command(t, os.Args[0], "-test.run=TestReadError") - cmd.Env = append(os.Environ(), "GO_TEST_READ_ERROR=1", "GODEBUG=randcrash=0") - out, err = cmd.CombinedOutput() - if err != nil { - t.Errorf("subprocess failed: %v\n%s", err, out) - } } func BenchmarkRead(b *testing.B) { diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 3af86d070a..59d4fa7d5b 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -47,7 +47,6 @@ var All = []Info{ {Name: "netedns0", Package: "net", Changed: 19, Old: "0"}, {Name: "panicnil", Package: "runtime", Changed: 21, Old: "1"}, {Name: "randautoseed", Package: "math/rand"}, - {Name: "randcrash", Package: "crypto/rand", Changed: 24, Old: "0"}, {Name: "randseednop", Package: "math/rand", Changed: 24, Old: "0"}, {Name: "tarinsecurepath", Package: "archive/tar"}, {Name: "tls10server", Package: "crypto/tls", Changed: 22, Old: "1"}, diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index 3d2cc6159f..906abb4102 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -306,10 +306,6 @@ Below is the full list of supported metrics, ordered lexicographically. The number of non-default behaviors executed by the math/rand package due to a non-default GODEBUG=randautoseed=... setting. - /godebug/non-default-behavior/randcrash:events - The number of non-default behaviors executed by the crypto/rand - package due to a non-default GODEBUG=randcrash=... setting. - /godebug/non-default-behavior/randseednop:events The number of non-default behaviors executed by the math/rand package due to a non-default GODEBUG=randseednop=... setting.