diff --git a/helpers/helpers.go b/helpers/helpers.go index 486029cfd..5aee7c4eb 100644 --- a/helpers/helpers.go +++ b/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 ¬After } +// 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 { diff --git a/helpers/helpers_test.go b/helpers/helpers_test.go index a4a6471f0..c872fd97c 100644 --- a/helpers/helpers_test.go +++ b/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) { diff --git a/scan/pki.go b/scan/pki.go index 4cfe4b677..f086e56d4 100644 --- a/scan/pki.go +++ b/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)) diff --git a/ubiquity/ubiquity_crypto.go b/ubiquity/ubiquity_crypto.go index c0cf5a930..779d1c2a8 100644 --- a/ubiquity/ubiquity_crypto.go +++ b/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 { diff --git a/ubiquity/ubiquity_test.go b/ubiquity/ubiquity_test.go index 3e822aebc..1409aa00a 100644 --- a/ubiquity/ubiquity_test.go +++ b/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") }