Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(165)

Unified Diff: net/cert/internal/parse_certificate.cc

Issue 2337373003: Add error details to ParseCertificate test data. (Closed)
Patch Set: sigh, git rebase-update added junk file Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | net/cert/internal/parse_certificate_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/cert/internal/parse_certificate.cc
diff --git a/net/cert/internal/parse_certificate.cc b/net/cert/internal/parse_certificate.cc
index af2f1c21dae8d8fc6d8dcb3733839cc3158e1cfb..c888d73e3e7cbc64e9a830859fa363ef2510ee1e 100644
--- a/net/cert/internal/parse_certificate.cc
+++ b/net/cert/internal/parse_certificate.cc
@@ -7,6 +7,7 @@
#include <utility>
#include "base/strings/string_util.h"
+#include "net/cert/internal/cert_errors.h"
#include "net/der/input.h"
#include "net/der/parse_values.h"
#include "net/der/parser.h"
@@ -15,6 +16,20 @@ namespace net {
namespace {
+DEFINE_CERT_ERROR_ID(kCertificateNotSequence,
+ "Failed parsing Certificate SEQUENCE");
+DEFINE_CERT_ERROR_ID(kUnconsumedDataInsideCertificateSequence,
+ "Unconsumed data inside Certificate SEQUENCE");
+DEFINE_CERT_ERROR_ID(kUnconsumedDataAfterCertificateSequence,
+ "Unconsumed data after Certificate SEQUENCE");
+DEFINE_CERT_ERROR_ID(kTbsCertificateNotSequence,
+ "Couldn't read tbsCertificate 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");
+
// Returns true if |input| is a SEQUENCE and nothing else.
WARN_UNUSED_RESULT bool IsSequenceTLV(const der::Input& input) {
der::Parser parser(input);
@@ -172,35 +187,54 @@ bool ParseCertificate(const der::Input& certificate_tlv,
der::Input* out_signature_algorithm_tlv,
der::BitString* out_signature_value,
CertErrors* out_errors) {
- // TODO(crbug.com/634443): Fill |out_errors| (which may be null) with error
- // information.
+ // |out_errors| is optional. But ensure it is non-null for the remainder of
+ // this function.
+ if (!out_errors) {
+ CertErrors unused_errors;
+ return ParseCertificate(certificate_tlv, out_tbs_certificate_tlv,
+ out_signature_algorithm_tlv, out_signature_value,
+ &unused_errors);
+ }
+
der::Parser parser(certificate_tlv);
// Certificate ::= SEQUENCE {
der::Parser certificate_parser;
- if (!parser.ReadSequence(&certificate_parser))
+ if (!parser.ReadSequence(&certificate_parser)) {
+ out_errors->AddError(kCertificateNotSequence);
return false;
+ }
// tbsCertificate TBSCertificate,
- if (!ReadSequenceTLV(&certificate_parser, out_tbs_certificate_tlv))
+ if (!ReadSequenceTLV(&certificate_parser, out_tbs_certificate_tlv)) {
+ out_errors->AddError(kTbsCertificateNotSequence);
return false;
+ }
// signatureAlgorithm AlgorithmIdentifier,
- if (!ReadSequenceTLV(&certificate_parser, out_signature_algorithm_tlv))
+ if (!ReadSequenceTLV(&certificate_parser, out_signature_algorithm_tlv)) {
+ out_errors->AddError(kSignatureAlgorithmNotSequence);
return false;
+ }
// signatureValue BIT STRING }
- if (!certificate_parser.ReadBitString(out_signature_value))
+ if (!certificate_parser.ReadBitString(out_signature_value)) {
+ out_errors->AddError(kSignatureValueNotBitString);
return false;
+ }
// There isn't an extension point at the end of Certificate.
- if (certificate_parser.HasMore())
+ if (certificate_parser.HasMore()) {
+ out_errors->AddError(kUnconsumedDataInsideCertificateSequence);
return false;
+ }
// By definition the input was a single Certificate, so there shouldn't be
// unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
+ out_errors->AddError(kUnconsumedDataAfterCertificateSequence);
return false;
+ }
return true;
}
« no previous file with comments | « no previous file | net/cert/internal/parse_certificate_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698