Add new ParsedCertificate class, move TrustStore to own file.

net/cert/internal/parsed_certificate.h

Index: net/cert/internal/parsed_certificate.h
diff --git a/net/cert/internal/parsed_certificate.h b/net/cert/internal/parsed_certificate.h
new file mode 100644
index 0000000000000000000000000000000000000000..11d09359ddb2bf5d7c6ef09e0571b897e9031d66
--- /dev/null
+++ b/net/cert/internal/parsed_certificate.h
@@ -0,0 +1,181 @@
+// Copyright 2016 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.
+
+#ifndef NET_CERT_INTERNAL_PARSED_CERTIFICATE_H_
+#define NET_CERT_INTERNAL_PARSED_CERTIFICATE_H_
+
+#include <map>
+#include <memory>
+#include <vector>
+
+#include "base/memory/ref_counted.h"
+#include "net/base/net_export.h"
+#include "net/cert/internal/parse_certificate.h"

[eroman, 2016/05/12 18:12:30]

This is a weird dependency, but I guess unavoidable right now.
We will need to further smooth out the differences in these files.

+#include "net/der/input.h"
+
+namespace net {
+
+struct GeneralNames;
+class NameConstraints;
+class SignatureAlgorithm;
+
+// XXX Rename.

[eroman, 2016/05/12 18:12:29]

TODO. That said I think you can just remove this line.

[mattm, 2016/05/13 02:17:36]

oops, yeah. I forgot to remove that.

+// Represents a certificate, including Certificate, TBSCertificate, and standard
+// extensions.
+// The certificate is not completely parsed and validated, only the
+// validation performed by ParseCertificate, ParseTbsCertificate,
+// NormalizeName, ParseBasicConstraints, ParseKeyUsage,
+// GeneralNames::CreateFromDer(subjectAltName), and
+// NameConstraints::CreateFromDer is done.

[eroman, 2016/05/12 18:12:29]

Is there maybe a more general description we can give?

These aren't the right words, but something along the lines of accessors that
return der::Input are unvalidated, whereas accessors that return objects have
been validated.

[mattm, 2016/05/13 02:17:36]

I'll try to come up with a better wording. In the meantime I wanted to upload
the rest of my changes for review...

+class NET_EXPORT ParsedCertificate
+    : public base::RefCountedThreadSafe<ParsedCertificate> {
+ public:
+  // Map from OID to ParsedExtension.
+  using ExtensionsMap = std::map<der::Input, ParsedExtension>;
+
+  // The certificate data for this trust anchor may either be owned internally
+  // (INTERNAL_COPY) or owned externally (EXTERNAL_REFERENCE). When it is
+  // owned internally the data is held by |cert_data_|
+  enum class DataSource {
+    INTERNAL_COPY,
+    EXTERNAL_REFERENCE,
+  };
+
+  // Creates a ParsedCertificate given a DER-encoded Certificate. Returns
+  // nullptr on

[eroman, 2016/05/12 18:12:30]

line wrap.

[mattm, 2016/05/13 02:17:37]

Done.

+  // failure. Failure will occur if the standard certificate fields and
+  // supported extensions cannot be parsed.
+  //
+  // The provided certificate data is either copied, or aliased, depending on
+  // the value of |source|. See the comments for DataSource for details.
+  static scoped_refptr<ParsedCertificate> CreateFromCertificateData(
+      const uint8_t* data,
+      size_t length,
+      DataSource source);
+  static scoped_refptr<ParsedCertificate> CreateFromCertificateCopy(
+      const base::StringPiece& data);
+
+  // Returns the DER-encoded certificate data for this cert.
+  const der::Input& der_cert() const { return cert_; }
+
+  // Accessors for raw fields of the Certificate.
+  const der::Input& tbs_certificate_tlv() const { return tbs_certificate_tlv_; }
+  const der::Input& signature_algorithm_tlv() const {
+    return signature_algorithm_tlv_;
+  }
+  const der::BitString& signature_value() const { return signature_value_; }
+  // Accessor for struct containing raw fields of the TbsCertificate.
+  const ParsedTbsCertificate& parsed_tbs() const { return parsed_tbs_; }
+
+  // Returns true if the signatureAlgorithm of the Certificate is supported.
+  bool has_supported_signature_algorithm() const {

[eroman, 2016/05/12 18:12:29]

It is a bit ambiguous from the name how this relates to validity. i.e. an
invalid algorithm (but of a supported type) might have been provided and
has_supported_signature_algorithm() will be false.

The current behavior is good, just wondering if we should name it differently.

[mattm, 2016/05/13 02:17:36]

Done.

+    return signature_algorithm_ != nullptr;
+  }
+  // Returns the signatureAlgorithm of the Certificate (not the tbsCertificate).
+  // Must not be called if has_supported_signature_algorithm() is false.
+  const SignatureAlgorithm& signature_algorithm() const {

[eroman, 2016/05/12 18:12:29]

Add a DCHECK() ?
(de-referencing wont' necessarily cause a crash, since return value is a const
reference).

[mattm, 2016/05/13 02:17:37]

Done.

+    return *signature_algorithm_;
+  }
+
+  // Returns the DER-encoded normalized subject value (not including outer
+  // Sequence tag).
+  const std::string& normalized_subject() const { return normalized_subject_; }

[eroman, 2016/05/12 18:12:30]

der::Input ?

[mattm, 2016/05/13 02:17:36]

Done.

+  // Returns the DER-encoded normalized issuer value (not including outer
+  // Sequence tag).
+  const std::string& normalized_issuer() const { return normalized_issuer_; }

[eroman, 2016/05/12 18:12:30]

der::Input ?

[mattm, 2016/05/13 02:17:37]

Done.

+
+  // Returns true if the certificate had a BasicConstraints extension.
+  bool has_basic_constraints() const { return has_basic_constraints_; }
+  // Returns the ParsedBasicConstraints struct. Caller should check

[eroman, 2016/05/12 18:12:30]

should --> must

[mattm, 2016/05/13 02:17:36]

Done.

+  // has_basic_constraints() before accessing this.
+  const ParsedBasicConstraints& basic_constraints() const {
+    return basic_constraints_;

[eroman, 2016/05/12 18:12:29]

Add a CHECK or DCHECK ?

[mattm, 2016/05/13 02:17:36]

Done.

+  }
+
+  // Returns true if the certificate had a KeyUsage extension.
+  bool has_key_usage() const { return has_key_usage_; }
+  // Returns the KeyUsage BitString. Caller should check

[eroman, 2016/05/12 18:12:30]

should--> must

Or alternately we could guarantee that the key usage is empty bitstring when not
provided?

[mattm, 2016/05/13 02:17:37]

done.
I think it's better to just require checking, since an empty key usage isn't
equivalent to not having a key usage.

+  // has_key_usage() before accessing this.
+  const der::BitString& key_usage() const { return key_usage_; }
+
+  // Returns true if the certificate had a SubjectAltName extension.
+  bool has_subject_alt_names() const { return subject_alt_names_ != nullptr; }
+  // Returns the ParsedExtension struct for the SubjectAltName extension.
+  // If the cert did not have a SubjectAltName extension, this will be a
+  // default-initialized ParsedExtension struct.
+  const ParsedExtension& subject_alt_names_extension() const {
+    return subject_alt_names_extension_;
+  }
+  // Returns the GeneralNames class parsed from SubjectAltName extension, or
+  // nullptr if no SubjectAltName extension was present.
+  const GeneralNames* subject_alt_names() const {
+    return subject_alt_names_.get();
+  }
+
+  // Returns true if the certificate had a NameConstraints extension.

[eroman, 2016/05/12 18:12:29]

nit: had --> has.
All of these properties correspond directly to the wrapped DER data.

[mattm, 2016/05/13 02:17:37]

Done.

+  bool has_name_constraints() const { return name_constraints_ != nullptr; }
+  // Returns the parsed NameConstraints extension. Must not be called if
+  // has_name_constraints() is false.
+  const NameConstraints& name_constraints() const { return *name_constraints_; }
+
+  // Returns a map of unhandled extensions (excludes the ones above).
+  const ExtensionsMap& unconsumed_extensions() const {
+    return unconsumed_extensions_;
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<ParsedCertificate>;
+  ParsedCertificate();
+  ~ParsedCertificate();
+
+  // The backing store for the certificate data. This is only applicable when
+  // the trust anchor was initialized using DataSource::INTERNAL_COPY.
+  std::vector<uint8_t> cert_data_;
+
+  // Note that the backing data for |cert_| and |name_| may come either form
+  // |cert_data_| or some external buffer (depending on how the anchor was
+  // created).
+
+  // Points to the raw certificate DER.
+  der::Input cert_;
+
+  der::Input tbs_certificate_tlv_;
+  der::Input signature_algorithm_tlv_;
+  der::BitString signature_value_;
+  ParsedTbsCertificate parsed_tbs_;
+
+  // The signatureAlgorithm from the Certificate.
+  std::unique_ptr<SignatureAlgorithm> signature_algorithm_;
+
+  // Normalized DER-encoded Subject (not including outer Sequence tag).
+  std::string normalized_subject_;
+  // Normalized DER-encoded Issuer (not including outer Sequence tag).
+  std::string normalized_issuer_;
+
+  // BasicConstraints extension.
+  bool has_basic_constraints_ = false;
+  ParsedBasicConstraints basic_constraints_;
+
+  // KeyUsage extension.
+  bool has_key_usage_ = false;
+  der::BitString key_usage_;
+
+  // Raw SubjectAltName extension.
+  ParsedExtension subject_alt_names_extension_;
+  // Parsed SubjectAltName extension.
+  std::unique_ptr<GeneralNames> subject_alt_names_;
+
+  // NameConstraints extension.
+  bool has_name_constraints_ = false;
+  std::unique_ptr<NameConstraints> name_constraints_;
+
+  // The remaining extensions (excludes the standard ones above).
+  ExtensionsMap unconsumed_extensions_;

[eroman, 2016/05/12 18:12:30]

From this layer "unconsumed" is a bit ambiguous.
Maybe unparsed_extensions ?

[mattm, 2016/05/13 02:17:36]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ParsedCertificate);
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_INTERNAL_PARSED_CERTIFICATE_H_