diff options
| author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2017-07-12 14:07:37 +0200 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2017-07-17 10:29:26 +0000 |
| commit | ec02ee4181c49b61fce1c8fb99292dbb8139cc90 (patch) | |
| tree | 25cde714b2b71eb639d1cd53f5a22e9ba76e14ef /chromium/net/cert | |
| parent | bb09965444b5bb20b096a291445170876225268d (diff) | |
| download | qtwebengine-chromium-ec02ee4181c49b61fce1c8fb99292dbb8139cc90.tar.gz | |
BASELINE: Update Chromium to 59.0.3071.134
Change-Id: Id02ef6fb2204c5fd21668a1c3e6911c83b17585a
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Diffstat (limited to 'chromium/net/cert')
72 files changed, 2779 insertions, 1290 deletions
diff --git a/chromium/net/cert/cert_verify_proc.cc b/chromium/net/cert/cert_verify_proc.cc index da4968850ad..a2a8a9e61a2 100644 --- a/chromium/net/cert/cert_verify_proc.cc +++ b/chromium/net/cert/cert_verify_proc.cc @@ -833,7 +833,7 @@ bool CertVerifyProc::HasNameConstraintsViolation( kDomainsIndiaCCA, }, // Not a real certificate - just for testing. This is the SPKI hash of - // the keys used in net/data/ssl/certificates/name_constraint_*.crt. + // the keys used in net/data/ssl/certificates/name_constraint_*.pem. { {0x48, 0x49, 0x4a, 0xc5, 0x5a, 0x3e, 0xcd, 0xc5, 0x62, 0x9f, 0xef, 0x23, 0x14, 0xad, 0x05, 0xa9, 0x2a, 0x5c, 0x39, 0xc0}, diff --git a/chromium/net/cert/cert_verify_proc_android.cc b/chromium/net/cert/cert_verify_proc_android.cc index 430ae631630..31d68d7071a 100644 --- a/chromium/net/cert/cert_verify_proc_android.cc +++ b/chromium/net/cert/cert_verify_proc_android.cc @@ -300,14 +300,18 @@ bool VerifyFromAndroidTrustManager( scoped_refptr<X509Certificate> verified_cert = X509Certificate::CreateFromDERCertChain(verified_chain_pieces); if (verified_cert.get()) - verify_result->verified_cert = verified_cert; + verify_result->verified_cert = std::move(verified_cert); + else + verify_result->cert_status |= CERT_STATUS_INVALID; } // Extract the public key hashes. for (size_t i = 0; i < verified_chain.size(); i++) { base::StringPiece spki_bytes; - if (!asn1::ExtractSPKIFromDERCert(verified_chain[i], &spki_bytes)) + if (!asn1::ExtractSPKIFromDERCert(verified_chain[i], &spki_bytes)) { + verify_result->cert_status |= CERT_STATUS_INVALID; continue; + } HashValue sha1(HASH_VALUE_SHA1); base::SHA1HashBytes(reinterpret_cast<const uint8_t*>(spki_bytes.data()), diff --git a/chromium/net/cert/cert_verify_proc_builtin.cc b/chromium/net/cert/cert_verify_proc_builtin.cc new file mode 100644 index 00000000000..08a2feed63c --- /dev/null +++ b/chromium/net/cert/cert_verify_proc_builtin.cc @@ -0,0 +1,445 @@ +// Copyright (c) 2017 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/cert_verify_proc_builtin.h" + +#include <string> +#include <vector> + +#if defined(USE_NSS_CERTS) +#include <cert.h> +#include <pk11pub.h> +#endif + +#include "base/logging.h" +#include "base/memory/ptr_util.h" +#include "base/sha1.h" +#include "base/strings/string_piece.h" +#include "crypto/sha2.h" +#include "net/base/net_errors.h" +#include "net/cert/asn1_util.h" +#include "net/cert/cert_status_flags.h" +#include "net/cert/cert_verify_proc.h" +#include "net/cert/cert_verify_result.h" +#include "net/cert/internal/cert_errors.h" +#include "net/cert/internal/cert_issuer_source_static.h" +#include "net/cert/internal/parsed_certificate.h" +#include "net/cert/internal/path_builder.h" +#include "net/cert/internal/signature_policy.h" +#include "net/cert/internal/trust_store_collection.h" +#include "net/cert/internal/trust_store_in_memory.h" +#include "net/cert/internal/verify_certificate_chain.h" +#include "net/cert/x509_certificate.h" +#include "net/cert/x509_util.h" +#include "net/der/encode_values.h" + +#if defined(USE_NSS_CERTS) +#include "crypto/nss_util.h" +#include "net/cert/internal/cert_issuer_source_nss.h" +#include "net/cert/internal/trust_store_nss.h" +#include "net/cert/scoped_nss_types.h" +#endif + +namespace net { + +namespace { + +class CertVerifyProcBuiltin : public CertVerifyProc { + public: + CertVerifyProcBuiltin(); + + bool SupportsAdditionalTrustAnchors() const override; + bool SupportsOCSPStapling() const override; + + protected: + ~CertVerifyProcBuiltin() override; + + private: + int VerifyInternal(X509Certificate* cert, + const std::string& hostname, + const std::string& ocsp_response, + int flags, + CRLSet* crl_set, + const CertificateList& additional_trust_anchors, + CertVerifyResult* verify_result) override; +}; + +CertVerifyProcBuiltin::CertVerifyProcBuiltin() {} + +CertVerifyProcBuiltin::~CertVerifyProcBuiltin() {} + +bool CertVerifyProcBuiltin::SupportsAdditionalTrustAnchors() const { + return true; +} + +bool CertVerifyProcBuiltin::SupportsOCSPStapling() const { + // TODO(crbug.com/649017): Implement. + return false; +} + +scoped_refptr<ParsedCertificate> ParseCertificateFromOSHandle( + X509Certificate::OSCertHandle cert_handle, + CertErrors* errors) { + std::string cert_bytes; + if (!X509Certificate::GetDEREncoded(cert_handle, &cert_bytes)) + return nullptr; + return ParsedCertificate::Create(x509_util::CreateCryptoBuffer(cert_bytes), + {}, errors); +} + +void AddIntermediatesToIssuerSource(X509Certificate* x509_cert, + CertIssuerSourceStatic* intermediates) { + const X509Certificate::OSCertHandles& cert_handles = + x509_cert->GetIntermediateCertificates(); + CertErrors errors; + for (auto it = cert_handles.begin(); it != cert_handles.end(); ++it) { + scoped_refptr<ParsedCertificate> cert = + ParseCertificateFromOSHandle(*it, &errors); + if (cert) + intermediates->AddCert(std::move(cert)); + // TODO(crbug.com/634443): Surface these parsing errors? + } +} + +// The SystemTrustStore interface augments the TrustStore interface with some +// additional functionality: +// +// * Determine if a trust anchor was one of the known roots +// * Determine if a trust anchor was one of the "extra" ones that +// was specified during verification. +// +// Implementations of SystemTrustStore create an effective trust +// store that is the composition of: +// +// (1) System trust store +// (2) |additional_trust_anchors|. +// (3) Test certificates (if they are separate from system trust store) +class SystemTrustStore { + public: + virtual ~SystemTrustStore() {} + + virtual TrustStore* GetTrustStore() = 0; + + // TODO(eroman): Can this be exposed through the TrustStore + // interface instead? + virtual CertIssuerSource* GetCertIssuerSource() = 0; + + // IsKnownRoot returns true if the given trust anchor is a standard one (as + // opposed to a user-installed root) + virtual bool IsKnownRoot( + const scoped_refptr<TrustAnchor>& trust_anchor) const = 0; + + virtual bool IsAdditionalTrustAnchor( + const scoped_refptr<TrustAnchor>& trust_anchor) const = 0; +}; + +#if defined(USE_NSS_CERTS) +class SystemTrustStoreNSS : public SystemTrustStore { + public: + explicit SystemTrustStoreNSS(const CertificateList& additional_trust_anchors) + : trust_store_nss_(trustSSL) { + CertErrors errors; + + trust_store_.AddTrustStore(&additional_trust_store_); + for (const auto& x509_cert : additional_trust_anchors) { + scoped_refptr<ParsedCertificate> cert = + ParseCertificateFromOSHandle(x509_cert->os_cert_handle(), &errors); + if (cert) { + additional_trust_store_.AddTrustAnchor( + TrustAnchor::CreateFromCertificateNoConstraints(std::move(cert))); + } + // TODO(eroman): Surface parsing errors of additional trust anchor. + } + + trust_store_.AddTrustStore(&trust_store_nss_); + } + + TrustStore* GetTrustStore() override { return &trust_store_; } + + CertIssuerSource* GetCertIssuerSource() override { + return &cert_issuer_source_nss_; + } + + // IsKnownRoot returns true if the given trust anchor is a standard one (as + // opposed to a user-installed root) + bool IsKnownRoot( + const scoped_refptr<TrustAnchor>& trust_anchor) const override { + // TODO(eroman): Based on how the TrustAnchors are created by this + // integration, there will always be an associated certificate. However this + // contradicts the API for TrustAnchor that states it is optional. + DCHECK(trust_anchor->cert()); + + // TODO(eroman): The overall approach of IsKnownRoot() is inefficient -- it + // requires searching for the trust anchor by DER in NSS, however path + // building already had a handle to it. + SECItem der_cert; + der_cert.data = + const_cast<uint8_t*>(trust_anchor->cert()->der_cert().UnsafeData()); + der_cert.len = trust_anchor->cert()->der_cert().Length(); + der_cert.type = siDERCertBuffer; + ScopedCERTCertificate nss_cert( + CERT_FindCertByDERCert(CERT_GetDefaultCertDB(), &der_cert)); + if (!nss_cert) + return false; + + return IsKnownRoot(nss_cert.get()); + } + + bool IsAdditionalTrustAnchor( + const scoped_refptr<TrustAnchor>& trust_anchor) const override { + return additional_trust_store_.Contains(trust_anchor.get()); + } + + private: + // TODO(eroman): This function was copied verbatim from + // cert_verify_proc_nss.cc + // + // IsKnownRoot returns true if the given certificate is one that we believe + // is a standard (as opposed to user-installed) root. + bool IsKnownRoot(CERTCertificate* root) const { + if (!root || !root->slot) + return false; + + // This magic name is taken from + // http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ckfw/builtins/constants.c&rev=1.13&mark=86,89#79 + return 0 == strcmp(PK11_GetSlotName(root->slot), "NSS Builtin Objects"); + } + + TrustStoreCollection trust_store_; + TrustStoreInMemory additional_trust_store_; + + TrustStoreNSS trust_store_nss_; + CertIssuerSourceNSS cert_issuer_source_nss_; +}; +#endif + +std::unique_ptr<SystemTrustStore> CreateSystemTrustStore( + const CertificateList& additional_trust_anchors) { +#if defined(USE_NSS_CERTS) + return base::MakeUnique<SystemTrustStoreNSS>(additional_trust_anchors); +#else + // TODO(crbug.com/649017): Integrate with other system trust stores. + NOTIMPLEMENTED(); + return nullptr; +#endif +} + +// Appends the SHA1 and SHA256 hashes of |spki_bytes| to |*hashes|. +void AppendPublicKeyHashes(const der::Input& spki_bytes, + HashValueVector* hashes) { + HashValue sha1(HASH_VALUE_SHA1); + base::SHA1HashBytes(spki_bytes.UnsafeData(), spki_bytes.Length(), + sha1.data()); + hashes->push_back(sha1); + + HashValue sha256(HASH_VALUE_SHA256); + crypto::SHA256HashString(spki_bytes.AsStringPiece(), sha256.data(), + crypto::kSHA256Length); + hashes->push_back(sha256); +} + +// Appends the SubjectPublicKeyInfo hashes for all certificates (and trust +// anchor) in |partial_path| to |*hashes|. +void AppendPublicKeyHashes(const CertPathBuilder::ResultPath& partial_path, + HashValueVector* hashes) { + for (const scoped_refptr<ParsedCertificate>& cert : partial_path.path.certs) + AppendPublicKeyHashes(cert->tbs().spki_tlv, hashes); + + if (partial_path.path.trust_anchor) + AppendPublicKeyHashes(partial_path.path.trust_anchor->spki(), hashes); +} + +// Sets the bits on |cert_status| for all the errors present in |errors| (the +// errors for a particular path). +void MapPathBuilderErrorsToCertStatus(const CertPathErrors& errors, + CertStatus* cert_status) { + // If there were no errors, nothing to do. + if (!errors.ContainsHighSeverityErrors()) + return; + + if (errors.ContainsError(kRsaModulusTooSmall)) + *cert_status |= CERT_STATUS_WEAK_KEY; + + if (errors.ContainsError(kValidityFailedNotAfter) || + errors.ContainsError(kValidityFailedNotBefore)) { + *cert_status |= CERT_STATUS_DATE_INVALID; + } + + // IMPORTANT: If the path was invalid for a reason that was not + // explicity checked above, set a general error. This is important as + // |cert_status| is what ultimately indicates whether verification was + // successful or not (absense of errors implies success). + if (!IsCertStatusError(*cert_status)) + *cert_status |= CERT_STATUS_INVALID; +} + +X509Certificate::OSCertHandle CreateOSCertHandle( + const scoped_refptr<ParsedCertificate>& certificate) { + return X509Certificate::CreateOSCertHandleFromBytes( + reinterpret_cast<const char*>(certificate->der_cert().UnsafeData()), + certificate->der_cert().Length()); +} + +// Creates a X509Certificate (chain) to return as the verified result. +// +// * |target_cert|: The original X509Certificate that was passed in to +// VerifyInternal() +// * |path|: The result (possibly failed) from path building. +scoped_refptr<X509Certificate> CreateVerifiedCertChain( + X509Certificate* target_cert, + const CertPathBuilder::ResultPath& path) { + X509Certificate::OSCertHandles intermediates; + + // Skip the first certificate in the path as that is the target certificate + for (size_t i = 1; i < path.path.certs.size(); ++i) + intermediates.push_back(CreateOSCertHandle(path.path.certs[i])); + + if (path.path.trust_anchor) { + // TODO(eroman): This assumes that TrustAnchor::cert() cannot be null, + // which disagrees with the documentation. + intermediates.push_back(CreateOSCertHandle(path.path.trust_anchor->cert())); + } + + scoped_refptr<X509Certificate> result = X509Certificate::CreateFromHandle( + target_cert->os_cert_handle(), intermediates); + // |target_cert| was already successfully parsed, so this should never fail. + DCHECK(result); + + for (const X509Certificate::OSCertHandle handle : intermediates) + X509Certificate::FreeOSCertHandle(handle); + + return result; +} + +// TODO(crbug.com/649017): Make use of |flags|, |crl_set|, and |ocsp_response|. +// Also handle key usages, policies and EV. +// +// Any failure short-circuits from the function must set +// |verify_result->cert_status|. +void DoVerify(X509Certificate* input_cert, + const std::string& hostname, + const std::string& ocsp_response, + int flags, + CRLSet* crl_set, + const CertificateList& additional_trust_anchors, + CertVerifyResult* verify_result) { + CertErrors parsing_errors; + + // Parse the target certificate. + scoped_refptr<ParsedCertificate> target = ParseCertificateFromOSHandle( + input_cert->os_cert_handle(), &parsing_errors); + if (!target) { + // TODO(crbug.com/634443): Surface these parsing errors? + verify_result->cert_status |= CERT_STATUS_INVALID; + return; + } + + std::unique_ptr<SystemTrustStore> trust_store = + CreateSystemTrustStore(additional_trust_anchors); + + // TODO(eroman): The path building code in this file enforces its idea of weak + // keys, and separately cert_verify_proc.cc also checks the chains with its + // own policy. These policies should be aligned, to give path building the + // best chance of finding a good path. + // Another difference to resolve is the path building here does not check the + // target certificate's key strength, whereas cert_verify_proc.cc does. + SimpleSignaturePolicy signature_policy(1024); + + // Use the current time. + der::GeneralizedTime verification_time; + if (!der::EncodeTimeAsGeneralizedTime(base::Time::Now(), + &verification_time)) { + // This really shouldn't be possible unless Time::Now() returned + // something crazy. + verify_result->cert_status |= CERT_STATUS_DATE_INVALID; + return; + } + + // Initialize the path builder. + CertPathBuilder::Result result; + CertPathBuilder path_builder(target, trust_store->GetTrustStore(), + &signature_policy, verification_time, + KeyPurpose::SERVER_AUTH, &result); + + // Allow the path builder to discover intermediates from the trust store. + if (trust_store->GetCertIssuerSource()) + path_builder.AddCertIssuerSource(trust_store->GetCertIssuerSource()); + + // Allow the path builder to discover the explicitly provided intermediates in + // |input_cert|. + CertIssuerSourceStatic intermediates; + AddIntermediatesToIssuerSource(input_cert, &intermediates); + path_builder.AddCertIssuerSource(&intermediates); + + // TODO(crbug.com/649017): Allow the path builder to discover intermediates + // through AIA fetching. + + path_builder.Run(); + + if (result.best_result_index >= result.paths.size()) { + // TODO(crbug.com/634443): What errors to communicate? Maybe the path + // builder should always return some partial path (even if just containing + // the target), then there is a CertErrors to test. + verify_result->cert_status |= CERT_STATUS_AUTHORITY_INVALID; + return; + } + + // Use the best path that was built. This could be a partial path, or it could + // be a valid complete path. + const CertPathBuilder::ResultPath& partial_path = + *result.paths[result.best_result_index].get(); + + if (partial_path.path.trust_anchor) { + verify_result->is_issued_by_known_root = + trust_store->IsKnownRoot(partial_path.path.trust_anchor); + + verify_result->is_issued_by_additional_trust_anchor = + trust_store->IsAdditionalTrustAnchor(partial_path.path.trust_anchor); + } else { + // TODO(eroman): This shouldn't be necessary -- partial_path.errors should + // contain an error if it didn't chain to trust anchor. + verify_result->cert_status |= CERT_STATUS_AUTHORITY_INVALID; + } + + verify_result->verified_cert = + CreateVerifiedCertChain(input_cert, partial_path); + + AppendPublicKeyHashes(partial_path, &verify_result->public_key_hashes); + MapPathBuilderErrorsToCertStatus(partial_path.errors, + &verify_result->cert_status); + + // TODO(eroman): Is it possible that IsValid() fails but no errors were set in + // partial_path.errors? + CHECK(partial_path.IsValid() || + IsCertStatusError(verify_result->cert_status)); + + if (partial_path.errors.ContainsHighSeverityErrors()) { + LOG(ERROR) << "CertVerifyProcBuiltin for " << hostname << " failed:\n" + << partial_path.errors.ToDebugString(partial_path.path.certs); + } +} + +int CertVerifyProcBuiltin::VerifyInternal( + X509Certificate* input_cert, + const std::string& hostname, + const std::string& ocsp_response, + int flags, + CRLSet* crl_set, + const CertificateList& additional_trust_anchors, + CertVerifyResult* verify_result) { + DoVerify(input_cert, hostname, ocsp_response, flags, crl_set, + additional_trust_anchors, verify_result); + + return IsCertStatusError(verify_result->cert_status) + ? MapCertStatusToNetError(verify_result->cert_status) + : OK; +} + +} // namespace + +scoped_refptr<CertVerifyProc> CreateCertVerifyProcBuiltin() { + return scoped_refptr<CertVerifyProc>(new CertVerifyProcBuiltin()); +} + +} // namespace net diff --git a/chromium/net/cert/cert_verify_proc_builtin.h b/chromium/net/cert/cert_verify_proc_builtin.h new file mode 100644 index 00000000000..63bfc2cae73 --- /dev/null +++ b/chromium/net/cert/cert_verify_proc_builtin.h @@ -0,0 +1,22 @@ +// Copyright (c) 2017 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_CERT_VERIFY_PROC_BUILTIN_H_ +#define NET_CERT_CERT_VERIFY_PROC_BUILTIN_H_ + +#include "base/memory/ref_counted.h" +#include "net/base/net_export.h" + +namespace net { + +class CertVerifyProc; + +// TODO(crbug.com/649017): This is not how other cert_verify_proc_*.h are +// implemented -- they expose the type in the header. Use a consistent style +// here too. +NET_EXPORT scoped_refptr<CertVerifyProc> CreateCertVerifyProcBuiltin(); + +} // namespace net + +#endif // NET_CERT_CERT_VERIFY_PROC_BUILTIN_H_ diff --git a/chromium/net/cert/cert_verify_proc_ios.cc b/chromium/net/cert/cert_verify_proc_ios.cc index 84ecd2aea84..527326e6bb8 100644 --- a/chromium/net/cert/cert_verify_proc_ios.cc +++ b/chromium/net/cert/cert_verify_proc_ios.cc @@ -116,12 +116,16 @@ void GetCertChainInfo(CFArrayRef cert_chain, CertVerifyResult* verify_result) { } std::string der_bytes; - if (!X509Certificate::GetDEREncoded(chain_cert, &der_bytes)) + if (!X509Certificate::GetDEREncoded(chain_cert, &der_bytes)) { + verify_result->cert_status |= CERT_STATUS_INVALID; return; + } base::StringPiece spki_bytes; - if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki_bytes)) - continue; + if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki_bytes)) { + verify_result->cert_status |= CERT_STATUS_INVALID; + return; + } HashValue sha1(HASH_VALUE_SHA1); CC_SHA1(spki_bytes.data(), spki_bytes.size(), sha1.data()); @@ -139,11 +143,16 @@ void GetCertChainInfo(CFArrayRef cert_chain, CertVerifyResult* verify_result) { } if (!verified_cert) { NOTREACHED(); + verify_result->cert_status |= CERT_STATUS_INVALID; return; } - verify_result->verified_cert = + scoped_refptr<X509Certificate> verified_cert_with_chain = X509Certificate::CreateFromHandle(verified_cert, verified_chain); + if (verified_cert_with_chain) + verify_result->verified_cert = std::move(verified_cert_with_chain); + else + verify_result->cert_status |= CERT_STATUS_INVALID; } } // namespace diff --git a/chromium/net/cert/cert_verify_proc_mac.cc b/chromium/net/cert/cert_verify_proc_mac.cc index c6a3f0e37a3..0ed65a6039c 100644 --- a/chromium/net/cert/cert_verify_proc_mac.cc +++ b/chromium/net/cert/cert_verify_proc_mac.cc @@ -197,17 +197,23 @@ void CopyCertChainToVerifyResult(CFArrayRef cert_chain, } if (!verified_cert) { NOTREACHED(); + verify_result->cert_status |= CERT_STATUS_INVALID; return; } - verify_result->verified_cert = - X509Certificate::CreateFromHandle(verified_cert, verified_chain); + scoped_refptr<X509Certificate> verified_cert_with_chain = + x509_util::CreateX509CertificateFromSecCertificate(verified_cert, + verified_chain); + if (verified_cert_with_chain) + verify_result->verified_cert = std::move(verified_cert_with_chain); + else + verify_result->cert_status |= CERT_STATUS_INVALID; } // Returns true if the certificate uses MD2, MD4, MD5, or SHA1, and false // otherwise. A return of false also includes the case where the signature // algorithm couldn't be conclusively labeled as weak. -bool CertUsesWeakHash(X509Certificate::OSCertHandle cert_handle) { +bool CertUsesWeakHash(SecCertificateRef cert_handle) { x509_util::CSSMCachedCertificate cached_cert; OSStatus status = cached_cert.Init(cert_handle); if (status) @@ -273,34 +279,14 @@ bool IsWeakChainBasedOnHashingAlgorithms( return !leaf_uses_weak_hash && intermediates_contain_weak_hash; } -using ExtensionsMap = std::map<net::der::Input, net::ParsedExtension>; - -// Helper that looks up an extension by OID given a map of extensions. -bool GetExtensionValue(const ExtensionsMap& extensions, - const net::der::Input& oid, - net::der::Input* value) { - auto it = extensions.find(oid); - if (it == extensions.end()) - return false; - *value = it->second.value; - return true; -} - // Checks if |*cert| has a Certificate Policies extension containing either // of |ev_policy_oid| or anyPolicy. bool HasPolicyOrAnyPolicy(const ParsedCertificate* cert, const der::Input& ev_policy_oid) { - der::Input extension_value; - if (!GetExtensionValue(cert->unparsed_extensions(), CertificatePoliciesOid(), - &extension_value)) { + if (!cert->has_policy_oids()) return false; - } - std::vector<der::Input> policies; - if (!ParseCertificatePoliciesExtension(extension_value, &policies)) - return false; - - for (const der::Input& policy_oid : policies) { + for (const der::Input& policy_oid : cert->policy_oids()) { if (policy_oid == ev_policy_oid || policy_oid == AnyPolicy()) return true; } @@ -324,18 +310,11 @@ void GetCandidateEVPolicy(const X509Certificate* cert_input, if (!cert) return; - der::Input extension_value; - if (!GetExtensionValue(cert->unparsed_extensions(), CertificatePoliciesOid(), - &extension_value)) { - return; - } - - std::vector<der::Input> policies; - if (!ParseCertificatePoliciesExtension(extension_value, &policies)) + if (!cert->has_policy_oids()) return; EVRootCAMetadata* metadata = EVRootCAMetadata::GetInstance(); - for (const der::Input& policy_oid : policies) { + for (const der::Input& policy_oid : cert->policy_oids()) { if (metadata->IsEVPolicyOID(policy_oid)) { *ev_policy_oid = policy_oid.AsString(); @@ -638,12 +617,12 @@ class OSXKnownRootHelper { return false; SecCertificateRef root_ref = reinterpret_cast<SecCertificateRef>( const_cast<void*>(CFArrayGetValueAtIndex(chain, n - 1))); - SHA256HashValue hash = X509Certificate::CalculateFingerprint256(root_ref); + SHA256HashValue hash = x509_util::CalculateFingerprint256(root_ref); return known_roots_.find(hash) != known_roots_.end(); } private: - friend struct base::DefaultLazyInstanceTraits<OSXKnownRootHelper>; + friend struct base::LazyInstanceTraitsBase<OSXKnownRootHelper>; OSXKnownRootHelper() { CFArrayRef cert_array = NULL; @@ -658,7 +637,7 @@ class OSXKnownRootHelper { for (CFIndex i = 0, size = CFArrayGetCount(cert_array); i < size; ++i) { SecCertificateRef cert = reinterpret_cast<SecCertificateRef>( const_cast<void*>(CFArrayGetValueAtIndex(cert_array, i))); - known_roots_.insert(X509Certificate::CalculateFingerprint256(cert)); + known_roots_.insert(x509_util::CalculateFingerprint256(cert)); } } @@ -804,7 +783,9 @@ int VerifyWithGivenFlags(X509Certificate* cert, } ScopedCFTypeRef<CFMutableArrayRef> cert_array( - cert->CreateOSCertChainForCert()); + x509_util::CreateSecCertificateArrayForX509Certificate(cert)); + if (!cert_array) + return ERR_CERT_INVALID; // Beginning with the certificate chain as supplied by the server, attempt // to verify the chain. If a failure is encountered, trim a certificate diff --git a/chromium/net/cert/cert_verify_proc_nss.cc b/chromium/net/cert/cert_verify_proc_nss.cc index 8357b78f249..48aca1dfd43 100644 --- a/chromium/net/cert/cert_verify_proc_nss.cc +++ b/chromium/net/cert/cert_verify_proc_nss.cc @@ -195,8 +195,13 @@ void GetCertChainInfo(CERTCertList* cert_list, if (root_cert) verified_chain.push_back(root_cert); - verify_result->verified_cert = + + scoped_refptr<X509Certificate> verified_cert_with_chain = X509Certificate::CreateFromHandle(verified_cert, verified_chain); + if (verified_cert_with_chain) + verify_result->verified_cert = std::move(verified_cert_with_chain); + else + verify_result->cert_status |= CERT_STATUS_INVALID; } // IsKnownRoot returns true if the given certificate is one that we believe diff --git a/chromium/net/cert/cert_verify_proc_openssl.cc b/chromium/net/cert/cert_verify_proc_openssl.cc index 13a19d8e163..20c0ad5b1d0 100644 --- a/chromium/net/cert/cert_verify_proc_openssl.cc +++ b/chromium/net/cert/cert_verify_proc_openssl.cc @@ -109,8 +109,12 @@ void GetCertChainInfo(X509_STORE_CTX* store_ctx, // Set verify_result->verified_cert and // verify_result->is_issued_by_known_root. if (verified_cert) { - verify_result->verified_cert = + scoped_refptr<X509Certificate> verified_cert_with_chain = X509Certificate::CreateFromHandle(verified_cert, verified_chain); + if (verified_cert_with_chain) + verify_result->verified_cert = std::move(verified_cert_with_chain); + else + verify_result->cert_status |= CERT_STATUS_INVALID; // For OpenSSL builds, only certificates used for unit tests are treated // as not issued by known roots. The only way to determine whether a diff --git a/chromium/net/cert/cert_verify_proc_unittest.cc b/chromium/net/cert/cert_verify_proc_unittest.cc index 93437474b33..fdb254d1bc5 100644 --- a/chromium/net/cert/cert_verify_proc_unittest.cc +++ b/chromium/net/cert/cert_verify_proc_unittest.cc @@ -21,6 +21,7 @@ #include "net/cert/asn1_util.h" #include "net/cert/cert_status_flags.h" #include "net/cert/cert_verifier.h" +#include "net/cert/cert_verify_proc_builtin.h" #include "net/cert/cert_verify_result.h" #include "net/cert/crl_set.h" #include "net/cert/crl_set_storage.h" @@ -48,6 +49,9 @@ #include "base/win/windows_version.h" #endif +// TODO(crbug.com/649017): Add tests that only certificates with +// serverAuth are accepted. + using net::test::IsError; using net::test::IsOk; @@ -113,6 +117,7 @@ enum CertVerifyProcType { CERT_VERIFY_PROC_IOS, CERT_VERIFY_PROC_MAC, CERT_VERIFY_PROC_WIN, + CERT_VERIFY_PROC_BUILTIN, }; // Returns the CertVerifyProcType corresponding to what @@ -162,6 +167,8 @@ std::string VerifyProcTypeToName( return "CertVerifyProcMac"; case CERT_VERIFY_PROC_WIN: return "CertVerifyProcWin"; + case CERT_VERIFY_PROC_BUILTIN: + return "CertVerifyProcBuiltin"; } return nullptr; @@ -170,13 +177,21 @@ std::string VerifyProcTypeToName( // The set of all CertVerifyProcTypes that tests should be // parameterized on. const std::vector<CertVerifyProcType> kAllCertVerifiers = { - GetDefaultCertVerifyProcType()}; + GetDefaultCertVerifyProcType() + +// TODO(crbug.com/649017): Enable this everywhere. Right now this is +// gated on having CertVerifyProcBuiltin understand the roots added +// via TestRootCerts. +#if defined(USE_NSS_CERTS) + , + CERT_VERIFY_PROC_BUILTIN +#endif +}; } // namespace // This fixture is for tests that apply to concrete implementations of -// CertVerifyProc. It will be run for all of the concrete -// CertVerifyProc types. +// CertVerifyProc. It will be run for all of the concrete CertVerifyProc types. // // It is called "Internal" as it tests the internal methods like // "VerifyInternal()". @@ -184,8 +199,14 @@ class CertVerifyProcInternalTest : public testing::TestWithParam<CertVerifyProcType> { protected: void SetUp() override { - EXPECT_EQ(verify_proc_type(), GetDefaultCertVerifyProcType()); - verify_proc_ = CertVerifyProc::CreateDefault(); + CertVerifyProcType type = verify_proc_type(); + if (type == CERT_VERIFY_PROC_BUILTIN) { + verify_proc_ = CreateCertVerifyProcBuiltin(); + } else if (type == GetDefaultCertVerifyProcType()) { + verify_proc_ = CertVerifyProc::CreateDefault(); + } else { + ADD_FAILURE() << "Unhandled CertVerifyProcType"; + } } int Verify(X509Certificate* cert, @@ -243,12 +264,14 @@ class CertVerifyProcInternalTest } bool SupportsCRLSet() const { + // TODO(crbug.com/649017): Return true for CERT_VERIFY_PROC_BUILTIN. return verify_proc_type() == CERT_VERIFY_PROC_NSS || verify_proc_type() == CERT_VERIFY_PROC_WIN || verify_proc_type() == CERT_VERIFY_PROC_MAC; } bool SupportsCRLSetsInPathBuilding() const { + // TODO(crbug.com/649017): Return true for CERT_VERIFY_PROC_BUILTIN. return verify_proc_type() == CERT_VERIFY_PROC_WIN || verify_proc_type() == CERT_VERIFY_PROC_NSS; } @@ -280,18 +303,11 @@ TEST_P(CertVerifyProcInternalTest, DISABLED_EVVerification) { return; } - CertificateList certs = - CreateCertificateListFromFile(GetTestCertsDirectory(), "comodo.chain.pem", - X509Certificate::FORMAT_PEM_CERT_SEQUENCE); - ASSERT_EQ(3U, certs.size()); - - X509Certificate::OSCertHandles intermediates; - intermediates.push_back(certs[1]->os_cert_handle()); - intermediates.push_back(certs[2]->os_cert_handle()); - - scoped_refptr<X509Certificate> comodo_chain = - X509Certificate::CreateFromHandle(certs[0]->os_cert_handle(), - intermediates); + scoped_refptr<X509Certificate> comodo_chain = CreateCertificateChainFromFile( + GetTestCertsDirectory(), "comodo.chain.pem", + X509Certificate::FORMAT_PEM_CERT_SEQUENCE); + ASSERT_TRUE(comodo_chain); + ASSERT_EQ(2U, comodo_chain->GetIntermediateCertificates().size()); scoped_refptr<CRLSet> crl_set(CRLSet::ForTesting(false, NULL, "")); CertVerifyResult verify_result; @@ -424,13 +440,10 @@ TEST_P(CertVerifyProcInternalTest, RejectExpiredCert) { ScopedTestRoot test_root( ImportCertFromFile(certs_dir, "root_ca_cert.pem").get()); - CertificateList certs = CreateCertificateListFromFile( + scoped_refptr<X509Certificate> cert = CreateCertificateChainFromFile( certs_dir, "expired_cert.pem", X509Certificate::FORMAT_AUTO); - ASSERT_EQ(1U, certs.size()); - - X509Certificate::OSCertHandles intermediates; - scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle( - certs[0]->os_cert_handle(), intermediates); + ASSERT_TRUE(cert); + ASSERT_EQ(0U, cert->GetIntermediateCertificates().size()); int flags = 0; CertVerifyResult verify_result; @@ -498,6 +511,7 @@ TEST_P(CertVerifyProcInternalTest, RejectWeakKeys) { scoped_refptr<X509Certificate> cert_chain = X509Certificate::CreateFromHandle(ee_cert->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain); CertVerifyResult verify_result; int error = Verify(cert_chain.get(), "127.0.0.1", 0, NULL, @@ -553,6 +567,7 @@ TEST_P(CertVerifyProcInternalTest, ExtraneousMD5RootCert) { intermediates.push_back(extra_cert->os_cert_handle()); scoped_refptr<X509Certificate> cert_chain = X509Certificate::CreateFromHandle( server_cert->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain); CertVerifyResult verify_result; int flags = 0; @@ -587,6 +602,7 @@ TEST_P(CertVerifyProcInternalTest, GoogleDigiNotarTest) { intermediates.push_back(intermediate_cert->os_cert_handle()); scoped_refptr<X509Certificate> cert_chain = X509Certificate::CreateFromHandle( server_cert->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain); CertVerifyResult verify_result; int flags = CertVerifier::VERIFY_REV_CHECKING_ENABLED; @@ -653,14 +669,11 @@ TEST_P(CertVerifyProcInternalTest, NameConstraintsOk) { ASSERT_EQ(1U, ca_cert_list.size()); ScopedTestRoot test_root(ca_cert_list[0].get()); - CertificateList cert_list = CreateCertificateListFromFile( + scoped_refptr<X509Certificate> leaf = CreateCertificateChainFromFile( GetTestCertsDirectory(), "name_constraint_good.pem", X509Certificate::FORMAT_AUTO); - ASSERT_EQ(1U, cert_list.size()); - - X509Certificate::OSCertHandles intermediates; - scoped_refptr<X509Certificate> leaf = X509Certificate::CreateFromHandle( - cert_list[0]->os_cert_handle(), intermediates); + ASSERT_TRUE(leaf); + ASSERT_EQ(0U, leaf->GetIntermediateCertificates().size()); int flags = 0; CertVerifyResult verify_result; @@ -693,13 +706,20 @@ class CertVerifyProcInspectSignatureAlgorithmsTest : public ::testing::Test { DigestAlgorithm tbs_algorithm; }; - // On iOS trying to import a certificate with mismatched signature will - // fail. Consequently the rest of the tests can't be performed. + // On some platforms trying to import a certificate with mismatched signature + // will fail. Consequently the rest of the tests can't be performed. WARN_UNUSED_RESULT bool SupportsImportingMismatchedAlgorithms() const { #if defined(OS_IOS) LOG(INFO) << "Skipping test on iOS because certs with mismatched " "algorithms cannot be imported"; return false; +#elif defined(OS_MACOSX) + if (base::mac::IsAtLeastOS10_12()) { + LOG(INFO) << "Skipping test on macOS >= 10.12 because certs with " + "mismatched algorithms cannot be imported"; + return false; + } + return true; #else return true; #endif @@ -1052,6 +1072,7 @@ TEST_P(CertVerifyProcInternalTest, NameConstraintsFailure) { X509Certificate::OSCertHandles intermediates; scoped_refptr<X509Certificate> leaf = X509Certificate::CreateFromHandle( cert_list[0]->os_cert_handle(), intermediates); + ASSERT_TRUE(leaf); int flags = 0; CertVerifyResult verify_result; @@ -1109,6 +1130,7 @@ TEST_P(CertVerifyProcInternalTest, DISABLED_TestKnownRoot) { scoped_refptr<X509Certificate> cert_chain = X509Certificate::CreateFromHandle( certs[0]->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain); int flags = 0; CertVerifyResult verify_result; @@ -1180,6 +1202,11 @@ TEST_P(CertVerifyProcInternalTest, PublicKeyHashes) { // The Key Usage extension in this RSA SSL server certificate does not have // the keyEncipherment bit. TEST_P(CertVerifyProcInternalTest, InvalidKeyUsage) { + if (verify_proc_type() == CERT_VERIFY_PROC_BUILTIN) { + LOG(INFO) << "TODO(crbug.com/649017): Skipping test as not yet implemented " + "in builting verifier"; + return; + } base::FilePath certs_dir = GetTestCertsDirectory(); scoped_refptr<X509Certificate> server_cert = diff --git a/chromium/net/cert/cert_verify_proc_win.cc b/chromium/net/cert/cert_verify_proc_win.cc index 28867677b14..7f4d98d2334 100644 --- a/chromium/net/cert/cert_verify_proc_win.cc +++ b/chromium/net/cert/cert_verify_proc_win.cc @@ -369,8 +369,12 @@ void GetCertChainInfo(PCCERT_CHAIN_CONTEXT chain_context, // Add the root certificate, if present, as it was not added above. if (has_root_ca) verified_chain.push_back(element[num_elements]->pCertContext); - verify_result->verified_cert = - X509Certificate::CreateFromHandle(verified_cert, verified_chain); + scoped_refptr<X509Certificate> verified_cert_with_chain = + X509Certificate::CreateFromHandle(verified_cert, verified_chain); + if (verified_cert_with_chain) + verify_result->verified_cert = std::move(verified_cert_with_chain); + else + verify_result->cert_status |= CERT_STATUS_INVALID; } } @@ -662,7 +666,7 @@ class RevocationInjector { void SetCRLSet(CRLSet* crl_set) { thread_local_crlset.Set(crl_set); } private: - friend struct base::DefaultLazyInstanceTraits<RevocationInjector>; + friend struct base::LazyInstanceTraitsBase<RevocationInjector>; RevocationInjector() { const CRYPT_OID_FUNC_ENTRY kInterceptFunction[] = { diff --git a/chromium/net/cert/ct_known_logs_static-inc.h b/chromium/net/cert/ct_known_logs_static-inc.h index 08b42960037..c6b0864255a 100644 --- a/chromium/net/cert/ct_known_logs_static-inc.h +++ b/chromium/net/cert/ct_known_logs_static-inc.h @@ -64,26 +64,6 @@ const CTLogInfo kCTLogList[] = { "\x99\x08\x3d\x21\x14\x86", 91, "Symantec log", "https://ct.ws.symantec.com/", "symantec.ct.googleapis.com"}, - {"\x30\x82\x01\x22\x30\x0d\x06\x09\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01" - "\x05\x00\x03\x82\x01\x0f\x00\x30\x82\x01\x0a\x02\x82\x01\x01\x00\xa2" - "\x5a\x48\x1f\x17\x52\x95\x35\xcb\xa3\x5b\x3a\x1f\x53\x82\x76\x94\xa3" - "\xff\x80\xf2\x1c\x37\x3c\xc0\xb1\xbd\xc1\x59\x8b\xab\x2d\x65\x93\xd7" - "\xf3\xe0\x04\xd5\x9a\x6f\xbf\xd6\x23\x76\x36\x4f\x23\x99\xcb\x54\x28" - "\xad\x8c\x15\x4b\x65\x59\x76\x41\x4a\x9c\xa6\xf7\xb3\x3b\x7e\xb1\xa5" - "\x49\xa4\x17\x51\x6c\x80\xdc\x2a\x90\x50\x4b\x88\x24\xe9\xa5\x12\x32" - "\x93\x04\x48\x90\x02\xfa\x5f\x0e\x30\x87\x8e\x55\x76\x05\xee\x2a\x4c" - "\xce\xa3\x6a\x69\x09\x6e\x25\xad\x82\x76\x0f\x84\x92\xfa\x38\xd6\x86" - "\x4e\x24\x8f\x9b\xb0\x72\xcb\x9e\xe2\x6b\x3f\xe1\x6d\xc9\x25\x75\x23" - "\x88\xa1\x18\x58\x06\x23\x33\x78\xda\x00\xd0\x38\x91\x67\xd2\xa6\x7d" - "\x27\x97\x67\x5a\xc1\xf3\x2f\x17\xe6\xea\xd2\x5b\xe8\x81\xcd\xfd\x92" - "\x68\xe7\xf3\x06\xf0\xe9\x72\x84\xee\x01\xa5\xb1\xd8\x33\xda\xce\x83" - "\xa5\xdb\xc7\xcf\xd6\x16\x7e\x90\x75\x18\xbf\x16\xdc\x32\x3b\x6d\x8d" - "\xab\x82\x17\x1f\x89\x20\x8d\x1d\x9a\xe6\x4d\x23\x08\xdf\x78\x6f\xc6" - "\x05\xbf\x5f\xae\x94\x97\xdb\x5f\x64\xd4\xee\x16\x8b\xa3\x84\x6c\x71" - "\x2b\xf1\xab\x7f\x5d\x0d\x32\xee\x04\xe2\x90\xec\x41\x9f\xfb\x39\xc1" - "\x02\x03\x01\x00\x01", - 294, "Venafi log", "https://ctlog.api.venafi.com/", - "venafi.ct.googleapis.com"}, {"\x30\x59\x30\x13\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x08\x2a\x86" "\x48\xce\x3d\x03\x01\x07\x03\x42\x00\x04\xea\x95\x9e\x02\xff\xee\xf1" "\x33\x6d\x4b\x87\xbc\xcd\xfd\x19\x17\x62\xff\x94\xd3\xd0\x59\x07\x3f" @@ -163,7 +143,15 @@ const CTLogInfo kCTLogList[] = { "\x78\x35\x2d\x4a\xe7\x40\x99\x11\x95\x34\xd4\x2f\x7f\xf9\x5f\x35\x37" "\x02\x03\x01\x00\x01", 294, "PuChuangSiDa CT Log 1", "https://www.certificatetransparency.cn/ct/", - "puchuangsida1.ct.googleapis.com"}}; + "puchuangsida1.ct.googleapis.com"}, + {"\x30\x59\x30\x13\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x08\x2a\x86" + "\x48\xce\x3d\x03\x01\x07\x03\x42\x00\x04\x8e\x27\x27\x7a\xb6\x55\x09" + "\x74\xeb\x6c\x4b\x94\x84\x65\xbc\xe4\x15\xf1\xea\x5a\xd8\x7c\x0e\x37" + "\xce\xba\x3f\x6c\x09\xda\xe7\x29\x96\xd3\x45\x50\x6f\xde\x1e\xb4\x1c" + "\xd2\x83\x88\xff\x29\x2f\xce\xa9\xff\xdf\x34\xde\x75\x0f\xc0\xcc\x18" + "\x0d\x94\x2e\xfc\x37\x01", + 91, "Venafi Gen2 CT log", "https://ctlog-gen2.api.venafi.com/", + "venafi2.ct.googleapis.com"}}; // Information related to previously-qualified, but now disqualified, CT // logs. @@ -196,6 +184,32 @@ const DisqualifiedCTLogInfo kDisqualifiedCTLogList[] = { base::TimeDelta::FromSeconds(1464566400), }, { + "\xac\x3b\x9a\xed\x7f\xa9\x67\x47\x57\x15\x9e\x6d\x7d\x57\x56\x72\xf9" + "\xd9\x81\x00\x94\x1e\x9b\xde\xff\xec\xa1\x31\x3b\x75\x78\x2d", + {"\x30\x82\x01\x22\x30\x0d\x06\x09\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01" + "\x05\x00\x03\x82\x01\x0f\x00\x30\x82\x01\x0a\x02\x82\x01\x01\x00\xa2" + "\x5a\x48\x1f\x17\x52\x95\x35\xcb\xa3\x5b\x3a\x1f\x53\x82\x76\x94\xa3" + "\xff\x80\xf2\x1c\x37\x3c\xc0\xb1\xbd\xc1\x59\x8b\xab\x2d\x65\x93\xd7" + "\xf3\xe0\x04\xd5\x9a\x6f\xbf\xd6\x23\x76\x36\x4f\x23\x99\xcb\x54\x28" + "\xad\x8c\x15\x4b\x65\x59\x76\x41\x4a\x9c\xa6\xf7\xb3\x3b\x7e\xb1\xa5" + "\x49\xa4\x17\x51\x6c\x80\xdc\x2a\x90\x50\x4b\x88\x24\xe9\xa5\x12\x32" + "\x93\x04\x48\x90\x02\xfa\x5f\x0e\x30\x87\x8e\x55\x76\x05\xee\x2a\x4c" + "\xce\xa3\x6a\x69\x09\x6e\x25\xad\x82\x76\x0f\x84\x92\xfa\x38\xd6\x86" + "\x4e\x24\x8f\x9b\xb0\x72\xcb\x9e\xe2\x6b\x3f\xe1\x6d\xc9\x25\x75\x23" + "\x88\xa1\x18\x58\x06\x23\x33\x78\xda\x00\xd0\x38\x91\x67\xd2\xa6\x7d" + "\x27\x97\x67\x5a\xc1\xf3\x2f\x17\xe6\xea\xd2\x5b\xe8\x81\xcd\xfd\x92" + "\x68\xe7\xf3\x06\xf0\xe9\x72\x84\xee\x01\xa5\xb1\xd8\x33\xda\xce\x83" + "\xa5\xdb\xc7\xcf\xd6\x16\x7e\x90\x75\x18\xbf\x16\xdc\x32\x3b\x6d\x8d" + "\xab\x82\x17\x1f\x89\x20\x8d\x1d\x9a\xe6\x4d\x23\x08\xdf\x78\x6f\xc6" + "\x05\xbf\x5f\xae\x94\x97\xdb\x5f\x64\xd4\xee\x16\x8b\xa3\x84\x6c\x71" + "\x2b\xf1\xab\x7f\x5d\x0d\x32\xee\x04\xe2\x90\xec\x41\x9f\xfb\x39\xc1" + "\x02\x03\x01\x00\x01", + 294, "Venafi log", "https://ctlog.api.venafi.com/", + "venafi.ct.googleapis.com"}, + // 2017-02-28 18:42:26 UTC + base::TimeDelta::FromSeconds(1488307346), + }, + { "\xcd\xb5\x17\x9b\x7f\xc1\xc0\x46\xfe\xea\x31\x13\x6a\x3f\x8f\x00\x2e" "\x61\x82\xfa\xf8\x89\x6f\xec\xc8\xb2\xf5\xb5\xab\x60\x49\x00", {"\x30\x59\x30\x13\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x08\x2a\x86" diff --git a/chromium/net/cert/ct_objects_extractor.cc b/chromium/net/cert/ct_objects_extractor.cc index 1a6e6e4772d..852bf827015 100644 --- a/chromium/net/cert/ct_objects_extractor.cc +++ b/chromium/net/cert/ct_objects_extractor.cc @@ -41,7 +41,9 @@ bool StringEqualToCBS(const std::string& value1, const CBS* value2) { bssl::UniquePtr<X509> OSCertHandleToOpenSSL( X509Certificate::OSCertHandle os_handle) { -#if defined(USE_OPENSSL_CERTS) +#if BUILDFLAG(USE_BYTE_CERTS) + return bssl::UniquePtr<X509>(X509_parse_from_buffer(os_handle)); +#elif defined(USE_OPENSSL_CERTS) return bssl::UniquePtr<X509>(X509Certificate::DupOSCertHandle(os_handle)); #else std::string der_encoded; diff --git a/chromium/net/cert/ct_objects_extractor_unittest.cc b/chromium/net/cert/ct_objects_extractor_unittest.cc index 8f8896938cd..107364511de 100644 --- a/chromium/net/cert/ct_objects_extractor_unittest.cc +++ b/chromium/net/cert/ct_objects_extractor_unittest.cc @@ -30,6 +30,7 @@ class CTObjectsExtractorTest : public ::testing::Test { std::string der_test_cert(ct::GetDerEncodedX509Cert()); test_cert_ = X509Certificate::CreateFromBytes(der_test_cert.data(), der_test_cert.length()); + ASSERT_TRUE(test_cert_); log_ = CTLogVerifier::Create(ct::GetTestPublicKey(), "testlog", "https://ct.example.com", "dns.example.com"); @@ -129,10 +130,12 @@ TEST_F(CTObjectsExtractorTest, ExtractSCTListFromOCSPResponse) { scoped_refptr<X509Certificate> subject_cert = X509Certificate::CreateFromBytes(der_subject_cert.data(), der_subject_cert.length()); + ASSERT_TRUE(subject_cert); std::string der_issuer_cert(ct::GetDerEncodedFakeOCSPResponseIssuerCert()); scoped_refptr<X509Certificate> issuer_cert = X509Certificate::CreateFromBytes(der_issuer_cert.data(), der_issuer_cert.length()); + ASSERT_TRUE(issuer_cert); std::string fake_sct_list = ct::GetFakeOCSPExtensionValue(); ASSERT_FALSE(fake_sct_list.empty()); @@ -151,6 +154,7 @@ TEST_F(CTObjectsExtractorTest, ExtractSCTListFromOCSPResponseMatchesSerial) { scoped_refptr<X509Certificate> issuer_cert = X509Certificate::CreateFromBytes(der_issuer_cert.data(), der_issuer_cert.length()); + ASSERT_TRUE(issuer_cert); std::string ocsp_response = ct::GetDerEncodedFakeOCSPResponse(); @@ -166,6 +170,7 @@ TEST_F(CTObjectsExtractorTest, ExtractSCTListFromOCSPResponseMatchesIssuer) { scoped_refptr<X509Certificate> subject_cert = X509Certificate::CreateFromBytes(der_subject_cert.data(), der_subject_cert.length()); + ASSERT_TRUE(subject_cert); std::string ocsp_response = ct::GetDerEncodedFakeOCSPResponse(); diff --git a/chromium/net/cert/ev_root_ca_metadata.h b/chromium/net/cert/ev_root_ca_metadata.h index 413f85dd92f..4fb5fc2f1a6 100644 --- a/chromium/net/cert/ev_root_ca_metadata.h +++ b/chromium/net/cert/ev_root_ca_metadata.h @@ -22,7 +22,7 @@ namespace base { template <typename T> -struct DefaultLazyInstanceTraits; +struct LazyInstanceTraitsBase; } // namespace base namespace net { @@ -71,7 +71,7 @@ class NET_EXPORT_PRIVATE EVRootCAMetadata { bool RemoveEVCA(const SHA1HashValue& fingerprint); private: - friend struct base::DefaultLazyInstanceTraits<EVRootCAMetadata>; + friend struct base::LazyInstanceTraitsBase<EVRootCAMetadata>; EVRootCAMetadata(); ~EVRootCAMetadata(); diff --git a/chromium/net/cert/internal/cert_error_scoper.cc b/chromium/net/cert/internal/cert_error_scoper.cc deleted file mode 100644 index c970f9acde6..00000000000 --- a/chromium/net/cert/internal/cert_error_scoper.cc +++ /dev/null @@ -1,54 +0,0 @@ -// 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. - -#include "net/cert/internal/cert_error_scoper.h" - -#include <memory> - -#include "base/logging.h" -#include "base/memory/ptr_util.h" -#include "net/cert/internal/cert_error_params.h" -#include "net/cert/internal/cert_errors.h" - -namespace net { - -CertErrorScoper::CertErrorScoper(CertErrors* parent_errors) { - DCHECK(parent_errors); - parent_errors_ = parent_errors; - parent_scoper_ = parent_errors->SetScoper(this); -} - -CertErrorScoper::~CertErrorScoper() { - CertErrorScoper* prev = parent_errors_->SetScoper(parent_scoper_); - DCHECK_EQ(prev, this); -} - -CertErrorNode* CertErrorScoper::LazyGetRootNode() { - if (!root_node_) { - // Create the node. - auto root_node = BuildRootNode(); - root_node_ = root_node.get(); - - // Attach it to the node hiearchy (ownership of this node is passed off - // to its parent, which is ultimately rooted in the CertErrors object). - if (parent_scoper_) { - parent_scoper_->LazyGetRootNode()->AddChild(std::move(root_node)); - } else { - parent_errors_->nodes_.push_back(std::move(root_node)); - } - } - - return root_node_; -} - -CertErrorScoperNoParams::CertErrorScoperNoParams(CertErrors* parent_errors, - CertErrorId id) - : CertErrorScoper(parent_errors), id_(id) {} - -std::unique_ptr<CertErrorNode> CertErrorScoperNoParams::BuildRootNode() { - return base::MakeUnique<CertErrorNode>(CertErrorNodeType::TYPE_CONTEXT, id_, - nullptr); -} - -} // namespace net diff --git a/chromium/net/cert/internal/cert_error_scoper.h b/chromium/net/cert/internal/cert_error_scoper.h deleted file mode 100644 index 141581270df..00000000000 --- a/chromium/net/cert/internal/cert_error_scoper.h +++ /dev/null @@ -1,59 +0,0 @@ -// 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_CERT_ERROR_SCOPER_H_ -#define NET_CERT_INTERNAL_CERT_ERROR_SCOPER_H_ - -#include <memory> - -#include "base/compiler_specific.h" -#include "base/macros.h" -#include "net/base/net_export.h" -#include "net/cert/internal/cert_error_id.h" - -namespace net { - -class CertErrors; -struct CertErrorNode; - -// CertErrorScoper is a base class for adding parent nodes into a CertErrors -// object. -class NET_EXPORT CertErrorScoper { - public: - explicit CertErrorScoper(CertErrors* parent_errors); - virtual ~CertErrorScoper(); - - // BuildRootNode() will be called at most once, to create the desired parent - // node. It may never be called if no errors are added to the CertErrors - // parent. - virtual std::unique_ptr<CertErrorNode> BuildRootNode() = 0; - - // Returns the parent node for this scoper (the one created by - // BuildRootNode()). - CertErrorNode* LazyGetRootNode(); - - private: - CertErrorScoper* parent_scoper_ = nullptr; - CertErrors* parent_errors_ = nullptr; - CertErrorNode* root_node_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(CertErrorScoper); -}; - -// Implementation of CertErrorScoper that creates a simple parent node with no -// parameters (just an ID). -class NET_EXPORT CertErrorScoperNoParams : public CertErrorScoper { - public: - CertErrorScoperNoParams(CertErrors* parent_errors, CertErrorId id); - std::unique_ptr<CertErrorNode> BuildRootNode() override; - - private: - CertErrorId id_; - - DISALLOW_COPY_AND_ASSIGN(CertErrorScoperNoParams); -}; - -} // namespace net - -#endif // NET_CERT_INTERNAL_CERT_ERROR_SCOPER_H_ diff --git a/chromium/net/cert/internal/cert_errors.cc b/chromium/net/cert/internal/cert_errors.cc index e66ab827fcf..be4814dc9c6 100644 --- a/chromium/net/cert/internal/cert_errors.cc +++ b/chromium/net/cert/internal/cert_errors.cc @@ -7,25 +7,15 @@ #include "base/logging.h" #include "base/memory/ptr_util.h" #include "base/strings/string_split.h" +#include "base/strings/stringprintf.h" #include "net/cert/internal/cert_error_params.h" -#include "net/cert/internal/cert_error_scoper.h" +#include "net/cert/internal/parse_name.h" +#include "net/cert/internal/parsed_certificate.h" namespace net { namespace { -// Helpers for pretty-printing CertErrors to a string. -void AppendNodeToDebugString(CertErrorNode* node, - const std::string& indentation, - std::string* out); - -void AppendChildrenToDebugString(const CertErrorNodes& children, - const std::string& indentation, - std::string* out) { - for (const auto& child : children) - AppendNodeToDebugString(child.get(), indentation, out); -} - void AppendLinesWithIndentation(const std::string& text, const std::string& indentation, std::string* out) { @@ -39,66 +29,54 @@ void AppendLinesWithIndentation(const std::string& text, } } -const char* CertErrorNodeTypeToString(CertErrorNodeType type) { - switch (type) { - case CertErrorNodeType::TYPE_CONTEXT: - return "[Context] "; - case CertErrorNodeType::TYPE_WARNING: - return "[Warning] "; - case CertErrorNodeType::TYPE_ERROR: - return "[Error] "; - } - return nullptr; -} - -void AppendNodeToDebugString(CertErrorNode* node, - const std::string& indentation, - std::string* out) { - std::string cur_indentation = indentation; +} // namespace - *out += cur_indentation; - *out += CertErrorNodeTypeToString(node->node_type); - *out += CertErrorIdToDebugString(node->id); - *out += +"\n"; +CertError::CertError() = default; - if (node->params) { - cur_indentation += " "; - AppendLinesWithIndentation(node->params->ToDebugString(), cur_indentation, - out); - } +CertError::CertError(Severity severity, + CertErrorId id, + std::unique_ptr<CertErrorParams> params) + : severity(severity), id(id), params(std::move(params)) {} - cur_indentation += " "; +CertError::CertError(CertError&& other) = default; - AppendChildrenToDebugString(node->children, cur_indentation, out); -} +CertError& CertError::operator=(CertError&&) = default; -} // namespace +CertError::~CertError() = default; -CertErrorNode::CertErrorNode(CertErrorNodeType node_type, - CertErrorId id, - std::unique_ptr<CertErrorParams> params) - : node_type(node_type), id(id), params(std::move(params)) {} +std::string CertError::ToDebugString() const { + std::string result; + switch (severity) { + case SEVERITY_WARNING: + result += "WARNING: "; + break; + case SEVERITY_HIGH: + result += "ERROR: "; + break; + } + result += CertErrorIdToDebugString(id); + result += +"\n"; -CertErrorNode::~CertErrorNode() = default; + if (params) + AppendLinesWithIndentation(params->ToDebugString(), " ", &result); -void CertErrorNode::AddChild(std::unique_ptr<CertErrorNode> child) { - DCHECK_EQ(CertErrorNodeType::TYPE_CONTEXT, node_type); - children.push_back(std::move(child)); + return result; } CertErrors::CertErrors() = default; - +CertErrors::CertErrors(CertErrors&& other) = default; +CertErrors& CertErrors::operator=(CertErrors&&) = default; CertErrors::~CertErrors() = default; -void CertErrors::Add(CertErrorNodeType node_type, +void CertErrors::Add(CertError::Severity severity, CertErrorId id, std::unique_ptr<CertErrorParams> params) { - AddNode(base::MakeUnique<CertErrorNode>(node_type, id, std::move(params))); + nodes_.push_back(CertError(severity, id, std::move(params))); } void CertErrors::AddError(CertErrorId id, std::unique_ptr<CertErrorParams> params) { - Add(CertErrorNodeType::TYPE_ERROR, id, std::move(params)); + Add(CertError::SEVERITY_HIGH, id, std::move(params)); } void CertErrors::AddError(CertErrorId id) { @@ -107,34 +85,124 @@ void CertErrors::AddError(CertErrorId id) { void CertErrors::AddWarning(CertErrorId id, std::unique_ptr<CertErrorParams> params) { - Add(CertErrorNodeType::TYPE_WARNING, id, std::move(params)); + Add(CertError::SEVERITY_WARNING, id, std::move(params)); } void CertErrors::AddWarning(CertErrorId id) { AddWarning(id, nullptr); } -bool CertErrors::empty() const { - return nodes_.empty(); -} - std::string CertErrors::ToDebugString() const { std::string result; - AppendChildrenToDebugString(nodes_, std::string(), &result); + for (const CertError& node : nodes_) + result += node.ToDebugString(); + return result; } -void CertErrors::AddNode(std::unique_ptr<CertErrorNode> node) { - if (current_scoper_) - current_scoper_->LazyGetRootNode()->AddChild(std::move(node)); - else - nodes_.push_back(std::move(node)); +bool CertErrors::ContainsError(CertErrorId id) const { + for (const CertError& node : nodes_) { + if (node.id == id) + return true; + } + return false; +} + +bool CertErrors::ContainsAnyErrorWithSeverity( + CertError::Severity severity) const { + for (const CertError& node : nodes_) { + if (node.severity == severity) + return true; + } + return false; +} + +CertPathErrors::CertPathErrors() = default; + +CertPathErrors::CertPathErrors(CertPathErrors&& other) = default; +CertPathErrors& CertPathErrors::operator=(CertPathErrors&&) = default; + +CertPathErrors::~CertPathErrors() = default; + +CertErrors* CertPathErrors::GetErrorsForCert(size_t cert_index) { + if (cert_index >= cert_errors_.size()) + cert_errors_.resize(cert_index + 1); + return &cert_errors_[cert_index]; +} + +CertErrors* CertPathErrors::GetOtherErrors() { + return &other_errors_; } -CertErrorScoper* CertErrors::SetScoper(CertErrorScoper* scoper) { - CertErrorScoper* prev = current_scoper_; - current_scoper_ = scoper; - return prev; +bool CertPathErrors::ContainsError(CertErrorId id) const { + for (const CertErrors& errors : cert_errors_) { + if (errors.ContainsError(id)) + return true; + } + + if (other_errors_.ContainsError(id)) + return true; + + return false; +} + +bool CertPathErrors::ContainsAnyErrorWithSeverity( + CertError::Severity severity) const { + for (const CertErrors& errors : cert_errors_) { + if (errors.ContainsAnyErrorWithSeverity(severity)) + return true; + } + + if (other_errors_.ContainsAnyErrorWithSeverity(severity)) + return true; + + return false; +} + +std::string CertPathErrors::ToDebugString( + const ParsedCertificateList& certs) const { + std::string result; + + for (size_t i = 0; i < cert_errors_.size(); ++i) { + // Pretty print the current CertErrors. If there were no errors/warnings, + // then continue. + const CertErrors& errors = cert_errors_[i]; + std::string cert_errors_string = errors.ToDebugString(); + if (cert_errors_string.empty()) + continue; + + // Add a header for the CertErrors that describes which certificate they + // apply to. + // + // TODO(eroman): Show the subject for trust anchor (which currently uses the + // bucket cert_errors_[certs.size()]). + std::string cert_name_debug_str; + if (i < certs.size() && certs[i]) { + RDNSequence subject; + if (ParseName(certs[i]->tbs().subject_tlv, &subject) && + ConvertToRFC2253(subject, &cert_name_debug_str)) { + cert_name_debug_str = " (" + cert_name_debug_str + ")"; + } + } + + result += + base::StringPrintf("----- Certificate i=%d%s -----\n", + static_cast<int>(i), cert_name_debug_str.c_str()); + + result += cert_errors_string; + result += "\n"; + } + + // Print any other errors that aren't associated with a particular certificate + // in the chain. + std::string other_errors = other_errors_.ToDebugString(); + if (!other_errors.empty()) { + result += "----- Other errors (not certificate specific) -----\n"; + result += other_errors; + result += "\n"; + } + + return result; } } // namespace net diff --git a/chromium/net/cert/internal/cert_errors.h b/chromium/net/cert/internal/cert_errors.h index cd713ed42ec..438417fd381 100644 --- a/chromium/net/cert/internal/cert_errors.h +++ b/chromium/net/cert/internal/cert_errors.h @@ -6,46 +6,29 @@ // Overview of error design // ---------------------------- // -// Certificate path validation/parsing may emit a sequence of -// errors/warnings/context. These are represented by a tree of CertErrorNodes. -// Each node is comprised of: +// Certificate path building/validation/parsing may emit a sequence of errors +// and warnings. +// +// Each individual error/warning entry (CertError) is comprised of: // // * A unique identifier. // -// This serves similarly to an error code, and is useful for querying if a -// particular error occurred. +// This serves similarly to an error code, and is used to query if a +// particular error/warning occurred. // // * [optional] A parameters object. // -// Nodes may attach a heap-allocated subclass of CertErrorParams, to carry -// extra information that is useful when reporting the error. For instance -// a parsing error may want to describe where in the DER the failure -// happened, or what the unexpected value was. -// -// * [optional] Child nodes. -// -// Error nodes are arranged in a tree. The parent/child hierarchy is used to -// group errors that share some common state. -// For instance during path processing it is useful to group the -// errors/warnings that happened while processing certificate "i" as -// children of a shared "context" node. The context node in this case -// doesn't describe a particular error, but rather some shared event and -// its parameters. -// -// ---------------------------- -// Using errors in other APIs -// ---------------------------- -// -// The top level object used in APIs is CertErrors. A pointer to a CertErrors -// object is typically given as an out-parameter for code that may generate -// errors. +// Nodes may attach a heap-allocated subclass of CertErrorParams to carry +// extra information that is used when reporting the error. For instance +// a parsing error may describe where in the DER the failure happened, or +// what the unexpected value was. // -// Note that CertErrors gives a non-hiearhical interface for emitting errors. -// In other words, it doesn't let you create parent/child relationships -// directly. +// A collection of errors is represented by the CertErrors object. This may be +// used to group errors that have a common context, such as all the +// errors/warnings that apply to a specific certificate. // -// To change the parent node for subsequently emitted errors in the CertErrors -// object, one constructs a CertErrorScoper on the stack. +// Lastly, CertPathErrors composes multiple CertErrors -- one for each +// certificate in the verified chain. // // ---------------------------- // Defining new errors @@ -70,92 +53,110 @@ #include "base/macros.h" #include "net/base/net_export.h" #include "net/cert/internal/cert_error_id.h" +#include "net/cert/internal/parsed_certificate.h" namespace net { class CertErrorParams; -class CertErrorScoper; - -// The type of a particular CertErrorNode. -enum class CertErrorNodeType { - // Note the TYPE_ prefix is to avoid compile errors. Because ERROR() is a - // commonly used macro name. - - // Node that represents a single error. - TYPE_ERROR, - - // Node that represents a single non-fatal error. - TYPE_WARNING, - - // Parent node for other errors/warnings. - TYPE_CONTEXT, -}; -struct CertErrorNode; -using CertErrorNodes = std::vector<std::unique_ptr<CertErrorNode>>; - -// CertErrorNode represents a node in the error tree. This could be an error, -// warning, or simply contextual parent node. See the error design overview for -// a better description of how this is used. -struct NET_EXPORT CertErrorNode { - CertErrorNode(CertErrorNodeType node_type, - CertErrorId id, - std::unique_ptr<CertErrorParams> params); - ~CertErrorNode(); - - void AddChild(std::unique_ptr<CertErrorNode> child); +// CertError represents either an error or a warning. +struct NET_EXPORT CertError { + enum Severity { + SEVERITY_HIGH, + SEVERITY_WARNING, + }; + + CertError(); + CertError(Severity severity, + CertErrorId id, + std::unique_ptr<CertErrorParams> params); + CertError(CertError&& other); + CertError& operator=(CertError&&); + ~CertError(); + + // Pretty-prints the error and its parameters. + std::string ToDebugString() const; - CertErrorNodeType node_type; + Severity severity; CertErrorId id; std::unique_ptr<CertErrorParams> params; - CertErrorNodes children; }; -// CertErrors is the main object for emitting errors and internally builds up -// the error tree. +// CertErrors is a collection of CertError, along with convenience methods to +// add and inspect errors. class NET_EXPORT CertErrors { public: CertErrors(); + CertErrors(CertErrors&& other); + CertErrors& operator=(CertErrors&&); ~CertErrors(); - // Adds a node to the current insertion point in the error tree. |params| may - // be null. - void Add(CertErrorNodeType node_type, + // Adds an error/warning. |params| may be null. + void Add(CertError::Severity severity, CertErrorId id, std::unique_ptr<CertErrorParams> params); + // Adds a high severity error. void AddError(CertErrorId id, std::unique_ptr<CertErrorParams> params); void AddError(CertErrorId id); + // Adds a low severity error. void AddWarning(CertErrorId id, std::unique_ptr<CertErrorParams> params); void AddWarning(CertErrorId id); - // Returns true if the tree is empty. Note that emptiness of the error tree - // is NOT equivalent to success for some call, and vice versa. (For instance - // consumers may forget to emit errors on failures, or some errors may be - // non-fatal warnings). - bool empty() const; - // Dumps a textual representation of the errors for debugging purposes. std::string ToDebugString() const; + // Returns true if the error |id| was added to this CertErrors (of any + // severity). + bool ContainsError(CertErrorId id) const; + + // Returns true if this contains any errors of the given severity level. + bool ContainsAnyErrorWithSeverity(CertError::Severity severity) const; + private: - // CertErrorScoper manipulates the CertErrors object. - friend class CertErrorScoper; + std::vector<CertError> nodes_; +}; + +// CertPathErrors is a collection of CertErrors, to group errors into different +// buckets for different certificates. The "index" should correspond with that +// of the certificate relative to its chain. +class NET_EXPORT CertPathErrors { + public: + CertPathErrors(); + CertPathErrors(CertPathErrors&& other); + CertPathErrors& operator=(CertPathErrors&&); + ~CertPathErrors(); - void AddNode(std::unique_ptr<CertErrorNode> node); + // Gets a bucket to put errors in for |cert_index|. This will lookup and + // return the existing error bucket if one exists, or create a new one for the + // specified index. It is expected that |cert_index| is the corresponding + // index in a certificate chain (with 0 being the target). + CertErrors* GetErrorsForCert(size_t cert_index); - // Used by CertErrorScoper to register itself as the top-level scoper. - // Returns the previously set scoper, or nullptr if there was none. - CertErrorScoper* SetScoper(CertErrorScoper* scoper); + // Returns a bucket to put errors that are not associated with a particular + // certificate. + CertErrors* GetOtherErrors(); - CertErrorNodes nodes_; + // Returns true if CertPathErrors contains the specified error (of any + // severity). + bool ContainsError(CertErrorId id) const; - // The top-most CertErrorScoper that is currently in scope (and which affects - // the parent node for newly added errors). - CertErrorScoper* current_scoper_ = nullptr; + // Returns true if this contains any errors of the given severity level. + bool ContainsAnyErrorWithSeverity(CertError::Severity severity) const; - DISALLOW_COPY_AND_ASSIGN(CertErrors); + // Shortcut for ContainsAnyErrorWithSeverity(CertError::SEVERITY_HIGH). + bool ContainsHighSeverityErrors() const { + return ContainsAnyErrorWithSeverity(CertError::SEVERITY_HIGH); + } + + // Pretty-prints all the errors in the CertPathErrors. If there were no + // errors/warnings, returns an empty string. + std::string ToDebugString(const ParsedCertificateList& certs) const; + + private: + std::vector<CertErrors> cert_errors_; + CertErrors other_errors_; }; } // namespace net diff --git a/chromium/net/cert/internal/parse_certificate_unittest.cc b/chromium/net/cert/internal/parse_certificate_unittest.cc index a54d895acba..6ddc768f68f 100644 --- a/chromium/net/cert/internal/parse_certificate_unittest.cc +++ b/chromium/net/cert/internal/parse_certificate_unittest.cc @@ -214,27 +214,6 @@ TEST(ParseTbsCertificateTest, Version3WithExtensions) { RunTbsCertificateTest("tbs_v3_extensions.pem"); } -// Tests parsing a TBSCertificate for v3 that contains no optional fields, and -// has a negative serial number. -// -// CAs are not supposed to include negative serial numbers, however RFC 5280 -// expects consumers to deal with it anyway). -TEST(ParseTbsCertificateTest, NegativeSerialNumber) { - RunTbsCertificateTest("tbs_negative_serial_number.pem"); -} - -// Tests parsing a TBSCertificate with a serial number that is 21 octets long -// (and the first byte is 0). -TEST(ParseTbCertificateTest, SerialNumber21OctetsLeading0) { - RunTbsCertificateTest("tbs_serial_number_21_octets_leading_0.pem"); -} - -// Tests parsing a TBSCertificate with a serial number that is 26 octets long -// (and does not contain a leading 0). -TEST(ParseTbsCertificateTest, SerialNumber26Octets) { - RunTbsCertificateTest("tbs_serial_number_26_octets.pem"); -} - // Tests parsing a TBSCertificate which lacks a version number (causing it to // default to v1). TEST(ParseTbsCertificateTest, Version1) { diff --git a/chromium/net/cert/internal/parse_name.cc b/chromium/net/cert/internal/parse_name.cc index bb9b64051ac..f3aa518f1c3 100644 --- a/chromium/net/cert/internal/parse_name.cc +++ b/chromium/net/cert/internal/parse_name.cc @@ -12,6 +12,10 @@ #include "base/sys_byteorder.h" #include "base/third_party/icu/icu_utf.h" +#if !defined(OS_NACL) +#include "net/base/net_string_util.h" +#endif + namespace net { namespace { @@ -134,6 +138,12 @@ der::Input TypeStateOrProvinceNameOid() { return der::Input(oid); } +der::Input TypeStreetAddressOid() { + // street (streetAddress): 2.5.4.9 (RFC 4519) + static const uint8_t oid[] = {0x55, 0x04, 0x09}; + return der::Input(oid); +} + der::Input TypeOrganizationNameOid() { // id-at-organizationName: 2.5.4.10 (RFC 5280) static const uint8_t oid[] = {0x55, 0x04, 0x0a}; @@ -176,6 +186,52 @@ der::Input TypeGenerationQualifierOid() { return der::Input(oid); } +der::Input TypeDomainComponentOid() { + // dc (domainComponent): 0.9.2342.19200300.100.1.25 (RFC 4519) + static const uint8_t oid[] = {0x09, 0x92, 0x26, 0x89, 0x93, + 0xF2, 0x2C, 0x64, 0x01, 0x19}; + return der::Input(oid); +} + +bool X509NameAttribute::ValueAsString(std::string* out) const { + switch (value_tag) { + case der::kTeletexString: +#if !defined(OS_NACL) + return ConvertToUtf8(value.AsString(), kCharsetLatin1, out); +#else +// For nacl, just fall through to treating like IA5String (ascii). +// (The nacl build does not include net_string_util and its deps, and a test of +// adding them increased nacl build size by 100KB.) +// TODO(mattm): Remove this behavioral difference. +#endif + case der::kIA5String: + for (char c : value.AsStringPiece()) { + if (static_cast<uint8_t>(c) > 127) + return false; + } + *out = value.AsString(); + return true; + case der::kPrintableString: + for (char c : value.AsStringPiece()) { + if (!(base::IsAsciiAlpha(c) || c == ' ' || (c >= '\'' && c <= ':') || + c == '=' || c == '?')) { + return false; + } + } + *out = value.AsString(); + return true; + case der::kUtf8String: + *out = value.AsString(); + return true; + case der::kUniversalString: + return ConvertUniversalStringValue(value, out); + case der::kBmpString: + return ConvertBmpStringValue(value, out); + default: + return false; + } +} + bool X509NameAttribute::ValueAsStringUnsafe(std::string* out) const { switch (value_tag) { case der::kIA5String: @@ -197,6 +253,7 @@ bool X509NameAttribute::ValueAsStringUnsafe(std::string* out) const { bool X509NameAttribute::AsRFC2253String(std::string* out) const { std::string type_string; std::string value_string; + // TODO(mattm): Add streetAddress and domainComponent here? if (type == TypeCommonNameOid()) { type_string = "CN"; } else if (type == TypeSurnameOid()) { diff --git a/chromium/net/cert/internal/parse_name.h b/chromium/net/cert/internal/parse_name.h index ce09bc82d4d..3a188c6d50a 100644 --- a/chromium/net/cert/internal/parse_name.h +++ b/chromium/net/cert/internal/parse_name.h @@ -20,6 +20,7 @@ NET_EXPORT der::Input TypeSerialNumberOid(); NET_EXPORT der::Input TypeCountryNameOid(); NET_EXPORT der::Input TypeLocalityNameOid(); NET_EXPORT der::Input TypeStateOrProvinceNameOid(); +NET_EXPORT der::Input TypeStreetAddressOid(); NET_EXPORT der::Input TypeOrganizationNameOid(); NET_EXPORT der::Input TypeOrganizationUnitNameOid(); NET_EXPORT der::Input TypeTitleOid(); @@ -27,6 +28,7 @@ NET_EXPORT der::Input TypeNameOid(); NET_EXPORT der::Input TypeGivenNameOid(); NET_EXPORT der::Input TypeInitialsOid(); NET_EXPORT der::Input TypeGenerationQualifierOid(); +NET_EXPORT der::Input TypeDomainComponentOid(); // X509NameAttribute contains a representation of a DER-encoded RFC 2253 // "AttributeTypeAndValue". @@ -42,9 +44,17 @@ struct NET_EXPORT X509NameAttribute { : type(in_type), value_tag(in_value_tag), value(in_value) {} // Attempts to convert the value represented by this struct into a + // UTF-8 string and store it in |out|, returning whether the conversion + // was successful. + bool ValueAsString(std::string* out) const WARN_UNUSED_RESULT; + + // Attempts to convert the value represented by this struct into a // std::string and store it in |out|, returning whether the conversion was // successful. Due to some encodings being incompatible, the caller must - // verify the attribute |type|. + // verify the attribute |value_tag|. + // + // Note: Don't use this function unless you know what you're doing. Use + // ValueAsString instead. // // Note: The conversion doesn't verify that the value corresponds to the // ASN.1 definition of the value type. diff --git a/chromium/net/cert/internal/parse_name_unittest.cc b/chromium/net/cert/internal/parse_name_unittest.cc index b1d768fda61..dab04212f6e 100644 --- a/chromium/net/cert/internal/parse_name_unittest.cc +++ b/chromium/net/cert/internal/parse_name_unittest.cc @@ -31,13 +31,92 @@ namespace { } } +TEST(ParseNameTest, IA5SafeStringValue) { + const uint8_t der[] = { + 0x46, 0x6f, 0x6f, 0x20, 0x62, 0x61, 0x72, + }; + X509NameAttribute value(der::Input(), der::kIA5String, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("Foo bar", result_unsafe); + std::string result; + ASSERT_TRUE(value.ValueAsString(&result)); + ASSERT_EQ("Foo bar", result); +} + +TEST(ParseNameTest, IA5UnsafeStringValue) { + const uint8_t der[] = { + 0x46, 0x6f, 0xFF, 0x20, 0x62, 0x61, 0x72, + }; + X509NameAttribute value(der::Input(), der::kIA5String, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("Fo\377 bar", result_unsafe); + std::string result; + ASSERT_FALSE(value.ValueAsString(&result)); +} + +TEST(ParseNameTest, PrintableSafeStringValue) { + const uint8_t der[] = { + 0x46, 0x6f, 0x6f, 0x20, 0x62, 0x61, 0x72, + }; + X509NameAttribute value(der::Input(), der::kPrintableString, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("Foo bar", result_unsafe); + std::string result; + ASSERT_TRUE(value.ValueAsString(&result)); + ASSERT_EQ("Foo bar", result); +} + +TEST(ParseNameTest, PrintableUnsafeStringValue) { + const uint8_t der[] = { + 0x46, 0x6f, 0x5f, 0x20, 0x62, 0x61, 0x72, + }; + X509NameAttribute value(der::Input(), der::kPrintableString, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("Fo_ bar", result_unsafe); + std::string result; + ASSERT_FALSE(value.ValueAsString(&result)); +} + +TEST(ParseNameTest, TeletexSafeStringValue) { + const uint8_t der[] = { + 0x46, 0x6f, 0x6f, 0x20, 0x62, 0x61, 0x72, + }; + X509NameAttribute value(der::Input(), der::kTeletexString, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("Foo bar", result_unsafe); + std::string result; + ASSERT_TRUE(value.ValueAsString(&result)); + ASSERT_EQ("Foo bar", result); +} + +TEST(ParseNameTest, TeletexLatin1StringValue) { + const uint8_t der[] = { + 0x46, 0x6f, 0xd6, 0x20, 0x62, 0x61, 0x72, + }; + X509NameAttribute value(der::Input(), der::kTeletexString, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("Fo\xd6 bar", result_unsafe); + std::string result; + ASSERT_TRUE(value.ValueAsString(&result)); + ASSERT_EQ("FoÖ bar", result); +} + TEST(ParseNameTest, ConvertBmpString) { const uint8_t der[] = { 0x00, 0x66, 0x00, 0x6f, 0x00, 0x6f, 0x00, 0x62, 0x00, 0x61, 0x00, 0x72, }; X509NameAttribute value(der::Input(), der::kBmpString, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("foobar", result_unsafe); std::string result; - ASSERT_TRUE(value.ValueAsStringUnsafe(&result)); + ASSERT_TRUE(value.ValueAsString(&result)); ASSERT_EQ("foobar", result); } @@ -47,6 +126,7 @@ TEST(ParseNameTest, ConvertInvalidBmpString) { X509NameAttribute value(der::Input(), der::kBmpString, der::Input(der)); std::string result; ASSERT_FALSE(value.ValueAsStringUnsafe(&result)); + ASSERT_FALSE(value.ValueAsString(&result)); } TEST(ParseNameTest, ConvertUniversalString) { @@ -54,8 +134,12 @@ TEST(ParseNameTest, ConvertUniversalString) { 0x00, 0x00, 0x00, 0x6f, 0x00, 0x00, 0x00, 0x62, 0x00, 0x00, 0x00, 0x61, 0x00, 0x00, 0x00, 0x72}; X509NameAttribute value(der::Input(), der::kUniversalString, der::Input(der)); + std::string result_unsafe; + ASSERT_TRUE(value.ValueAsStringUnsafe(&result_unsafe)); + ASSERT_EQ("foobar", result_unsafe); std::string result; - ASSERT_TRUE(value.ValueAsStringUnsafe(&result)); + ASSERT_TRUE(value.ValueAsString(&result)); + ASSERT_EQ("foobar", result); } // UniversalString must encode characters in pairs of 4 bytes. @@ -64,6 +148,7 @@ TEST(ParseNameTest, ConvertInvalidUniversalString) { X509NameAttribute value(der::Input(), der::kUniversalString, der::Input(der)); std::string result; ASSERT_FALSE(value.ValueAsStringUnsafe(&result)); + ASSERT_FALSE(value.ValueAsString(&result)); } TEST(ParseNameTest, EmptyName) { diff --git a/chromium/net/cert/internal/parsed_certificate.cc b/chromium/net/cert/internal/parsed_certificate.cc index 0c655079e3c..97a432aa6d2 100644 --- a/chromium/net/cert/internal/parsed_certificate.cc +++ b/chromium/net/cert/internal/parsed_certificate.cc @@ -4,6 +4,8 @@ #include "net/cert/internal/parsed_certificate.h" +#include "net/cert/internal/certificate_policies.h" +#include "net/cert/internal/extended_key_usage.h" #include "net/cert/internal/name_constraints.h" #include "net/cert/internal/signature_algorithm.h" #include "net/cert/internal/verify_name_match.h" @@ -22,6 +24,21 @@ WARN_UNUSED_RESULT bool GetSequenceValue(const der::Input& tlv, } // namespace +bool ParsedCertificate::GetExtension(const der::Input& extension_oid, + ParsedExtension* parsed_extension) const { + if (!tbs_.has_extensions) + return false; + + auto it = extensions_.find(extension_oid); + if (it == extensions_.end()) { + *parsed_extension = ParsedExtension(); + return false; + } + + *parsed_extension = it->second; + return true; +} + ParsedCertificate::ParsedCertificate() {} ParsedCertificate::~ParsedCertificate() {} @@ -102,37 +119,40 @@ scoped_refptr<ParsedCertificate> ParsedCertificate::CreateInternal( return nullptr; } - // Parse the standard X.509 extensions and remove them from - // |unparsed_extensions|. + // Parse the standard X.509 extensions. if (result->tbs_.has_extensions) { // ParseExtensions() ensures there are no duplicates, and maps the (unique) // OID to the extension value. - if (!ParseExtensions(result->tbs_.extensions_tlv, - &result->unparsed_extensions_)) { + if (!ParseExtensions(result->tbs_.extensions_tlv, &result->extensions_)) { return nullptr; } ParsedExtension extension; // Basic constraints. - if (ConsumeExtension(BasicConstraintsOid(), &result->unparsed_extensions_, - &extension)) { + if (result->GetExtension(BasicConstraintsOid(), &extension)) { result->has_basic_constraints_ = true; if (!ParseBasicConstraints(extension.value, &result->basic_constraints_)) return nullptr; } - // KeyUsage. - if (ConsumeExtension(KeyUsageOid(), &result->unparsed_extensions_, - &extension)) { + // Key Usage. + if (result->GetExtension(KeyUsageOid(), &extension)) { result->has_key_usage_ = true; if (!ParseKeyUsage(extension.value, &result->key_usage_)) return nullptr; } + // Extended Key Usage. + if (result->GetExtension(ExtKeyUsageOid(), &extension)) { + result->has_extended_key_usage_ = true; + if (!ParseEKUExtension(extension.value, &result->extended_key_usage_)) + return nullptr; + } + // Subject alternative name. - if (ConsumeExtension(SubjectAltNameOid(), &result->unparsed_extensions_, - &result->subject_alt_names_extension_)) { + if (result->GetExtension(SubjectAltNameOid(), + &result->subject_alt_names_extension_)) { // RFC 5280 section 4.2.1.6: // SubjectAltName ::= GeneralNames result->subject_alt_names_ = @@ -151,8 +171,7 @@ scoped_refptr<ParsedCertificate> ParsedCertificate::CreateInternal( } // Name constraints. - if (ConsumeExtension(NameConstraintsOid(), &result->unparsed_extensions_, - &extension)) { + if (result->GetExtension(NameConstraintsOid(), &extension)) { result->name_constraints_ = NameConstraints::Create(extension.value, extension.critical); if (!result->name_constraints_) @@ -160,9 +179,8 @@ scoped_refptr<ParsedCertificate> ParsedCertificate::CreateInternal( } // Authority information access. - if (ConsumeExtension(AuthorityInfoAccessOid(), - &result->unparsed_extensions_, - &result->authority_info_access_extension_)) { + if (result->GetExtension(AuthorityInfoAccessOid(), + &result->authority_info_access_extension_)) { result->has_authority_info_access_ = true; if (!ParseAuthorityInfoAccess( result->authority_info_access_extension_.value, @@ -170,10 +188,14 @@ scoped_refptr<ParsedCertificate> ParsedCertificate::CreateInternal( return nullptr; } - // NOTE: if additional extensions are consumed here, the verification code - // must be updated to process those extensions, since the - // VerifyNoUnconsumedCriticalExtensions uses the unparsed_extensions_ - // variable to tell which extensions were processed. + // Policies. + if (result->GetExtension(CertificatePoliciesOid(), &extension)) { + result->has_policy_oids_ = true; + if (!ParseCertificatePoliciesExtension(extension.value, + &result->policy_oids_)) { + return nullptr; + } + } } return result; diff --git a/chromium/net/cert/internal/parsed_certificate.h b/chromium/net/cert/internal/parsed_certificate.h index 06bd37890b9..9b561a03751 100644 --- a/chromium/net/cert/internal/parsed_certificate.h +++ b/chromium/net/cert/internal/parsed_certificate.h @@ -142,6 +142,16 @@ class NET_EXPORT ParsedCertificate return key_usage_; } + // Returns true if the certificate has a ExtendedKeyUsage extension. + bool has_extended_key_usage() const { return has_extended_key_usage_; } + + // Returns the ExtendedKeyUsage key purpose OIDs. Caller must check + // has_extended_key_usage() before accessing this. + const std::vector<der::Input>& extended_key_usage() const { + DCHECK(has_extended_key_usage_); + return extended_key_usage_; + } + // Returns true if the certificate has a SubjectAltName extension. bool has_subject_alt_names() const { return subject_alt_names_ != nullptr; } @@ -184,11 +194,24 @@ class NET_EXPORT ParsedCertificate // Returns any OCSP URIs from the AuthorityInfoAccess extension. const std::vector<base::StringPiece>& ocsp_uris() const { return ocsp_uris_; } - // Returns a map of unhandled extensions (excludes the ones above). - const ExtensionsMap& unparsed_extensions() const { - return unparsed_extensions_; + // Returns true if the certificate has a Policies extension. + bool has_policy_oids() const { return has_policy_oids_; } + + // Returns the policy OIDs. Caller must check has_policy_oids() before + // accessing this. + const std::vector<der::Input>& policy_oids() const { + DCHECK(has_policy_oids()); + return policy_oids_; } + // Returns a map of all the extensions in the certificate. + const ExtensionsMap& extensions() const { return extensions_; } + + // Gets the value for extension matching |extension_oid|. Returns false if the + // extension is not present. + bool GetExtension(const der::Input& extension_oid, + ParsedExtension* parsed_extension) const; + private: friend class base::RefCountedThreadSafe<ParsedCertificate>; ParsedCertificate(); @@ -232,6 +255,10 @@ class NET_EXPORT ParsedCertificate bool has_key_usage_ = false; der::BitString key_usage_; + // ExtendedKeyUsage extension. + bool has_extended_key_usage_ = false; + std::vector<der::Input> extended_key_usage_; + // Raw SubjectAltName extension. ParsedExtension subject_alt_names_extension_; // Parsed SubjectAltName extension. @@ -249,8 +276,12 @@ class NET_EXPORT ParsedCertificate std::vector<base::StringPiece> ca_issuers_uris_; std::vector<base::StringPiece> ocsp_uris_; - // The remaining extensions (excludes the standard ones above). - ExtensionsMap unparsed_extensions_; + // Policies extension. + bool has_policy_oids_ = false; + std::vector<der::Input> policy_oids_; + + // All of the extensions. + ExtensionsMap extensions_; DISALLOW_COPY_AND_ASSIGN(ParsedCertificate); }; diff --git a/chromium/net/cert/internal/parsed_certificate_unittest.cc b/chromium/net/cert/internal/parsed_certificate_unittest.cc index 5212caab8e1..1508378c194 100644 --- a/chromium/net/cert/internal/parsed_certificate_unittest.cc +++ b/chromium/net/cert/internal/parsed_certificate_unittest.cc @@ -24,7 +24,8 @@ std::string GetFilePath(const std::string& file_name) { // Returns nullptr if the certificate parsing failed, and verifies that any // errors match the ERRORS block in the .pem file. scoped_refptr<ParsedCertificate> ParseCertificateFromFile( - const std::string& file_name) { + const std::string& file_name, + const ParseCertificateOptions& options) { std::string data; std::string expected_errors; @@ -39,7 +40,7 @@ scoped_refptr<ParsedCertificate> ParseCertificateFromFile( scoped_refptr<ParsedCertificate> cert = ParsedCertificate::Create( bssl::UniquePtr<CRYPTO_BUFFER>(CRYPTO_BUFFER_new( reinterpret_cast<const uint8_t*>(data.data()), data.size(), nullptr)), - {}, &errors); + options, &errors); EXPECT_EQ(expected_errors, errors.ToDebugString()) << "Test file: " << test_file_path; @@ -63,14 +64,13 @@ der::Input DavidBenOid() { // Parses an Extension whose critical field is true (255). TEST(ParsedCertificateTest, ExtensionCritical) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("extension_critical.pem"); + ParseCertificateFromFile("extension_critical.pem", {}); ASSERT_TRUE(cert); const uint8_t kExpectedValue[] = {0x30, 0x00}; - auto it = cert->unparsed_extensions().find(DavidBenOid()); - ASSERT_NE(cert->unparsed_extensions().end(), it); - const auto& extension = it->second; + ParsedExtension extension; + ASSERT_TRUE(cert->GetExtension(DavidBenOid(), &extension)); EXPECT_TRUE(extension.critical); EXPECT_EQ(DavidBenOid(), extension.oid); @@ -80,14 +80,13 @@ TEST(ParsedCertificateTest, ExtensionCritical) { // Parses an Extension whose critical field is false (omitted). TEST(ParsedCertificateTest, ExtensionNotCritical) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("extension_not_critical.pem"); + ParseCertificateFromFile("extension_not_critical.pem", {}); ASSERT_TRUE(cert); const uint8_t kExpectedValue[] = {0x30, 0x00}; - auto it = cert->unparsed_extensions().find(DavidBenOid()); - ASSERT_NE(cert->unparsed_extensions().end(), it); - const auto& extension = it->second; + ParsedExtension extension; + ASSERT_TRUE(cert->GetExtension(DavidBenOid(), &extension)); EXPECT_FALSE(extension.critical); EXPECT_EQ(DavidBenOid(), extension.oid); @@ -98,55 +97,60 @@ TEST(ParsedCertificateTest, ExtensionNotCritical) { // however because critical has DEFAULT of false this is in fact invalid // DER-encoding. TEST(ParsedCertificateTest, ExtensionCritical0) { - ASSERT_FALSE(ParseCertificateFromFile("extension_critical_0.pem")); + ASSERT_FALSE(ParseCertificateFromFile("extension_critical_0.pem", {})); } // Parses an Extension whose critical field is 3. Under DER-encoding BOOLEAN // values must an octet of either all zero bits, or all 1 bits, so this is not // valid. TEST(ParsedCertificateTest, ExtensionCritical3) { - ASSERT_FALSE(ParseCertificateFromFile("extension_critical_3.pem")); + ASSERT_FALSE(ParseCertificateFromFile("extension_critical_3.pem", {})); } // Parses an Extensions that is an empty sequence. TEST(ParsedCertificateTest, ExtensionsEmptySequence) { - ASSERT_FALSE(ParseCertificateFromFile("extensions_empty_sequence.pem")); + ASSERT_FALSE(ParseCertificateFromFile("extensions_empty_sequence.pem", {})); } // Parses an Extensions that is not a sequence. TEST(ParsedCertificateTest, ExtensionsNotSequence) { - ASSERT_FALSE(ParseCertificateFromFile("extensions_not_sequence.pem")); + ASSERT_FALSE(ParseCertificateFromFile("extensions_not_sequence.pem", {})); } // Parses an Extensions that has data after the sequence. TEST(ParsedCertificateTest, ExtensionsDataAfterSequence) { - ASSERT_FALSE(ParseCertificateFromFile("extensions_data_after_sequence.pem")); + ASSERT_FALSE( + ParseCertificateFromFile("extensions_data_after_sequence.pem", {})); } // Parses an Extensions that contains duplicated key usages. TEST(ParsedCertificateTest, ExtensionsDuplicateKeyUsage) { - ASSERT_FALSE(ParseCertificateFromFile("extensions_duplicate_key_usage.pem")); + ASSERT_FALSE( + ParseCertificateFromFile("extensions_duplicate_key_usage.pem", {})); } // Parses an Extensions that contains an extended key usages. TEST(ParsedCertificateTest, ExtendedKeyUsage) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("extended_key_usage.pem"); + ParseCertificateFromFile("extended_key_usage.pem", {}); ASSERT_TRUE(cert); - const auto& extensions = cert->unparsed_extensions(); - ASSERT_EQ(3u, extensions.size()); + ASSERT_EQ(4u, cert->extensions().size()); + + ParsedExtension extension; + ASSERT_TRUE(cert->GetExtension(ExtKeyUsageOid(), &extension)); + + EXPECT_FALSE(extension.critical); + EXPECT_EQ(45u, extension.value.Length()); - auto iter = extensions.find(ExtKeyUsageOid()); - ASSERT_TRUE(iter != extensions.end()); - EXPECT_FALSE(iter->second.critical); - EXPECT_EQ(45u, iter->second.value.Length()); + EXPECT_TRUE(cert->has_extended_key_usage()); + EXPECT_EQ(4u, cert->extended_key_usage().size()); } // Parses an Extensions that contains a key usage. TEST(ParsedCertificateTest, KeyUsage) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("key_usage.pem"); + ParseCertificateFromFile("key_usage.pem", {}); ASSERT_TRUE(cert); ASSERT_TRUE(cert->has_key_usage()); @@ -163,22 +167,25 @@ TEST(ParsedCertificateTest, KeyUsage) { // Parses an Extensions that contains a policies extension. TEST(ParsedCertificateTest, Policies) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("policies.pem"); + ParseCertificateFromFile("policies.pem", {}); ASSERT_TRUE(cert); - const auto& extensions = cert->unparsed_extensions(); - ASSERT_EQ(3u, extensions.size()); + ASSERT_EQ(4u, cert->extensions().size()); - auto iter = extensions.find(CertificatePoliciesOid()); - ASSERT_TRUE(iter != extensions.end()); - EXPECT_FALSE(iter->second.critical); - EXPECT_EQ(95u, iter->second.value.Length()); + ParsedExtension extension; + ASSERT_TRUE(cert->GetExtension(CertificatePoliciesOid(), &extension)); + + EXPECT_FALSE(extension.critical); + EXPECT_EQ(95u, extension.value.Length()); + + EXPECT_TRUE(cert->has_policy_oids()); + EXPECT_EQ(2u, cert->policy_oids().size()); } // Parses an Extensions that contains a subjectaltname extension. TEST(ParsedCertificateTest, SubjectAltName) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("subject_alt_name.pem"); + ParseCertificateFromFile("subject_alt_name.pem", {}); ASSERT_TRUE(cert); ASSERT_TRUE(cert->has_subject_alt_names()); @@ -188,19 +195,20 @@ TEST(ParsedCertificateTest, SubjectAltName) { // real-world certificate. TEST(ParsedCertificateTest, ExtensionsReal) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("extensions_real.pem"); + ParseCertificateFromFile("extensions_real.pem", {}); ASSERT_TRUE(cert); - const auto& extensions = cert->unparsed_extensions(); - ASSERT_EQ(4u, extensions.size()); + ASSERT_EQ(7u, cert->extensions().size()); EXPECT_TRUE(cert->has_key_usage()); EXPECT_TRUE(cert->has_basic_constraints()); + EXPECT_TRUE(cert->has_policy_oids()); - auto iter = extensions.find(CertificatePoliciesOid()); - ASSERT_TRUE(iter != extensions.end()); - EXPECT_FALSE(iter->second.critical); - EXPECT_EQ(16u, iter->second.value.Length()); + ParsedExtension extension; + ASSERT_TRUE(cert->GetExtension(CertificatePoliciesOid(), &extension)); + + EXPECT_FALSE(extension.critical); + EXPECT_EQ(16u, extension.value.Length()); // TODO(eroman): Verify the other 4 extensions' values. } @@ -208,7 +216,7 @@ TEST(ParsedCertificateTest, ExtensionsReal) { // Parses a BasicConstraints with no CA or pathlen. TEST(ParsedCertificateTest, BasicConstraintsNotCa) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("basic_constraints_not_ca.pem"); + ParseCertificateFromFile("basic_constraints_not_ca.pem", {}); ASSERT_TRUE(cert); EXPECT_TRUE(cert->has_basic_constraints()); @@ -219,7 +227,7 @@ TEST(ParsedCertificateTest, BasicConstraintsNotCa) { // Parses a BasicConstraints with CA but no pathlen. TEST(ParsedCertificateTest, BasicConstraintsCaNoPath) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("basic_constraints_ca_no_path.pem"); + ParseCertificateFromFile("basic_constraints_ca_no_path.pem", {}); ASSERT_TRUE(cert); EXPECT_TRUE(cert->has_basic_constraints()); @@ -230,7 +238,7 @@ TEST(ParsedCertificateTest, BasicConstraintsCaNoPath) { // Parses a BasicConstraints with CA and pathlen of 9. TEST(ParsedCertificateTest, BasicConstraintsCaPath9) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("basic_constraints_ca_path_9.pem"); + ParseCertificateFromFile("basic_constraints_ca_path_9.pem", {}); ASSERT_TRUE(cert); EXPECT_TRUE(cert->has_basic_constraints()); @@ -242,7 +250,7 @@ TEST(ParsedCertificateTest, BasicConstraintsCaPath9) { // Parses a BasicConstraints with CA and pathlen of 255 (largest allowed size). TEST(ParsedCertificateTest, BasicConstraintsPathlen255) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("basic_constraints_pathlen_255.pem"); + ParseCertificateFromFile("basic_constraints_pathlen_255.pem", {}); ASSERT_TRUE(cert); EXPECT_TRUE(cert->has_basic_constraints()); @@ -253,26 +261,28 @@ TEST(ParsedCertificateTest, BasicConstraintsPathlen255) { // Parses a BasicConstraints with CA and pathlen of 256 (too large). TEST(ParsedCertificateTest, BasicConstraintsPathlen256) { - ASSERT_FALSE(ParseCertificateFromFile("basic_constraints_pathlen_256.pem")); + ASSERT_FALSE( + ParseCertificateFromFile("basic_constraints_pathlen_256.pem", {})); } // Parses a BasicConstraints with CA and a negative pathlen. TEST(ParsedCertificateTest, BasicConstraintsNegativePath) { - ASSERT_FALSE(ParseCertificateFromFile("basic_constraints_negative_path.pem")); + ASSERT_FALSE( + ParseCertificateFromFile("basic_constraints_negative_path.pem", {})); } // Parses a BasicConstraints with CA and pathlen that is very large (and // couldn't fit in a 64-bit integer). TEST(ParsedCertificateTest, BasicConstraintsPathTooLarge) { ASSERT_FALSE( - ParseCertificateFromFile("basic_constraints_path_too_large.pem")); + ParseCertificateFromFile("basic_constraints_path_too_large.pem", {})); } // Parses a BasicConstraints with CA explicitly set to false. This violates // DER-encoding rules, however is commonly used, so it is accepted. TEST(ParsedCertificateTest, BasicConstraintsCaFalse) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("basic_constraints_ca_false.pem"); + ParseCertificateFromFile("basic_constraints_ca_false.pem", {}); ASSERT_TRUE(cert); EXPECT_TRUE(cert->has_basic_constraints()); @@ -284,7 +294,7 @@ TEST(ParsedCertificateTest, BasicConstraintsCaFalse) { // the end. TEST(ParsedCertificateTest, BasicConstraintsUnconsumedData) { ASSERT_FALSE( - ParseCertificateFromFile("basic_constraints_unconsumed_data.pem")); + ParseCertificateFromFile("basic_constraints_unconsumed_data.pem", {})); } // Parses a BasicConstraints with CA omitted (false), but with a pathlen of 1. @@ -292,7 +302,7 @@ TEST(ParsedCertificateTest, BasicConstraintsUnconsumedData) { // BasicConstraints at a higher level. TEST(ParsedCertificateTest, BasicConstraintsPathLenButNotCa) { scoped_refptr<ParsedCertificate> cert = - ParseCertificateFromFile("basic_constraints_pathlen_not_ca.pem"); + ParseCertificateFromFile("basic_constraints_pathlen_not_ca.pem", {}); ASSERT_TRUE(cert); EXPECT_TRUE(cert->has_basic_constraints()); @@ -301,6 +311,70 @@ TEST(ParsedCertificateTest, BasicConstraintsPathLenButNotCa) { EXPECT_EQ(1u, cert->basic_constraints().path_len); } +// Tests a certificate with a serial number with a leading 0 padding byte in +// the encoding since it is not negative. +TEST(ParsedCertificateTest, SerialNumberZeroPadded) { + scoped_refptr<ParsedCertificate> cert = + ParseCertificateFromFile("serial_zero_padded.pem", {}); + ASSERT_TRUE(cert); + + static const uint8_t expected_serial[3] = {0x00, 0x80, 0x01}; + EXPECT_EQ(der::Input(expected_serial), cert->tbs().serial_number); +} + +// Tests a serial number where the MSB is >= 0x80, causing the encoded +// length to be 21 bytes long. This is an error, as RFC 5280 specifies a +// maximum of 20 bytes. +TEST(ParsedCertificateTest, SerialNumberZeroPadded21BytesLong) { + scoped_refptr<ParsedCertificate> cert = + ParseCertificateFromFile("serial_zero_padded_21_bytes.pem", {}); + ASSERT_FALSE(cert); + + // Try again with allow_invalid_serial_numbers=true. Parsing should succeed. + ParseCertificateOptions options; + options.allow_invalid_serial_numbers = true; + cert = ParseCertificateFromFile("serial_zero_padded_21_bytes.pem", options); + ASSERT_TRUE(cert); + + static const uint8_t expected_serial[21] = { + 0x00, 0x80, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, + 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13}; + EXPECT_EQ(der::Input(expected_serial), cert->tbs().serial_number); +} + +// Tests a serial number which is negative. CAs are not supposed to include +// negative serial numbers, however RFC 5280 expects consumers to deal with it +// anyway. +TEST(ParsedCertificateTest, SerialNumberNegative) { + scoped_refptr<ParsedCertificate> cert = + ParseCertificateFromFile("serial_negative.pem", {}); + ASSERT_TRUE(cert); + + static const uint8_t expected_serial[2] = {0x80, 0x01}; + EXPECT_EQ(der::Input(expected_serial), cert->tbs().serial_number); +} + +// Tests a serial number which is very long. RFC 5280 specifies a maximum of 20 +// bytes. +TEST(ParsedCertificateTest, SerialNumber37BytesLong) { + scoped_refptr<ParsedCertificate> cert = + ParseCertificateFromFile("serial_37_bytes.pem", {}); + ASSERT_FALSE(cert); + + // Try again with allow_invalid_serial_numbers=true. Parsing should succeed. + ParseCertificateOptions options; + options.allow_invalid_serial_numbers = true; + cert = ParseCertificateFromFile("serial_37_bytes.pem", options); + ASSERT_TRUE(cert); + + static const uint8_t expected_serial[37] = { + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, + 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, + 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, + 0x1f, 0x20, 0x21, 0x22, 0x23, 0x24, 0x25}; + EXPECT_EQ(der::Input(expected_serial), cert->tbs().serial_number); +} + } // namespace } // namespace net diff --git a/chromium/net/cert/internal/path_builder.cc b/chromium/net/cert/internal/path_builder.cc index bb4283eeed6..291bcf53606 100644 --- a/chromium/net/cert/internal/path_builder.cc +++ b/chromium/net/cert/internal/path_builder.cc @@ -459,6 +459,12 @@ void CertPathIter::DoBackTrack() { CertPathBuilder::ResultPath::ResultPath() = default; CertPathBuilder::ResultPath::~ResultPath() = default; + +bool CertPathBuilder::ResultPath::IsValid() const { + return !path.certs.empty() && path.trust_anchor && + !errors.ContainsHighSeverityErrors(); +} + CertPathBuilder::Result::Result() = default; CertPathBuilder::Result::~Result() = default; @@ -471,7 +477,7 @@ const CertPathBuilder::ResultPath* CertPathBuilder::Result::GetBestValidPath() return nullptr; const ResultPath* result_path = paths[best_result_index].get(); - if (result_path->valid) + if (result_path->IsValid()) return result_path; return nullptr; @@ -485,10 +491,12 @@ CertPathBuilder::CertPathBuilder(scoped_refptr<ParsedCertificate> cert, const TrustStore* trust_store, const SignaturePolicy* signature_policy, const der::GeneralizedTime& time, + KeyPurpose key_purpose, Result* result) : cert_path_iter_(new CertPathIter(std::move(cert), trust_store)), signature_policy_(signature_policy), time_(time), + key_purpose_(key_purpose), next_state_(STATE_NONE), out_result_(result) {} @@ -535,13 +543,13 @@ void CertPathBuilder::DoGetNextPathComplete() { // Verify the entire certificate chain. auto result_path = base::MakeUnique<ResultPath>(); - bool verify_result = - VerifyCertificateChain(next_path_.certs, next_path_.trust_anchor.get(), - signature_policy_, time_, &result_path->errors); + bool verify_result = VerifyCertificateChain( + next_path_.certs, next_path_.trust_anchor.get(), signature_policy_, time_, + key_purpose_, &result_path->errors); DVLOG(1) << "CertPathBuilder VerifyCertificateChain result = " - << result_path->valid; + << verify_result; result_path->path = next_path_; - result_path->valid = verify_result; + DCHECK_EQ(verify_result, !result_path->errors.ContainsHighSeverityErrors()); AddResultPath(std::move(result_path)); if (verify_result) { @@ -559,7 +567,7 @@ void CertPathBuilder::DoGetNextPathComplete() { void CertPathBuilder::AddResultPath(std::unique_ptr<ResultPath> result_path) { // TODO(mattm): set best_result_index based on number or severity of errors. - if (result_path->valid) + if (result_path->IsValid()) out_result_->best_result_index = out_result_->paths.size(); // TODO(mattm): add flag to only return a single path or all attempted paths? out_result_->paths.push_back(std::move(result_path)); diff --git a/chromium/net/cert/internal/path_builder.h b/chromium/net/cert/internal/path_builder.h index ee39ed67ee5..361e745602f 100644 --- a/chromium/net/cert/internal/path_builder.h +++ b/chromium/net/cert/internal/path_builder.h @@ -13,6 +13,7 @@ #include "net/cert/internal/cert_errors.h" #include "net/cert/internal/parsed_certificate.h" #include "net/cert/internal/trust_store.h" +#include "net/cert/internal/verify_certificate_chain.h" #include "net/der/input.h" #include "net/der/parse_values.h" @@ -32,6 +33,9 @@ class SignaturePolicy; // certs[0] is the target certificate // certs[i] was issued by certs[i+1] // certs.back() was issued by trust_anchor +// +// TODO(eroman): The current code doesn't allow for the target certificate to +// be the trust anchor. Should it? struct NET_EXPORT CertPath { CertPath(); ~CertPath(); @@ -44,7 +48,7 @@ struct NET_EXPORT CertPath { // Resets the path to empty path (same as if default constructed). void Clear(); - // Returns true if the path is empty. + // TODO(eroman): Can we remove this? Unclear on how this relates to validity. bool IsEmpty() const; }; @@ -61,19 +65,17 @@ class NET_EXPORT CertPathBuilder { ResultPath(); ~ResultPath(); + // Returns true if the candidate path is valid, false otherwise. + bool IsValid() const; + // The (possibly partial) certificate path. Consumers must always test - // |valid| before using |path|. When |!valid| path.trust_anchor may be - // nullptr, and the path may be otherwise incomplete/invalid. + // |errors.IsValid()| before using |path|. When invalid, + // |path.trust_anchor| may be null, and the path may be incomplete. CertPath path; - // The errors/warnings from this path. Note that the list of errors is - // independent of whether the path was |valid| (a valid path may - // contain errors/warnings, and vice versa an invalid path may not have - // logged any errors). - CertErrors errors; - - // True if |path| is a correct verified certificate chain. - bool valid = false; + // The errors/warnings from this path. Use |IsValid()| to determine if the + // path is valid. + CertPathErrors errors; }; // Provides the overall result of path building. This includes the paths that @@ -117,6 +119,7 @@ class NET_EXPORT CertPathBuilder { const TrustStore* trust_store, const SignaturePolicy* signature_policy, const der::GeneralizedTime& time, + KeyPurpose key_purpose, Result* result); ~CertPathBuilder(); @@ -151,6 +154,7 @@ class NET_EXPORT CertPathBuilder { std::unique_ptr<CertPathIter> cert_path_iter_; const SignaturePolicy* signature_policy_; const der::GeneralizedTime time_; + const KeyPurpose key_purpose_; // Stores the next complete path to attempt verification on. This is filled in // by |cert_path_iter_| during the STATE_GET_NEXT_PATH step, and thus should diff --git a/chromium/net/cert/internal/path_builder_pkits_unittest.cc b/chromium/net/cert/internal/path_builder_pkits_unittest.cc index 2d16ee9bf4e..a7b02e3c3b7 100644 --- a/chromium/net/cert/internal/path_builder_pkits_unittest.cc +++ b/chromium/net/cert/internal/path_builder_pkits_unittest.cc @@ -92,7 +92,8 @@ class PathBuilderPkitsTestDelegate { CertPathBuilder::Result result; CertPathBuilder path_builder(std::move(target_cert), &trust_store, - &signature_policy, time, &result); + &signature_policy, time, KeyPurpose::ANY_EKU, + &result); path_builder.AddCertIssuerSource(&cert_issuer_source); path_builder.Run(); diff --git a/chromium/net/cert/internal/path_builder_unittest.cc b/chromium/net/cert/internal/path_builder_unittest.cc index be5432b59d8..44471fa8767 100644 --- a/chromium/net/cert/internal/path_builder_unittest.cc +++ b/chromium/net/cert/internal/path_builder_unittest.cc @@ -160,7 +160,7 @@ TEST_F(PathBuilderMultiRootTest, TargetHasNameAndSpkiOfTrustAnchor) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.Run(); @@ -180,7 +180,7 @@ TEST_F(PathBuilderMultiRootTest, TargetWithSameNameAsTrustAnchorFails) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.Run(); @@ -210,7 +210,7 @@ TEST_F(PathBuilderMultiRootTest, SelfSignedTrustAnchorSupplementalCert) { CertPathBuilder::Result result; CertPathBuilder path_builder(b_by_c_, &trust_store, &signature_policy_, - expired_time, &result); + expired_time, KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -218,7 +218,7 @@ TEST_F(PathBuilderMultiRootTest, SelfSignedTrustAnchorSupplementalCert) { EXPECT_FALSE(result.HasValidPath()); ASSERT_EQ(2U, result.paths.size()); - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path0 = result.paths[0]->path; ASSERT_EQ(2U, path0.certs.size()); EXPECT_EQ(b_by_c_, path0.certs[0]); @@ -243,7 +243,7 @@ TEST_F(PathBuilderMultiRootTest, TargetIsSelfSignedTrustAnchor) { CertPathBuilder::Result result; CertPathBuilder path_builder(e_by_e_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.Run(); @@ -262,7 +262,7 @@ TEST_F(PathBuilderMultiRootTest, TargetDirectlySignedByTrustAnchor) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.Run(); @@ -289,7 +289,7 @@ TEST_F(PathBuilderMultiRootTest, TriesSyncFirst) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&async_certs); path_builder.AddCertIssuerSource(&sync_certs); @@ -317,7 +317,7 @@ TEST_F(PathBuilderMultiRootTest, TestAsyncSimultaneous) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&async_certs1); path_builder.AddCertIssuerSource(&async_certs2); path_builder.AddCertIssuerSource(&sync_certs); @@ -344,7 +344,7 @@ TEST_F(PathBuilderMultiRootTest, TestLongChain) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -377,7 +377,7 @@ TEST_F(PathBuilderMultiRootTest, TestBacktracking) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.AddCertIssuerSource(&async_certs); @@ -416,7 +416,7 @@ TEST_F(PathBuilderMultiRootTest, TestCertIssuerOrdering) { CertPathBuilder::Result result; CertPathBuilder path_builder(a_by_b_, &trust_store, &signature_policy_, - time_, &result); + time_, KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -439,12 +439,15 @@ class PathBuilderKeyRolloverTest : public ::testing::Test { void SetUp() override { ParsedCertificateList path; - bool unused_result; - std::string unused_errors; + VerifyCertChainTest test; ReadVerifyCertChainTestFromFile( "net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem", - &path, &oldroot_, &time_, &unused_result, &unused_errors); + &test); + path = test.chain; + oldroot_ = test.trust_anchor; + time_ = test.time; + ASSERT_EQ(2U, path.size()); target_ = path[0]; oldintermediate_ = path[1]; @@ -454,7 +457,9 @@ class PathBuilderKeyRolloverTest : public ::testing::Test { ReadVerifyCertChainTestFromFile( "net/data/verify_certificate_chain_unittest/" "key-rollover-longrolloverchain.pem", - &path, &oldroot_, &time_, &unused_result, &unused_errors); + &test); + path = test.chain; + ASSERT_EQ(4U, path.size()); newintermediate_ = path[1]; newroot_ = path[2]; @@ -500,7 +505,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -511,7 +516,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) { // but it will fail since newintermediate is signed by newroot. ASSERT_EQ(2U, result.paths.size()); const auto& path0 = result.paths[0]->path; - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); ASSERT_EQ(2U, path0.certs.size()); EXPECT_EQ(target_, path0.certs[0]); EXPECT_EQ(newintermediate_, path0.certs[1]); @@ -522,7 +527,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverOnlyOldRootTrusted) { // which will succeed. const auto& path1 = result.paths[1]->path; EXPECT_EQ(1U, result.best_result_index); - EXPECT_TRUE(result.paths[1]->valid); + EXPECT_TRUE(result.paths[1]->IsValid()); ASSERT_EQ(3U, path1.certs.size()); EXPECT_EQ(target_, path1.certs[0]); EXPECT_EQ(newintermediate_, path1.certs[1]); @@ -548,7 +553,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -561,7 +566,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverBothRootsTrusted) { // either will succeed. ASSERT_EQ(1U, result.paths.size()); const auto& path = result.paths[0]->path; - EXPECT_TRUE(result.paths[0]->valid); + EXPECT_TRUE(result.paths[0]->IsValid()); ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(target_, path.certs[0]); if (path.certs[1] != newintermediate_) { @@ -584,7 +589,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestAnchorsNoMatchAndNoIssuerSources) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.Run(); @@ -616,7 +621,8 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store_collection, - &signature_policy_, time_, &result); + &signature_policy_, time_, KeyPurpose::ANY_EKU, + &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -627,7 +633,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) { { // Path builder may first attempt: target <- oldintermediate <- newroot // but it will fail since oldintermediate is signed by oldroot. - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path = result.paths[0]->path; ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(target_, path.certs[0]); @@ -639,7 +645,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleRootMatchesOnlyOneWorks) { // Path builder will next attempt: // target <- old intermediate <- oldroot // which should succeed. - EXPECT_TRUE(result.paths[result.best_result_index]->valid); + EXPECT_TRUE(result.paths[result.best_result_index]->IsValid()); const auto& path = result.paths[result.best_result_index]->path; ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(target_, path.certs[0]); @@ -666,7 +672,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.AddCertIssuerSource(&async_certs); @@ -677,7 +683,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) { // Path builder will first attempt: target <- newintermediate <- oldroot // but it will fail since newintermediate is signed by newroot. - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path0 = result.paths[0]->path; ASSERT_EQ(2U, path0.certs.size()); EXPECT_EQ(target_, path0.certs[0]); @@ -687,7 +693,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) { // Path builder will next attempt: // target <- newintermediate <- newroot <- oldroot // but it will fail since newroot is self-signed. - EXPECT_FALSE(result.paths[1]->valid); + EXPECT_FALSE(result.paths[1]->IsValid()); const auto& path1 = result.paths[1]->path; ASSERT_EQ(3U, path1.certs.size()); EXPECT_EQ(target_, path1.certs[0]); @@ -702,7 +708,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestRolloverLongChain) { // Finally path builder will use: // target <- newintermediate <- newrootrollover <- oldroot EXPECT_EQ(2U, result.best_result_index); - EXPECT_TRUE(result.paths[2]->valid); + EXPECT_TRUE(result.paths[2]->IsValid()); const auto& path2 = result.paths[2]->path; ASSERT_EQ(3U, path2.certs.size()); EXPECT_EQ(target_, path2.certs[0]); @@ -723,7 +729,8 @@ TEST_F(PathBuilderKeyRolloverTest, TestEndEntityIsTrustRoot) { CertPathBuilder::Result result; // Newintermediate is also the target cert. CertPathBuilder path_builder(newintermediate_, &trust_store, - &signature_policy_, time_, &result); + &signature_policy_, time_, KeyPurpose::ANY_EKU, + &result); path_builder.Run(); @@ -747,7 +754,7 @@ TEST_F(PathBuilderKeyRolloverTest, CertPathBuilder::Result result; // Newroot is the target cert. CertPathBuilder path_builder(newroot_, &trust_store, &signature_policy_, - time_, &result); + time_, KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -768,7 +775,7 @@ TEST_F(PathBuilderKeyRolloverTest, CertPathBuilder::Result result; // Newroot is the target cert. CertPathBuilder path_builder(newroot_, &trust_store, &signature_policy_, - time_, &result); + time_, KeyPurpose::ANY_EKU, &result); path_builder.Run(); @@ -778,7 +785,7 @@ TEST_F(PathBuilderKeyRolloverTest, // Newroot has same name+SPKI as newrootrollover, thus the path is valid and // only contains newroot. - EXPECT_TRUE(best_result->valid); + EXPECT_TRUE(best_result->IsValid()); ASSERT_EQ(1U, best_result->path.certs.size()); EXPECT_EQ(newroot_, best_result->path.certs[0]); EXPECT_EQ(newrootrollover_, best_result->path.trust_anchor->cert()); @@ -816,7 +823,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs1); path_builder.AddCertIssuerSource(&sync_certs2); path_builder.AddCertIssuerSource(&async_certs); @@ -828,7 +835,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) { // Path builder will first attempt: target <- oldintermediate <- newroot // but it will fail since oldintermediate is signed by oldroot. - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path0 = result.paths[0]->path; ASSERT_EQ(2U, path0.certs.size()); @@ -841,7 +848,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) { // Path builder will next attempt: target <- newintermediate <- newroot // which will succeed. EXPECT_EQ(1U, result.best_result_index); - EXPECT_TRUE(result.paths[1]->valid); + EXPECT_TRUE(result.paths[1]->IsValid()); const auto& path1 = result.paths[1]->path; ASSERT_EQ(2U, path1.certs.size()); EXPECT_EQ(target_, path1.certs[0]); @@ -870,7 +877,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&sync_certs); path_builder.Run(); @@ -881,7 +888,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) { // Path builder attempt: target <- oldintermediate <- newroot // but it will fail since oldintermediate is signed by oldroot. - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path = result.paths[0]->path; ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(target_, path.certs[0]); @@ -945,7 +952,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&cert_issuer_source); // Create the mock CertIssuerSource::Request... @@ -996,7 +1003,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) { // Path builder first attempts: target <- oldintermediate <- newroot // but it will fail since oldintermediate is signed by oldroot. - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path0 = result.paths[0]->path; ASSERT_EQ(2U, path0.certs.size()); EXPECT_EQ(target_, path0.certs[0]); @@ -1005,7 +1012,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) { // After the second batch of async results, path builder will attempt: // target <- newintermediate <- newroot which will succeed. - EXPECT_TRUE(result.paths[1]->valid); + EXPECT_TRUE(result.paths[1]->IsValid()); const auto& path1 = result.paths[1]->path; ASSERT_EQ(2U, path1.certs.size()); EXPECT_EQ(target_, path1.certs[0]); @@ -1024,7 +1031,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) { CertPathBuilder::Result result; CertPathBuilder path_builder(target_, &trust_store, &signature_policy_, time_, - &result); + KeyPurpose::ANY_EKU, &result); path_builder.AddCertIssuerSource(&cert_issuer_source); // Create the mock CertIssuerSource::Request... @@ -1082,7 +1089,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) { // Path builder first attempts: target <- oldintermediate <- newroot // but it will fail since oldintermediate is signed by oldroot. - EXPECT_FALSE(result.paths[0]->valid); + EXPECT_FALSE(result.paths[0]->IsValid()); const auto& path0 = result.paths[0]->path; ASSERT_EQ(2U, path0.certs.size()); EXPECT_EQ(target_, path0.certs[0]); @@ -1093,7 +1100,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) { // After the third batch of async results, path builder will attempt: // target <- newintermediate <- newroot which will succeed. - EXPECT_TRUE(result.paths[1]->valid); + EXPECT_TRUE(result.paths[1]->IsValid()); const auto& path1 = result.paths[1]->path; ASSERT_EQ(2U, path1.certs.size()); EXPECT_EQ(target_, path1.certs[0]); diff --git a/chromium/net/cert/internal/path_builder_verify_certificate_chain_unittest.cc b/chromium/net/cert/internal/path_builder_verify_certificate_chain_unittest.cc index 003157b8dfa..43ba879a971 100644 --- a/chromium/net/cert/internal/path_builder_verify_certificate_chain_unittest.cc +++ b/chromium/net/cert/internal/path_builder_verify_certificate_chain_unittest.cc @@ -15,30 +15,27 @@ namespace { class PathBuilderDelegate { public: - static void Verify(const ParsedCertificateList& chain, - const scoped_refptr<TrustAnchor>& trust_anchor, - const der::GeneralizedTime& time, - bool expected_result, - const std::string& expected_errors, + static void Verify(const VerifyCertChainTest& test, const std::string& test_file_path) { SimpleSignaturePolicy signature_policy(1024); - ASSERT_FALSE(chain.empty()); + ASSERT_FALSE(test.chain.empty()); TrustStoreInMemory trust_store; - trust_store.AddTrustAnchor(trust_anchor); + trust_store.AddTrustAnchor(test.trust_anchor); CertIssuerSourceStatic intermediate_cert_issuer_source; - for (size_t i = 1; i < chain.size(); ++i) - intermediate_cert_issuer_source.AddCert(chain[i]); + for (size_t i = 1; i < test.chain.size(); ++i) + intermediate_cert_issuer_source.AddCert(test.chain[i]); CertPathBuilder::Result result; // First cert in the |chain| is the target. - CertPathBuilder path_builder(chain.front(), &trust_store, &signature_policy, - time, &result); + CertPathBuilder path_builder(test.chain.front(), &trust_store, + &signature_policy, test.time, test.key_purpose, + &result); path_builder.AddCertIssuerSource(&intermediate_cert_issuer_source); path_builder.Run(); - EXPECT_EQ(expected_result, result.HasValidPath()); + EXPECT_EQ(test.expected_result, result.HasValidPath()); } }; diff --git a/chromium/net/cert/internal/signature_policy.cc b/chromium/net/cert/internal/signature_policy.cc index b98cb7ae052..8f86b208f14 100644 --- a/chromium/net/cert/internal/signature_policy.cc +++ b/chromium/net/cert/internal/signature_policy.cc @@ -11,11 +11,12 @@ namespace net { +DEFINE_CERT_ERROR_ID(kRsaModulusTooSmall, "RSA modulus too small"); + namespace { DEFINE_CERT_ERROR_ID(kUnacceptableCurveForEcdsa, "Only P-256, P-384, P-521 are supported for ECDSA"); -DEFINE_CERT_ERROR_ID(kRsaModulusTooSmall, "RSA modulus too small"); bool IsModulusSizeGreaterOrEqual(size_t modulus_length_bits, size_t min_length_bits, diff --git a/chromium/net/cert/internal/signature_policy.h b/chromium/net/cert/internal/signature_policy.h index 86d6c32b19f..fbc94ad976f 100644 --- a/chromium/net/cert/internal/signature_policy.h +++ b/chromium/net/cert/internal/signature_policy.h @@ -9,6 +9,7 @@ #include "base/compiler_specific.h" #include "net/base/net_export.h" +#include "net/cert/internal/cert_errors.h" #include "net/cert/internal/signature_algorithm.h" namespace net { @@ -61,6 +62,9 @@ class NET_EXPORT SimpleSignaturePolicy : public SignaturePolicy { const size_t min_rsa_modulus_length_bits_; }; +// TODO(crbug.com/634443): Move exported errors to a central location? +extern CertErrorId kRsaModulusTooSmall; + } // namespace net #endif // NET_CERT_INTERNAL_SIGNATURE_POLICY_H_ diff --git a/chromium/net/cert/internal/test_helpers.cc b/chromium/net/cert/internal/test_helpers.cc index 24d4f329063..80948c2318d 100644 --- a/chromium/net/cert/internal/test_helpers.cc +++ b/chromium/net/cert/internal/test_helpers.cc @@ -102,15 +102,13 @@ der::Input SequenceValueFromString(const std::string* s) { return ::testing::AssertionSuccess(); } +VerifyCertChainTest::VerifyCertChainTest() = default; +VerifyCertChainTest::~VerifyCertChainTest() = default; + void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, - ParsedCertificateList* chain, - scoped_refptr<TrustAnchor>* trust_anchor, - der::GeneralizedTime* time, - bool* verify_result, - std::string* expected_errors) { - chain->clear(); - *trust_anchor = nullptr; - expected_errors->clear(); + VerifyCertChainTest* test) { + // Reset all the out parameters to their defaults. + *test = {}; std::string file_data = ReadTestFileToString(file_path_ascii); @@ -124,6 +122,7 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, const char kTimeHeader[] = "TIME"; const char kResultHeader[] = "VERIFY_RESULT"; const char kErrorsHeader[] = "ERRORS"; + const char kKeyPurpose[] = "KEY_PURPOSE"; pem_headers.push_back(kCertificateHeader); pem_headers.push_back(kTrustAnchorUnconstrained); @@ -131,10 +130,12 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, pem_headers.push_back(kTimeHeader); pem_headers.push_back(kResultHeader); pem_headers.push_back(kErrorsHeader); + pem_headers.push_back(kKeyPurpose); bool has_time = false; bool has_result = false; bool has_errors = false; + bool has_key_purpose = false; PEMTokenizer pem_tokenizer(file_data, pem_headers); while (pem_tokenizer.GetNext()) { @@ -147,11 +148,11 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, bssl::UniquePtr<CRYPTO_BUFFER>(CRYPTO_BUFFER_new( reinterpret_cast<const uint8_t*>(block_data.data()), block_data.size(), nullptr)), - {}, chain, &errors)) + {}, &test->chain, &errors)) << errors.ToDebugString(); } else if (block_type == kTrustAnchorUnconstrained || block_type == kTrustAnchorConstrained) { - ASSERT_FALSE(*trust_anchor) << "Duplicate trust anchor"; + ASSERT_FALSE(test->trust_anchor) << "Duplicate trust anchor"; CertErrors errors; scoped_refptr<ParsedCertificate> root = net::ParsedCertificate::Create( bssl::UniquePtr<CRYPTO_BUFFER>(CRYPTO_BUFFER_new( @@ -159,7 +160,7 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, block_data.size(), nullptr)), {}, &errors); ASSERT_TRUE(root) << errors.ToDebugString(); - *trust_anchor = + test->trust_anchor = block_type == kTrustAnchorUnconstrained ? TrustAnchor::CreateFromCertificateNoConstraints(std::move(root)) : TrustAnchor::CreateFromCertificateWithConstraints( @@ -167,23 +168,37 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, } else if (block_type == kTimeHeader) { ASSERT_FALSE(has_time) << "Duplicate " << kTimeHeader; has_time = true; - ASSERT_TRUE(der::ParseUTCTime(der::Input(&block_data), time)); + ASSERT_TRUE(der::ParseUTCTime(der::Input(&block_data), &test->time)); + } else if (block_type == kKeyPurpose) { + ASSERT_FALSE(has_key_purpose) << "Duplicate " << kKeyPurpose; + has_key_purpose = true; + + if (block_data == "anyExtendedKeyUsage") { + test->key_purpose = KeyPurpose::ANY_EKU; + } else if (block_data == "serverAuth") { + test->key_purpose = KeyPurpose::SERVER_AUTH; + } else if (block_data == "clientAuth") { + test->key_purpose = KeyPurpose::CLIENT_AUTH; + } else { + ADD_FAILURE() << "Unrecognized " << block_type << ": " << block_data; + } } else if (block_type == kResultHeader) { ASSERT_FALSE(has_result) << "Duplicate " << kResultHeader; ASSERT_TRUE(block_data == "SUCCESS" || block_data == "FAIL") << "Unrecognized result: " << block_data; has_result = true; - *verify_result = block_data == "SUCCESS"; + test->expected_result = block_data == "SUCCESS"; } else if (block_type == kErrorsHeader) { ASSERT_FALSE(has_errors) << "Duplicate " << kErrorsHeader; has_errors = true; - *expected_errors = block_data; + test->expected_errors = block_data; } } ASSERT_TRUE(has_time); ASSERT_TRUE(has_result); - ASSERT_TRUE(*trust_anchor); + ASSERT_TRUE(test->trust_anchor); + ASSERT_TRUE(has_key_purpose); } std::string ReadTestFileToString(const std::string& file_path_ascii) { diff --git a/chromium/net/cert/internal/test_helpers.h b/chromium/net/cert/internal/test_helpers.h index 0e4cd17a58d..25afc28886d 100644 --- a/chromium/net/cert/internal/test_helpers.h +++ b/chromium/net/cert/internal/test_helpers.h @@ -13,6 +13,7 @@ #include "net/cert/internal/parsed_certificate.h" #include "net/cert/internal/trust_store.h" +#include "net/cert/internal/verify_certificate_chain.h" #include "net/der/input.h" #include "testing/gtest/include/gtest/gtest.h" @@ -76,17 +77,36 @@ template <size_t N> return ReadTestDataFromPemFile(file_path_ascii, mappings, N); } -// Reads a test case from |file_path_ascii| (which is relative to //src). Test -// cases are comprised of a certificate chain, trust anchor, a timestamp to -// validate at, and the expected result of verification. +// Test cases are comprised of all the parameters to certificate +// verification, as well as the expected outputs. +struct VerifyCertChainTest { + VerifyCertChainTest(); + ~VerifyCertChainTest(); + + // The chain of certificates (with the zero-th being the target). + ParsedCertificateList chain; + + // The trust anchor to use when verifying the chain. + scoped_refptr<TrustAnchor> trust_anchor; + + // The time to use when verifying the chain. + der::GeneralizedTime time; + + // The Key Purpose to use when verifying the chain. + KeyPurpose key_purpose = KeyPurpose::ANY_EKU; + + // The expected result from verification. + bool expected_result = false; + + // The expected errors from verification (as a string). + std::string expected_errors; +}; + +// Reads a test case from |file_path_ascii| (which is relative to //src). // Generally |file_path_ascii| will start with: // net/data/verify_certificate_chain_unittest/ void ReadVerifyCertChainTestFromFile(const std::string& file_path_ascii, - ParsedCertificateList* chain, - scoped_refptr<TrustAnchor>* trust_anchor, - der::GeneralizedTime* time, - bool* verify_result, - std::string* expected_errors); + VerifyCertChainTest* test); // Reads a data file relative to the src root directory. std::string ReadTestFileToString(const std::string& file_path_ascii); diff --git a/chromium/net/cert/internal/trust_store.h b/chromium/net/cert/internal/trust_store.h index 6985301f35c..383e83a54d6 100644 --- a/chromium/net/cert/internal/trust_store.h +++ b/chromium/net/cert/internal/trust_store.h @@ -75,7 +75,7 @@ class NET_EXPORT TrustAnchor : public base::RefCountedThreadSafe<TrustAnchor> { // * Signature: No // * Validity (expiration): No // * Key usage: No - // * Extended key usage: No + // * Extended key usage: Yes (not part of RFC 5937) // * Basic constraints: Yes, but only the pathlen (CA=false is accepted) // * Name constraints: Yes // * Certificate policies: Not currently, TODO(crbug.com/634453) diff --git a/chromium/net/cert/internal/trust_store_collection_unittest.cc b/chromium/net/cert/internal/trust_store_collection_unittest.cc index c9cd85ccab8..198988e78c6 100644 --- a/chromium/net/cert/internal/trust_store_collection_unittest.cc +++ b/chromium/net/cert/internal/trust_store_collection_unittest.cc @@ -16,13 +16,14 @@ class TrustStoreCollectionTest : public testing::Test { public: void SetUp() override { ParsedCertificateList chain; - bool unused_verify_result; - der::GeneralizedTime unused_time; - std::string unused_errors; + VerifyCertChainTest test; ReadVerifyCertChainTestFromFile( "net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem", - &chain, &oldroot_, &unused_time, &unused_verify_result, &unused_errors); + &test); + chain = test.chain; + oldroot_ = test.trust_anchor; + ASSERT_EQ(2U, chain.size()); target_ = chain[0]; oldintermediate_ = chain[1]; @@ -30,12 +31,12 @@ class TrustStoreCollectionTest : public testing::Test { ASSERT_TRUE(oldintermediate_); ASSERT_TRUE(oldroot_); - scoped_refptr<TrustAnchor> unused_root; ReadVerifyCertChainTestFromFile( "net/data/verify_certificate_chain_unittest/" "key-rollover-longrolloverchain.pem", - &chain, &unused_root, &unused_time, &unused_verify_result, - &unused_errors); + &test); + chain = test.chain; + ASSERT_EQ(4U, chain.size()); newintermediate_ = chain[1]; newroot_ = TrustAnchor::CreateFromCertificateNoConstraints(chain[2]); diff --git a/chromium/net/cert/internal/trust_store_in_memory.cc b/chromium/net/cert/internal/trust_store_in_memory.cc index 3f94b6f0c49..a3e9e3eea07 100644 --- a/chromium/net/cert/internal/trust_store_in_memory.cc +++ b/chromium/net/cert/internal/trust_store_in_memory.cc @@ -27,4 +27,12 @@ void TrustStoreInMemory::FindTrustAnchorsForCert( matches->push_back(it->second); } +bool TrustStoreInMemory::Contains(const TrustAnchor* anchor) const { + for (const auto& it : anchors_) { + if (anchor == it.second.get()) + return true; + } + return false; +} + } // namespace net diff --git a/chromium/net/cert/internal/trust_store_in_memory.h b/chromium/net/cert/internal/trust_store_in_memory.h index 45b5123caf1..214d73e7a14 100644 --- a/chromium/net/cert/internal/trust_store_in_memory.h +++ b/chromium/net/cert/internal/trust_store_in_memory.h @@ -30,6 +30,11 @@ class NET_EXPORT TrustStoreInMemory : public TrustStore { void FindTrustAnchorsForCert(const scoped_refptr<ParsedCertificate>& cert, TrustAnchors* matches) const override; + // Returns true if the trust store contains the given TrustAnchor instance. + // Note that this considers only pointer equality and not a more + // broad notion of equivalence based on the object's content. + bool Contains(const TrustAnchor* anchor) const; + private: // Multimap from normalized subject -> TrustAnchor. std::unordered_multimap<base::StringPiece, diff --git a/chromium/net/cert/internal/trust_store_mac.cc b/chromium/net/cert/internal/trust_store_mac.cc index a088a32f4c5..ba31ffb1d59 100644 --- a/chromium/net/cert/internal/trust_store_mac.cc +++ b/chromium/net/cert/internal/trust_store_mac.cc @@ -16,8 +16,8 @@ #include "net/cert/internal/parse_name.h" #include "net/cert/internal/parsed_certificate.h" #include "net/cert/test_keychain_search_list_mac.h" -#include "net/cert/x509_certificate.h" #include "net/cert/x509_util.h" +#include "net/cert/x509_util_mac.h" namespace net { @@ -146,7 +146,7 @@ TrustStatus IsTrustSettingsTrustedForPolicy(CFArrayRef trust_settings, // |policy_oid|. TrustStatus IsSecCertificateTrustedForPolicy(SecCertificateRef cert_handle, const CFStringRef policy_oid) { - const bool is_self_signed = X509Certificate::IsSelfSigned(cert_handle); + const bool is_self_signed = x509_util::IsSelfSigned(cert_handle); // Evaluate trust domains in user, admin, system order. Admin settings can // override system ones, and user settings can override both admin and system. for (const auto& trust_domain : @@ -320,8 +320,8 @@ base::ScopedCFTypeRef<CFDataRef> TrustStoreMac::GetMacNormalizedIssuer( // There does not appear to be any public API to get the normalized version // of a Name without creating a SecCertificate. base::ScopedCFTypeRef<SecCertificateRef> cert_handle( - X509Certificate::CreateOSCertHandleFromBytes( - cert->der_cert().AsStringPiece().data(), cert->der_cert().Length())); + x509_util::CreateSecCertificateFromBytes(cert->der_cert().UnsafeData(), + cert->der_cert().Length())); if (!cert_handle) { LOG(ERROR) << "CreateOSCertHandleFromBytes"; return name_data; diff --git a/chromium/net/cert/internal/trust_store_mac_unittest.cc b/chromium/net/cert/internal/trust_store_mac_unittest.cc index 1d1bc4916d9..bba5995eb6c 100644 --- a/chromium/net/cert/internal/trust_store_mac_unittest.cc +++ b/chromium/net/cert/internal/trust_store_mac_unittest.cc @@ -18,6 +18,7 @@ #include "net/cert/test_keychain_search_list_mac.h" #include "net/cert/x509_certificate.h" #include "net/cert/x509_util.h" +#include "net/cert/x509_util_mac.h" #include "net/test/test_data_directory.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -263,9 +264,8 @@ TEST(TrustStoreMacTest, SystemCerts) { } base::ScopedCFTypeRef<SecCertificateRef> cert_handle( - X509Certificate::CreateOSCertHandleFromBytes( - cert->der_cert().AsStringPiece().data(), - cert->der_cert().Length())); + x509_util::CreateSecCertificateFromBytes(cert->der_cert().UnsafeData(), + cert->der_cert().Length())); if (!cert_handle) { ADD_FAILURE() << "CreateOSCertHandleFromBytes " << hash_text; continue; diff --git a/chromium/net/cert/internal/trust_store_nss_unittest.cc b/chromium/net/cert/internal/trust_store_nss_unittest.cc index f9d1f272d74..e83ba672865 100644 --- a/chromium/net/cert/internal/trust_store_nss_unittest.cc +++ b/chromium/net/cert/internal/trust_store_nss_unittest.cc @@ -24,14 +24,14 @@ class TrustStoreNSSTest : public testing::Test { void SetUp() override { ASSERT_TRUE(test_nssdb_.is_open()); + VerifyCertChainTest test; ParsedCertificateList chain; - bool unused_verify_result; - der::GeneralizedTime unused_time; - std::string unused_errors; - ReadVerifyCertChainTestFromFile( "net/data/verify_certificate_chain_unittest/key-rollover-oldchain.pem", - &chain, &oldroot_, &unused_time, &unused_verify_result, &unused_errors); + &test); + chain = test.chain; + oldroot_ = test.trust_anchor; + ASSERT_EQ(2U, chain.size()); target_ = chain[0]; oldintermediate_ = chain[1]; @@ -39,12 +39,12 @@ class TrustStoreNSSTest : public testing::Test { ASSERT_TRUE(oldintermediate_); ASSERT_TRUE(oldroot_); - scoped_refptr<TrustAnchor> unused_root; ReadVerifyCertChainTestFromFile( "net/data/verify_certificate_chain_unittest/" "key-rollover-longrolloverchain.pem", - &chain, &unused_root, &unused_time, &unused_verify_result, - &unused_errors); + &test); + chain = test.chain; + ASSERT_EQ(4U, chain.size()); newintermediate_ = chain[1]; newroot_ = TrustAnchor::CreateFromCertificateNoConstraints(chain[2]); diff --git a/chromium/net/cert/internal/verify_certificate_chain.cc b/chromium/net/cert/internal/verify_certificate_chain.cc index 041a6fc7842..5f2b7d81bd7 100644 --- a/chromium/net/cert/internal/verify_certificate_chain.cc +++ b/chromium/net/cert/internal/verify_certificate_chain.cc @@ -9,8 +9,8 @@ #include "base/logging.h" #include "base/memory/ptr_util.h" #include "net/cert/internal/cert_error_params.h" -#include "net/cert/internal/cert_error_scoper.h" #include "net/cert/internal/cert_errors.h" +#include "net/cert/internal/extended_key_usage.h" #include "net/cert/internal/name_constraints.h" #include "net/cert/internal/parse_certificate.h" #include "net/cert/internal/signature_algorithm.h" @@ -22,6 +22,9 @@ namespace net { +DEFINE_CERT_ERROR_ID(kValidityFailedNotAfter, "Time is after notAfter"); +DEFINE_CERT_ERROR_ID(kValidityFailedNotBefore, "Time is before notBefore"); + namespace { // ----------------------------------------------- @@ -50,51 +53,44 @@ DEFINE_CERT_ERROR_ID(kNotPermittedByNameConstraints, DEFINE_CERT_ERROR_ID(kSubjectDoesNotMatchIssuer, "subject does not match issuer"); DEFINE_CERT_ERROR_ID(kVerifySignedDataFailed, "VerifySignedData failed"); -DEFINE_CERT_ERROR_ID(kValidityFailedNotAfter, "Time is after notAfter"); -DEFINE_CERT_ERROR_ID(kValidityFailedNotBefore, "Time is before notBefore"); DEFINE_CERT_ERROR_ID(kSignatureAlgorithmsDifferentEncoding, "Certificate.signatureAlgorithm is encoded differently " "than TBSCertificate.signature"); +DEFINE_CERT_ERROR_ID(kEkuLacksServerAuth, + "The extended key usage does not include server auth"); +DEFINE_CERT_ERROR_ID(kEkuLacksClientAuth, + "The extended key usage does not include client auth"); -DEFINE_CERT_ERROR_ID(kContextTrustAnchor, "Processing Trust Anchor"); -DEFINE_CERT_ERROR_ID(kContextCertificate, "Processing Certificate"); - -// This class changes the error scope to indicate which certificate in the -// chain is currently being processed. -class CertErrorScoperForCert : public CertErrorScoper { - public: - CertErrorScoperForCert(CertErrors* parent_errors, size_t index) - : CertErrorScoper(parent_errors), index_(index) {} - - std::unique_ptr<CertErrorNode> BuildRootNode() override { - return base::MakeUnique<CertErrorNode>( - CertErrorNodeType::TYPE_CONTEXT, kContextCertificate, - CreateCertErrorParams1SizeT("index", index_)); - } - - private: - size_t index_; +bool IsHandledCriticalExtensionOid(const der::Input& oid) { + if (oid == BasicConstraintsOid()) + return true; + if (oid == KeyUsageOid()) + return true; + if (oid == ExtKeyUsageOid()) + return true; + if (oid == NameConstraintsOid()) + return true; + // TODO(eroman): SubjectAltName isn't actually used here, but rather is being + // checked by a higher layer. + if (oid == SubjectAltNameOid()) + return true; - DISALLOW_COPY_AND_ASSIGN(CertErrorScoperForCert); -}; + // TODO(eroman): Make this more complete. + return false; +} -// Returns true if the certificate does not contain any unconsumed _critical_ +// Adds errors to |errors| if the certificate contains unconsumed _critical_ // extensions. -WARN_UNUSED_RESULT bool VerifyNoUnconsumedCriticalExtensions( - const ParsedCertificate& cert, - CertErrors* errors) { - bool has_unconsumed_critical_extensions = false; - - for (const auto& entry : cert.unparsed_extensions()) { - if (entry.second.critical) { - has_unconsumed_critical_extensions = true; +void VerifyNoUnconsumedCriticalExtensions(const ParsedCertificate& cert, + CertErrors* errors) { + for (const auto& it : cert.extensions()) { + const ParsedExtension& extension = it.second; + if (extension.critical && !IsHandledCriticalExtensionOid(extension.oid)) { errors->AddError(kUnconsumedCriticalExtension, - CreateCertErrorParams2Der("oid", entry.second.oid, - "value", entry.second.value)); + CreateCertErrorParams2Der("oid", extension.oid, "value", + extension.value)); } } - - return !has_unconsumed_critical_extensions; } // Returns true if |cert| was self-issued. The definition of self-issuance @@ -112,30 +108,25 @@ WARN_UNUSED_RESULT bool IsSelfIssued(const ParsedCertificate& cert) { return cert.normalized_subject() == cert.normalized_issuer(); } -// Returns true if |cert| is valid at time |time|. +// Adds errors to |errors| if |cert| is not valid at time |time|. // // The certificate's validity requirements are described by RFC 5280 section // 4.1.2.5: // // The validity period for a certificate is the period of time from // notBefore through notAfter, inclusive. -WARN_UNUSED_RESULT bool VerifyTimeValidity(const ParsedCertificate& cert, - const der::GeneralizedTime time, - CertErrors* errors) { - if (time < cert.tbs().validity_not_before) { +void VerifyTimeValidity(const ParsedCertificate& cert, + const der::GeneralizedTime time, + CertErrors* errors) { + if (time < cert.tbs().validity_not_before) errors->AddError(kValidityFailedNotBefore); - return false; - } - if (cert.tbs().validity_not_after < time) { + if (cert.tbs().validity_not_after < time) errors->AddError(kValidityFailedNotAfter); - return false; - } - - return true; } -// Returns true if |cert| has internally consistent signature algorithms. +// Adds errors to |errors| if |cert| has internally inconsistent signature +// algorithms. // // X.509 certificates contain two different signature algorithms: // (1) The signatureAlgorithm field of Certificate @@ -154,16 +145,15 @@ WARN_UNUSED_RESULT bool VerifyTimeValidity(const ParsedCertificate& cert, // In practice however there are certificates which use different encodings for // specifying RSA with SHA1 (different OIDs). This is special-cased for // compatibility sake. -WARN_UNUSED_RESULT bool VerifySignatureAlgorithmsMatch( - const ParsedCertificate& cert, - CertErrors* errors) { +void VerifySignatureAlgorithmsMatch(const ParsedCertificate& cert, + CertErrors* errors) { const der::Input& alg1_tlv = cert.signature_algorithm_tlv(); const der::Input& alg2_tlv = cert.tbs().signature_algorithm_tlv; // Ensure that the two DER-encoded signature algorithms are byte-for-byte // equal. if (alg1_tlv == alg2_tlv) - return true; + return; // But make a compatibility concession if alternate encodings are used // TODO(eroman): Turn this warning into an error. @@ -173,20 +163,58 @@ WARN_UNUSED_RESULT bool VerifySignatureAlgorithmsMatch( kSignatureAlgorithmsDifferentEncoding, CreateCertErrorParams2Der("Certificate.algorithm", alg1_tlv, "TBSCertificate.signature", alg2_tlv)); - return true; + return; } errors->AddError( kSignatureAlgorithmMismatch, CreateCertErrorParams2Der("Certificate.algorithm", alg1_tlv, "TBSCertificate.signature", alg2_tlv)); +} - return false; +// Verify that |cert| can be used for |required_key_purpose|. +void VerifyExtendedKeyUsage(const ParsedCertificate& cert, + KeyPurpose required_key_purpose, + CertErrors* errors) { + switch (required_key_purpose) { + case KeyPurpose::ANY_EKU: + return; + case KeyPurpose::SERVER_AUTH: { + // TODO(eroman): Is it OK for the target certificate to omit the EKU? + if (!cert.has_extended_key_usage()) + return; + + for (const auto& key_purpose_oid : cert.extended_key_usage()) { + if (key_purpose_oid == AnyEKU()) + return; + if (key_purpose_oid == ServerAuth()) + return; + } + + errors->AddError(kEkuLacksServerAuth); + break; + } + case KeyPurpose::CLIENT_AUTH: { + // TODO(eroman): Is it OK for the target certificate to omit the EKU? + if (!cert.has_extended_key_usage()) + return; + + for (const auto& key_purpose_oid : cert.extended_key_usage()) { + if (key_purpose_oid == AnyEKU()) + return; + if (key_purpose_oid == ClientAuth()) + return; + } + + errors->AddError(kEkuLacksClientAuth); + break; + } + } } // This function corresponds to RFC 5280 section 6.1.3's "Basic Certificate // Processing" procedure. -WARN_UNUSED_RESULT bool BasicCertificateProcessing( +void BasicCertificateProcessing( const ParsedCertificate& cert, bool is_target_cert, const SignaturePolicy* signature_policy, @@ -198,8 +226,7 @@ WARN_UNUSED_RESULT bool BasicCertificateProcessing( // Check that the signature algorithms in Certificate vs TBSCertificate // match. This isn't part of RFC 5280 section 6.1.3, but is mandated by // sections 4.1.1.2 and 4.1.2.3. - if (!VerifySignatureAlgorithmsMatch(cert, errors)) - return false; + VerifySignatureAlgorithmsMatch(cert, errors); // Verify the digital signature using the previous certificate's key (RFC // 5280 section 6.1.3 step a.1). @@ -207,30 +234,25 @@ WARN_UNUSED_RESULT bool BasicCertificateProcessing( errors->AddError( kInvalidOrUnsupportedSignatureAlgorithm, CreateCertErrorParams1Der("algorithm", cert.signature_algorithm_tlv())); - return false; - } - - if (!VerifySignedData(cert.signature_algorithm(), cert.tbs_certificate_tlv(), - cert.signature_value(), working_spki, signature_policy, - errors)) { - errors->AddError(kVerifySignedDataFailed); - return false; + } else { + if (!VerifySignedData(cert.signature_algorithm(), + cert.tbs_certificate_tlv(), cert.signature_value(), + working_spki, signature_policy, errors)) { + errors->AddError(kVerifySignedDataFailed); + } } // Check the time range for the certificate's validity, ensuring it is valid // at |time|. // (RFC 5280 section 6.1.3 step a.2) - if (!VerifyTimeValidity(cert, time, errors)) - return false; + VerifyTimeValidity(cert, time, errors); // TODO(eroman): Check revocation (RFC 5280 section 6.1.3 step a.3) // Verify the certificate's issuer name matches the issuing certificate's // subject name. (RFC 5280 section 6.1.3 step a.4) - if (cert.normalized_issuer() != working_normalized_issuer_name) { + if (cert.normalized_issuer() != working_normalized_issuer_name) errors->AddError(kSubjectDoesNotMatchIssuer); - return false; - } // Name constraints (RFC 5280 section 6.1.3 step b & c) // If certificate i is self-issued and it is not the final certificate in the @@ -241,20 +263,17 @@ WARN_UNUSED_RESULT bool BasicCertificateProcessing( if (!nc->IsPermittedCert(cert.normalized_subject(), cert.subject_alt_names())) { errors->AddError(kNotPermittedByNameConstraints); - return false; } } } // TODO(eroman): Steps d-f are omitted, as policy constraints are not yet // implemented. - - return true; } // This function corresponds to RFC 5280 section 6.1.4's "Preparation for // Certificate i+1" procedure. |cert| is expected to be an intermediate. -WARN_UNUSED_RESULT bool PrepareForNextCertificate( +void PrepareForNextCertificate( const ParsedCertificate& cert, size_t* max_path_length_ptr, der::Input* working_spki, @@ -300,12 +319,8 @@ WARN_UNUSED_RESULT bool PrepareForNextCertificate( // can't contain a BasicConstraints extension. if (!cert.has_basic_constraints()) { errors->AddError(kMissingBasicConstraints); - return false; - } - - if (!cert.basic_constraints().is_ca) { + } else if (!cert.basic_constraints().is_ca) { errors->AddError(kBasicConstraintsIndicatesNotCa); - return false; } // From RFC 5280 section 6.1.4 step l: @@ -316,9 +331,9 @@ WARN_UNUSED_RESULT bool PrepareForNextCertificate( if (!IsSelfIssued(cert)) { if (*max_path_length_ptr == 0) { errors->AddError(kMaxPathLengthViolated); - return false; + } else { + --(*max_path_length_ptr); } - --(*max_path_length_ptr); } // From RFC 5280 section 6.1.4 step m: @@ -326,7 +341,7 @@ WARN_UNUSED_RESULT bool PrepareForNextCertificate( // If pathLenConstraint is present in the certificate and is // less than max_path_length, set max_path_length to the value // of pathLenConstraint. - if (cert.basic_constraints().has_path_len && + if (cert.has_basic_constraints() && cert.basic_constraints().has_path_len && cert.basic_constraints().path_len < *max_path_length_ptr) { *max_path_length_ptr = cert.basic_constraints().path_len; } @@ -338,7 +353,6 @@ WARN_UNUSED_RESULT bool PrepareForNextCertificate( if (cert.has_key_usage() && !cert.key_usage().AssertsBit(KEY_USAGE_BIT_KEY_CERT_SIGN)) { errors->AddError(kKeyCertSignBitNotSet); - return false; } // From RFC 5280 section 6.1.4 step o: @@ -347,15 +361,12 @@ WARN_UNUSED_RESULT bool PrepareForNextCertificate( // the certificate. Process any other recognized non-critical // extension present in the certificate that is relevant to path // processing. - if (!VerifyNoUnconsumedCriticalExtensions(cert, errors)) - return false; - - return true; + VerifyNoUnconsumedCriticalExtensions(cert, errors); } // Checks that if the target certificate has properties that only a CA should // have (keyCertSign, CA=true, pathLenConstraint), then its other properties -// are consistent with being a CA. +// are consistent with being a CA. If it does, adds errors to |errors|. // // This follows from some requirements in RFC 5280 section 4.2.1.9. In // particular: @@ -375,9 +386,8 @@ WARN_UNUSED_RESULT bool PrepareForNextCertificate( // TODO(eroman): I don't believe Firefox enforces the keyCertSign restriction // for compatibility reasons. Investigate if we need to similarly relax this // constraint. -WARN_UNUSED_RESULT bool VerifyTargetCertHasConsistentCaBits( - const ParsedCertificate& cert, - CertErrors* errors) { +void VerifyTargetCertHasConsistentCaBits(const ParsedCertificate& cert, + CertErrors* errors) { // Check if the certificate contains any property specific to CAs. bool has_ca_property = (cert.has_basic_constraints() && @@ -397,17 +407,12 @@ WARN_UNUSED_RESULT bool VerifyTargetCertHasConsistentCaBits( // TODO(eroman): Add DER for basic constraints and key usage. errors->AddError(kTargetCertInconsistentCaBits); } - - return success; } - - return true; } // This function corresponds with RFC 5280 section 6.1.5's "Wrap-Up Procedure". // It does processing for the final certificate (the target cert). -WARN_UNUSED_RESULT bool WrapUp(const ParsedCertificate& cert, - CertErrors* errors) { +void WrapUp(const ParsedCertificate& cert, CertErrors* errors) { // TODO(crbug.com/634452): Steps a-b are omitted as policy constraints are not // yet implemented. @@ -423,40 +428,38 @@ WARN_UNUSED_RESULT bool WrapUp(const ParsedCertificate& cert, // // Note that this is duplicated by PrepareForNextCertificate() so as to // directly match the procedures in RFC 5280's section 6.1. - if (!VerifyNoUnconsumedCriticalExtensions(cert, errors)) - return false; + VerifyNoUnconsumedCriticalExtensions(cert, errors); // TODO(eroman): Step g is omitted, as policy constraints are not yet // implemented. // The following check is NOT part of RFC 5280 6.1.5's "Wrap-Up Procedure", // however is implied by RFC 5280 section 4.2.1.9. - if (!VerifyTargetCertHasConsistentCaBits(cert, errors)) - return false; - - return true; + VerifyTargetCertHasConsistentCaBits(cert, errors); } // Initializes the path validation algorithm given anchor constraints. This // follows the description in RFC 5937 -WARN_UNUSED_RESULT bool ProcessTrustAnchorConstraints( +void ProcessTrustAnchorConstraints( const TrustAnchor& trust_anchor, + KeyPurpose required_key_purpose, size_t* max_path_length_ptr, std::vector<const NameConstraints*>* name_constraints_list, CertErrors* errors) { - // Set the trust anchor as the current context for any subsequent errors. - CertErrorScoperNoParams error_context(errors, kContextTrustAnchor); - // In RFC 5937 the enforcement of anchor constraints is governed by the input // enforceTrustAnchorConstraints to path validation. In our implementation // this is always on, and enforcement is controlled solely by whether or not // the trust anchor specified constraints. if (!trust_anchor.enforces_constraints()) - return true; + return; // Anchor constraints are encoded via the attached certificate. const ParsedCertificate& cert = *trust_anchor.cert(); + // This is not part of RFC 5937 nor RFC 5280, but matches the EKU handling + // done for intermediates (described in Web PKI's Baseline Requirements). + VerifyExtendedKeyUsage(cert, required_key_purpose, errors); + // The following enforcements follow from RFC 5937 (primarily section 3.2): // Initialize name constraints initial-permitted/excluded-subtrees. @@ -491,29 +494,26 @@ WARN_UNUSED_RESULT bool ProcessTrustAnchorConstraints( // Extensions may be marked critical or not critical. When trust anchor // constraints are enforced, clients MUST reject certification paths // containing a trust anchor with unrecognized critical extensions. - if (!VerifyNoUnconsumedCriticalExtensions(cert, errors)) - return false; - - return true; + VerifyNoUnconsumedCriticalExtensions(cert, errors); } -} // namespace - // This implementation is structured to mimic the description of certificate // path verification given by RFC 5280 section 6.1. -bool VerifyCertificateChain(const ParsedCertificateList& certs, - const TrustAnchor* trust_anchor, - const SignaturePolicy* signature_policy, - const der::GeneralizedTime& time, - CertErrors* errors) { +void VerifyCertificateChainNoReturnValue( + const ParsedCertificateList& certs, + const TrustAnchor* trust_anchor, + const SignaturePolicy* signature_policy, + const der::GeneralizedTime& time, + KeyPurpose required_key_purpose, + CertPathErrors* errors) { DCHECK(trust_anchor); DCHECK(signature_policy); DCHECK(errors); // An empty chain is necessarily invalid. if (certs.empty()) { - errors->AddError(kChainIsEmpty); - return false; + errors->GetOtherErrors()->AddError(kChainIsEmpty); + return; } // Will contain a NameConstraints for each previous cert in the chain which @@ -556,10 +556,13 @@ bool VerifyCertificateChain(const ParsedCertificateList& certs, size_t max_path_length = certs.size(); // Apply any trust anchor constraints per RFC 5937. - if (!ProcessTrustAnchorConstraints(*trust_anchor, &max_path_length, - &name_constraints_list, errors)) { - return false; - } + // + // TODO(eroman): Errors on the trust anchor are put into a certificate bucket + // GetErrorsForCert(certs.size()). This is a bit magical, and + // has some integration issues. + ProcessTrustAnchorConstraints(*trust_anchor, required_key_purpose, + &max_path_length, &name_constraints_list, + errors->GetErrorsForCert(certs.size())); // Iterate over all the certificates in the reverse direction: starting from // the certificate signed by trust anchor and progressing towards the target @@ -579,28 +582,33 @@ bool VerifyCertificateChain(const ParsedCertificateList& certs, const ParsedCertificate& cert = *certs[index_into_certs]; - // Set the current certificate as the context for any subsequent errors. - CertErrorScoperForCert error_context(errors, i); + // Output errors for the current certificate into an error bucket that is + // associated with that certificate. + CertErrors* cert_errors = errors->GetErrorsForCert(index_into_certs); // Per RFC 5280 section 6.1: // * Do basic processing for each certificate // * If it is the last certificate in the path (target certificate) // - Then run "Wrap up" // - Otherwise run "Prepare for Next cert" - if (!BasicCertificateProcessing( - cert, is_target_cert, signature_policy, time, working_spki, - working_normalized_issuer_name, name_constraints_list, errors)) { - return false; - } + BasicCertificateProcessing(cert, is_target_cert, signature_policy, time, + working_spki, working_normalized_issuer_name, + name_constraints_list, cert_errors); + + // The key purpose is checked not just for the end-entity certificate, but + // also interpreted as a constraint when it appears in intermediates. This + // goes beyond what RFC 5280 describes, but is the de-facto standard. See + // https://wiki.mozilla.org/CA:CertificatePolicyV2.1#Frequently_Asked_Questions + VerifyExtendedKeyUsage(cert, required_key_purpose, cert_errors); + if (!is_target_cert) { - if (!PrepareForNextCertificate(cert, &max_path_length, &working_spki, - &working_normalized_issuer_name, - &name_constraints_list, errors)) { - return false; - } + PrepareForNextCertificate(cert, &max_path_length, &working_spki, + &working_normalized_issuer_name, + &name_constraints_list, cert_errors); } else { - if (!WrapUp(cert, errors)) - return false; + WrapUp(cert, cert_errors); + // TODO(eroman): Verify the Key Usage on target is consistent with + // key_purpose. } } @@ -608,8 +616,22 @@ bool VerifyCertificateChain(const ParsedCertificateList& certs, // // A certificate MUST NOT appear more than once in a prospective // certification path. +} + +} // namespace - return true; +bool VerifyCertificateChain(const ParsedCertificateList& certs, + const TrustAnchor* trust_anchor, + const SignaturePolicy* signature_policy, + const der::GeneralizedTime& time, + KeyPurpose required_key_purpose, + CertPathErrors* errors) { + // TODO(eroman): This function requires that |errors| is empty upon entry, + // which is not part of the API contract. + DCHECK(!errors->ContainsHighSeverityErrors()); + VerifyCertificateChainNoReturnValue(certs, trust_anchor, signature_policy, + time, required_key_purpose, errors); + return !errors->ContainsHighSeverityErrors(); } } // namespace net diff --git a/chromium/net/cert/internal/verify_certificate_chain.h b/chromium/net/cert/internal/verify_certificate_chain.h index 428bd7b0214..7abeede6dd4 100644 --- a/chromium/net/cert/internal/verify_certificate_chain.h +++ b/chromium/net/cert/internal/verify_certificate_chain.h @@ -23,6 +23,13 @@ struct GeneralizedTime; class SignaturePolicy; class TrustAnchor; +// The key purpose (extended key usage) to check for during verification. +enum class KeyPurpose { + ANY_EKU, + SERVER_AUTH, + CLIENT_AUTH, +}; + // VerifyCertificateChain() verifies a certificate path (chain) based on the // rules in RFC 5280. The caller is responsible for building the path and // finding the trust anchor. @@ -56,23 +63,31 @@ class TrustAnchor; // time: // The UTC time to use for expiration checks. // +// key_purpose: +// The key purpose that the target certificate needs to be valid for. +// // --------- // Outputs // --------- // // Returns true if the target certificate can be verified. +// TODO(eroman): This return value is redundant with the |errors| parameter. // // errors: // Must be non-null. The set of errors/warnings encountered while -// validating the path are appended to this structure. There is no -// guarantee that on success |errors| is empty, or conversely that -// on failure |errors| is non-empty. Consumers must only use the -// boolean return value to determine success/failure. +// validating the path are appended to this structure. If verification +// failed, then there is guaranteed to be at least 1 error written to +// |errors|. NET_EXPORT bool VerifyCertificateChain(const ParsedCertificateList& certs, const TrustAnchor* trust_anchor, const SignaturePolicy* signature_policy, const der::GeneralizedTime& time, - CertErrors* errors) WARN_UNUSED_RESULT; + KeyPurpose required_key_purpose, + CertPathErrors* errors); + +// TODO(crbug.com/634443): Move exported errors to a central location? +extern CertErrorId kValidityFailedNotAfter; +extern CertErrorId kValidityFailedNotBefore; } // namespace net diff --git a/chromium/net/cert/internal/verify_certificate_chain_pkits_unittest.cc b/chromium/net/cert/internal/verify_certificate_chain_pkits_unittest.cc index 52a5ff88a0d..97a81cc73b7 100644 --- a/chromium/net/cert/internal/verify_certificate_chain_pkits_unittest.cc +++ b/chromium/net/cert/internal/verify_certificate_chain_pkits_unittest.cc @@ -57,14 +57,15 @@ class VerifyCertificateChainPkitsTestDelegate { // PKITS lists chains from trust anchor to target, VerifyCertificateChain // takes them starting with the target and not including the trust anchor. std::vector<scoped_refptr<net::ParsedCertificate>> input_chain; - CertErrors errors; + CertErrors parsing_errors; for (auto i = cert_ders.rbegin(); i != cert_ders.rend(); ++i) { if (!net::ParsedCertificate::CreateAndAddToVector( bssl::UniquePtr<CRYPTO_BUFFER>( CRYPTO_BUFFER_new(reinterpret_cast<const uint8_t*>(i->data()), i->size(), nullptr)), - {}, &input_chain, &errors)) { - ADD_FAILURE() << "Cert failed to parse:\n" << errors.ToDebugString(); + {}, &input_chain, &parsing_errors)) { + ADD_FAILURE() << "Cert failed to parse:\n" + << parsing_errors.ToDebugString(); return false; } } @@ -78,13 +79,13 @@ class VerifyCertificateChainPkitsTestDelegate { // Run all tests at the time the PKITS was published. der::GeneralizedTime time = {2011, 4, 15, 0, 0, 0}; + CertPathErrors path_errors; bool result = VerifyCertificateChain(input_chain, trust_anchor.get(), - &signature_policy, time, &errors); + &signature_policy, time, + KeyPurpose::ANY_EKU, &path_errors); // TODO(crbug.com/634443): Test errors on failure? - if (!result) - EXPECT_FALSE(errors.empty()); - + EXPECT_EQ(result, !path_errors.ContainsHighSeverityErrors()); return result; } }; diff --git a/chromium/net/cert/internal/verify_certificate_chain_typed_unittest.h b/chromium/net/cert/internal/verify_certificate_chain_typed_unittest.h index 8c64d916cc3..48e401869e3 100644 --- a/chromium/net/cert/internal/verify_certificate_chain_typed_unittest.h +++ b/chromium/net/cert/internal/verify_certificate_chain_typed_unittest.h @@ -8,6 +8,7 @@ #include "net/cert/internal/parsed_certificate.h" #include "net/cert/internal/test_helpers.h" #include "net/cert/internal/trust_store.h" +#include "net/cert/internal/verify_certificate_chain.h" #include "net/cert/pem_tokenizer.h" #include "net/der/input.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,20 +19,16 @@ template <typename TestDelegate> class VerifyCertificateChainTest : public ::testing::Test { public: void RunTest(const char* file_name) { - ParsedCertificateList chain; - scoped_refptr<TrustAnchor> trust_anchor; - der::GeneralizedTime time; - bool expected_result; - std::string expected_errors; + VerifyCertChainTest test; std::string path = std::string("net/data/verify_certificate_chain_unittest/") + file_name; - ReadVerifyCertChainTestFromFile(path, &chain, &trust_anchor, &time, - &expected_result, &expected_errors); + SCOPED_TRACE("Test file: " + path); - TestDelegate::Verify(chain, trust_anchor, time, expected_result, - expected_errors, path); + ReadVerifyCertChainTestFromFile(path, &test); + + TestDelegate::Verify(test, path); } }; @@ -43,60 +40,42 @@ class VerifyCertificateChainSingleRootTest TYPED_TEST_CASE_P(VerifyCertificateChainSingleRootTest); -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetAndIntermediate) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, Simple) { this->RunTest("target-and-intermediate.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IntermediateLacksBasicConstraints) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, BasicConstraintsCa) { this->RunTest("intermediate-lacks-basic-constraints.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IntermediateBasicConstraintsCaFalse) { this->RunTest("intermediate-basic-constraints-ca-false.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IntermediateBasicConstraintsNotCritical) { this->RunTest("intermediate-basic-constraints-not-critical.pem"); + this->RunTest("unconstrained-root-lacks-basic-constraints.pem"); + this->RunTest("constrained-root-lacks-basic-constraints.pem"); + this->RunTest("unconstrained-root-basic-constraints-ca-false.pem"); + this->RunTest("constrained-root-basic-constraints-ca-false.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IntermediateLacksSigningKeyUsage) { - this->RunTest("intermediate-lacks-signing-key-usage.pem"); +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, BasicConstraintsPathlen) { + this->RunTest("violates-basic-constraints-pathlen-0.pem"); + this->RunTest("basic-constraints-pathlen-0-self-issued.pem"); + this->RunTest("target-has-pathlen-but-not-ca.pem"); + this->RunTest("violates-pathlen-1-constrained-root.pem"); + this->RunTest("violates-pathlen-1-unconstrained-root.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IntermediateUnknownCriticalExtension) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, UnknownExtension) { this->RunTest("intermediate-unknown-critical-extension.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IntermediateUnknownNonCriticalExtension) { this->RunTest("intermediate-unknown-non-critical-extension.pem"); + this->RunTest("target-unknown-critical-extension.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - ViolatesBasicConstraintsPathlen0) { - this->RunTest("violates-basic-constraints-pathlen-0.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - BasicConstraintsPathlen0SelfIssued) { - this->RunTest("basic-constraints-pathlen-0-self-issued.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetSignedWithMd5) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, Md5) { this->RunTest("target-signed-with-md5.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, IntermediateSignedWithMd5) { this->RunTest("intermediate-signed-with-md5.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetWrongSignature) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, WrongSignature) { this->RunTest("target-wrong-signature.pem"); + this->RunTest("incorrect-trust-anchor.pem"); } TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetSignedBy512bitRsa) { @@ -107,23 +86,11 @@ TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetSignedUsingEcdsa) { this->RunTest("target-signed-using-ecdsa.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, ExpiredIntermediate) { - this->RunTest("expired-intermediate.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, ExpiredTarget) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, Expired) { this->RunTest("expired-target.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, ExpiredTargetNotBefore) { + this->RunTest("expired-intermediate.pem"); this->RunTest("expired-target-notBefore.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, ExpiredUnconstrainedRoot) { this->RunTest("expired-unconstrained-root.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, ExpiredConstrainedRoot) { this->RunTest("expired-constrained-root.pem"); } @@ -131,138 +98,60 @@ TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetNotEndEntity) { this->RunTest("target-not-end-entity.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - TargetHasKeyCertSignButNotCa) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, KeyUsage) { + this->RunTest("intermediate-lacks-signing-key-usage.pem"); this->RunTest("target-has-keycertsign-but-not-ca.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetHasPathlenButNotCa) { - this->RunTest("target-has-pathlen-but-not-ca.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - TargetUnknownCriticalExtension) { - this->RunTest("target-unknown-critical-extension.pem"); +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, ExtendedKeyUsage) { + this->RunTest("target-lacks-eku.pem"); + this->RunTest("target-restricts-eku-fail.pem"); + this->RunTest("intermediate-restricts-eku-fail.pem"); + this->RunTest("intermediate-restricts-eku-ok.pem"); + this->RunTest("intermediate-sets-eku-any.pem"); + this->RunTest("target-sets-eku-any.pem"); + this->RunTest("constrained-root-bad-eku.pem"); + this->RunTest("unconstrained-root-bad-eku.pem"); } TYPED_TEST_P(VerifyCertificateChainSingleRootTest, IssuerAndSubjectNotByteForByteEqual) { this->RunTest("issuer-and-subject-not-byte-for-byte-equal.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - IssuerAndSubjectNotByteForByteEqualAnchor) { this->RunTest("issuer-and-subject-not-byte-for-byte-equal-anchor.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - ViolatesPathlen1UnconstrainedRoot) { - this->RunTest("violates-pathlen-1-unconstrained-root.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - ViolatesPathlen1ConstrainedRoot) { - this->RunTest("violates-pathlen-1-constrained-root.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, NonSelfSignedRoot) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TrustAnchorNotSelfSigned) { this->RunTest("non-self-signed-root.pem"); + this->RunTest("unconstrained-non-self-signed-root.pem"); + this->RunTest("constrained-non-self-signed-root.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, KeyRolloverOldChain) { +TYPED_TEST_P(VerifyCertificateChainSingleRootTest, KeyRollover) { this->RunTest("key-rollover-oldchain.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, KeyRolloverRolloverChain) { this->RunTest("key-rollover-rolloverchain.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - KeyRolloverLongRolloverChain) { this->RunTest("key-rollover-longrolloverchain.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, KeyRolloverNewChain) { this->RunTest("key-rollover-newchain.pem"); } -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, IncorrectTrustAnchor) { - this->RunTest("incorrect-trust-anchor.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - UnconstrainedRootLacksBasicConstraints) { - this->RunTest("unconstrained-root-lacks-basic-constraints.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - ConstrainedRootLacksBasicConstraints) { - this->RunTest("constrained-root-lacks-basic-constraints.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - UnconstrainedRootBasicConstraintsCaFalse) { - this->RunTest("unconstrained-root-basic-constraints-ca-false.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - ConstrainedRootBasicConstraintsCaFalse) { - this->RunTest("constrained-root-basic-constraints-ca-false.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - UnconstrainedNonSelfSignedRoot) { - this->RunTest("unconstrained-non-self-signed-root.pem"); -} - -TYPED_TEST_P(VerifyCertificateChainSingleRootTest, - ConstrainedNonSelfSignedRoot) { - this->RunTest("constrained-non-self-signed-root.pem"); -} - // TODO(eroman): Add test that invalid validity dates where the day or month // ordinal not in range, like "March 39, 2016" are rejected. REGISTER_TYPED_TEST_CASE_P(VerifyCertificateChainSingleRootTest, - TargetAndIntermediate, - IntermediateLacksBasicConstraints, - IntermediateBasicConstraintsCaFalse, - IntermediateBasicConstraintsNotCritical, - IntermediateLacksSigningKeyUsage, - IntermediateUnknownCriticalExtension, - IntermediateUnknownNonCriticalExtension, - ViolatesBasicConstraintsPathlen0, - BasicConstraintsPathlen0SelfIssued, - TargetSignedWithMd5, - IntermediateSignedWithMd5, - TargetWrongSignature, + Simple, + BasicConstraintsCa, + BasicConstraintsPathlen, + UnknownExtension, + Md5, + WrongSignature, TargetSignedBy512bitRsa, TargetSignedUsingEcdsa, - ExpiredIntermediate, - ExpiredTarget, - ExpiredTargetNotBefore, - ExpiredUnconstrainedRoot, - ExpiredConstrainedRoot, + Expired, TargetNotEndEntity, - TargetHasKeyCertSignButNotCa, - TargetHasPathlenButNotCa, - TargetUnknownCriticalExtension, + KeyUsage, + ExtendedKeyUsage, IssuerAndSubjectNotByteForByteEqual, - IssuerAndSubjectNotByteForByteEqualAnchor, - ViolatesPathlen1UnconstrainedRoot, - ViolatesPathlen1ConstrainedRoot, - NonSelfSignedRoot, - KeyRolloverOldChain, - KeyRolloverRolloverChain, - KeyRolloverLongRolloverChain, - KeyRolloverNewChain, - IncorrectTrustAnchor, - UnconstrainedRootLacksBasicConstraints, - ConstrainedRootLacksBasicConstraints, - UnconstrainedRootBasicConstraintsCaFalse, - ConstrainedRootBasicConstraintsCaFalse, - UnconstrainedNonSelfSignedRoot, - ConstrainedNonSelfSignedRoot); + TrustAnchorNotSelfSigned, + KeyRollover); } // namespace net diff --git a/chromium/net/cert/internal/verify_certificate_chain_unittest.cc b/chromium/net/cert/internal/verify_certificate_chain_unittest.cc index ec5f637a5a5..62d3b3b3bbe 100644 --- a/chromium/net/cert/internal/verify_certificate_chain_unittest.cc +++ b/chromium/net/cert/internal/verify_certificate_chain_unittest.cc @@ -14,24 +14,20 @@ namespace { class VerifyCertificateChainDelegate { public: - static void Verify(const ParsedCertificateList& chain, - const scoped_refptr<TrustAnchor>& trust_anchor, - const der::GeneralizedTime& time, - bool expected_result, - const std::string& expected_errors, + static void Verify(const VerifyCertChainTest& test, const std::string& test_file_path) { - ASSERT_TRUE(trust_anchor); + ASSERT_TRUE(test.trust_anchor); SimpleSignaturePolicy signature_policy(1024); - CertErrors errors; - bool result = VerifyCertificateChain(chain, trust_anchor.get(), - &signature_policy, time, &errors); - EXPECT_EQ(expected_result, result); - EXPECT_EQ(expected_errors, errors.ToDebugString()) << "Test file: " - << test_file_path; - if (!result) - EXPECT_FALSE(errors.empty()); + CertPathErrors errors; + bool result = VerifyCertificateChain(test.chain, test.trust_anchor.get(), + &signature_policy, test.time, + test.key_purpose, &errors); + EXPECT_EQ(test.expected_result, result); + EXPECT_EQ(test.expected_errors, errors.ToDebugString(test.chain)) + << "Test file: " << test_file_path; + EXPECT_EQ(result, !errors.ContainsHighSeverityErrors()); } }; diff --git a/chromium/net/cert/nss_cert_database.cc b/chromium/net/cert/nss_cert_database.cc index 7a580d6b28e..2b90e008f29 100644 --- a/chromium/net/cert/nss_cert_database.cc +++ b/chromium/net/cert/nss_cert_database.cc @@ -22,7 +22,6 @@ #include "base/task_runner_util.h" #include "base/threading/worker_pool.h" #include "crypto/scoped_nss_types.h" -#include "net/base/crypto_module.h" #include "net/base/net_errors.h" #include "net/cert/cert_database.h" #include "net/cert/x509_certificate.h" @@ -421,8 +420,13 @@ void NSSCertDatabase::ListCertsImpl(crypto::ScopedPK11Slot slot, CERTCertListNode* node; for (node = CERT_LIST_HEAD(cert_list); !CERT_LIST_END(node, cert_list); node = CERT_LIST_NEXT(node)) { - certs->push_back(X509Certificate::CreateFromHandle( - node->cert, X509Certificate::OSCertHandles())); + scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle( + node->cert, X509Certificate::OSCertHandles()); + if (!cert) { + LOG(ERROR) << "X509Certificate::CreateFromHandle failed"; + continue; + } + certs->push_back(cert); } CERT_DestroyCertList(cert_list); } diff --git a/chromium/net/cert/nss_cert_database_chromeos.cc b/chromium/net/cert/nss_cert_database_chromeos.cc index 9f4f9161c9c..a3b0c4cb089 100644 --- a/chromium/net/cert/nss_cert_database_chromeos.cc +++ b/chromium/net/cert/nss_cert_database_chromeos.cc @@ -14,8 +14,8 @@ #include "base/bind.h" #include "base/callback.h" #include "base/location.h" +#include "base/stl_util.h" #include "base/task_runner.h" -#include "net/base/crypto_module.h" #include "net/cert/x509_certificate.h" namespace net { @@ -67,13 +67,9 @@ void NSSCertDatabaseChromeOS::ListModules( NSSCertDatabase::ListModules(modules, need_rw); size_t pre_size = modules->size(); - modules->erase( - std::remove_if( - modules->begin(), - modules->end(), - NSSProfileFilterChromeOS::ModuleNotAllowedForProfilePredicate( - profile_filter_)), - modules->end()); + base::EraseIf(*modules, + NSSProfileFilterChromeOS::ModuleNotAllowedForProfilePredicate( + profile_filter_)); DVLOG(1) << "filtered " << pre_size - modules->size() << " of " << pre_size << " modules"; } @@ -84,12 +80,9 @@ void NSSCertDatabaseChromeOS::ListCertsImpl( NSSCertDatabase::ListCertsImpl(crypto::ScopedPK11Slot(), certs); size_t pre_size = certs->size(); - certs->erase(std::remove_if( - certs->begin(), - certs->end(), - NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate( - profile_filter)), - certs->end()); + base::EraseIf(*certs, + NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate( + profile_filter)); DVLOG(1) << "filtered " << pre_size - certs->size() << " of " << pre_size << " certs"; } diff --git a/chromium/net/cert/nss_cert_database_unittest.cc b/chromium/net/cert/nss_cert_database_unittest.cc index 235ea44cc0d..2bcda9e35aa 100644 --- a/chromium/net/cert/nss_cert_database_unittest.cc +++ b/chromium/net/cert/nss_cert_database_unittest.cc @@ -22,7 +22,6 @@ #include "base/threading/thread_task_runner_handle.h" #include "crypto/scoped_nss_types.h" #include "crypto/scoped_test_nss_db.h" -#include "net/base/crypto_module.h" #include "net/base/hash_value.h" #include "net/base/net_errors.h" #include "net/cert/cert_status_flags.h" @@ -108,8 +107,13 @@ class CertDatabaseNSSTest : public testing::Test { for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list); !CERT_LIST_END(node, cert_list); node = CERT_LIST_NEXT(node)) { - result.push_back(X509Certificate::CreateFromHandle( - node->cert, X509Certificate::OSCertHandles())); + scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle( + node->cert, X509Certificate::OSCertHandles()); + if (!cert) { + ADD_FAILURE() << "X509Certificate::CreateFromHandle failed"; + continue; + } + result.push_back(cert); } CERT_DestroyCertList(cert_list); diff --git a/chromium/net/cert/nss_profile_filter_chromeos_unittest.cc b/chromium/net/cert/nss_profile_filter_chromeos_unittest.cc index 2de8039f5fe..2443ed33dde 100644 --- a/chromium/net/cert/nss_profile_filter_chromeos_unittest.cc +++ b/chromium/net/cert/nss_profile_filter_chromeos_unittest.cc @@ -46,8 +46,13 @@ CertificateList ListCertsInSlot(PK11SlotInfo* slot) { for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list); !CERT_LIST_END(node, cert_list); node = CERT_LIST_NEXT(node)) { - result.push_back(X509Certificate::CreateFromHandle( - node->cert, X509Certificate::OSCertHandles())); + scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromHandle( + node->cert, X509Certificate::OSCertHandles()); + if (!cert) { + ADD_FAILURE() << "X509Certificate::CreateFromHandle failed"; + continue; + } + result.push_back(cert); } CERT_DestroyCertList(cert_list); diff --git a/chromium/net/cert/test_root_certs.h b/chromium/net/cert/test_root_certs.h index c403deac25f..6fa22c1ea00 100644 --- a/chromium/net/cert/test_root_certs.h +++ b/chromium/net/cert/test_root_certs.h @@ -67,7 +67,7 @@ class NET_EXPORT TestRootCerts { #if defined(USE_NSS_CERTS) bool Contains(CERTCertificate* cert) const; -#elif defined(OS_MACOSX) && !defined(USE_NSS_CERTS) +#elif defined(OS_MACOSX) CFArrayRef temporary_roots() const { return temporary_roots_; } // Modifies the root certificates of |trust_ref| to include the @@ -94,7 +94,7 @@ class NET_EXPORT TestRootCerts { #endif private: - friend struct base::DefaultLazyInstanceTraits<TestRootCerts>; + friend struct base::LazyInstanceTraitsBase<TestRootCerts>; TestRootCerts(); ~TestRootCerts(); diff --git a/chromium/net/cert/test_root_certs_mac.cc b/chromium/net/cert/test_root_certs_mac.cc index af1e9c18dbe..f3a51e1f59c 100644 --- a/chromium/net/cert/test_root_certs_mac.cc +++ b/chromium/net/cert/test_root_certs_mac.cc @@ -9,49 +9,25 @@ #include "base/logging.h" #include "net/cert/x509_certificate.h" -namespace net { - -namespace { - -typedef OSStatus (*SecTrustSetAnchorCertificatesOnlyFuncPtr)(SecTrustRef, - Boolean); - -Boolean OurSecCertificateEqual(const void* value1, const void* value2) { - if (CFGetTypeID(value1) != SecCertificateGetTypeID() || - CFGetTypeID(value2) != SecCertificateGetTypeID()) - return CFEqual(value1, value2); - return X509Certificate::IsSameOSCert( - reinterpret_cast<SecCertificateRef>(const_cast<void*>(value1)), - reinterpret_cast<SecCertificateRef>(const_cast<void*>(value2))); -} - -const void* RetainWrapper(CFAllocatorRef unused, const void* value) { - return CFRetain(value); -} - -void ReleaseWrapper(CFAllocatorRef unused, const void* value) { - CFRelease(value); -} +#if defined(OS_IOS) +#include "net/cert/x509_util_ios.h" +#else +#include "net/cert/x509_util_mac.h" +#endif -// CFEqual prior to 10.6 only performed pointer checks on SecCertificateRefs, -// rather than checking if they were the same (logical) certificate, so a -// custom structure is used for the array callbacks. -const CFArrayCallBacks kCertArrayCallbacks = { - 0, // version - RetainWrapper, - ReleaseWrapper, - CFCopyDescription, - OurSecCertificateEqual, -}; - -} // namespace +namespace net { bool TestRootCerts::Add(X509Certificate* certificate) { + base::ScopedCFTypeRef<SecCertificateRef> os_cert( + x509_util::CreateSecCertificateFromX509Certificate(certificate)); + if (!os_cert) + return false; + if (CFArrayContainsValue(temporary_roots_, CFRangeMake(0, CFArrayGetCount(temporary_roots_)), - certificate->os_cert_handle())) + os_cert.get())) return true; - CFArrayAppendValue(temporary_roots_, certificate->os_cert_handle()); + CFArrayAppendValue(temporary_roots_, os_cert.get()); return true; } @@ -80,8 +56,8 @@ void TestRootCerts::SetAllowSystemTrust(bool allow_system_trust) { TestRootCerts::~TestRootCerts() {} void TestRootCerts::Init() { - temporary_roots_.reset(CFArrayCreateMutable(kCFAllocatorDefault, 0, - &kCertArrayCallbacks)); + temporary_roots_.reset( + CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks)); allow_system_trust_ = true; } diff --git a/chromium/net/cert/test_root_certs_nss.cc b/chromium/net/cert/test_root_certs_nss.cc index afa37f13ea2..6f9cc8ad501 100644 --- a/chromium/net/cert/test_root_certs_nss.cc +++ b/chromium/net/cert/test_root_certs_nss.cc @@ -12,10 +12,6 @@ #include "crypto/nss_util.h" #include "net/cert/x509_certificate.h" -#if defined(OS_IOS) -#include "net/cert/x509_util_ios.h" -#endif - namespace net { @@ -30,12 +26,7 @@ TestRootCerts::TrustEntry::~TrustEntry() { } bool TestRootCerts::Add(X509Certificate* certificate) { -#if defined(OS_IOS) - x509_util_ios::NSSCertificate nss_certificate(certificate->os_cert_handle()); - CERTCertificate* cert_handle = nss_certificate.cert_handle(); -#else CERTCertificate* cert_handle = certificate->os_cert_handle(); -#endif // Preserve the original trust bits so that they can be restored when // the certificate is removed. CERTCertTrust original_trust; @@ -92,7 +83,6 @@ bool TestRootCerts::IsEmpty() const { return trust_cache_.empty(); } -#if defined(USE_NSS_CERTS) bool TestRootCerts::Contains(CERTCertificate* cert) const { for (const auto& item : trust_cache_) if (X509Certificate::IsSameOSCert(cert, item->certificate())) @@ -100,7 +90,6 @@ bool TestRootCerts::Contains(CERTCertificate* cert) const { return false; } -#endif TestRootCerts::~TestRootCerts() { Clear(); diff --git a/chromium/net/cert/test_root_certs_win.cc b/chromium/net/cert/test_root_certs_win.cc index 8535725cb4b..e13f173b832 100644 --- a/chromium/net/cert/test_root_certs_win.cc +++ b/chromium/net/cert/test_root_certs_win.cc @@ -41,7 +41,7 @@ struct CryptoAPIInjector { HCRYPTOIDFUNCADDR original_handle; private: - friend struct base::DefaultLazyInstanceTraits<CryptoAPIInjector>; + friend struct base::LazyInstanceTraitsBase<CryptoAPIInjector>; CryptoAPIInjector() : original_function(NULL), diff --git a/chromium/net/cert/x509_certificate.cc b/chromium/net/cert/x509_certificate.cc index 782f4a4c0fa..cf5d2f6222e 100644 --- a/chromium/net/cert/x509_certificate.cc +++ b/chromium/net/cert/x509_certificate.cc @@ -48,7 +48,7 @@ const char kCertificateHeader[] = "CERTIFICATE"; // The PEM block header used for PKCS#7 data const char kPKCS7Header[] = "PKCS7"; -#if !defined(USE_NSS_CERTS) +#if !defined(USE_NSS_CERTS) && !BUILDFLAG(USE_BYTE_CERTS) // A thread-safe cache for OS certificate handles. // // Within each of the supported underlying crypto libraries, a certificate @@ -102,7 +102,7 @@ class X509CertificateCache { // Obtain an instance of X509CertificateCache via a LazyInstance. X509CertificateCache() {} ~X509CertificateCache() {} - friend struct base::DefaultLazyInstanceTraits<X509CertificateCache>; + friend struct base::LazyInstanceTraitsBase<X509CertificateCache>; // You must acquire this lock before using any private data of this object // You must not block while holding this lock. @@ -189,19 +189,20 @@ void X509CertificateCache::Remove(X509Certificate::OSCertHandle cert_handle) { cache_.erase(pos); } } -#endif // !defined(USE_NSS_CERTS) +#endif // !defined(USE_NSS_CERTS) && !BUILDFLAG(USE_BYTE_CERTS) // See X509CertificateCache::InsertOrUpdate. NSS has a built-in cache, so there -// is no point in wrapping another cache around it. +// is no point in wrapping another cache around it. With USE_BYTE_CERTS, the +// CYRPTO_BUFFERs are deduped by a CRYPTO_BUFFER_POOL. void InsertOrUpdateCache(X509Certificate::OSCertHandle* cert_handle) { -#if !defined(USE_NSS_CERTS) +#if !defined(USE_NSS_CERTS) && !BUILDFLAG(USE_BYTE_CERTS) g_x509_certificate_cache.Pointer()->InsertOrUpdate(cert_handle); #endif } // See X509CertificateCache::Remove. void RemoveFromCache(X509Certificate::OSCertHandle cert_handle) { -#if !defined(USE_NSS_CERTS) +#if !defined(USE_NSS_CERTS) && !BUILDFLAG(USE_BYTE_CERTS) g_x509_certificate_cache.Pointer()->Remove(cert_handle); #endif } @@ -230,7 +231,11 @@ scoped_refptr<X509Certificate> X509Certificate::CreateFromHandle( OSCertHandle cert_handle, const OSCertHandles& intermediates) { DCHECK(cert_handle); - return new X509Certificate(cert_handle, intermediates); + scoped_refptr<X509Certificate> cert( + new X509Certificate(cert_handle, intermediates)); + if (!cert->os_cert_handle()) + return nullptr; // Initialize() failed. + return cert; } // static @@ -445,7 +450,10 @@ CertificateList X509Certificate::CreateCertificateListFromBytes( for (OSCertHandles::iterator it = certificates.begin(); it != certificates.end(); ++it) { - results.push_back(CreateFromHandle(*it, OSCertHandles())); + scoped_refptr<X509Certificate> cert = + CreateFromHandle(*it, OSCertHandles()); + if (cert) + results.push_back(std::move(cert)); FreeOSCertHandle(*it); } @@ -711,7 +719,12 @@ X509Certificate::X509Certificate(OSCertHandle cert_handle, intermediate_ca_certs_.push_back(intermediate); } // Platform-specific initialization. - Initialize(); + if (!Initialize() && cert_handle_) { + // Signal initialization failure by clearing cert_handle_. + RemoveFromCache(cert_handle_); + FreeOSCertHandle(cert_handle_); + cert_handle_ = nullptr; + } } X509Certificate::~X509Certificate() { diff --git a/chromium/net/cert/x509_certificate.h b/chromium/net/cert/x509_certificate.h index e11f1042b12..0cb93b5adb7 100644 --- a/chromium/net/cert/x509_certificate.h +++ b/chromium/net/cert/x509_certificate.h @@ -19,8 +19,11 @@ #include "net/base/net_export.h" #include "net/cert/cert_type.h" #include "net/cert/x509_cert_types.h" +#include "net/net_features.h" -#if defined(OS_WIN) +#if BUILDFLAG(USE_BYTE_CERTS) +#include "third_party/boringssl/src/include/openssl/base.h" +#elif defined(OS_WIN) #include <windows.h> #include "crypto/wincrypt_shim.h" #elif defined(OS_MACOSX) @@ -56,7 +59,11 @@ class NET_EXPORT X509Certificate // An OSCertHandle is a handle to a certificate object in the underlying // crypto library. We assume that OSCertHandle is a pointer type on all // platforms and that NULL represents an invalid OSCertHandle. -#if defined(OS_WIN) +#if BUILDFLAG(USE_BYTE_CERTS) + // TODO(mattm): Remove OSCertHandle type and clean up the interfaces once all + // platforms use the CRYPTO_BUFFER version. + typedef CRYPTO_BUFFER* OSCertHandle; +#elif defined(OS_WIN) typedef PCCERT_CONTEXT OSCertHandle; #elif defined(OS_MACOSX) typedef SecCertificateRef OSCertHandle; @@ -127,7 +134,11 @@ class NET_EXPORT X509Certificate }; // Create an X509Certificate from a handle to the certificate object in the - // underlying crypto library. + // underlying crypto library. Returns NULL on failure to parse or extract + // data from the the certificate. Note that this does not guarantee the + // certificate is fully parsed and validated, only that the members of this + // class, such as subject, issuer, expiry times, and serial number, could be + // successfully initialized from the certificate. static scoped_refptr<X509Certificate> CreateFromHandle( OSCertHandle cert_handle, const OSCertHandles& intermediates); @@ -220,10 +231,7 @@ class NET_EXPORT X509Certificate return intermediate_ca_certs_; } -#if defined(OS_MACOSX) - // Does this certificate's usage allow SSL client authentication? - bool SupportsSSLClientAuth() const; - +#if defined(OS_IOS) // Returns a new CFMutableArrayRef containing this certificate and its // intermediate certificates in the form expected by Security.framework // and Keychain Services, or NULL on failure. @@ -391,7 +399,7 @@ class NET_EXPORT X509Certificate ~X509Certificate(); // Common object initialization code. Called by the constructors only. - void Initialize(); + bool Initialize(); #if defined(USE_OPENSSL_CERTS) // Resets the store returned by cert_store() to default state. Used by diff --git a/chromium/net/cert/x509_certificate_bytes.cc b/chromium/net/cert/x509_certificate_bytes.cc new file mode 100644 index 00000000000..ef4e006713f --- /dev/null +++ b/chromium/net/cert/x509_certificate_bytes.cc @@ -0,0 +1,491 @@ +// Copyright 2017 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/x509_certificate.h" + +#include "base/numerics/safe_conversions.h" +#include "base/pickle.h" +#include "crypto/openssl_util.h" +#include "net/base/ip_address.h" +#include "net/cert/asn1_util.h" +#include "net/cert/internal/cert_errors.h" +#include "net/cert/internal/name_constraints.h" +#include "net/cert/internal/parse_name.h" +#include "net/cert/internal/parsed_certificate.h" +#include "net/cert/internal/signature_policy.h" +#include "net/cert/internal/verify_name_match.h" +#include "net/cert/internal/verify_signed_data.h" +#include "net/cert/x509_util.h" +#include "net/cert/x509_util_openssl.h" +#include "net/der/parser.h" +#include "third_party/boringssl/src/include/openssl/evp.h" +#include "third_party/boringssl/src/include/openssl/pool.h" +#include "third_party/boringssl/src/include/openssl/sha.h" + +namespace net { + +namespace { + +// Converts a GeneralizedTime struct to a base::Time, returning true on success +// or false if |generalized| was invalid or cannot be represented by +// base::Time. +bool GeneralizedTimeToBaseTime(const der::GeneralizedTime& generalized, + base::Time* result) { + base::Time::Exploded exploded = {0}; + exploded.year = generalized.year; + exploded.month = generalized.month; + exploded.day_of_month = generalized.day; + exploded.hour = generalized.hours; + exploded.minute = generalized.minutes; + exploded.second = generalized.seconds; + return base::Time::FromUTCExploded(exploded, result); +} + +ParseCertificateOptions DefaultParseCertificateOptions() { + ParseCertificateOptions options; + options.allow_invalid_serial_numbers = true; + return options; +} + +// Sets |value| to the Value from a DER Sequence Tag-Length-Value and return +// true, or return false if the TLV was not a valid DER Sequence. +WARN_UNUSED_RESULT bool GetSequenceValue(const der::Input& tlv, + der::Input* value) { + der::Parser parser(tlv); + return parser.ReadTag(der::kSequence, value) && !parser.HasMore(); +} + +// Normalize |cert|'s Issuer and store it in |out_normalized_issuer|, returning +// true on success or false if there was a parsing error. +bool GetNormalizedCertIssuer(CRYPTO_BUFFER* cert, + std::string* out_normalized_issuer) { + der::Input tbs_certificate_tlv; + der::Input signature_algorithm_tlv; + der::BitString signature_value; + if (!ParseCertificate( + der::Input(CRYPTO_BUFFER_data(cert), CRYPTO_BUFFER_len(cert)), + &tbs_certificate_tlv, &signature_algorithm_tlv, &signature_value, + nullptr)) { + return false; + } + ParsedTbsCertificate tbs; + if (!ParseTbsCertificate(tbs_certificate_tlv, + DefaultParseCertificateOptions(), &tbs, nullptr)) + return false; + + der::Input issuer_value; + if (!GetSequenceValue(tbs.issuer_tlv, &issuer_value)) + return false; + + return NormalizeName(issuer_value, out_normalized_issuer); +} + +// Fills |principal| from the DER encoded |name_tlv|, returning true on success +// or false if parsing failed or some of the values could not be converted to +// UTF-8. +bool ParsePrincipal(const der::Input& name_tlv, CertPrincipal* principal) { + RDNSequence rdns; + if (!ParseName(name_tlv, &rdns)) + return false; + + for (const RelativeDistinguishedName& rdn : rdns) { + for (const X509NameAttribute& name_attribute : rdn) { + if (name_attribute.type == TypeCommonNameOid()) { + if (principal->common_name.empty() && + !name_attribute.ValueAsString(&principal->common_name)) { + return false; + } + } else if (name_attribute.type == TypeLocalityNameOid()) { + if (principal->locality_name.empty() && + !name_attribute.ValueAsString(&principal->locality_name)) { + return false; + } + } else if (name_attribute.type == TypeStateOrProvinceNameOid()) { + if (principal->state_or_province_name.empty() && + !name_attribute.ValueAsString(&principal->state_or_province_name)) { + return false; + } + } else if (name_attribute.type == TypeCountryNameOid()) { + if (principal->country_name.empty() && + !name_attribute.ValueAsString(&principal->country_name)) { + return false; + } + } else if (name_attribute.type == TypeStreetAddressOid()) { + std::string s; + if (!name_attribute.ValueAsString(&s)) + return false; + principal->street_addresses.push_back(s); + } else if (name_attribute.type == TypeOrganizationNameOid()) { + std::string s; + if (!name_attribute.ValueAsString(&s)) + return false; + principal->organization_names.push_back(s); + } else if (name_attribute.type == TypeOrganizationUnitNameOid()) { + std::string s; + if (!name_attribute.ValueAsString(&s)) + return false; + principal->organization_unit_names.push_back(s); + } else if (name_attribute.type == TypeDomainComponentOid()) { + std::string s; + if (!name_attribute.ValueAsString(&s)) + return false; + principal->domain_components.push_back(s); + } + } + } + return true; +} + +// Parses certificates from a PKCS#7 SignedData structure, appending them to +// |handles|. +void CreateOSCertHandlesFromPKCS7Bytes( + const char* data, + size_t length, + X509Certificate::OSCertHandles* handles) { + crypto::EnsureOpenSSLInit(); + crypto::OpenSSLErrStackTracer err_cleaner(FROM_HERE); + + CBS der_data; + CBS_init(&der_data, reinterpret_cast<const uint8_t*>(data), length); + STACK_OF(X509)* certs = sk_X509_new_null(); + + if (PKCS7_get_certificates(certs, &der_data)) { + for (size_t i = 0; i < sk_X509_num(certs); ++i) { + base::StringPiece stringpiece; + x509_util::GetDER(sk_X509_value(certs, i), &stringpiece); + handles->push_back(x509_util::CreateCryptoBuffer(stringpiece).release()); + } + } + sk_X509_pop_free(certs, X509_free); +} + +} // namespace + +bool X509Certificate::Initialize() { + der::Input tbs_certificate_tlv; + der::Input signature_algorithm_tlv; + der::BitString signature_value; + + if (!ParseCertificate(der::Input(CRYPTO_BUFFER_data(cert_handle_), + CRYPTO_BUFFER_len(cert_handle_)), + &tbs_certificate_tlv, &signature_algorithm_tlv, + &signature_value, nullptr)) { + return false; + } + + ParsedTbsCertificate tbs; + if (!ParseTbsCertificate(tbs_certificate_tlv, + DefaultParseCertificateOptions(), &tbs, nullptr)) + return false; + + if (!ParsePrincipal(tbs.subject_tlv, &subject_) || + !ParsePrincipal(tbs.issuer_tlv, &issuer_)) { + return false; + } + + if (!GeneralizedTimeToBaseTime(tbs.validity_not_before, &valid_start_) || + !GeneralizedTimeToBaseTime(tbs.validity_not_after, &valid_expiry_)) { + return false; + } + serial_number_ = tbs.serial_number.AsString(); + return true; +} + +bool X509Certificate::GetSubjectAltName( + std::vector<std::string>* dns_names, + std::vector<std::string>* ip_addrs) const { + if (dns_names) + dns_names->clear(); + if (ip_addrs) + ip_addrs->clear(); + + der::Input tbs_certificate_tlv; + der::Input signature_algorithm_tlv; + der::BitString signature_value; + if (!ParseCertificate(der::Input(CRYPTO_BUFFER_data(cert_handle_), + CRYPTO_BUFFER_len(cert_handle_)), + &tbs_certificate_tlv, &signature_algorithm_tlv, + &signature_value, nullptr)) { + return false; + } + + ParsedTbsCertificate tbs; + if (!ParseTbsCertificate(tbs_certificate_tlv, + DefaultParseCertificateOptions(), &tbs, nullptr)) + return false; + if (!tbs.has_extensions) + return false; + + std::map<der::Input, ParsedExtension> extensions; + if (!ParseExtensions(tbs.extensions_tlv, &extensions)) + return false; + + ParsedExtension subject_alt_names_extension; + if (!ConsumeExtension(SubjectAltNameOid(), &extensions, + &subject_alt_names_extension)) { + return false; + } + + std::unique_ptr<GeneralNames> subject_alt_names = + GeneralNames::Create(subject_alt_names_extension.value); + if (!subject_alt_names) + return false; + + if (dns_names) + *dns_names = subject_alt_names->dns_names; + if (ip_addrs) { + for (const IPAddress& addr : subject_alt_names->ip_addresses) { + ip_addrs->push_back( + std::string(reinterpret_cast<const char*>(addr.bytes().data()), + addr.bytes().size())); + } + } + + return !subject_alt_names->dns_names.empty() || + !subject_alt_names->ip_addresses.empty(); +} + +bool X509Certificate::IsIssuedByEncoded( + const std::vector<std::string>& valid_issuers) { + std::vector<std::string> normalized_issuers; + for (const auto& raw_issuer : valid_issuers) { + der::Input issuer_value; + std::string normalized_issuer; + if (!GetSequenceValue(der::Input(&raw_issuer), &issuer_value) || + !NormalizeName(issuer_value, &normalized_issuer)) { + continue; + } + normalized_issuers.push_back(std::move(normalized_issuer)); + } + + std::string normalized_cert_issuer; + if (!GetNormalizedCertIssuer(cert_handle_, &normalized_cert_issuer)) + return false; + if (std::find(normalized_issuers.begin(), normalized_issuers.end(), + normalized_cert_issuer) != normalized_issuers.end()) + return true; + + for (CRYPTO_BUFFER* intermediate : intermediate_ca_certs_) { + if (!GetNormalizedCertIssuer(intermediate, &normalized_cert_issuer)) + return false; + if (std::find(normalized_issuers.begin(), normalized_issuers.end(), + normalized_cert_issuer) != normalized_issuers.end()) + return true; + } + return false; +} + +// static +bool X509Certificate::GetDEREncoded(X509Certificate::OSCertHandle cert_handle, + std::string* encoded) { + if (!cert_handle) + return false; + encoded->assign( + reinterpret_cast<const char*>(CRYPTO_BUFFER_data(cert_handle)), + CRYPTO_BUFFER_len(cert_handle)); + return true; +} + +// static +void X509Certificate::GetPublicKeyInfo(OSCertHandle cert_handle, + size_t* size_bits, + PublicKeyType* type) { + *type = kPublicKeyTypeUnknown; + *size_bits = 0; + + base::StringPiece spki; + if (!asn1::ExtractSPKIFromDERCert( + base::StringPiece( + reinterpret_cast<const char*>(CRYPTO_BUFFER_data(cert_handle)), + CRYPTO_BUFFER_len(cert_handle)), + &spki)) { + return; + } + + bssl::UniquePtr<EVP_PKEY> pkey; + crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE); + CBS cbs; + CBS_init(&cbs, reinterpret_cast<const uint8_t*>(spki.data()), spki.size()); + pkey.reset(EVP_parse_public_key(&cbs)); + if (!pkey) + return; + + switch (pkey->type) { + case EVP_PKEY_RSA: + *type = kPublicKeyTypeRSA; + break; + case EVP_PKEY_DSA: + *type = kPublicKeyTypeDSA; + break; + case EVP_PKEY_EC: + *type = kPublicKeyTypeECDSA; + break; + case EVP_PKEY_DH: + *type = kPublicKeyTypeDH; + break; + } + *size_bits = base::saturated_cast<size_t>(EVP_PKEY_bits(pkey.get())); +} + +// static +bool X509Certificate::IsSameOSCert(X509Certificate::OSCertHandle a, + X509Certificate::OSCertHandle b) { + DCHECK(a && b); + if (a == b) + return true; + return CRYPTO_BUFFER_len(a) == CRYPTO_BUFFER_len(b) && + memcmp(CRYPTO_BUFFER_data(a), CRYPTO_BUFFER_data(b), + CRYPTO_BUFFER_len(a)) == 0; +} + +// static +X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes( + const char* data, + size_t length) { + der::Input tbs_certificate_tlv; + der::Input signature_algorithm_tlv; + der::BitString signature_value; + // Do a bare minimum of DER parsing here to make sure the input is not + // completely crazy. (This is required for at least + // CreateCertificateListFromBytes with FORMAT_AUTO, if not more.) + if (!ParseCertificate( + der::Input(reinterpret_cast<const uint8_t*>(data), length), + &tbs_certificate_tlv, &signature_algorithm_tlv, &signature_value, + nullptr)) { + return nullptr; + } + + return CRYPTO_BUFFER_new(reinterpret_cast<const uint8_t*>(data), length, + x509_util::GetBufferPool()); +} + +// static +X509Certificate::OSCertHandles X509Certificate::CreateOSCertHandlesFromBytes( + const char* data, + size_t length, + Format format) { + OSCertHandles results; + + switch (format) { + case FORMAT_SINGLE_CERTIFICATE: { + OSCertHandle handle = CreateOSCertHandleFromBytes(data, length); + if (handle) + results.push_back(handle); + break; + } + case FORMAT_PKCS7: { + CreateOSCertHandlesFromPKCS7Bytes(data, length, &results); + break; + } + default: { + NOTREACHED() << "Certificate format " << format << " unimplemented"; + break; + } + } + + return results; +} + +// static +X509Certificate::OSCertHandle X509Certificate::DupOSCertHandle( + OSCertHandle cert_handle) { + CRYPTO_BUFFER_up_ref(cert_handle); + return cert_handle; +} + +// static +void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) { + CRYPTO_BUFFER_free(cert_handle); +} + +// static +SHA256HashValue X509Certificate::CalculateFingerprint256(OSCertHandle cert) { + SHA256HashValue sha256; + + SHA256(CRYPTO_BUFFER_data(cert), CRYPTO_BUFFER_len(cert), sha256.data); + return sha256; +} + +// static +SHA256HashValue X509Certificate::CalculateCAFingerprint256( + const OSCertHandles& intermediates) { + SHA256HashValue sha256; + memset(sha256.data, 0, sizeof(sha256.data)); + + SHA256_CTX sha256_ctx; + SHA256_Init(&sha256_ctx); + for (CRYPTO_BUFFER* cert : intermediates) { + SHA256_Update(&sha256_ctx, CRYPTO_BUFFER_data(cert), + CRYPTO_BUFFER_len(cert)); + } + SHA256_Final(sha256.data, &sha256_ctx); + + return sha256; +} + +// static +bool X509Certificate::IsSelfSigned(OSCertHandle cert_handle) { + der::Input tbs_certificate_tlv; + der::Input signature_algorithm_tlv; + der::BitString signature_value; + if (!ParseCertificate(der::Input(CRYPTO_BUFFER_data(cert_handle), + CRYPTO_BUFFER_len(cert_handle)), + &tbs_certificate_tlv, &signature_algorithm_tlv, + &signature_value, nullptr)) { + return false; + } + ParsedTbsCertificate tbs; + if (!ParseTbsCertificate(tbs_certificate_tlv, + DefaultParseCertificateOptions(), &tbs, nullptr)) { + return false; + } + + der::Input subject_value; + std::string normalized_subject; + if (!GetSequenceValue(tbs.subject_tlv, &subject_value) || + !NormalizeName(subject_value, &normalized_subject)) { + return false; + } + der::Input issuer_value; + std::string normalized_issuer; + if (!GetSequenceValue(tbs.issuer_tlv, &issuer_value) || + !NormalizeName(issuer_value, &normalized_issuer)) { + return false; + } + + if (normalized_subject != normalized_issuer) + return false; + + std::unique_ptr<SignatureAlgorithm> signature_algorithm = + SignatureAlgorithm::Create(signature_algorithm_tlv, nullptr /* errors */); + if (!signature_algorithm) + return false; + + SimpleSignaturePolicy signature_policy(1024); + CertErrors unused_errors; + return VerifySignedData(*signature_algorithm, tbs_certificate_tlv, + signature_value, tbs.spki_tlv, &signature_policy, + &unused_errors); +} + +// static +X509Certificate::OSCertHandle X509Certificate::ReadOSCertHandleFromPickle( + base::PickleIterator* pickle_iter) { + const char* data; + int length; + if (!pickle_iter->ReadData(&data, &length)) + return NULL; + + return CreateOSCertHandleFromBytes(data, length); +} + +// static +bool X509Certificate::WriteOSCertHandleToPickle(OSCertHandle cert_handle, + base::Pickle* pickle) { + return pickle->WriteData( + reinterpret_cast<const char*>(CRYPTO_BUFFER_data(cert_handle)), + CRYPTO_BUFFER_len(cert_handle)); +} + +} // namespace net diff --git a/chromium/net/cert/x509_certificate_ios.cc b/chromium/net/cert/x509_certificate_ios.cc index 1ea686a266d..0c3f162b205 100644 --- a/chromium/net/cert/x509_certificate_ios.cc +++ b/chromium/net/cert/x509_certificate_ios.cc @@ -75,11 +75,11 @@ void ParsePrincipalValues(X509_NAME* name, } } -void ParsePrincipal(X509Certificate::OSCertHandle os_cert, +bool ParsePrincipal(X509Certificate::OSCertHandle os_cert, X509_NAME* x509_name, CertPrincipal* principal) { if (!x509_name) - return; + return false; ParsePrincipalValues(x509_name, NID_streetAddress, &principal->street_addresses); @@ -98,6 +98,7 @@ void ParsePrincipal(X509Certificate::OSCertHandle os_cert, &principal->state_or_province_name); x509_util::ParsePrincipalValueByNID(x509_name, NID_countryName, &principal->country_name); + return true; } bool ParseSubjectAltName(X509Certificate::OSCertHandle os_cert, @@ -165,31 +166,34 @@ void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) { CFRelease(cert_handle); } -void X509Certificate::Initialize() { +bool X509Certificate::Initialize() { crypto::EnsureOpenSSLInit(); bssl::UniquePtr<X509> x509_cert = OSCertHandleToOpenSSL(cert_handle_); if (!x509_cert) - return; + return false; ASN1_INTEGER* serial_num = X509_get_serialNumber(x509_cert.get()); - if (serial_num) { - // ASN1_INTEGERS represent the decoded number, in a format internal to - // OpenSSL. Most notably, this may have leading zeroes stripped off for - // numbers whose first byte is >= 0x80. Thus, it is necessary to - // re-encoded the integer back into DER, which is what the interface - // of X509Certificate exposes, to ensure callers get the proper (DER) - // value. - int bytes_required = i2c_ASN1_INTEGER(serial_num, nullptr); - unsigned char* buffer = reinterpret_cast<unsigned char*>( - base::WriteInto(&serial_number_, bytes_required + 1)); - int bytes_written = i2c_ASN1_INTEGER(serial_num, &buffer); - DCHECK_EQ(static_cast<size_t>(bytes_written), serial_number_.size()); - } - - ParsePrincipal(cert_handle_, X509_get_subject_name(x509_cert.get()), - &subject_); - ParsePrincipal(cert_handle_, X509_get_issuer_name(x509_cert.get()), &issuer_); - x509_util::ParseDate(X509_get_notBefore(x509_cert.get()), &valid_start_); - x509_util::ParseDate(X509_get_notAfter(x509_cert.get()), &valid_expiry_); + if (!serial_num) + return false; + // ASN1_INTEGERS represent the decoded number, in a format internal to + // OpenSSL. Most notably, this may have leading zeroes stripped off for + // numbers whose first byte is >= 0x80. Thus, it is necessary to + // re-encoded the integer back into DER, which is what the interface + // of X509Certificate exposes, to ensure callers get the proper (DER) + // value. + int bytes_required = i2c_ASN1_INTEGER(serial_num, nullptr); + unsigned char* buffer = reinterpret_cast<unsigned char*>( + base::WriteInto(&serial_number_, bytes_required + 1)); + int bytes_written = i2c_ASN1_INTEGER(serial_num, &buffer); + DCHECK_EQ(static_cast<size_t>(bytes_written), serial_number_.size()); + + return ( + ParsePrincipal(cert_handle_, X509_get_subject_name(x509_cert.get()), + &subject_) && + ParsePrincipal(cert_handle_, X509_get_issuer_name(x509_cert.get()), + &issuer_) && + x509_util::ParseDate(X509_get_notBefore(x509_cert.get()), + &valid_start_) && + x509_util::ParseDate(X509_get_notAfter(x509_cert.get()), &valid_expiry_)); } // static @@ -362,10 +366,6 @@ void X509Certificate::GetPublicKeyInfo(OSCertHandle os_cert, *size_bits = EVP_PKEY_bits(key); } -bool X509Certificate::SupportsSSLClientAuth() const { - return false; -} - CFMutableArrayRef X509Certificate::CreateOSCertChainForCert() const { CFMutableArrayRef cert_list = CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks); diff --git a/chromium/net/cert/x509_certificate_mac.cc b/chromium/net/cert/x509_certificate_mac.cc index 3c0b6739aa4..73dbe464f02 100644 --- a/chromium/net/cert/x509_certificate_mac.cc +++ b/chromium/net/cert/x509_certificate_mac.cc @@ -35,16 +35,17 @@ namespace net { namespace { -void GetCertDistinguishedName( +bool GetCertDistinguishedName( const x509_util::CSSMCachedCertificate& cached_cert, const CSSM_OID* oid, CertPrincipal* result) { x509_util::CSSMFieldValue distinguished_name; OSStatus status = cached_cert.GetField(oid, &distinguished_name); if (status || !distinguished_name.field()) - return; + return false; result->ParseDistinguishedName(distinguished_name.field()->Data, distinguished_name.field()->Length); + return true; } bool IsCertIssuerInEncodedList(X509Certificate::OSCertHandle cert_handle, @@ -73,7 +74,7 @@ bool IsCertIssuerInEncodedList(X509Certificate::OSCertHandle cert_handle, return false; } -void GetCertDateForOID(const x509_util::CSSMCachedCertificate& cached_cert, +bool GetCertDateForOID(const x509_util::CSSMCachedCertificate& cached_cert, const CSSM_OID* oid, Time* result) { *result = Time(); @@ -81,14 +82,14 @@ void GetCertDateForOID(const x509_util::CSSMCachedCertificate& cached_cert, x509_util::CSSMFieldValue field; OSStatus status = cached_cert.GetField(oid, &field); if (status) - return; + return false; const CSSM_X509_TIME* x509_time = field.GetAs<CSSM_X509_TIME>(); if (x509_time->timeType != BER_TAG_UTC_TIME && x509_time->timeType != BER_TAG_GENERALIZED_TIME) { LOG(ERROR) << "Unsupported date/time format " << x509_time->timeType; - return; + return false; } base::StringPiece time_string( @@ -96,8 +97,11 @@ void GetCertDateForOID(const x509_util::CSSMCachedCertificate& cached_cert, x509_time->time.Length); CertDateFormat format = x509_time->timeType == BER_TAG_UTC_TIME ? CERT_DATE_FORMAT_UTC_TIME : CERT_DATE_FORMAT_GENERALIZED_TIME; - if (!ParseCertificateDate(time_string, format, result)) + if (!ParseCertificateDate(time_string, format, result)) { LOG(ERROR) << "Invalid certificate date/time " << time_string; + return false; + } + return true; } std::string GetCertSerialNumber( @@ -113,37 +117,6 @@ std::string GetCertSerialNumber( serial_number.field()->Length); } -// Returns true if |purpose| is listed as allowed in |usage|. This -// function also considers the "Any" purpose. If the attribute is -// present and empty, we return false. -bool ExtendedKeyUsageAllows(const CE_ExtendedKeyUsage* usage, - const CSSM_OID* purpose) { - for (unsigned p = 0; p < usage->numPurposes; ++p) { - if (CSSMOIDEqual(&usage->purposes[p], purpose)) - return true; - if (CSSMOIDEqual(&usage->purposes[p], &CSSMOID_ExtendedKeyUsageAny)) - return true; - } - return false; -} - -// Test that a given |cert_handle| is actually a valid X.509 certificate, and -// return true if it is. -// -// On OS X, SecCertificateCreateFromData() does not return any errors if -// called with invalid data, as long as data is present. The actual decoding -// of the certificate does not happen until an API that requires a CSSM -// handle is called. While SecCertificateGetCLHandle is the most likely -// candidate, as it performs the parsing, it does not check whether the -// parsing was actually successful. Instead, SecCertificateGetSubject is -// used (supported since 10.3), as a means to check that the certificate -// parsed as a valid X.509 certificate. -bool IsValidOSCertHandle(SecCertificateRef cert_handle) { - const CSSM_X509_NAME* sanity_check = NULL; - OSStatus status = SecCertificateGetSubject(cert_handle, &sanity_check); - return status == noErr && sanity_check; -} - // Parses |data| of length |length|, attempting to decode it as the specified // |format|. If |data| is in the specified format, any certificates contained // within are stored into |output|. @@ -192,7 +165,7 @@ void AddCertificatesFromBytes(const char* data, size_t length, // |input_format|, causing decode to succeed. On OS X 10.6, the data // is properly decoded as a PKCS#7, whether PEM or not, which avoids // the need to fallback to internal decoding. - if (IsValidOSCertHandle(cert)) { + if (x509_util::IsValidSecCertificate(cert)) { CFRetain(cert); output->push_back(cert); } @@ -202,19 +175,21 @@ void AddCertificatesFromBytes(const char* data, size_t length, } // namespace -void X509Certificate::Initialize() { +bool X509Certificate::Initialize() { x509_util::CSSMCachedCertificate cached_cert; - if (cached_cert.Init(cert_handle_) == CSSM_OK) { - GetCertDistinguishedName(cached_cert, &CSSMOID_X509V1SubjectNameStd, - &subject_); - GetCertDistinguishedName(cached_cert, &CSSMOID_X509V1IssuerNameStd, - &issuer_); - GetCertDateForOID(cached_cert, &CSSMOID_X509V1ValidityNotBefore, - &valid_start_); - GetCertDateForOID(cached_cert, &CSSMOID_X509V1ValidityNotAfter, - &valid_expiry_); - serial_number_ = GetCertSerialNumber(cached_cert); - } + if (cached_cert.Init(cert_handle_) != CSSM_OK) + return false; + serial_number_ = GetCertSerialNumber(cached_cert); + + return (!serial_number_.empty() && + GetCertDistinguishedName(cached_cert, &CSSMOID_X509V1SubjectNameStd, + &subject_) && + GetCertDistinguishedName(cached_cert, &CSSMOID_X509V1IssuerNameStd, + &issuer_) && + GetCertDateForOID(cached_cert, &CSSMOID_X509V1ValidityNotBefore, + &valid_start_) && + GetCertDateForOID(cached_cert, &CSSMOID_X509V1ValidityNotAfter, + &valid_expiry_)); } bool X509Certificate::IsIssuedByEncoded( @@ -299,37 +274,16 @@ bool X509Certificate::GetDEREncoded(X509Certificate::OSCertHandle cert_handle, bool X509Certificate::IsSameOSCert(X509Certificate::OSCertHandle a, X509Certificate::OSCertHandle b) { DCHECK(a && b); - if (a == b) - return true; - if (CFEqual(a, b)) - return true; - CSSM_DATA a_data, b_data; - return SecCertificateGetData(a, &a_data) == noErr && - SecCertificateGetData(b, &b_data) == noErr && - a_data.Length == b_data.Length && - memcmp(a_data.Data, b_data.Data, a_data.Length) == 0; + return CFEqual(a, b); } // static X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes( const char* data, size_t length) { - CSSM_DATA cert_data; - cert_data.Data = const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(data)); - cert_data.Length = length; - - OSCertHandle cert_handle = NULL; - OSStatus status = SecCertificateCreateFromData(&cert_data, - CSSM_CERT_X_509v3, - CSSM_CERT_ENCODING_DER, - &cert_handle); - if (status != noErr) - return NULL; - if (!IsValidOSCertHandle(cert_handle)) { - CFRelease(cert_handle); - return NULL; - } - return cert_handle; + return x509_util::CreateSecCertificateFromBytes( + reinterpret_cast<const uint8_t*>(data), length) + .release(); } // static @@ -373,20 +327,7 @@ void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) { // static SHA256HashValue X509Certificate::CalculateFingerprint256(OSCertHandle cert) { - SHA256HashValue sha256; - memset(sha256.data, 0, sizeof(sha256.data)); - - CSSM_DATA cert_data; - OSStatus status = SecCertificateGetData(cert, &cert_data); - if (status) - return sha256; - - DCHECK(cert_data.Data); - DCHECK_NE(cert_data.Length, 0U); - - CC_SHA256(cert_data.Data, cert_data.Length, sha256.data); - - return sha256; + return x509_util::CalculateFingerprint256(cert); } // static @@ -411,56 +352,6 @@ SHA256HashValue X509Certificate::CalculateCAFingerprint256( return sha256; } -bool X509Certificate::SupportsSSLClientAuth() const { - x509_util::CSSMCachedCertificate cached_cert; - OSStatus status = cached_cert.Init(cert_handle_); - if (status) - return false; - - // RFC5280 says to take the intersection of the two extensions. - // - // Our underlying crypto libraries don't expose - // ClientCertificateType, so for now we will not support fixed - // Diffie-Hellman mechanisms. For rsa_sign, we need the - // digitalSignature bit. - // - // In particular, if a key has the nonRepudiation bit and not the - // digitalSignature one, we will not offer it to the user. - x509_util::CSSMFieldValue key_usage; - status = cached_cert.GetField(&CSSMOID_KeyUsage, &key_usage); - if (status == CSSM_OK && key_usage.field()) { - const CSSM_X509_EXTENSION* ext = key_usage.GetAs<CSSM_X509_EXTENSION>(); - const CE_KeyUsage* key_usage_value = - reinterpret_cast<const CE_KeyUsage*>(ext->value.parsedValue); - if (!((*key_usage_value) & CE_KU_DigitalSignature)) - return false; - } - - status = cached_cert.GetField(&CSSMOID_ExtendedKeyUsage, &key_usage); - if (status == CSSM_OK && key_usage.field()) { - const CSSM_X509_EXTENSION* ext = key_usage.GetAs<CSSM_X509_EXTENSION>(); - const CE_ExtendedKeyUsage* ext_key_usage = - reinterpret_cast<const CE_ExtendedKeyUsage*>(ext->value.parsedValue); - if (!ExtendedKeyUsageAllows(ext_key_usage, &CSSMOID_ClientAuth)) - return false; - } - return true; -} - -CFMutableArrayRef X509Certificate::CreateOSCertChainForCert() const { - CFMutableArrayRef cert_list = - CFArrayCreateMutable(kCFAllocatorDefault, 0, - &kCFTypeArrayCallBacks); - if (!cert_list) - return NULL; - - CFArrayAppendValue(cert_list, os_cert_handle()); - for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) - CFArrayAppendValue(cert_list, intermediate_ca_certs_[i]); - - return cert_list; -} - // static X509Certificate::OSCertHandle X509Certificate::ReadOSCertHandleFromPickle( base::PickleIterator* pickle_iter) { @@ -533,39 +424,7 @@ void X509Certificate::GetPublicKeyInfo(OSCertHandle cert_handle, // static bool X509Certificate::IsSelfSigned(OSCertHandle cert_handle) { - x509_util::CSSMCachedCertificate cached_cert; - OSStatus status = cached_cert.Init(cert_handle); - if (status != noErr) - return false; - - x509_util::CSSMFieldValue subject; - status = cached_cert.GetField(&CSSMOID_X509V1SubjectNameStd, &subject); - if (status != CSSM_OK || !subject.field()) - return false; - - x509_util::CSSMFieldValue issuer; - status = cached_cert.GetField(&CSSMOID_X509V1IssuerNameStd, &issuer); - if (status != CSSM_OK || !issuer.field()) - return false; - - if (subject.field()->Length != issuer.field()->Length || - memcmp(subject.field()->Data, issuer.field()->Data, - issuer.field()->Length) != 0) { - return false; - } - - CSSM_CL_HANDLE cl_handle = CSSM_INVALID_HANDLE; - status = SecCertificateGetCLHandle(cert_handle, &cl_handle); - if (status) - return false; - CSSM_DATA cert_data; - status = SecCertificateGetData(cert_handle, &cert_data); - if (status) - return false; - - if (CSSM_CL_CertVerify(cl_handle, 0, &cert_data, &cert_data, NULL, 0)) - return false; - return true; + return x509_util::IsSelfSigned(cert_handle); } #pragma clang diagnostic pop // "-Wdeprecated-declarations" diff --git a/chromium/net/cert/x509_certificate_nss.cc b/chromium/net/cert/x509_certificate_nss.cc index baa0a5953c4..4f3339f6a62 100644 --- a/chromium/net/cert/x509_certificate_nss.cc +++ b/chromium/net/cert/x509_certificate_nss.cc @@ -26,14 +26,15 @@ namespace net { -void X509Certificate::Initialize() { - x509_util::ParsePrincipal(&cert_handle_->subject, &subject_); - x509_util::ParsePrincipal(&cert_handle_->issuer, &issuer_); - - x509_util::ParseDate(&cert_handle_->validity.notBefore, &valid_start_); - x509_util::ParseDate(&cert_handle_->validity.notAfter, &valid_expiry_); - +bool X509Certificate::Initialize() { serial_number_ = x509_util::ParseSerialNumber(cert_handle_); + + return ( + !serial_number_.empty() && + x509_util::ParsePrincipal(&cert_handle_->subject, &subject_) && + x509_util::ParsePrincipal(&cert_handle_->issuer, &issuer_) && + x509_util::ParseDate(&cert_handle_->validity.notBefore, &valid_start_) && + x509_util::ParseDate(&cert_handle_->validity.notAfter, &valid_expiry_)); } std::string X509Certificate::GetDefaultNickname(CertType type) const { diff --git a/chromium/net/cert/x509_certificate_openssl.cc b/chromium/net/cert/x509_certificate_openssl.cc index 49e5ad7c979..1dd46e0c745 100644 --- a/chromium/net/cert/x509_certificate_openssl.cc +++ b/chromium/net/cert/x509_certificate_openssl.cc @@ -15,6 +15,7 @@ #include "crypto/openssl_util.h" #include "net/base/ip_address.h" #include "net/base/net_errors.h" +#include "net/cert/x509_util.h" #include "net/cert/x509_util_openssl.h" #include "third_party/boringssl/src/include/openssl/asn1.h" #include "third_party/boringssl/src/include/openssl/bytestring.h" @@ -67,11 +68,11 @@ void ParsePrincipalValues(X509_NAME* name, } } -void ParsePrincipal(X509Certificate::OSCertHandle cert, +bool ParsePrincipal(X509Certificate::OSCertHandle cert, X509_NAME* x509_name, CertPrincipal* principal) { if (!x509_name) - return; + return false; ParsePrincipalValues(x509_name, NID_streetAddress, &principal->street_addresses); @@ -90,6 +91,7 @@ void ParsePrincipal(X509Certificate::OSCertHandle cert, &principal->state_or_province_name); x509_util::ParsePrincipalValueByNID(x509_name, NID_countryName, &principal->country_name); + return true; } bool ParseSubjectAltName(X509Certificate::OSCertHandle cert, @@ -185,28 +187,31 @@ void X509Certificate::FreeOSCertHandle(OSCertHandle cert_handle) { X509_free(cert_handle); } -void X509Certificate::Initialize() { +bool X509Certificate::Initialize() { crypto::EnsureOpenSSLInit(); ASN1_INTEGER* serial_num = X509_get_serialNumber(cert_handle_); - if (serial_num) { - // ASN1_INTEGERS represent the decoded number, in a format internal to - // OpenSSL. Most notably, this may have leading zeroes stripped off for - // numbers whose first byte is >= 0x80. Thus, it is necessary to - // re-encoded the integer back into DER, which is what the interface - // of X509Certificate exposes, to ensure callers get the proper (DER) - // value. - int bytes_required = i2c_ASN1_INTEGER(serial_num, NULL); - unsigned char* buffer = reinterpret_cast<unsigned char*>( - base::WriteInto(&serial_number_, bytes_required + 1)); - int bytes_written = i2c_ASN1_INTEGER(serial_num, &buffer); - DCHECK_EQ(static_cast<size_t>(bytes_written), serial_number_.size()); - } - - ParsePrincipal(cert_handle_, X509_get_subject_name(cert_handle_), &subject_); - ParsePrincipal(cert_handle_, X509_get_issuer_name(cert_handle_), &issuer_); - x509_util::ParseDate(X509_get_notBefore(cert_handle_), &valid_start_); - x509_util::ParseDate(X509_get_notAfter(cert_handle_), &valid_expiry_); + if (!serial_num) + return false; + // ASN1_INTEGERS represent the decoded number, in a format internal to + // OpenSSL. Most notably, this may have leading zeroes stripped off for + // numbers whose first byte is >= 0x80. Thus, it is necessary to + // re-encoded the integer back into DER, which is what the interface + // of X509Certificate exposes, to ensure callers get the proper (DER) + // value. + int bytes_required = i2c_ASN1_INTEGER(serial_num, NULL); + unsigned char* buffer = reinterpret_cast<unsigned char*>( + base::WriteInto(&serial_number_, bytes_required + 1)); + int bytes_written = i2c_ASN1_INTEGER(serial_num, &buffer); + DCHECK_EQ(static_cast<size_t>(bytes_written), serial_number_.size()); + + return ( + ParsePrincipal(cert_handle_, X509_get_subject_name(cert_handle_), + &subject_) && + ParsePrincipal(cert_handle_, X509_get_issuer_name(cert_handle_), + &issuer_) && + x509_util::ParseDate(X509_get_notBefore(cert_handle_), &valid_start_) && + x509_util::ParseDate(X509_get_notAfter(cert_handle_), &valid_expiry_)); } // static @@ -248,12 +253,9 @@ X509Certificate::OSCertHandle X509Certificate::CreateOSCertHandleFromBytes( const char* data, size_t length) { crypto::EnsureOpenSSLInit(); - const unsigned char* d2i_data = - reinterpret_cast<const unsigned char*>(data); - // Don't cache this data for x509_util::GetDER as this wire format - // may be not be identical from the i2d_X509 roundtrip. - X509* cert = d2i_X509(NULL, &d2i_data, base::checked_cast<long>(length)); - return cert; + bssl::UniquePtr<CRYPTO_BUFFER> buffer = x509_util::CreateCryptoBuffer( + reinterpret_cast<const uint8_t*>(data), length); + return X509_parse_from_buffer(buffer.get()); } // static diff --git a/chromium/net/cert/x509_certificate_unittest.cc b/chromium/net/cert/x509_certificate_unittest.cc index 62a2c2f150d..91bdefc43b5 100644 --- a/chromium/net/cert/x509_certificate_unittest.cc +++ b/chromium/net/cert/x509_certificate_unittest.cc @@ -22,6 +22,7 @@ #include "net/test/test_certificate_data.h" #include "net/test/test_data_directory.h" #include "testing/gtest/include/gtest/gtest.h" +#include "url/url_features.h" #if defined(USE_NSS_CERTS) #include <cert.h> @@ -267,10 +268,91 @@ TEST(X509CertificateTest, UnescapedSpecialCharacters) { EXPECT_EQ(0U, subject.domain_components.size()); } +TEST(X509CertificateTest, TeletexStringIsLatin1) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "subject_t61string.pem"); + ASSERT_TRUE(cert); + + const CertPrincipal& subject = cert->subject(); + EXPECT_EQ( + " !\"#$%&'()*+,-./" + "0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`" + "abcdefghijklmnopqrstuvwxyz{|}~" + " ¡¢£¤¥¦§¨©ª«¬®¯°±²³´µ¶·¸¹º»¼½¾¿ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßàáâãäåæç" + "èéêëìíîïðñòóôõö÷øùúûüýþÿ", + subject.organization_names[0]); +} + +TEST(X509CertificateTest, TeletexStringControlChars) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "subject_t61string_1-32.pem"); + ASSERT_TRUE(cert); + + const CertPrincipal& subject = cert->subject(); + EXPECT_EQ( + "\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12" + "\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20", + subject.organization_names[0]); +} + +TEST(X509CertificateTest, TeletexStringIsLatin1OrCp1252) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "subject_t61string_126-160.pem"); + ASSERT_TRUE(cert); + + const CertPrincipal& subject = cert->subject(); +#if (defined(OS_MACOSX) && !defined(OS_IOS)) || \ + (BUILDFLAG(USE_BYTE_CERTS) && !BUILDFLAG(USE_PLATFORM_ICU_ALTERNATIVES)) + // Mac: TeletexString is decoded as CP1252. + // use_byte_certs: ICU ISO-8859-1 seems to be CP1252 actually. + // (but with use_platform_icu_alternatives it's not.) + EXPECT_EQ( + "~\x7F\xE2\x82\xAC\xC2\x81\xE2\x80\x9A\xC6\x92\xE2\x80\x9E\xE2\x80\xA6" + "\xE2\x80\xA0\xE2\x80\xA1\xCB\x86\xE2\x80\xB0\xC5\xA0\xE2\x80\xB9\xC5\x92" + "\xC2\x8D\xC5\xBD\xC2\x8F\xC2\x90\xE2\x80\x98\xE2\x80\x99\xE2\x80\x9C\xE2" + "\x80\x9D\xE2\x80\xA2\xE2\x80\x93\xE2\x80\x94\xCB\x9C\xE2\x84\xA2\xC5\xA1" + "\xE2\x80\xBA\xC5\x93\xC2\x9D\xC5\xBE\xC5\xB8\xC2\xA0", + subject.organization_names[0]); +#else + // NSS, Win, Android, iOS: TeletexString is decoded as latin1, so 127-160 get + // decoded to equivalent unicode control chars. + EXPECT_EQ( + "~\x7F\xC2\x80\xC2\x81\xC2\x82\xC2\x83\xC2\x84\xC2\x85\xC2\x86\xC2\x87" + "\xC2\x88\xC2\x89\xC2\x8A\xC2\x8B\xC2\x8C\xC2\x8D\xC2\x8E\xC2\x8F\xC2\x90" + "\xC2\x91\xC2\x92\xC2\x93\xC2\x94\xC2\x95\xC2\x96\xC2\x97\xC2\x98\xC2\x99" + "\xC2\x9A\xC2\x9B\xC2\x9C\xC2\x9D\xC2\x9E\xC2\x9F\xC2\xA0", + subject.organization_names[0]); +#endif +} + +TEST(X509CertificateTest, TeletexStringIsNotARealT61String) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "subject_t61string_actual.pem"); + ASSERT_TRUE(cert); + + const CertPrincipal& subject = cert->subject(); + // If TeletexStrings were actually parsed according to T.61, this would be + // "あ". (Probably. Not verified against a real implementation.) + EXPECT_EQ("\x1B$@$\"", subject.organization_names[0]); +} + TEST(X509CertificateTest, SerialNumbers) { scoped_refptr<X509Certificate> google_cert( X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(google_der), sizeof(google_der))); + ASSERT_TRUE(google_cert); static const uint8_t google_serial[16] = { 0x01,0x2a,0x39,0x76,0x0d,0x3f,0x4f,0xc9, @@ -280,24 +362,79 @@ TEST(X509CertificateTest, SerialNumbers) { ASSERT_EQ(sizeof(google_serial), google_cert->serial_number().size()); EXPECT_TRUE(memcmp(google_cert->serial_number().data(), google_serial, sizeof(google_serial)) == 0); +} - // We also want to check a serial number where the first byte is >= 0x80 in - // case the underlying library tries to pad it. - scoped_refptr<X509Certificate> paypal_null_cert( - X509Certificate::CreateFromBytes( - reinterpret_cast<const char*>(paypal_null_der), - sizeof(paypal_null_der))); - - static const uint8_t paypal_null_serial[3] = {0x00, 0xf0, 0x9b}; - ASSERT_EQ(sizeof(paypal_null_serial), - paypal_null_cert->serial_number().size()); - EXPECT_TRUE(memcmp(paypal_null_cert->serial_number().data(), - paypal_null_serial, sizeof(paypal_null_serial)) == 0); +TEST(X509CertificateTest, SerialNumberZeroPadded) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "serial_zero_padded.pem"); + ASSERT_TRUE(cert); + + // Check a serial number where the first byte is >= 0x80, the DER returned by + // serial() should contain the leading 0 padding byte. + static const uint8_t expected_serial[3] = {0x00, 0x80, 0x01}; + ASSERT_EQ(sizeof(expected_serial), cert->serial_number().size()); + EXPECT_TRUE(memcmp(cert->serial_number().data(), expected_serial, + sizeof(expected_serial)) == 0); +} + +TEST(X509CertificateTest, SerialNumberZeroPadded21BytesLong) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "serial_zero_padded_21_bytes.pem"); + ASSERT_TRUE(cert); + + // Check a serial number where the first byte is >= 0x80, causing the encoded + // length to be 21 bytes long. This should be an error, but serial number + // parsing is currently permissive. + static const uint8_t expected_serial[21] = { + 0x00, 0x80, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, + 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13}; + ASSERT_EQ(sizeof(expected_serial), cert->serial_number().size()); + EXPECT_TRUE(memcmp(cert->serial_number().data(), expected_serial, + sizeof(expected_serial)) == 0); +} + +TEST(X509CertificateTest, SerialNumberNegative) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "serial_negative.pem"); + ASSERT_TRUE(cert); + + // RFC 5280 does not allow serial numbers to be negative, but serial number + // parsing is currently permissive, so this does not cause an error. + static const uint8_t expected_serial[2] = {0x80, 0x01}; + ASSERT_EQ(sizeof(expected_serial), cert->serial_number().size()); + EXPECT_TRUE(memcmp(cert->serial_number().data(), expected_serial, + sizeof(expected_serial)) == 0); +} + +TEST(X509CertificateTest, SerialNumber37BytesLong) { + base::FilePath certs_dir = + GetTestNetDataDirectory().AppendASCII("parse_certificate_unittest"); + scoped_refptr<X509Certificate> cert = + ImportCertFromFile(certs_dir, "serial_37_bytes.pem"); + ASSERT_TRUE(cert); + + // Check a serial number which is very long. This should be an error, but + // serial number parsing is currently permissive. + static const uint8_t expected_serial[37] = { + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, + 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, + 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, + 0x1f, 0x20, 0x21, 0x22, 0x23, 0x24, 0x25}; + ASSERT_EQ(sizeof(expected_serial), cert->serial_number().size()); + EXPECT_TRUE(memcmp(cert->serial_number().data(), expected_serial, + sizeof(expected_serial)) == 0); } TEST(X509CertificateTest, SHA256FingerprintsCorrectly) { scoped_refptr<X509Certificate> google_cert(X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(google_der), sizeof(google_der))); + ASSERT_TRUE(google_cert); const SHA256HashValue google_sha256_fingerprint = { {0x21, 0xaf, 0x58, 0x74, 0xea, 0x6b, 0xad, 0xbd, 0xe4, 0xb3, 0xb1, @@ -328,18 +465,21 @@ TEST(X509CertificateTest, CAFingerprints) { scoped_refptr<X509Certificate> cert_chain1 = X509Certificate::CreateFromHandle(server_cert->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain1); intermediates.clear(); intermediates.push_back(intermediate_cert2->os_cert_handle()); scoped_refptr<X509Certificate> cert_chain2 = X509Certificate::CreateFromHandle(server_cert->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain2); // No intermediate CA certicates. intermediates.clear(); scoped_refptr<X509Certificate> cert_chain3 = X509Certificate::CreateFromHandle(server_cert->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain3); SHA256HashValue cert_chain1_ca_fingerprint_256 = { {0x51, 0x15, 0x30, 0x49, 0x97, 0x54, 0xf8, 0xb4, 0x17, 0x41, 0x6b, @@ -547,6 +687,7 @@ TEST(X509CertificateTest, Cache) { scoped_refptr<X509Certificate> cert1(X509Certificate::CreateFromHandle( google_cert_handle, X509Certificate::OSCertHandles())); X509Certificate::FreeOSCertHandle(google_cert_handle); + ASSERT_TRUE(cert1); // Add the same certificate, but as a new handle. google_cert_handle = X509Certificate::CreateOSCertHandleFromBytes( @@ -554,6 +695,7 @@ TEST(X509CertificateTest, Cache) { scoped_refptr<X509Certificate> cert2(X509Certificate::CreateFromHandle( google_cert_handle, X509Certificate::OSCertHandles())); X509Certificate::FreeOSCertHandle(google_cert_handle); + ASSERT_TRUE(cert2); // A new X509Certificate should be returned. EXPECT_NE(cert1.get(), cert2.get()); @@ -575,6 +717,7 @@ TEST(X509CertificateTest, Cache) { google_cert_handle, intermediates)); X509Certificate::FreeOSCertHandle(google_cert_handle); X509Certificate::FreeOSCertHandle(thawte_cert_handle); + ASSERT_TRUE(cert3); // Test that the new certificate, even with intermediates, results in the // same underlying handle being used. @@ -626,10 +769,12 @@ TEST(X509CertificateTest, IntermediateCertificates) { scoped_refptr<X509Certificate> webkit_cert( X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(webkit_der), sizeof(webkit_der))); + ASSERT_TRUE(webkit_cert); scoped_refptr<X509Certificate> thawte_cert( X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(thawte_der), sizeof(thawte_der))); + ASSERT_TRUE(thawte_cert); X509Certificate::OSCertHandle google_handle; // Create object with no intermediates: @@ -638,6 +783,7 @@ TEST(X509CertificateTest, IntermediateCertificates) { X509Certificate::OSCertHandles intermediates1; scoped_refptr<X509Certificate> cert1; cert1 = X509Certificate::CreateFromHandle(google_handle, intermediates1); + ASSERT_TRUE(cert1); EXPECT_EQ(0u, cert1->GetIntermediateCertificates().size()); // Create object with 2 intermediates: @@ -646,6 +792,7 @@ TEST(X509CertificateTest, IntermediateCertificates) { intermediates2.push_back(thawte_cert->os_cert_handle()); scoped_refptr<X509Certificate> cert2; cert2 = X509Certificate::CreateFromHandle(google_handle, intermediates2); + ASSERT_TRUE(cert2); // Verify it has all the intermediates: const X509Certificate::OSCertHandles& cert2_intermediates = @@ -758,6 +905,7 @@ TEST(X509CertificateTest, IsIssuedByEncodedWithIntermediates) { scoped_refptr<X509Certificate> cert_chain = X509Certificate::CreateFromHandle(policy_chain[0]->os_cert_handle(), intermediates); + ASSERT_TRUE(cert_chain); std::vector<std::string> issuers; @@ -924,6 +1072,7 @@ TEST_P(X509CertificateParseTest, CanParseFormat) { // A cert is expected - make sure that one was parsed. ASSERT_LT(i, certs.size()); + ASSERT_TRUE(certs[i]); // Compare the parsed certificate with the expected certificate, by // comparing fingerprints. @@ -1181,16 +1330,13 @@ const struct PublicKeyInfoTestData { size_t expected_bits; X509Certificate::PublicKeyType expected_type; } kPublicKeyInfoTestData[] = { - {"768-rsa-ee-by-768-rsa-intermediate.pem", - 768, + {"768-rsa-ee-by-768-rsa-intermediate.pem", 768, X509Certificate::kPublicKeyTypeRSA}, - {"1024-rsa-ee-by-768-rsa-intermediate.pem", - 1024, + {"1024-rsa-ee-by-768-rsa-intermediate.pem", 1024, X509Certificate::kPublicKeyTypeRSA}, - {"prime256v1-ecdsa-ee-by-1024-rsa-intermediate.pem", - 256, + {"prime256v1-ecdsa-ee-by-1024-rsa-intermediate.pem", 256, X509Certificate::kPublicKeyTypeECDSA}, -#if defined(OS_MACOSX) && !defined(OS_IOS) +#if defined(OS_MACOSX) && !defined(OS_IOS) && !BUILDFLAG(USE_BYTE_CERTS) // OS X has an key length limit of 4096 bits. This should manifest as an // unknown key. If a future version of OS X changes this, large_key.pem may // need to be renegerated with a larger key. See https://crbug.com/472291. diff --git a/chromium/net/cert/x509_certificate_win.cc b/chromium/net/cert/x509_certificate_win.cc index 3414734cc7e..05eec4e1b12 100644 --- a/chromium/net/cert/x509_certificate_win.cc +++ b/chromium/net/cert/x509_certificate_win.cc @@ -133,12 +133,14 @@ bool IsCertNameBlobInIssuerList( } // namespace -void X509Certificate::Initialize() { +bool X509Certificate::Initialize() { DCHECK(cert_handle_); - subject_.ParseDistinguishedName(cert_handle_->pCertInfo->Subject.pbData, - cert_handle_->pCertInfo->Subject.cbData); - issuer_.ParseDistinguishedName(cert_handle_->pCertInfo->Issuer.pbData, - cert_handle_->pCertInfo->Issuer.cbData); + if (!subject_.ParseDistinguishedName( + cert_handle_->pCertInfo->Subject.pbData, + cert_handle_->pCertInfo->Subject.cbData) || + !issuer_.ParseDistinguishedName(cert_handle_->pCertInfo->Issuer.pbData, + cert_handle_->pCertInfo->Issuer.cbData)) + return false; valid_start_ = Time::FromFileTime(cert_handle_->pCertInfo->NotBefore); valid_expiry_ = Time::FromFileTime(cert_handle_->pCertInfo->NotAfter); @@ -149,6 +151,8 @@ void X509Certificate::Initialize() { serial_bytes[i] = serial->pbData[serial->cbData - i - 1]; serial_number_ = std::string( reinterpret_cast<char*>(serial_bytes.get()), serial->cbData); + + return true; } bool X509Certificate::GetSubjectAltName( diff --git a/chromium/net/cert/x509_util.cc b/chromium/net/cert/x509_util.cc index a5d583d0eb1..4a1f755ef3a 100644 --- a/chromium/net/cert/x509_util.cc +++ b/chromium/net/cert/x509_util.cc @@ -30,7 +30,7 @@ bool GetCommonName(const der::Input& tlv, std::string* common_name) { for (const auto& rdn : rdn_sequence) { for (const auto& atv : rdn) { if (atv.type == TypeCommonNameOid()) { - return atv.ValueAsStringUnsafe(common_name); + return atv.ValueAsString(common_name); } } } diff --git a/chromium/net/cert/x509_util_ios.cc b/chromium/net/cert/x509_util_ios.cc new file mode 100644 index 00000000000..9bf41850efa --- /dev/null +++ b/chromium/net/cert/x509_util_ios.cc @@ -0,0 +1,22 @@ +// Copyright 2017 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/x509_util_ios.h" + +#include "net/cert/x509_certificate.h" + +namespace net { + +namespace x509_util { + +base::ScopedCFTypeRef<SecCertificateRef> +CreateSecCertificateFromX509Certificate(const X509Certificate* cert) { + return base::ScopedCFTypeRef<SecCertificateRef>( + reinterpret_cast<SecCertificateRef>( + const_cast<void*>(CFRetain(cert->os_cert_handle())))); +} + +} // namespace x509_util + +} // namespace net diff --git a/chromium/net/cert/x509_util_ios.h b/chromium/net/cert/x509_util_ios.h new file mode 100644 index 00000000000..bf3473ffcab --- /dev/null +++ b/chromium/net/cert/x509_util_ios.h @@ -0,0 +1,27 @@ +// Copyright 2017 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_X509_UTIL_IOS_H_ +#define NET_CERT_X509_UTIL_IOS_H_ + +#include <Security/Security.h> + +#include "base/mac/scoped_cftyperef.h" +#include "net/base/net_export.h" + +namespace net { + +class X509Certificate; + +namespace x509_util { + +// Returns a SecCertificate representing |cert|, or NULL on failure. +NET_EXPORT base::ScopedCFTypeRef<SecCertificateRef> +CreateSecCertificateFromX509Certificate(const X509Certificate* cert); + +} // namespace x509_util + +} // namespace net + +#endif // NET_CERT_X509_UTIL_IOS_H_ diff --git a/chromium/net/cert/x509_util_mac.cc b/chromium/net/cert/x509_util_mac.cc index f2ce0f3b4b4..c8f6a2f4a8e 100644 --- a/chromium/net/cert/x509_util_mac.cc +++ b/chromium/net/cert/x509_util_mac.cc @@ -4,11 +4,14 @@ #include "net/cert/x509_util_mac.h" +#include <CommonCrypto/CommonDigest.h> + #include "base/logging.h" #include "base/mac/mac_util.h" -#include "base/mac/scoped_cftyperef.h" #include "base/strings/sys_string_conversions.h" +#include "net/cert/x509_certificate.h" #include "third_party/apple_apsl/cssmapplePriv.h" +#include "third_party/boringssl/src/include/openssl/pool.h" namespace net { @@ -52,6 +55,163 @@ OSStatus CreatePolicy(const CSSM_OID* policy_oid, } // namespace +bool IsValidSecCertificate(SecCertificateRef cert_handle) { + const CSSM_X509_NAME* sanity_check = NULL; + OSStatus status = SecCertificateGetSubject(cert_handle, &sanity_check); + return status == noErr && sanity_check; +} + +base::ScopedCFTypeRef<SecCertificateRef> CreateSecCertificateFromBytes( + const uint8_t* data, + size_t length) { + CSSM_DATA cert_data; + cert_data.Data = const_cast<uint8_t*>(data); + cert_data.Length = length; + + base::ScopedCFTypeRef<SecCertificateRef> cert_handle; + OSStatus status = SecCertificateCreateFromData(&cert_data, CSSM_CERT_X_509v3, + CSSM_CERT_ENCODING_DER, + cert_handle.InitializeInto()); + if (status != noErr) + return base::ScopedCFTypeRef<SecCertificateRef>(); + if (!IsValidSecCertificate(cert_handle.get())) + return base::ScopedCFTypeRef<SecCertificateRef>(); + return cert_handle; +} + +base::ScopedCFTypeRef<SecCertificateRef> +CreateSecCertificateFromX509Certificate(const X509Certificate* cert) { +#if BUILDFLAG(USE_BYTE_CERTS) + return CreateSecCertificateFromBytes( + CRYPTO_BUFFER_data(cert->os_cert_handle()), + CRYPTO_BUFFER_len(cert->os_cert_handle())); +#else + return base::ScopedCFTypeRef<SecCertificateRef>( + reinterpret_cast<SecCertificateRef>( + const_cast<void*>(CFRetain(cert->os_cert_handle())))); +#endif +} + +base::ScopedCFTypeRef<CFMutableArrayRef> +CreateSecCertificateArrayForX509Certificate(X509Certificate* cert) { + base::ScopedCFTypeRef<CFMutableArrayRef> cert_list( + CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks)); + if (!cert_list) + return base::ScopedCFTypeRef<CFMutableArrayRef>(); +#if BUILDFLAG(USE_BYTE_CERTS) + std::string bytes; + base::ScopedCFTypeRef<SecCertificateRef> sec_cert( + CreateSecCertificateFromBytes(CRYPTO_BUFFER_data(cert->os_cert_handle()), + CRYPTO_BUFFER_len(cert->os_cert_handle()))); + if (!sec_cert) + return base::ScopedCFTypeRef<CFMutableArrayRef>(); + CFArrayAppendValue(cert_list, sec_cert); + for (X509Certificate::OSCertHandle intermediate : + cert->GetIntermediateCertificates()) { + base::ScopedCFTypeRef<SecCertificateRef> sec_cert( + CreateSecCertificateFromBytes(CRYPTO_BUFFER_data(intermediate), + CRYPTO_BUFFER_len(intermediate))); + if (!sec_cert) + return base::ScopedCFTypeRef<CFMutableArrayRef>(); + CFArrayAppendValue(cert_list, sec_cert); + } +#else + X509Certificate::OSCertHandles intermediate_ca_certs = + cert->GetIntermediateCertificates(); + CFArrayAppendValue(cert_list, cert->os_cert_handle()); + for (size_t i = 0; i < intermediate_ca_certs.size(); ++i) + CFArrayAppendValue(cert_list, intermediate_ca_certs[i]); +#endif + return cert_list; +} + +scoped_refptr<X509Certificate> CreateX509CertificateFromSecCertificate( + SecCertificateRef sec_cert, + const std::vector<SecCertificateRef>& sec_chain) { +#if BUILDFLAG(USE_BYTE_CERTS) + CSSM_DATA der_data; + if (!sec_cert || SecCertificateGetData(sec_cert, &der_data) != noErr) + return nullptr; + bssl::UniquePtr<CRYPTO_BUFFER> cert_handle( + X509Certificate::CreateOSCertHandleFromBytes( + reinterpret_cast<const char*>(der_data.Data), der_data.Length)); + if (!cert_handle) + return nullptr; + std::vector<bssl::UniquePtr<CRYPTO_BUFFER>> intermediates; + X509Certificate::OSCertHandles intermediates_raw; + for (const SecCertificateRef& sec_intermediate : sec_chain) { + if (!sec_intermediate || + SecCertificateGetData(sec_intermediate, &der_data) != noErr) { + return nullptr; + } + bssl::UniquePtr<CRYPTO_BUFFER> intermediate_cert_handle( + X509Certificate::CreateOSCertHandleFromBytes( + reinterpret_cast<const char*>(der_data.Data), der_data.Length)); + if (!intermediate_cert_handle) + return nullptr; + intermediates_raw.push_back(intermediate_cert_handle.get()); + intermediates.push_back(std::move(intermediate_cert_handle)); + } + scoped_refptr<X509Certificate> result( + X509Certificate::CreateFromHandle(cert_handle.get(), intermediates_raw)); + return result; +#else + return X509Certificate::CreateFromHandle(sec_cert, sec_chain); +#endif +} + +bool IsSelfSigned(SecCertificateRef cert_handle) { + CSSMCachedCertificate cached_cert; + OSStatus status = cached_cert.Init(cert_handle); + if (status != noErr) + return false; + + CSSMFieldValue subject; + status = cached_cert.GetField(&CSSMOID_X509V1SubjectNameStd, &subject); + if (status != CSSM_OK || !subject.field()) + return false; + + CSSMFieldValue issuer; + status = cached_cert.GetField(&CSSMOID_X509V1IssuerNameStd, &issuer); + if (status != CSSM_OK || !issuer.field()) + return false; + + if (subject.field()->Length != issuer.field()->Length || + memcmp(subject.field()->Data, issuer.field()->Data, + issuer.field()->Length) != 0) { + return false; + } + + CSSM_CL_HANDLE cl_handle = CSSM_INVALID_HANDLE; + status = SecCertificateGetCLHandle(cert_handle, &cl_handle); + if (status) + return false; + CSSM_DATA cert_data; + status = SecCertificateGetData(cert_handle, &cert_data); + if (status) + return false; + + if (CSSM_CL_CertVerify(cl_handle, 0, &cert_data, &cert_data, NULL, 0)) + return false; + return true; +} + +SHA256HashValue CalculateFingerprint256(SecCertificateRef cert) { + SHA256HashValue sha256; + memset(sha256.data, 0, sizeof(sha256.data)); + + CSSM_DATA cert_data; + OSStatus status = SecCertificateGetData(cert, &cert_data); + if (status) + return sha256; + + DCHECK(cert_data.Data); + DCHECK_NE(cert_data.Length, 0U); + + CC_SHA256(cert_data.Data, cert_data.Length, sha256.data); + + return sha256; +} OSStatus CreateSSLClientPolicy(SecPolicyRef* policy) { *policy = SecPolicyCreateSSL(false /* server */, nullptr); diff --git a/chromium/net/cert/x509_util_mac.h b/chromium/net/cert/x509_util_mac.h index 6b320a8cd6c..1700a6cce4f 100644 --- a/chromium/net/cert/x509_util_mac.h +++ b/chromium/net/cert/x509_util_mac.h @@ -10,18 +10,67 @@ #include <string> +#include "base/mac/scoped_cftyperef.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "net/base/hash_value.h" #include "net/base/net_export.h" namespace net { +class X509Certificate; + namespace x509_util { +// Tests that a given |cert_handle| is actually a valid X.509 certificate, and +// returns true if it is. +// +// On OS X, SecCertificateCreateFromData() does not return any errors if +// called with invalid data, as long as data is present. The actual decoding +// of the certificate does not happen until an API that requires a CSSM +// handle is called. While SecCertificateGetCLHandle is the most likely +// candidate, as it performs the parsing, it does not check whether the +// parsing was actually successful. Instead, SecCertificateGetSubject is +// used (supported since 10.3), as a means to check that the certificate +// parsed as a valid X.509 certificate. +NET_EXPORT bool IsValidSecCertificate(SecCertificateRef cert_handle); + +// Creates a SecCertificate handle from the DER-encoded representation. +// Returns NULL on failure. +NET_EXPORT base::ScopedCFTypeRef<SecCertificateRef> +CreateSecCertificateFromBytes(const uint8_t* data, size_t length); + +// Returns a SecCertificate representing |cert|, or NULL on failure. +NET_EXPORT base::ScopedCFTypeRef<SecCertificateRef> +CreateSecCertificateFromX509Certificate(const X509Certificate* cert); + +// Returns a new CFMutableArrayRef containing this certificate and its +// intermediate certificates in the form expected by Security.framework +// and Keychain Services, or NULL on failure. +// The first item in the array will be this certificate, followed by its +// intermediates, if any. +NET_EXPORT base::ScopedCFTypeRef<CFMutableArrayRef> +CreateSecCertificateArrayForX509Certificate(X509Certificate* cert); + +// Creates an X509Certificate representing |sec_cert| with intermediates +// |sec_chain|. +NET_EXPORT scoped_refptr<X509Certificate> +CreateX509CertificateFromSecCertificate( + SecCertificateRef sec_cert, + const std::vector<SecCertificateRef>& sec_chain); + +// Returns true if the certificate is self-signed. +NET_EXPORT bool IsSelfSigned(SecCertificateRef cert_handle); + +// Calculates the SHA-256 fingerprint of the certificate. Returns an empty +// (all zero) fingerprint on failure. +NET_EXPORT SHA256HashValue CalculateFingerprint256(SecCertificateRef cert); + // Creates a security policy for certificates used as client certificates // in SSL. // If a policy is successfully created, it will be stored in // |*policy| and ownership transferred to the caller. -OSStatus NET_EXPORT CreateSSLClientPolicy(SecPolicyRef* policy); +NET_EXPORT OSStatus CreateSSLClientPolicy(SecPolicyRef* policy); // Create an SSL server policy. While certificate name validation will be // performed by SecTrustEvaluate(), it has the following limitations: @@ -32,13 +81,13 @@ OSStatus NET_EXPORT CreateSSLClientPolicy(SecPolicyRef* policy); // system trust preferences, such as those created by Safari. Preferences // created by Keychain Access do not share this requirement. // On success, stores the resultant policy in |*policy| and returns noErr. -OSStatus NET_EXPORT CreateSSLServerPolicy(const std::string& hostname, +NET_EXPORT OSStatus CreateSSLServerPolicy(const std::string& hostname, SecPolicyRef* policy); // Creates a security policy for basic X.509 validation. If the policy is // successfully created, it will be stored in |*policy| and ownership // transferred to the caller. -OSStatus NET_EXPORT CreateBasicX509Policy(SecPolicyRef* policy); +NET_EXPORT OSStatus CreateBasicX509Policy(SecPolicyRef* policy); // Creates security policies to control revocation checking (OCSP and CRL). // If |enable_revocation_checking| is true, revocation checking will be @@ -47,7 +96,7 @@ OSStatus NET_EXPORT CreateBasicX509Policy(SecPolicyRef* policy); // the network or the local cache, if possible. // If the policies are successfully created, they will be appended to // |policies|. -OSStatus NET_EXPORT CreateRevocationPolicies(bool enable_revocation_checking, +NET_EXPORT OSStatus CreateRevocationPolicies(bool enable_revocation_checking, CFMutableArrayRef policies); // CSSM functions are deprecated as of OSX 10.7, but have no replacement. diff --git a/chromium/net/cert/x509_util_nss.cc b/chromium/net/cert/x509_util_nss.cc index 2988417673b..1175bd8054d 100644 --- a/chromium/net/cert/x509_util_nss.cc +++ b/chromium/net/cert/x509_util_nss.cc @@ -89,7 +89,7 @@ CERTName* CreateCertNameFromEncoded(PLArenaPool* arena, namespace x509_util { -void ParsePrincipal(CERTName* name, CertPrincipal* principal) { +bool ParsePrincipal(CERTName* name, CertPrincipal* principal) { // Starting in NSS 3.15, CERTGetNameFunc takes a const CERTName* argument. #if NSS_VMINOR >= 15 typedef char* (*CERTGetNameFunc)(const CERTName* name); @@ -120,7 +120,7 @@ void ParsePrincipal(CERTName* name, CertPrincipal* principal) { if (kOIDs[oid] == tag) { SECItem* decode_item = CERT_DecodeAVAValue(&avas[pair]->value); if (!decode_item) - break; + return false; // TODO(wtc): Pass decode_item to CERT_RFC1485_EscapeAndQuote. std::string value(reinterpret_cast<char*>(decode_item->data), decode_item->len); @@ -145,13 +145,17 @@ void ParsePrincipal(CERTName* name, CertPrincipal* principal) { PORT_Free(value); } } + + return true; } -void ParseDate(const SECItem* der_date, base::Time* result) { +bool ParseDate(const SECItem* der_date, base::Time* result) { PRTime prtime; SECStatus rv = DER_DecodeTimeChoice(&prtime, der_date); - DCHECK_EQ(SECSuccess, rv); + if (rv != SECSuccess) + return false; *result = crypto::PRTimeToBaseTime(prtime); + return true; } std::string ParseSerialNumber(const CERTCertificate* certificate) { diff --git a/chromium/net/cert/x509_util_nss.h b/chromium/net/cert/x509_util_nss.h index 41561b437a1..b5dfe795dfb 100644 --- a/chromium/net/cert/x509_util_nss.h +++ b/chromium/net/cert/x509_util_nss.h @@ -28,14 +28,13 @@ namespace net { namespace x509_util { -#if defined(USE_NSS_CERTS) // Parses the Principal attribute from |name| and outputs the result in -// |principal|. -void ParsePrincipal(CERTName* name, - CertPrincipal* principal); +// |principal|. Returns true on success. +bool ParsePrincipal(CERTName* name, CertPrincipal* principal); // Parses the date from |der_date| and outputs the result in |result|. -void ParseDate(const SECItem* der_date, base::Time* result); +// Returns true on success. +bool ParseDate(const SECItem* der_date, base::Time* result); // Parses the serial number from |certificate|. std::string ParseSerialNumber(const CERTCertificate* certificate); @@ -127,7 +126,6 @@ bool IsCertificateIssuedBy(const std::vector<CERTCertificate*>& cert_chain, std::string GetUniqueNicknameForSlot(const std::string& nickname, const SECItem* subject, PK11SlotInfo* slot); -#endif // defined(USE_NSS_CERTS) } // namespace x509_util diff --git a/chromium/net/cert/x509_util_openssl.cc b/chromium/net/cert/x509_util_openssl.cc index 72f93f26983..44dee713951 100644 --- a/chromium/net/cert/x509_util_openssl.cc +++ b/chromium/net/cert/x509_util_openssl.cc @@ -363,16 +363,15 @@ bool GetTLSServerEndPointChannelBinding(const X509Certificate& certificate, if (!digest_evp_md) return false; - std::vector<uint8_t> digest(EVP_MAX_MD_SIZE); - unsigned int out_size = digest.size(); + uint8_t digest[EVP_MAX_MD_SIZE]; + unsigned int out_size; if (!EVP_Digest(der_encoded_certificate.data(), - der_encoded_certificate.size(), digest.data(), &out_size, + der_encoded_certificate.size(), digest, &out_size, digest_evp_md, nullptr)) return false; - digest.resize(out_size); token->assign(kChannelBindingPrefix); - token->append(digest.begin(), digest.end()); + token->append(digest, digest + out_size); return true; } diff --git a/chromium/net/cert/x509_util_unittest.cc b/chromium/net/cert/x509_util_unittest.cc index 51097796fd4..2872051d104 100644 --- a/chromium/net/cert/x509_util_unittest.cc +++ b/chromium/net/cert/x509_util_unittest.cc @@ -282,6 +282,7 @@ TEST(X509UtilTest, CreateChannelBindings_SHA1) { scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(kCertificateDataDER), sizeof(kCertificateDataDER)); + ASSERT_TRUE(cert); std::string channel_bindings; ASSERT_TRUE( @@ -390,6 +391,7 @@ TEST(X509UtilTest, CreateChannelBindings_SHA256) { scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(kCertificateDataDER), sizeof(kCertificateDataDER)); + ASSERT_TRUE(cert); std::string channel_bindings; ASSERT_TRUE( @@ -506,6 +508,7 @@ TEST(X509UtilTest, CreateChannelBindings_SHA384) { scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(kCertificateDataDER), sizeof(kCertificateDataDER)); + ASSERT_TRUE(cert); std::string channel_bindings; ASSERT_TRUE( @@ -617,6 +620,7 @@ TEST(X509UtilTest, CreateChannelBindings_SHA512) { scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(kCertificateDataDER), sizeof(kCertificateDataDER)); + ASSERT_TRUE(cert); std::string channel_bindings; ASSERT_TRUE( @@ -717,6 +721,7 @@ TEST(X509UtilTest, CreateChannelBindings_Unsupported_MD4) { scoped_refptr<X509Certificate> cert = X509Certificate::CreateFromBytes( reinterpret_cast<const char*>(kCertificateDataDER), sizeof(kCertificateDataDER)); + ASSERT_TRUE(cert); std::string channel_bindings; ASSERT_FALSE( |