From 07b42666051841352077c0d04ba67d510247fd1d Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Fri, 22 Nov 2024 00:24:09 +0000 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov --- src/crypto/x509/x509.go | 31 +++++++++++++++++++++++++++---- src/crypto/x509/x509_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 3e2d9b4d71..f7ee1b511c 100644 --- 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 37dc717fa1..3eeeb02128 100644 --- 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), -- 2.48.1