Add parsing code for TBSCertificate's "validity" field.

net/cert/internal/parse_certificate.cc

Index: net/cert/internal/parse_certificate.cc
diff --git a/net/cert/internal/parse_certificate.cc b/net/cert/internal/parse_certificate.cc
index 3a04e3ad17977feca5d1756286421d871fb9578b..e790d8a7e043cecfbec838301a549b3d8f95a82a 100644
--- a/net/cert/internal/parse_certificate.cc
+++ b/net/cert/internal/parse_certificate.cc
@@ -124,6 +124,37 @@ WARN_UNUSED_RESULT bool VerifySerialNumber(const der::Input& value) {
   return true;
 }
 
+// Consumes a "Time" value (as defined by RFC 5280) from |parser|. On success
+// writes the result to |*out| and returns true. On failure no guarantees are
+// made about the state of |parser|.
+//
+// From RFC 5280:
+//
+//     Time ::= CHOICE {
+//          utcTime        UTCTime,
+//          generalTime    GeneralizedTime }
+WARN_UNUSED_RESULT bool ReadTime(der::Parser* parser,
+                                 der::GeneralizedTime* out) {
+  der::Input value;
+  der::Tag tag;
+
+  if (!parser->ReadTagAndValue(&tag, &value))
+    return false;
+
+  // TODO(eroman): Justify using the "relaxed" flavor rather
+  // than strict. The CT database of certificates do not
+  // require the relaxed parsing, however certificates in the
+  // wild may.

[davidben, 2015/08/18 17:24:27]

Nit: This probably can be word-wrapped more loosely.

[davidben, 2015/08/18 17:24:27]

[If we don't have example certs, where did this behavior come from? Some other
implementation I'm guessing?]

[eroman, 2015/08/18 17:34:51]

I am not sure, I will ask Nick about the history.

Also see the email thread titled "Has anyone written parsing for
TBSCertificate.validity?" where I raise the issue about whether "relaxed" flavor
is needed.

+  if (tag == der::kUtcTime)
+    return der::ParseUTCTimeRelaxed(value, out);
+
+  if (tag == der::kGeneralizedTime)
+    return der::ParseGeneralizedTime(value, out);
+
+  // Unrecognized tag.
+  return false;
+}
+
 }  // namespace
 
 ParsedTbsCertificate::ParsedTbsCertificate()
@@ -304,4 +335,43 @@ bool ParseTbsCertificate(const der::Input& tbs_tlv, ParsedTbsCertificate* out) {
   return true;
 }
 
+// From RFC 5280:
+//
+//     Validity ::= SEQUENCE {
+//          notBefore      Time,
+//          notAfter       Time }
+bool ParseValidity(const der::Input& validity_tlv,
+                   der::GeneralizedTime* not_before,
+                   der::GeneralizedTime* not_after) {
+  der::Parser parser(validity_tlv);
+
+  //     Validity ::= SEQUENCE {
+  der::Parser validity_parser;
+  if (!parser.ReadSequence(&validity_parser))
+    return false;
+
+  //          notBefore      Time,
+  if (!ReadTime(&validity_parser, not_before))
+    return false;
+
+  //          notAfter       Time }
+  if (!ReadTime(&validity_parser, not_after))
+    return false;
+
+  // By definition the input was a single Validity sequence, so there shouldn't
+  // be unconsumed data.
+  if (parser.HasMore())
+    return false;
+
+  // The Validity type does not have an extension point.
+  if (validity_parser.HasMore())
+    return false;
+
+  // Note that RFC 5280 doesn't require notBefore to be <=
+  // notAfter, so that will not be considered a "parsing" error here. Instead it
+  // will be considered an expired certificate later when testing against the
+  // current timestamp.
+  return true;
+}
+
 }  // namespace net