Add error details to TBSCertificate parsing function and tests.

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 <utility>
 
 #include "base/strings/string_util.h"
 #include "net/cert/internal/cert_errors.h"
@@ skipping 12 matching lines @@
 DEFINE_CERT_ERROR_ID(kUnconsumedDataAfterCertificateSequence,
                      "Unconsumed data after Certificate SEQUENCE");
 DEFINE_CERT_ERROR_ID(kTbsCertificateNotSequence,
                      "Couldn't read Certificate as SEQUENCE");
 DEFINE_CERT_ERROR_ID(
     kSignatureAlgorithmNotSequence,
     "Couldn't read Certificate.signatureAlgorithm as SEQUENCE");
 DEFINE_CERT_ERROR_ID(kSignatureValueNotBitString,
                      "Couldn't read Certificate.signatureValue as BIT STRING");
 
+DEFINE_CERT_ERROR_ID(kUnconsumedDataInsideTbsCertificateSequence,
+                     "Unconsumed data inside TBSCertificate");

[mattm, 2016/09/16 21:41:48]

should tbs be lowercase?

[eroman, 2016/09/16 22:07:18]

I intentionally upper-cased it to refer to the type, rather than the field name
in Certificate.

Because ParseTbsCertificate can be called independently, and hence technically
may not refer to the tbsCertificate field.

[mattm, 2016/09/16 22:16:44]

ah, sounds good.

+
 // Returns true if |input| is a SEQUENCE and nothing else.
 WARN_UNUSED_RESULT bool IsSequenceTLV(const der::Input& input) {
   der::Parser parser(input);
   der::Parser unused_sequence_parser;
   if (!parser.ReadSequence(&unused_sequence_parser))
     return false;
   // Should by a single SEQUENCE by definition of the function.
   return !parser.HasMore();
 }
 
@@ skipping 208 matching lines @@
 //        subjectPublicKeyInfo SubjectPublicKeyInfo,
 //        issuerUniqueID  [1]  IMPLICIT UniqueIdentifier OPTIONAL,
 //                             -- If present, version MUST be v2 or v3
 //        subjectUniqueID [2]  IMPLICIT UniqueIdentifier OPTIONAL,
 //                             -- If present, version MUST be v2 or v3
 //        extensions      [3]  EXPLICIT Extensions OPTIONAL
 //                             -- If present, version MUST be v3
 //        }
 bool ParseTbsCertificate(const der::Input& tbs_tlv,
                          const ParseCertificateOptions& options,
-                         ParsedTbsCertificate* out) {
+                         ParsedTbsCertificate* out,
+                         CertErrors* errors) {
+  // The rest of this function assumes that |errors| is non-null.
+  if (!errors) {
+    CertErrors unused_errors;
+    return ParseTbsCertificate(tbs_tlv, options, out, &unused_errors);
+  }
+
+  // TODO(crbug.com/634443): Add useful error information to |errors|.
+
   der::Parser parser(tbs_tlv);
 
   //   Certificate  ::=  SEQUENCE  {
   der::Parser tbs_parser;
   if (!parser.ReadSequence(&tbs_parser))
     return false;
 
   //        version         [0]  EXPLICIT Version DEFAULT v1,
   der::Input version;
   bool has_version;
@@ skipping 95 matching lines @@
       return false;
   }
 
   // Note that there IS an extension point at the end of TBSCertificate
   // (according to RFC 5912), so from that interpretation, unconsumed data would
   // be allowed in |tbs_parser|.
   //
   // However because only v1, v2, and v3 certificates are supported by the
   // parsing, there shouldn't be any subsequent data in those versions, so
   // reject.
-  if (tbs_parser.HasMore())
+  if (tbs_parser.HasMore()) {
+    errors->AddError(kUnconsumedDataInsideTbsCertificateSequence);
     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:
@@ skipping 320 matching lines @@
         out_ca_issuers_uris->push_back(uri);
       else if (access_method_oid == AdOcspOid())
         out_ocsp_uris->push_back(uri);
     }
   }
 
   return true;
 }
 
 }  // namespace net