From e146938e4744cb89d3508a1fe3c185cc1fc3e5e1 Mon Sep 17 00:00:00 2001
From: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Date: Mon, 8 Aug 2016 16:01:45 -0700
Subject: [PATCH] change ordering of expiry checks and add explicit error type
 for expired certs

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
---
 tuf/data/errors.go |  9 +++++++++
 tuf/tuf.go         |  9 ++++++++-
 tuf/utils/x509.go  | 24 ++++++++++++------------
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/tuf/data/errors.go b/tuf/data/errors.go
index 0cd0afba06..5c1397d3e9 100644
--- a/tuf/data/errors.go
+++ b/tuf/data/errors.go
@@ -42,3 +42,12 @@ func (e ErrMismatchedChecksum) Error() string {
 	return fmt.Sprintf("%s checksum for %s did not match: expected %s", e.alg, e.name,
 		e.expected)
 }
+
+// ErrCertExpired is the error to be returned when a certificate has expired
+type ErrCertExpired struct {
+	CN string
+}
+
+func (e ErrCertExpired) Error() string {
+	return fmt.Sprintf("certificate with CN %s is expired", e.CN)
+}
diff --git a/tuf/tuf.go b/tuf/tuf.go
index c76a485a4a..7bc1958b40 100644
--- a/tuf/tuf.go
+++ b/tuf/tuf.go
@@ -253,7 +253,14 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) {
 						continue
 					}
 					if err := utils.ValidateCertificate(certFromKey, true); err != nil {
-						logrus.Warnf("error with delegation %s key ID %d: %s", delgRole.Name, keyID, err)
+						switch err.(type) {
+						case data.ErrCertExpired:
+							logrus.Warnf("error with delegation %s key ID %d: %s", delgRole.Name, keyID, err)
+						default:
+							// skip delegation roles for other invalid cert errors
+							continue
+						}
+
 					}
 				}
 				foundRole = &delgRole
diff --git a/tuf/utils/x509.go b/tuf/utils/x509.go
index 98162328ce..4abb16defb 100644
--- a/tuf/utils/x509.go
+++ b/tuf/utils/x509.go
@@ -264,18 +264,6 @@ func ValidateCertificate(c *x509.Certificate, checkExpiry bool) error {
 	if (c.NotBefore).After(c.NotAfter) {
 		return fmt.Errorf("certificate validity window is invalid")
 	}
-	if checkExpiry {
-		now := time.Now()
-		tomorrow := now.AddDate(0, 0, 1)
-		// Give one day leeway on creation "before" time, check "after" against today
-		if (tomorrow).Before(c.NotBefore) || now.After(c.NotAfter) {
-			return fmt.Errorf("certificate with CN %s is expired", c.Subject.CommonName)
-		}
-		// If this certificate is expiring within 6 months, put out a warning
-		if (c.NotAfter).Before(time.Now().AddDate(0, 6, 0)) {
-			logrus.Warnf("certificate with CN %s is near expiry", c.Subject.CommonName)
-		}
-	}
 	// Can't have SHA1 sig algorithm
 	if c.SignatureAlgorithm == x509.SHA1WithRSA || c.SignatureAlgorithm == x509.DSAWithSHA1 || c.SignatureAlgorithm == x509.ECDSAWithSHA1 {
 		return fmt.Errorf("certificate with CN %s uses invalid SHA1 signature algorithm", c.Subject.CommonName)
@@ -290,6 +278,18 @@ func ValidateCertificate(c *x509.Certificate, checkExpiry bool) error {
 			return fmt.Errorf("RSA bit length is too short")
 		}
 	}
+	if checkExpiry {
+		now := time.Now()
+		tomorrow := now.AddDate(0, 0, 1)
+		// Give one day leeway on creation "before" time, check "after" against today
+		if (tomorrow).Before(c.NotBefore) || now.After(c.NotAfter) {
+			return data.ErrCertExpired{CN: c.Subject.CommonName}
+		}
+		// If this certificate is expiring within 6 months, put out a warning
+		if (c.NotAfter).Before(time.Now().AddDate(0, 6, 0)) {
+			logrus.Warnf("certificate with CN %s is near expiry", c.Subject.CommonName)
+		}
+	}
 	return nil
 }