Reject certificates that are valid for too long.

net/cert/cert_verify_proc.cc

Index: net/cert/cert_verify_proc.cc
diff --git a/net/cert/cert_verify_proc.cc b/net/cert/cert_verify_proc.cc
index 222ba47f51c98fcab412a52334146ee064d77696..39ff70a1430ab828be05f456e0468c223e2377e1 100644
--- a/net/cert/cert_verify_proc.cc
+++ b/net/cert/cert_verify_proc.cc
@@ -4,10 +4,13 @@
 
 #include "net/cert/cert_verify_proc.h"
 
+#include <stdint.h>
+
 #include "base/basictypes.h"
 #include "base/metrics/histogram.h"
 #include "base/sha1.h"
 #include "base/strings/stringprintf.h"
+#include "base/time/time.h"
 #include "build/build_config.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
@@ -33,7 +36,6 @@
 #error Implement certificate verification.
 #endif
 
-
 namespace net {
 
 namespace {
@@ -276,6 +278,13 @@ int CertVerifyProc::Verify(X509Certificate* cert,
     // now treat it as a warning and do not map it to an error return value.
   }
 
+  // Flag certificates using too long validity periods.
+  if (verify_result->is_issued_by_known_root && HasTooLongValidity(*cert)) {
+    verify_result->cert_status |= CERT_STATUS_VALIDITY_TOO_LONG;
+    if (rv == OK)
+      rv = MapCertStatusToNetError(verify_result->cert_status);
+  }
+
   return rv;
 }
 
@@ -614,4 +623,54 @@ bool CertVerifyProc::HasNameConstraintsViolation(
   return false;
 }
 
+// static
+bool CertVerifyProc::HasTooLongValidity(const X509Certificate& cert) {
+  const base::Time& start = cert.valid_start();
+  const base::Time& expiry = cert.valid_expiry();
+  if (start.is_max() || start.is_null() || expiry.is_max() ||
+      expiry.is_null() || start > expiry) {
+    return true;
+  }
+
+  base::Time::Exploded exploded_start;
+  base::Time::Exploded exploded_expiry;
+  cert.valid_start().UTCExplode(&exploded_start);
+  cert.valid_expiry().UTCExplode(&exploded_expiry);
+
+  if (exploded_expiry.year - exploded_start.year > 10) {
+    return true;
+  }

[Ryan Sleevi, 2014/11/26 12:25:36]

no braces for one-liners (consistency in //net uber alles). And you know, line
647 et all.

[palmer, 2014/12/15 22:55:58]

Done.

+  int month_diff = (exploded_expiry.year - exploded_start.year) * 12 +
+                   (exploded_expiry.month - exploded_start.month);
+
+  // Add any remainder as a full month.
+  if (exploded_expiry.day_of_month > exploded_start.day_of_month)
+    ++month_diff;
+
+  static const base::Time time_2012_07_01 =
+      base::Time::FromUTCExploded({ 2012, 7, 0, 1, 0, 0, 0, 0 });
+  static const base::Time time_2015_04_01 =
+      base::Time::FromUTCExploded({ 2015, 4, 0, 1, 0, 0, 0, 0 });
+  static const base::Time time_2019_07_01 =
+      base::Time::FromUTCExploded({ 2019, 7, 0, 1, 0, 0, 0, 0 });

[Ryan Sleevi, 2014/11/26 12:25:36]

Forces a lot of lib calls every time. That scared of another typo? :)

[palmer, 2014/12/15 22:55:58]

This code is much clearer. Our bug was (in part) due to the fact that base::Time
is based on the *Windows* epoch, not the POSIX epoch — it was not *just* a
microseconds vs. seconds mistake.

This explicit format avoids all such confusion.

As for repeated calls to FromUTCExploded, these variables are declared static
and so should be constructed only once. printf debugging indeed shows this to be
the case.

(Even if it were somewhat inefficient, clarity should override in this
situation. Thankfully, we get both clarity and efficiency this time.)

+
+  // For certificates issued before the BRs took effect.
+  if (start < time_2012_07_01 &&
+      (month_diff > 120 || expiry > time_2019_07_01)) {
+    return true;
+  }
+
+  // For certificates issued after 1 July 2012: 60 months.
+  if (start >= time_2012_07_01 && month_diff > 60) {
+    return true;
+  }

[Ryan Sleevi, 2014/11/26 12:25:36]

no brace

[palmer, 2014/12/15 22:55:58]

Done.

+
+  // For certificates issued after 1 April 2015: 39 months.
+  if (start >= time_2015_04_01 && month_diff > 39) {
+    return true;
+  }

[Ryan Sleevi, 2014/11/26 12:25:36]

no brace

[palmer, 2014/12/15 22:55:58]

Done.

+
+  return false;
+}
+
 }  // namespace net