crypto/x509: avoid crypto/rand.Int to generate serial number

It's probabyl safe enough, but just reading bytes from rand and then
using SetBytes is simpler, and doesn't require allowing calls from
crypto into math/big's Lsh, Sub, and Cmp.

Change-Id: I6a6a4656761f7073f9e149f288c48e97048ab13c
Reviewed-on: https://go-review.googlesource.com/c/go/+/643278
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Roland Shoemaker <roland@golang.org>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 2283f6cac3a353ca1054f86f1665c916e0c5a308..cbcc582a3ff68afc526a717576cc9738ad7bdd6a 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -27,7 +27,6 @@ import (
        "crypto/ecdsa"
        "crypto/ed25519"
        "crypto/elliptic"
-       cryptorand "crypto/rand"
        "crypto/rsa"
        "crypto/sha1"
        "crypto/x509/pkix"
@@ -1673,25 +1672,19 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
 
        serialNumber := template.SerialNumber
        if serialNumber == nil {
-               // Generate a serial number following RFC 5280 Section 4.1.2.2 if one is not provided.
-               // Requirements:
-               //   - serial number must be positive
-               //   - at most 20 octets when encoded
-               maxSerial := big.NewInt(1).Lsh(big.NewInt(1), 20*8)
-               for {
-                       var err error
-                       serialNumber, err = cryptorand.Int(rand, maxSerial)
-                       if err != nil {
-                               return nil, err
-                       }
-                       // If the serial is exactly 20 octets, check if the high bit of the first byte is set.
-                       // If so, generate a new serial, since it will be padded with a leading 0 byte during
-                       // encoding so that the serial is not interpreted as a negative integer, making it
-                       // 21 octets.
-                       if serialBytes := serialNumber.Bytes(); len(serialBytes) > 0 && (len(serialBytes) < 20 || serialBytes[0]&0x80 == 0) {
-                               break
-                       }
+               // Generate a serial number following RFC 5280, Section 4.1.2.2 if one
+               // is not provided. The serial number must be positive and at most 20
+               // octets *when encoded*.
+               serialBytes := make([]byte, 20)
+               if _, err := io.ReadFull(rand, serialBytes); err != nil {
+                       return nil, err
                }
+               // If the top bit is set, the serial will be padded with a leading zero
+               // byte during encoding, so that it's not interpreted as a negative
+               // integer. This padding would make the serial 21 octets so we clear the
+               // top bit to ensure the correct length in all cases.
+               serialBytes[0] &= 0b0111_1111
+               serialNumber = new(big.Int).SetBytes(serialBytes)
        }
 
        // RFC 5280 Section 4.1.2.2: serial number must be positive