Permalink
Please sign in to comment.
Browse files
crypto: randomly read an extra byte of randomness in some places.
Code has ended up depending on things like RSA's key generation being deterministic given a fixed random Reader. This was never guaranteed and would prevent us from ever changing anything about it. This change makes certain calls randomly (based on the internal fastrand) read an extra byte from the random Reader. This helps to ensure that code does not depend on internal details. I've not added this call in the key generation of ECDSA and DSA because, in those cases, key generation is so obvious that it probably is acceptable to do the obvious thing and not worry about code that depends on that. This does not affect tests that use a Reader of constant bytes (e.g. a zeroReader) because shifting such a stream is a no-op. The stdlib uses this internally (which is fine because it can be atomically updated if the crypto libraries change). It is possible that external tests could be doing the same and would thus break if we ever, say, tweaked the way RSA key generation worked. I feel that addressing that would be more effort than it's worth. Fixes #21915 Change-Id: I84cff2e249acc921ad6eb5527171e02e6d39c530 Reviewed-on: https://go-review.googlesource.com/64451 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
- Loading branch information...
Showing
with
66 additions
and 8 deletions.
- +4 −0 src/crypto/dsa/dsa.go
- +4 −0 src/crypto/ecdsa/ecdsa.go
- +38 −0 src/crypto/internal/randutil/randutil.go
- +4 −0 src/crypto/rsa/pkcs1v15.go
- +6 −0 src/crypto/rsa/rsa.go
- +10 −8 src/go/build/deps_test.go
| @@ -0,0 +1,38 @@ | ||
| // Copyright 2018 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 randutil contains internal randomness utilities for various | ||
| // crypto packages. | ||
| package randutil | ||
| import ( | ||
| "io" | ||
| "sync" | ||
| ) | ||
| var ( | ||
| closedChanOnce sync.Once | ||
| closedChan chan struct{} | ||
| ) | ||
| // MaybeReadByte reads a single byte from r with ~50% probability. This is used | ||
| // to ensure that callers do not depend on non-guaranteed behaviour, e.g. | ||
| // assuming that rsa.GenerateKey is deterministic w.r.t. a given random stream. | ||
| // | ||
| // This does not affect tests that pass a stream of fixed bytes as the random | ||
| // source (e.g. a zeroReader). | ||
| func MaybeReadByte(r io.Reader) { | ||
| closedChanOnce.Do(func() { | ||
| closedChan = make(chan struct{}) | ||
| close(closedChan) | ||
| }) | ||
| select { | ||
| case <-closedChan: | ||
| return | ||
| case <-closedChan: | ||
| var buf [1]byte | ||
| r.Read(buf[:]) | ||
| } | ||
| } |
0 comments on commit
6269dcd