Skip to content

Commit 0138c1a

Browse files
FiloSottilegopherbot
authored andcommitted
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 <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Auto-Submit: Filippo Valsorda <[email protected]> Reviewed-by: Daniel McCarney <[email protected]>
1 parent 7a256ad commit 0138c1a

File tree

5 files changed

+1
-26
lines changed

5 files changed

+1
-26
lines changed

Diff for: doc/godebug.md

-5
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,6 @@ For Go 1.24, it now defaults to multipathtcp="2", thus
168168
enabled by default on listerners. Using multipathtcp="0" reverts to the
169169
pre-Go 1.24 behavior.
170170

171-
Go 1.24 changed [`crypto/rand.Read`](/pkg/crypto/rand/#Read) to crash the
172-
program on any error. This setting is controlled by the `randcrash` setting.
173-
For Go 1.24 it defaults to `randcrash=1`.
174-
Using `randcrash=0` reverts to the pre-Go 1.24 behavior.
175-
176171
### Go 1.23
177172

178173
Go 1.23 changed the channels created by package time to be unbuffered

Diff for: src/crypto/rand/rand.go

-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package rand
88

99
import (
1010
"crypto/internal/boring"
11-
"internal/godebug"
1211
"io"
1312
"os"
1413
"sync"
@@ -65,8 +64,6 @@ func (r *reader) Read(b []byte) (n int, err error) {
6564
//go:linkname fatal
6665
func fatal(string)
6766

68-
var randcrash = godebug.New("randcrash")
69-
7067
// Read fills b with cryptographically secure random bytes. It never returns an
7168
// error, and always fills b entirely.
7269
//
@@ -86,10 +83,6 @@ func Read(b []byte) (n int, err error) {
8683
copy(b, bb)
8784
}
8885
if err != nil {
89-
if randcrash.Value() == "0" {
90-
randcrash.IncNonDefault()
91-
return 0, err
92-
}
9386
fatal("crypto/rand: failed to read random data (see https://go.dev/issue/66821): " + err.Error())
9487
panic("unreachable") // To be sure.
9588
}

Diff for: src/crypto/rand/rand_test.go

+1-9
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,7 @@ func TestReadError(t *testing.T) {
198198
}
199199
testenv.MustHaveExec(t)
200200

201-
// We run this test in a subprocess because it's expected to crash the
202-
// program unless the GODEBUG is set.
201+
// We run this test in a subprocess because it's expected to crash.
203202
if os.Getenv("GO_TEST_READ_ERROR") == "1" {
204203
defer func(r io.Reader) { Reader = r }(Reader)
205204
Reader = readerFunc(func([]byte) (int, error) {
@@ -221,13 +220,6 @@ func TestReadError(t *testing.T) {
221220
if !bytes.Contains(out, []byte(exp)) {
222221
t.Errorf("subprocess output does not contain %q: %s", exp, out)
223222
}
224-
225-
cmd = testenv.Command(t, os.Args[0], "-test.run=TestReadError")
226-
cmd.Env = append(os.Environ(), "GO_TEST_READ_ERROR=1", "GODEBUG=randcrash=0")
227-
out, err = cmd.CombinedOutput()
228-
if err != nil {
229-
t.Errorf("subprocess failed: %v\n%s", err, out)
230-
}
231223
}
232224

233225
func BenchmarkRead(b *testing.B) {

Diff for: src/internal/godebugs/table.go

-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ var All = []Info{
4747
{Name: "netedns0", Package: "net", Changed: 19, Old: "0"},
4848
{Name: "panicnil", Package: "runtime", Changed: 21, Old: "1"},
4949
{Name: "randautoseed", Package: "math/rand"},
50-
{Name: "randcrash", Package: "crypto/rand", Changed: 24, Old: "0"},
5150
{Name: "randseednop", Package: "math/rand", Changed: 24, Old: "0"},
5251
{Name: "tarinsecurepath", Package: "archive/tar"},
5352
{Name: "tls10server", Package: "crypto/tls", Changed: 22, Old: "1"},

Diff for: src/runtime/metrics/doc.go

-4
Original file line numberDiff line numberDiff line change
@@ -306,10 +306,6 @@ Below is the full list of supported metrics, ordered lexicographically.
306306
The number of non-default behaviors executed by the math/rand
307307
package due to a non-default GODEBUG=randautoseed=... setting.
308308
309-
/godebug/non-default-behavior/randcrash:events
310-
The number of non-default behaviors executed by the crypto/rand
311-
package due to a non-default GODEBUG=randcrash=... setting.
312-
313309
/godebug/non-default-behavior/randseednop:events
314310
The number of non-default behaviors executed by the math/rand
315311
package due to a non-default GODEBUG=randseednop=... setting.

0 commit comments

Comments
 (0)