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

net/cert/internal/parse_certificate.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/internal/parse_certificate.h"
 
 #include "net/der/input.h"
 #include "net/der/parse_values.h"
 #include "net/der/parser.h"
 
@@ skipping 106 matching lines @@
     if (!reader.ReadByte(&second_byte))
       return false;  // Unexpected
 
     if ((second_byte & 0x80) == 0)
       return false;  // MSB must be 1.
   }
 
   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()
     : version(CertificateVersion::V1),
       has_issuer_unique_id(false),
       has_subject_unique_id(false),
       has_extensions(false) {}
 
 ParsedTbsCertificate::~ParsedTbsCertificate() {}
 
@@ skipping 160 matching lines @@
     return false;
 
   // By definition the input was a single TBSCertificate, so there shouldn't be
   // unconsumed data.
   if (parser.HasMore())
     return false;
 
   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