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 <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 3e2d9b4d71e804c8824fe231cbb6bc9ce3ca39cd..f7ee1b511c22cb25edb8e59f105a468a32693566 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/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,8 +1671,27 @@ 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
@@ -1676,7 +1699,7 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
        // 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()},

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 37dc717fa1513d482fdc58260c2ac3193e141056..3eeeb0212878aef67169e42738ddc1fa5afe6712 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/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),