crypto/x509: don't allow too long serials

Don't create certificates that have serial numbers that are longer
than 20 octets (when encoded), since these are explicitly disallowed
by RFC 5280.

Change-Id: I292b7001f45bed0971b2d519b6de26f0b90860ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/400377
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 e28e213dc123a4f597e607a165274d8e181a4e8d..6d99191fefa59bccfbb56a72de6675c83296306d 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -1478,6 +1478,18 @@ 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 not be longer than 20 octets
+       //
+       // 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.
+       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 818a9750c34c9c9d43a4a53bd2ec505841ef6e6c..c294f91ed645de840d30bf39f2d342bad8d3eb7a 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -3592,3 +3592,39 @@ func TestOmitEmptyExtensions(t *testing.T) {
                t.Error("DER encoding contains the an empty extensions SEQUENCE")
        }
 }
+
+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)
+       }
+}