Merge pull request #296 from cloudflare/kyle/expiry-validation

kyle/expiry validation

helpers/helpers.go

@@ -27,6 +27,20 @@ const OneYear = 8760 * time.Hour
 // OneDay is a time.Duration representing a day's worth of seconds.
 const OneDay = 24 * time.Hour
 
+// InclusiveDate returns the time.Time representation of a date - 1
+// nanosecond. This allows time.After to be used inclusively.
+func InclusiveDate(year int, month time.Month, day int) time.Time {
+	return time.Date(year, month, day, 0, 0, 0, 0, time.UTC).Add(-1 * time.Nanosecond)
+}
+
+// Jul2012 is the July 2012 CAB Forum deadline for when CAs must stop
+// issuing certificates valid for more than 5 years.
+var Jul2012 = InclusiveDate(2012, time.July, 01)
+
+// April2015 is the April 2015 CAB Forum deadline for when CAs must stop
+// issuing certificates valid for more than 39 months.
+var Apr2015 = InclusiveDate(2015, time.April, 01)
+
 // KeyLength returns the bit size of ECDSA or RSA PublicKey
 func KeyLength(key interface{}) int {
 	if key == nil {
@@ -55,6 +69,42 @@ func ExpiryTime(chain []*x509.Certificate) *time.Time {
 	return &notAfter
 }
 
+// MonthsValid returns the number of months for which a certificate is valid.
+func MonthsValid(c *x509.Certificate) int {
+	issued := c.NotBefore
+	expiry := c.NotAfter
+	years := (expiry.Year() - issued.Year())
+	months := years*12 + int(expiry.Month()) - int(issued.Month())
+
+	// Round up if valid for less than a full month
+	if expiry.Day() > issued.Day() {
+		months++
+	}
+	return months
+}
+
+// ValidExpiry determines if a certificate is valid for an acceptable
+// length of time per the CA/Browser Forum baseline requirements.
+// See https://cabforum.org/wp-content/uploads/CAB-Forum-BR-1.3.0.pdf
+func ValidExpiry(c *x509.Certificate) bool {
+	issued := c.NotBefore
+
+	var maxMonths int
+	switch {
+	case issued.After(Apr2015):
+		maxMonths = 39
+	case issued.After(Jul2012):
+		maxMonths = 60
+	case issued.Before(Jul2012):
+		maxMonths = 120
+	}
+
+	if MonthsValid(c) > maxMonths {
+		return false
+	}
+	return true
+}
+
 // SignatureString returns the TLS signature string corresponding to
 // an X509 signature algorithm.
 func SignatureString(alg x509.SignatureAlgorithm) string {
@@ -157,7 +207,6 @@ func ParseCertificatesDER(certsDER []byte, password string) ([]*x509.Certificate
 		if err != nil {
 			certs, err = x509.ParseCertificates(certsDER)
 			if err != nil {
-				//fmt.Println("\n\n\n\n\n\nCRITICALZONE\n\n\n\n\n\n\n\n\n\n")
 				return nil, nil, cferr.New(cferr.CertificateError, cferr.DecodeFailed)
 			}
 		} else {
@@ -166,7 +215,7 @@ func ParseCertificatesDER(certsDER []byte, password string) ([]*x509.Certificate
 		}
 	} else {
 		if pkcs7data.ContentInfo != "SignedData" {
-			return nil, nil, cferr.Wrap(cferr.CertificateError, cferr.DecodeFailed, errors.New("Can only extract certificates from signed data content info"))
+			return nil, nil, cferr.Wrap(cferr.CertificateError, cferr.DecodeFailed, errors.New("can only extract certificates from signed data content info"))
 		}
 		certs = pkcs7data.Content.SignedData.Certificates
 	}
@@ -200,9 +249,9 @@ func ParseCertificatePEM(certPEM []byte) (*x509.Certificate, error) {
 	} else if cert == nil {
 		return nil, cferr.New(cferr.CertificateError, cferr.DecodeFailed)
 	} else if len(rest) > 0 {
-		return nil, cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, errors.New("The PEM file should contain only one object."))
+		return nil, cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, errors.New("the PEM file should contain only one object"))
 	} else if len(cert) > 1 {
-		return nil, cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, errors.New("The PKCS7 object in the PEM file should contain only one certificate"))
+		return nil, cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, errors.New("the PKCS7 object in the PEM file should contain only one certificate"))
 	}
 	return cert[0], nil
 }
@@ -225,7 +274,7 @@ func ParseOneCertificateFromPEM(certsPEM []byte) ([]*x509.Certificate, []byte, e
 			return nil, rest, err
 		}
 		if pkcs7data.ContentInfo != "SignedData" {
-			return nil, rest, errors.New("Only PKCS #7 Signed Data Content Info supported for certificate parsing")
+			return nil, rest, errors.New("only PKCS #7 Signed Data Content Info supported for certificate parsing")
 		}
 		certs := pkcs7data.Content.SignedData.Certificates
 		if certs == nil {

helpers/helpers_test.go

@@ -111,7 +111,58 @@ func TestExpiryTime(t *testing.T) {
 	if *out != expected {
 		t.Fatalf("Expected %v, got %v", expected, *out)
 	}
+}
+
+func TestMonthsValid(t *testing.T) {
+	var cert = &x509.Certificate{
+		NotBefore: time.Date(2015, time.April, 01, 0, 0, 0, 0, time.UTC),
+		NotAfter:  time.Date(2015, time.April, 01, 0, 0, 0, 0, time.UTC),
+	}
+
+	if MonthsValid(cert) != 0 {
+		t.Fail()
+	}
+
+	cert.NotAfter = time.Date(2016, time.April, 01, 0, 0, 0, 0, time.UTC)
+	if MonthsValid(cert) != 12 {
+		t.Fail()
+	}
+
+	// extra days should be rounded up to 1 month
+	cert.NotAfter = time.Date(2016, time.April, 02, 0, 0, 0, 0, time.UTC)
+	if MonthsValid(cert) != 13 {
+		t.Fail()
+	}
+}
 
+func HasValidExpiry(t *testing.T) {
+	// Issue period > April 1, 2015
+	var cert = &x509.Certificate{
+		NotBefore: time.Date(2015, time.April, 01, 0, 0, 0, 0, time.UTC),
+		NotAfter:  time.Date(2016, time.April, 01, 0, 0, 0, 0, time.UTC),
+	}
+
+	if !ValidExpiry(cert) {
+		t.Fail()
+	}
+
+	cert.NotAfter = time.Date(2019, time.April, 01, 01, 0, 0, 0, time.UTC)
+	if ValidExpiry(cert) {
+		t.Fail()
+	}
+
+	// Issue period < July 1, 2012
+	cert.NotBefore = time.Date(2009, time.March, 01, 0, 0, 0, 0, time.UTC)
+	if ValidExpiry(cert) {
+		t.Fail()
+	}
+
+	// Issue period July 1, 2012 - April 1, 2015
+	cert.NotBefore = time.Date(2012, time.July, 01, 0, 0, 0, 0, time.UTC)
+	cert.NotAfter = time.Date(2017, time.July, 01, 0, 0, 0, 0, time.UTC)
+	if !ValidExpiry(cert) {
+		t.Fail()
+	}
 }
 
 func TestHashAlgoString(t *testing.T) {

scan/pki.go

@@ -94,6 +94,11 @@ func chainValidation(host string) (grade Grade, output Output, err error) {
 	for i := 0; i < len(chain)-1; i++ {
 		cert, parent := chain[i], chain[i+1]
 
+		valid := helpers.ValidExpiry(cert)
+		if !valid {
+			warnings = append(warnings, fmt.Sprintf("Certificate for %s is valid for too long", cert.Subject.CommonName))
+		}
+
 		revoked, ok := revoke.VerifyCertificate(cert)
 		if !ok {
 			warnings = append(warnings, fmt.Sprintf("couldn't check if %s is revoked", cert.Subject.CommonName))

ubiquity/ubiquity_crypto.go

@@ -6,6 +6,7 @@ import (
 	"crypto/elliptic"
 	"crypto/rsa"
 	"crypto/x509"
+	"github.com/cloudflare/cfssl/helpers"
 	"math"
 )
 
@@ -139,9 +140,22 @@ func CompareExpiryUbiquity(chain1, chain2 []*x509.Certificate) int {
 		if i >= len(chain1) || i >= len(chain2) {
 			break
 		}
-		// Compare expiry dates, later is ranked higher.
-		t1 := &chain1[len(chain1)-1-i].NotAfter
-		t2 := &chain2[len(chain2)-1-i].NotAfter
+		c1 := chain1[len(chain1)-1-i]
+		c2 := chain2[len(chain2)-1-i]
+		t1 := &c1.NotAfter
+		t2 := &c2.NotAfter
+
+		// Check if expiry dates valid. Return if one or other is invalid.
+		// Otherwise rank by expiry date. Later is ranked higher.
+		c1Valid := helpers.ValidExpiry(c1)
+		c2Valid := helpers.ValidExpiry(c2)
+		if c1Valid && !c2Valid {
+			return 1
+		}
+		if !c1Valid && c2Valid {
+			return -1
+		}
+
 		r := compareTime(t1, t2)
 		// Return when we find rank difference.
 		if r != 0 {

ubiquity/ubiquity_test.go

@@ -185,11 +185,16 @@ func TestChainExpiryUbiquity(t *testing.T) {
 	// ecdsa256Cert expires at year 2019
 	chain1 := []*x509.Certificate{ecdsa256Cert, rsa2048Cert}
 	chain2 := []*x509.Certificate{ecdsa256Cert, rsa1024Cert}
-	if CompareExpiryUbiquity(chain1, chain2) >= 0 {
+
+	// CompareExpiryUbiquity should return > 0 because chain1
+	// has a better expiry ubiquity than chain2.
+	if CompareExpiryUbiquity(chain1, chain2) <= 0 {
 		t.Fatal("Incorrect chain expiry ubiquity")
 	}
 
-	if CompareExpiryUbiquity(chain2, chain1) <= 0 {
+	// CompareExpiryUbiquity should return < 0 because chain1 has
+	// a better expiry ubiquity than chain2.
+	if CompareExpiryUbiquity(chain2, chain1) >= 0 {
 		t.Fatal("Incorrect chain expiry ubiquity")
 	}