crypto/x509: revert serial length restriction

This reverts CL400377, which restricted serials passed to
x509.CreateCertificate to <= 20 octets. Unfortunately this turns out to
be something _a lot_ of people get wrong. Since it's not particularly
obvious how to properly generate conformant serials, until we provide
an easier way for people to get this right, reverting this restriction
makes sense (possible solution discussed in #52444.)

Change-Id: Ia85a0ffe61e2e547abdaf1389c3e1ad29e28a2be
Reviewed-on: https://go-review.googlesource.com/c/go/+/401657
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index ceb04ae20e92f6c72c2cfb5724d4b9285726f763..582e1b151959573a69415713951ab7ccd8c3c963 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -1478,21 +1478,14 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
                return nil, errors.New("x509: no SerialNumber given")
        }
 
-       // RFC 5280 Section 4.1.2.2: serial number must positive and should not be longer
-       // than 20 octets.
+       // RFC 5280 Section 4.1.2.2: serial number must positive
        //
-       // We cannot simply check for len(serialBytes) > 20, because encoding/asn1 may
-       // pad the slice in order to prevent the integer being mistaken for a negative
-       // number (DER uses the high bit of the left-most byte to indicate the sign.),
-       // so we need to double check the composition of the serial if it is exactly
-       // 20 bytes.
+       // 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 {
                return nil, errors.New("x509: serial number must be positive")
        }
-       serialBytes := template.SerialNumber.Bytes()
-       if len(serialBytes) > 20 || (len(serialBytes) == 20 && serialBytes[0]&0x80 != 0) {
-               return nil, errors.New("x509: serial number exceeds 20 octets")
-       }
 
        if template.BasicConstraintsValid && !template.IsCA && template.MaxPathLen != -1 && (template.MaxPathLen != 0 || template.MaxPathLenZero) {
                return nil, errors.New("x509: only CAs are allowed to specify MaxPathLen")

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index 486d6bf3d238b4d2396cf677a0f09febf248d52f..f68dd0299a69cd43b4dd9543c5ef5483bce95554 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -3589,42 +3589,6 @@ func TestOmitEmptyExtensions(t *testing.T) {
        }
 }
 
-func TestCreateCertificateLongSerial(t *testing.T) {
-       k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
-       if err != nil {
-               t.Fatal(err)
-       }
-
-       serialBytes := make([]byte, 21)
-       serialBytes[0] = 0x80
-       serialBytes[20] = 1
-       tooLong := big.NewInt(0).SetBytes(serialBytes)
-
-       tmpl := &Certificate{
-               SerialNumber: tooLong,
-               Subject: pkix.Name{
-                       CommonName: ":)",
-               },
-               NotAfter:  time.Now().Add(time.Hour),
-               NotBefore: time.Now().Add(-time.Hour),
-       }
-
-       expectedErr := "x509: serial number exceeds 20 octets"
-
-       _, err = CreateCertificate(rand.Reader, tmpl, tmpl, k.Public(), k)
-       if err == nil || err.Error() != expectedErr {
-               t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err)
-       }
-
-       serialBytes = serialBytes[:20]
-       tmpl.SerialNumber = big.NewInt(0).SetBytes(serialBytes)
-
-       _, err = CreateCertificate(rand.Reader, tmpl, tmpl, k.Public(), k)
-       if err == nil || err.Error() != expectedErr {
-               t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err)
-       }
-}
-
 var negativeSerialCert = `-----BEGIN CERTIFICATE-----
 MIIBBTCBraADAgECAgH/MAoGCCqGSM49BAMCMA0xCzAJBgNVBAMTAjopMB4XDTIy
 MDQxNDIzNTYwNFoXDTIyMDQxNTAxNTYwNFowDTELMAkGA1UEAxMCOikwWTATBgcq