crypto/x509: generate serial number for nil template SerialNumber

Fixes #67675

Change-Id: I976935d20eb6b9adcd19d47bcaeb7abcf78ec5bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/630995
Reviewed-by: Roland Shoemaker <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>

Author: seankhliao

src/crypto/x509/x509.go

@@ -27,6 +27,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/elliptic"
+	cryptorand "crypto/rand"
 	"crypto/rsa"
 	"crypto/sha1"
 	"crypto/x509/pkix"
@@ -1655,6 +1656,9 @@ var emptyASN1Subject = []byte{0x30, 0}
 // If SubjectKeyId from template is empty and the template is a CA, SubjectKeyId
 // will be generated from the hash of the public key.
 //
+// If template.SerialNumber is nil, a serial number will be generated which
+// conforms to RFC 5280, Section 4.1.2.2 using entropy from rand.
+//
 // The PolicyIdentifier and Policies fields can both be used to marshal certificate
 // policy OIDs. By default, only the Policies is marshaled, but if the
 // GODEBUG setting "x509usepolicies" has the value "0", the PolicyIdentifiers field will
@@ -1667,16 +1671,35 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
 		return nil, errors.New("x509: certificate private key does not implement crypto.Signer")
 	}
 
-	if template.SerialNumber == nil {
-		return nil, errors.New("x509: no SerialNumber given")
+	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
+			}
+		}
 	}
 
 	// RFC 5280 Section 4.1.2.2: serial number must positive
 	//
 	// We _should_ also restrict serials to <= 20 octets, but it turns out a lot of people
 	// get this wrong, in part because the encoding can itself alter the length of the
 	// serial. For now we accept these non-conformant serials.
-	if template.SerialNumber.Sign() == -1 {
+	if serialNumber.Sign() == -1 {
 		return nil, errors.New("x509: serial number must be positive")
 	}
 
@@ -1740,7 +1763,7 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
 	encodedPublicKey := asn1.BitString{BitLength: len(publicKeyBytes) * 8, Bytes: publicKeyBytes}
 	c := tbsCertificate{
 		Version:            2,
-		SerialNumber:       template.SerialNumber,
+		SerialNumber:       serialNumber,
 		SignatureAlgorithm: algorithmIdentifier,
 		Issuer:             asn1.RawValue{FullBytes: asn1Issuer},
 		Validity:           validity{template.NotBefore.UTC(), template.NotAfter.UTC()},

src/crypto/x509/x509_test.go

@@ -2323,6 +2323,37 @@ func TestAdditionFieldsInGeneralSubtree(t *testing.T) {
 	}
 }
 
+func TestEmptySerialNumber(t *testing.T) {
+	template := Certificate{
+		DNSNames: []string{"example.com"},
+	}
+
+	for range 100 {
+		derBytes, err := CreateCertificate(rand.Reader, &template, &template, &testPrivateKey.PublicKey, testPrivateKey)
+		if err != nil {
+			t.Fatalf("failed to create certificate: %s", err)
+		}
+
+		cert, err := ParseCertificate(derBytes)
+		if err != nil {
+			t.Fatalf("failed to parse certificate: %s", err)
+		}
+
+		if sign := cert.SerialNumber.Sign(); sign != 1 {
+			t.Fatalf("generated a non positive serial, sign: %d", sign)
+		}
+
+		b, err := asn1.Marshal(cert.SerialNumber)
+		if err != nil {
+			t.Fatalf("failed to marshal generated serial number: %s", err)
+		}
+		// subtract 2 for tag and length
+		if l := len(b) - 2; l > 20 {
+			t.Fatalf("generated serial number larger than 20 octets when encoded: %d", l)
+		}
+	}
+}
+
 func TestEmptySubject(t *testing.T) {
 	template := Certificate{
 		SerialNumber: big.NewInt(1),