Check EV and OCSP revocation in CertVerifyProcBuiltin.
Bug: 649017,649000,762380
Change-Id: Ic80908a4b883fbae23ba60f67c422d95478836db
Reviewed-on: https://chromium-review.googlesource.com/721850
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510165}
diff --git a/net/cert/cert_verify_proc_builtin.cc b/net/cert/cert_verify_proc_builtin.cc
index 6c821c7..9091d24 100644
--- a/net/cert/cert_verify_proc_builtin.cc
+++ b/net/cert/cert_verify_proc_builtin.cc
@@ -13,10 +13,12 @@
#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_net_fetcher.h"
#include "net/cert/cert_status_flags.h"
+#include "net/cert/cert_verifier.h"
#include "net/cert/cert_verify_proc.h"
#include "net/cert/cert_verify_result.h"
+#include "net/cert/ev_root_ca_metadata.h"
#include "net/cert/internal/cert_errors.h"
#include "net/cert/internal/cert_issuer_source_static.h"
#include "net/cert/internal/common_cert_errors.h"
@@ -33,6 +35,50 @@
namespace {
+DEFINE_CERT_ERROR_ID(kPathLacksEVPolicy, "Path does not have an EV policy");
+
+RevocationPolicy NoRevocationChecking() {
+ RevocationPolicy policy;
+ policy.check_revocation = false;
+ policy.networking_allowed = false;
+ policy.allow_missing_info = true;
+ policy.allow_network_failure = true;
+ return policy;
+}
+
+// Gets the set of policy OIDs in |cert| that are recognized as EV OIDs for some
+// root.
+void GetEVPolicyOids(const EVRootCAMetadata* ev_metadata,
+ const ParsedCertificate* cert,
+ std::set<der::Input>* oids) {
+ oids->clear();
+
+ if (!cert->has_policy_oids())
+ return;
+
+ for (const der::Input& oid : cert->policy_oids()) {
+ if (ev_metadata->IsEVPolicyOIDGivenBytes(oid))
+ oids->insert(oid);
+ }
+}
+
+// Returns true if |cert| could be an EV certificate, based on its policies
+// extension. A return of false means it definitely is not an EV certificate,
+// whereas a return of true means it could be EV.
+bool IsEVCandidate(const EVRootCAMetadata* ev_metadata,
+ const ParsedCertificate* cert) {
+ std::set<der::Input> oids;
+ GetEVPolicyOids(ev_metadata, cert, &oids);
+ return !oids.empty();
+}
+
+// Enum for whether path building is attempting to verify a certificate as EV or
+// as DV.
+enum class VerificationType {
+ kEV, // Extended Validation
+ kDV, // Domain Validation
+};
+
// 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
@@ -41,30 +87,178 @@
public:
// Uses the default policy from SimplePathBuilderDelegate, which requires RSA
// keys to be at least 1024-bits large, and accepts SHA1 certificates.
- PathBuilderDelegateImpl(CRLSet* crl_set)
- : SimplePathBuilderDelegate(1024), crl_set_(crl_set) {}
+ PathBuilderDelegateImpl(const CRLSet* crl_set,
+ CertNetFetcher* net_fetcher,
+ VerificationType verification_type,
+ int flags,
+ const SystemTrustStore* ssl_trust_store,
+ base::StringPiece stapled_leaf_ocsp_response,
+ const EVRootCAMetadata* ev_metadata,
+ bool* checked_revocation_for_some_path)
+ : SimplePathBuilderDelegate(1024),
+ crl_set_(crl_set),
+ net_fetcher_(net_fetcher),
+ verification_type_(verification_type),
+ flags_(flags),
+ ssl_trust_store_(ssl_trust_store),
+ stapled_leaf_ocsp_response_(stapled_leaf_ocsp_response),
+ ev_metadata_(ev_metadata),
+ checked_revocation_for_some_path_(checked_revocation_for_some_path) {}
// This is called for each built chain, including ones which failed. It is
// responsible for adding errors to the built chain if it is not acceptable.
void CheckPathAfterVerification(CertPathBuilderResultPath* path) override {
- CheckRevocation(path);
+ // If the path is already invalid, don't check revocation status. The chain
+ // is expected to be valid when doing revocation checks (since for instance
+ // the correct issuer for a certificate may need to be known). Also if
+ // certificates are already expired, obtaining their revocation status may
+ // fail.
+ //
+ // TODO(eroman): When CertVerifyProcBuiltin fails to find a valid path,
+ // whatever (partial/incomplete) path it does return should
+ // minimally be checked with the CRLSet.
+ if (!path->IsValid())
+ return;
+
+ // If EV was requested the certificate must chain to a recognized EV root
+ // and have one of its recognized EV policy OIDs.
+ if (verification_type_ == VerificationType::kEV) {
+ if (!ConformsToEVPolicy(path)) {
+ path->errors.GetErrorsForCert(0)->AddError(kPathLacksEVPolicy);
+ return;
+ }
+ }
+
+ // Select an appropriate revocation policy for this chain based on the
+ // verifier flags and root, and whether this is an EV or DV path building
+ // attempt.
+ bool crlset_leaf_coverage_sufficient;
+ RevocationPolicy policy =
+ ChooseRevocationPolicy(path->certs, &crlset_leaf_coverage_sufficient);
+
+ // Check for revocations using the CRLSet (if available).
+ if (crl_set_) {
+ switch (CheckChainRevocationUsingCRLSet(crl_set_, path->certs,
+ &path->errors)) {
+ case CRLSet::Result::REVOKED:
+ return;
+ case CRLSet::Result::GOOD:
+ if (crlset_leaf_coverage_sufficient) {
+ // Weaken the revocation checking requirement as it has been
+ // satisfied. (Don't early-return, since still want to consult
+ // cached OCSP/CRL if available).
+ policy = NoRevocationChecking();
+ }
+ break;
+ case CRLSet::Result::UNKNOWN:
+ // CRLSet was inconclusive.
+ break;
+ }
+ }
+
+ if (policy.check_revocation)
+ *checked_revocation_for_some_path_ = true;
+
+ // Check the revocation status for each certificate in the chain according
+ // to |policy|. Depending on the policy, errors will be added to the
+ // respective certificates, so |errors->ContainsHighSeverityErrors()| will
+ // reflect the revocation status of the chain after this call.
+ CheckCertChainRevocation(path->certs, path->last_cert_trust, policy,
+ stapled_leaf_ocsp_response_, net_fetcher_,
+ &path->errors);
}
private:
- // This method checks whether a certificate chain has been revoked, and if
- // so adds errors to the affected certificates.
- void CheckRevocation(CertPathBuilderResultPath* path) const {
- // First check for revocations using the CRLSet. This does not require
- // any network activity.
- if (crl_set_) {
- CheckChainRevocationUsingCRLSet(crl_set_, path->certs, &path->errors);
+ // Selects a revocation policy based on the CertVerifier flags and the given
+ // certificate chain.
+ RevocationPolicy ChooseRevocationPolicy(
+ const ParsedCertificateList& certs,
+ bool* crlset_leaf_coverage_sufficient) {
+ // The only case this is set to true is for EV.
+ *crlset_leaf_coverage_sufficient = false;
+
+ // Use hard-fail revocation checking for local trust anchors, if requested
+ // by the load flag and the chain uses a non-public root.
+ if ((flags_ & CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS) &&
+ !certs.empty() && !ssl_trust_store_->IsKnownRoot(certs.back().get())) {
+ RevocationPolicy policy;
+ policy.check_revocation = true;
+ policy.networking_allowed =
+ (flags_ & CertVerifier::VERIFY_CERT_IO_ENABLED);
+ policy.allow_missing_info = true;
+ policy.allow_network_failure = false;
+
+ // In practice EV verification won't succeed for local anchors. For
+ // completeness though use all the strictness of EV if this is an EV
+ // attempt.
+ if (verification_type_ == VerificationType::kEV)
+ policy.allow_missing_info = false;
+
+ return policy;
}
- // TODO(eroman): Next check revocation using OCSP and CRL.
+ // Use hard-fail revocation checking for EV certificates.
+ if (verification_type_ == VerificationType::kEV) {
+ // For EV verification leaf coverage is considered sufficient.
+ *crlset_leaf_coverage_sufficient = true;
+
+ RevocationPolicy policy;
+ policy.check_revocation = true;
+ // TODO(eroman): This definition for |networking_allowed| is redundant.
+ // Perhaps VERIFY_EV_CERT should imply revocation checking.
+ policy.networking_allowed =
+ (flags_ & CertVerifier::VERIFY_CERT_IO_ENABLED) &&
+ ((flags_ & CertVerifier::VERIFY_REV_CHECKING_ENABLED) ||
+ (flags_ & CertVerifier::VERIFY_REV_CHECKING_ENABLED_EV_ONLY));
+ policy.allow_missing_info = false;
+ policy.allow_network_failure = false;
+ return policy;
+ }
+
+ // Use soft-fail revocation checking for VERIFY_REV_CHECKING_ENABLED.
+ if (flags_ & CertVerifier::VERIFY_REV_CHECKING_ENABLED) {
+ RevocationPolicy policy;
+ policy.check_revocation = true;
+ policy.networking_allowed = flags_ & CertVerifier::VERIFY_CERT_IO_ENABLED;
+ policy.allow_missing_info = true;
+ policy.allow_network_failure = true;
+ return policy;
+ }
+
+ return NoRevocationChecking();
+ }
+
+ // Returns true if |path| chains to an EV root, and the chain conforms to one
+ // of its EV policy OIDs. When building paths all candidate EV policy OIDs
+ // were requested, so it is just a matter of testing each of the policies the
+ // chain conforms to.
+ bool ConformsToEVPolicy(const CertPathBuilderResultPath* path) {
+ const ParsedCertificate* root = path->GetTrustedCert();
+ if (!root)
+ return false;
+
+ SHA256HashValue root_fingerprint;
+ crypto::SHA256HashString(root->der_cert().AsStringPiece(),
+ root_fingerprint.data,
+ sizeof(root_fingerprint.data));
+
+ for (const der::Input& oid : path->user_constrained_policy_set) {
+ if (ev_metadata_->HasEVPolicyOIDGivenBytes(root_fingerprint, oid))
+ return true;
+ }
+
+ return false;
}
// The CRLSet may be null.
- CRLSet* crl_set_;
+ const CRLSet* crl_set_;
+ CertNetFetcher* net_fetcher_;
+ const VerificationType verification_type_;
+ const int flags_;
+ const SystemTrustStore* ssl_trust_store_;
+ const base::StringPiece stapled_leaf_ocsp_response_;
+ const EVRootCAMetadata* ev_metadata_;
+ bool* checked_revocation_for_some_path_;
};
class CertVerifyProcBuiltin : public CertVerifyProc {
@@ -96,8 +290,7 @@
}
bool CertVerifyProcBuiltin::SupportsOCSPStapling() const {
- // TODO(crbug.com/649017): Implement.
- return false;
+ return true;
}
scoped_refptr<ParsedCertificate> ParseCertificateFromOSHandle(
@@ -126,6 +319,9 @@
}
// Appends the SHA256 hashes of |spki_bytes| to |*hashes|.
+// TODO(eroman): Hashes are also calculated at other times (such as when
+// checking CRLSet). Consider caching to avoid recalculating (say
+// in the delegate's PathInfo).
void AppendPublicKeyHashes(const der::Input& spki_bytes,
HashValueVector* hashes) {
HashValue sha256(HASH_VALUE_SHA256);
@@ -153,6 +349,12 @@
if (errors.ContainsError(cert_errors::kCertificateRevoked))
*cert_status |= CERT_STATUS_REVOKED;
+ if (errors.ContainsError(cert_errors::kNoRevocationMechanism))
+ *cert_status |= CERT_STATUS_NO_REVOCATION_MECHANISM;
+
+ if (errors.ContainsError(cert_errors::kUnableToCheckRevocation))
+ *cert_status |= CERT_STATUS_UNABLE_TO_CHECK_REVOCATION;
+
if (errors.ContainsError(cert_errors::kUnacceptablePublicKey))
*cert_status |= CERT_STATUS_WEAK_KEY;
@@ -201,29 +403,150 @@
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) {
+void TryBuildPath(const scoped_refptr<ParsedCertificate>& target,
+ CertIssuerSourceStatic* intermediates,
+ SystemTrustStore* ssl_trust_store,
+ base::Time verification_time,
+ VerificationType verification_type,
+ int flags,
+ const std::string& ocsp_response,
+ const CRLSet* crl_set,
+ CertNetFetcher* net_fetcher,
+ const EVRootCAMetadata* ev_metadata,
+ CertPathBuilder::Result* result,
+ bool* checked_revocation) {
+ der::GeneralizedTime der_verification_time;
+ if (!der::EncodeTimeAsGeneralizedTime(verification_time,
+ &der_verification_time)) {
+ // This shouldn't be possible.
+ result->Clear();
+ return;
+ }
+
+ // Path building will require candidate paths to conform to at least one of
+ // the policies in |user_initial_policy_set|.
+ std::set<der::Input> user_initial_policy_set;
+
+ if (verification_type == VerificationType::kEV) {
+ GetEVPolicyOids(ev_metadata, target.get(), &user_initial_policy_set);
+ } else {
+ user_initial_policy_set = {AnyPolicy()};
+ }
+
+ PathBuilderDelegateImpl path_builder_delegate(
+ crl_set, net_fetcher, verification_type, flags, ssl_trust_store,
+ ocsp_response, ev_metadata, checked_revocation);
+
+ // Initialize the path builder.
+ CertPathBuilder path_builder(
+ target, ssl_trust_store->GetTrustStore(), &path_builder_delegate,
+ der_verification_time, KeyPurpose::SERVER_AUTH,
+ InitialExplicitPolicy::kFalse, user_initial_policy_set,
+ InitialPolicyMappingInhibit::kFalse, InitialAnyPolicyInhibit::kFalse,
+ result);
+
+ // Allow the path builder to discover the explicitly provided intermediates in
+ // |input_cert|.
+ path_builder.AddCertIssuerSource(intermediates);
+
+ // TODO(crbug.com/649017): Allow the path builder to discover intermediates
+ // through AIA fetching.
+
+ path_builder.Run();
+}
+
+int AssignVerifyResult(X509Certificate* input_cert,
+ const std::string& hostname,
+ CertPathBuilder::Result& result,
+ VerificationType verification_type,
+ bool checked_revocation_for_some_path,
+ SystemTrustStore* ssl_trust_store,
+ CertVerifyResult* verify_result) {
+ 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 ERR_CERT_AUTHORITY_INVALID;
+ }
+
+ // Use the best path that was built. This could be a partial path, or it could
+ // be a valid complete path.
+ const CertPathBuilderResultPath& partial_path =
+ *result.paths[result.best_result_index].get();
+
+ bool path_is_valid = partial_path.IsValid();
+
+ const ParsedCertificate* trusted_cert = partial_path.GetTrustedCert();
+ if (trusted_cert) {
+ verify_result->is_issued_by_known_root =
+ ssl_trust_store->IsKnownRoot(trusted_cert);
+
+ verify_result->is_issued_by_additional_trust_anchor =
+ ssl_trust_store->IsAdditionalTrustAnchor(trusted_cert);
+ }
+
+ if (path_is_valid && (verification_type == VerificationType::kEV)) {
+ verify_result->cert_status |= CERT_STATUS_IS_EV;
+ }
+
+ // TODO(eroman): Add documentation for the meaning of
+ // CERT_STATUS_REV_CHECKING_ENABLED. Based on the current tests it appears to
+ // mean whether revocation checking was attempted during path building,
+ // although does not necessarily mean that revocation checking was done for
+ // the final returned path.
+ if (checked_revocation_for_some_path)
+ verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
+
+ 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(path_is_valid || IsCertStatusError(verify_result->cert_status));
+
+ if (!path_is_valid) {
+ LOG(ERROR) << "CertVerifyProcBuiltin for " << hostname << " failed:\n"
+ << partial_path.errors.ToDebugString(partial_path.certs);
+ }
+
+ return IsCertStatusError(verify_result->cert_status)
+ ? MapCertStatusToNetError(verify_result->cert_status)
+ : OK;
+}
+
+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) {
CertErrors parsing_errors;
+ // VerifyInternal() is expected to carry out verifications using the current
+ // time stamp.
+ base::Time verification_time = base::Time::Now();
+
// 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;
+ return ERR_CERT_INVALID;
}
+ // Parse the provided intermediates.
+ CertIssuerSourceStatic intermediates;
+ AddIntermediatesToIssuerSource(input_cert, &intermediates);
+
+ // Parse the additional trust anchors and setup trust store.
std::unique_ptr<SystemTrustStore> ssl_trust_store =
CreateSslSystemTrustStore();
@@ -235,92 +558,47 @@
// TODO(eroman): Surface parsing errors of additional trust anchor.
}
- PathBuilderDelegateImpl path_builder_delegate(crl_set);
+ // Get the global dependencies.
+ CertNetFetcher* net_fetcher = GetGlobalCertNetFetcher();
+ const EVRootCAMetadata* ev_metadata = EVRootCAMetadata::GetInstance();
- // 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;
- }
+ // This boolean tracks whether online revocation checking was performed for
+ // *any* of the built paths, and not just the final path returned (used for
+ // setting output flag CERT_STATUS_REV_CHECKING_ENABLED).
+ bool checked_revocation_for_some_path = false;
- // Initialize the path builder.
+ // Only attempt to build EV paths if it was requested by the caller AND the
+ // target could possibly be an EV certificate.
+ const bool should_try_ev = (flags & CertVerifier::VERIFY_EV_CERT) &&
+ IsEVCandidate(ev_metadata, target.get());
+
+ // Run path building with the different parameters (attempts) until a valid
+ // path is found. Earlier successful attempts have priority over later
+ // attempts.
+
CertPathBuilder::Result result;
- CertPathBuilder path_builder(
- target, ssl_trust_store->GetTrustStore(), &path_builder_delegate,
- verification_time, KeyPurpose::SERVER_AUTH, InitialExplicitPolicy::kFalse,
- {AnyPolicy()} /* user_initial_policy_set*/,
- InitialPolicyMappingInhibit::kFalse, InitialAnyPolicyInhibit::kFalse,
- &result);
+ VerificationType verification_type = VerificationType::kDV;
- // Allow the path builder to discover the explicitly provided intermediates in
- // |input_cert|.
- CertIssuerSourceStatic intermediates;
- AddIntermediatesToIssuerSource(input_cert, &intermediates);
- path_builder.AddCertIssuerSource(&intermediates);
+ for (VerificationType cur_attempt :
+ {VerificationType::kEV, VerificationType::kDV}) {
+ // Only attempt EV if it was requested.
+ if (cur_attempt == VerificationType::kEV && !should_try_ev)
+ continue;
- // TODO(crbug.com/649017): Allow the path builder to discover intermediates
- // through AIA fetching.
+ verification_type = cur_attempt;
+ TryBuildPath(target, &intermediates, ssl_trust_store.get(),
+ verification_time, verification_type, flags, ocsp_response,
+ crl_set, net_fetcher, ev_metadata, &result,
+ &checked_revocation_for_some_path);
- 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;
+ if (result.HasValidPath())
+ break;
}
- // Use the best path that was built. This could be a partial path, or it could
- // be a valid complete path.
- const CertPathBuilderResultPath& partial_path =
- *result.paths[result.best_result_index].get();
-
- const ParsedCertificate* trusted_cert = partial_path.GetTrustedCert();
- if (trusted_cert) {
- verify_result->is_issued_by_known_root =
- ssl_trust_store->IsKnownRoot(trusted_cert);
-
- verify_result->is_issued_by_additional_trust_anchor =
- ssl_trust_store->IsAdditionalTrustAnchor(trusted_cert);
- }
-
- 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.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;
+ // Write the results to |*verify_result|.
+ return AssignVerifyResult(input_cert, hostname, result, verification_type,
+ checked_revocation_for_some_path,
+ ssl_trust_store.get(), verify_result);
}
} // namespace
diff --git a/net/cert/cert_verify_proc_unittest.cc b/net/cert/cert_verify_proc_unittest.cc
index 7c35cff..2ac2e4d 100644
--- a/net/cert/cert_verify_proc_unittest.cc
+++ b/net/cert/cert_verify_proc_unittest.cc
@@ -286,11 +286,11 @@
}
bool SupportsEV() const {
- // TODO(crbug.com/649017): CertVerifyProcBuiltin does not support EV.
// TODO(crbug.com/117478): Android and iOS do not support EV.
return verify_proc_type() == CERT_VERIFY_PROC_NSS ||
verify_proc_type() == CERT_VERIFY_PROC_WIN ||
- verify_proc_type() == CERT_VERIFY_PROC_MAC;
+ verify_proc_type() == CERT_VERIFY_PROC_MAC ||
+ verify_proc_type() == CERT_VERIFY_PROC_BUILTIN;
}
CertVerifyProc* verify_proc() const { return verify_proc_.get(); }
diff --git a/net/cert/internal/common_cert_errors.cc b/net/cert/internal/common_cert_errors.cc
index bbf152cf..db2a4ae 100644
--- a/net/cert/internal/common_cert_errors.cc
+++ b/net/cert/internal/common_cert_errors.cc
@@ -55,6 +55,9 @@
"Unacceptable signature algorithm");
DEFINE_CERT_ERROR_ID(kUnacceptablePublicKey, "Unacceptable public key");
DEFINE_CERT_ERROR_ID(kCertificateRevoked, "Certificate is revoked");
+DEFINE_CERT_ERROR_ID(kNoRevocationMechanism,
+ "Certificate lacks a revocation mechanism");
+DEFINE_CERT_ERROR_ID(kUnableToCheckRevocation, "Unable to check revocation");
} // namespace cert_errors
diff --git a/net/cert/internal/common_cert_errors.h b/net/cert/internal/common_cert_errors.h
index 27f82a72..e7efb64 100644
--- a/net/cert/internal/common_cert_errors.h
+++ b/net/cert/internal/common_cert_errors.h
@@ -115,6 +115,15 @@
// The certificate has been revoked.
NET_EXPORT extern const CertErrorId kCertificateRevoked;
+// The certificate lacks a recognized revocation mechanism (i.e. OCSP/CRL).
+// Emitted as an error when revocation checking expects certificates to have
+// such info.
+NET_EXPORT extern const CertErrorId kNoRevocationMechanism;
+
+// The certificate had a revocation mechanism, but when used it was unable to
+// affirmatively say whether the certificate was unrevoked.
+NET_EXPORT extern const CertErrorId kUnableToCheckRevocation;
+
} // namespace cert_errors
} // namespace net
diff --git a/net/cert/internal/revocation_checker.cc b/net/cert/internal/revocation_checker.cc
index cf92ef63..83c4ebf 100644
--- a/net/cert/internal/revocation_checker.cc
+++ b/net/cert/internal/revocation_checker.cc
@@ -8,11 +8,218 @@
#include "base/strings/string_piece.h"
#include "crypto/sha2.h"
+#include "net/cert/cert_net_fetcher.h"
#include "net/cert/internal/common_cert_errors.h"
+#include "net/cert/internal/ocsp.h"
#include "net/cert/internal/parsed_certificate.h"
+#include "net/cert/internal/trust_store.h"
+#include "net/cert/ocsp_verify_result.h"
+#include "url/gurl.h"
namespace net {
+namespace {
+
+void MarkCertificateRevoked(CertErrors* errors) {
+ // TODO(eroman): Add a parameter to the error indicating which mechanism
+ // caused the revocation (i.e. CRLSet, OCSP, stapled OCSP, etc).
+ errors->AddError(cert_errors::kCertificateRevoked);
+}
+
+// Checks the revocation status of |cert| according to |policy|. If the checks
+// failed, returns false and adds errors to |cert_errors|.
+//
+// TODO(eroman): Make the verification time an input.
+bool CheckCertRevocation(const ParsedCertificate* cert,
+ const ParsedCertificate* issuer_cert,
+ const RevocationPolicy& policy,
+ base::StringPiece stapled_ocsp_response,
+ CertNetFetcher* net_fetcher,
+ CertErrors* cert_errors) {
+ // Check using stapled OCSP, if available.
+ if (!stapled_ocsp_response.empty() && issuer_cert) {
+ // TODO(eroman): CheckOCSP() re-parses the certificates, perhaps just pass
+ // ParsedCertificate into it.
+ OCSPVerifyResult::ResponseStatus response_details;
+ OCSPRevocationStatus ocsp_status =
+ CheckOCSP(stapled_ocsp_response, cert->der_cert().AsStringPiece(),
+ issuer_cert->der_cert().AsStringPiece(), base::Time::Now(),
+ &response_details);
+
+ // TODO(eroman): Save the stapled OCSP response to cache.
+ switch (ocsp_status) {
+ case OCSPRevocationStatus::REVOKED:
+ MarkCertificateRevoked(cert_errors);
+ return false;
+ case OCSPRevocationStatus::GOOD:
+ return true;
+ case OCSPRevocationStatus::UNKNOWN:
+ // TODO(eroman): If the OCSP response was invalid, should we keep
+ // looking or fail?
+ break;
+ }
+ }
+
+ // TODO(eroman): Check CRL.
+
+ if (!policy.check_revocation) {
+ // TODO(eroman): Should still check CRL/OCSP caches.
+ return true;
+ }
+
+ bool found_revocation_info = false;
+ bool failed_network_fetch = false;
+
+ // Check OCSP.
+ if (cert->has_authority_info_access()) {
+ // Try each of the OCSP URIs
+ for (const auto& ocsp_uri : cert->ocsp_uris()) {
+ // Only consider http:// URLs (https:// could create a circular
+ // dependency).
+ GURL responder_url(ocsp_uri);
+ if (!responder_url.is_valid() ||
+ !responder_url.SchemeIs(url::kHttpScheme)) {
+ continue;
+ }
+
+ found_revocation_info = true;
+
+ if (!policy.networking_allowed)
+ continue;
+
+ if (!net_fetcher) {
+ LOG(ERROR) << "Cannot fetch OCSP as didn't specify a |net_fetcher|";
+ continue;
+ }
+
+ // TODO(eroman): Duplication of work if there are multiple URLs to try.
+ // TODO(eroman): Are there cases where we would need to POST instead?
+ GURL get_url = CreateOCSPGetURL(cert, issuer_cert, responder_url);
+ if (!get_url.is_valid()) {
+ // A failure here is unexpected, but could happen from BoringSSL.
+ continue;
+ }
+
+ // Fetch it over network.
+ //
+ // TODO(eroman): Issue POST instead of GET if request is larger than 255
+ // bytes?
+ // TODO(eroman): Improve interplay with HTTP cache.
+ //
+ // TODO(eroman): Bound the maximum time allowed spent doing network
+ // requests.
+ std::unique_ptr<CertNetFetcher::Request> net_ocsp_request =
+ net_fetcher->FetchOcsp(get_url, CertNetFetcher::DEFAULT,
+ CertNetFetcher::DEFAULT);
+
+ Error net_error;
+ std::vector<uint8_t> ocsp_response_bytes;
+ net_ocsp_request->WaitForResult(&net_error, &ocsp_response_bytes);
+
+ if (net_error != OK) {
+ failed_network_fetch = true;
+ continue;
+ }
+
+ OCSPVerifyResult::ResponseStatus response_details;
+
+ OCSPRevocationStatus ocsp_status = CheckOCSP(
+ base::StringPiece(
+ reinterpret_cast<const char*>(ocsp_response_bytes.data()),
+ ocsp_response_bytes.size()),
+ cert->der_cert().AsStringPiece(),
+ issuer_cert->der_cert().AsStringPiece(), base::Time::Now(),
+ &response_details);
+
+ switch (ocsp_status) {
+ case OCSPRevocationStatus::REVOKED:
+ MarkCertificateRevoked(cert_errors);
+ return false;
+ case OCSPRevocationStatus::GOOD:
+ return true;
+ case OCSPRevocationStatus::UNKNOWN:
+ break;
+ }
+ }
+ }
+
+ // Reaching here means that revocation checking was inconclusive. Determine
+ // whether failure to complete revocation checking constitutes an error.
+
+ if (!found_revocation_info) {
+ if (policy.allow_missing_info) {
+ // If the certificate lacked any (recognized) revocation mechanisms, and
+ // the policy permits it, consider revocation checking a success.
+ return true;
+ } else {
+ // If the certificate lacked any (recognized) revocation mechanisms, and
+ // the policy forbids it, fail revocation checking.
+ cert_errors->AddError(cert_errors::kNoRevocationMechanism);
+ return false;
+ }
+ }
+
+ // In soft-fail mode permit failures due to network errors.
+ // TODO(eroman): Add a warning to |cert_errors| indicating the failure.
+ if (failed_network_fetch && policy.allow_network_failure)
+ return true;
+
+ // Otherwise the policy doesn't allow revocation checking to fail.
+ cert_errors->AddError(cert_errors::kUnableToCheckRevocation);
+ return false;
+}
+
+} // namespace
+
+// The default values specify a strict revocation checking mode, in case users
+// fail to fully set the parameters.
+RevocationPolicy::RevocationPolicy()
+ : check_revocation(true),
+ networking_allowed(false),
+ allow_missing_info(false),
+ allow_network_failure(false) {}
+
+void CheckCertChainRevocation(const ParsedCertificateList& certs,
+ const CertificateTrust& last_cert_trust,
+ const RevocationPolicy& policy,
+ base::StringPiece stapled_leaf_ocsp_response,
+ CertNetFetcher* net_fetcher,
+ CertPathErrors* errors) {
+ // Check each certificate for revocation using OCSP/CRL. Checks proceed
+ // from the root certificate towards the leaf certificate. Revocation errors
+ // are added to |errors|.
+ for (size_t reverse_i = 0; reverse_i < certs.size(); ++reverse_i) {
+ size_t i = certs.size() - reverse_i - 1;
+ const ParsedCertificate* cert = certs[i].get();
+ const ParsedCertificate* issuer_cert =
+ i + 1 < certs.size() ? certs[i + 1].get() : nullptr;
+
+ // Trust anchors bypass OCSP/CRL revocation checks. (The only way to revoke
+ // trust anchors is via CRLSet or the built-in SPKI blacklist).
+ if (reverse_i == 0 && last_cert_trust.IsTrustAnchor())
+ continue;
+
+ // TODO(eroman): Plumb stapled OCSP for non-leaf certificates from TLS?
+ base::StringPiece stapled_ocsp =
+ (i == 0) ? stapled_leaf_ocsp_response : base::StringPiece();
+
+ // Check whether this certificate's revocation status complies with the
+ // policy.
+ bool cert_ok =
+ CheckCertRevocation(cert, issuer_cert, policy, stapled_ocsp,
+ net_fetcher, errors->GetErrorsForCert(i));
+
+ if (!cert_ok) {
+ // If any certificate in the chain fails revocation checks, the chain is
+ // revoked and no need to check revocation status for the remaining
+ // certificates.
+ DCHECK(errors->GetErrorsForCert(i)->ContainsAnyErrorWithSeverity(
+ CertError::SEVERITY_HIGH));
+ break;
+ }
+ }
+}
+
CRLSet::Result CheckChainRevocationUsingCRLSet(
const CRLSet* crl_set,
const ParsedCertificateList& certs,
@@ -46,7 +253,7 @@
switch (result) {
case CRLSet::REVOKED:
- errors->GetErrorsForCert(i)->AddError(cert_errors::kCertificateRevoked);
+ MarkCertificateRevoked(errors->GetErrorsForCert(i));
return CRLSet::Result::REVOKED;
case CRLSet::UNKNOWN:
// If the status is unknown, advance to the subordinate certificate.
diff --git a/net/cert/internal/revocation_checker.h b/net/cert/internal/revocation_checker.h
index 2252f38..2deddbb 100644
--- a/net/cert/internal/revocation_checker.h
+++ b/net/cert/internal/revocation_checker.h
@@ -13,6 +13,60 @@
namespace net {
class CertPathErrors;
+class CertNetFetcher;
+struct CertificateTrust;
+
+// RevocationPolicy describes how revocation should be carried out for a
+// particular chain.
+struct NET_EXPORT_PRIVATE RevocationPolicy {
+ // Callers should not rely on the default-initialized value, but should fully
+ // specify all the parameters.
+ RevocationPolicy();
+
+ // If |check_revocation| is true, then revocation checking is mandatory. This
+ // means that every certificate in the chain (excluding trust anchors) must
+ // have valid (unexpired) revocation information proving it to be unrevoked.
+ //
+ // The mechanisms used for checking revocation may include stapled OCSP,
+ // cached OCSP, online OCSP, cached CRL, online CRL.
+ //
+ // The other properties of RevocationPolicy place further constraints on how
+ // revocation checking may proceed.
+ bool check_revocation : 1;
+
+ // If |networking_allowed| is true then revocation checking is allowed to
+ // issue network requests in order to fetch fresh OCSP/CRL. Otherwise
+ // networking is not permitted in the course of revocation checking.
+ bool networking_allowed : 1;
+
+ // If set to true, considers certificates lacking URLs for OCSP/CRL to be
+ // unrevoked. Otherwise will fail for certificates lacking revocation
+ // mechanisms.
+ bool allow_missing_info : 1;
+
+ // If set to true, failure to perform online revocation checks (due to a
+ // network level failure) is considered equivalent to a successful revocation
+ // check.
+ //
+ // TODO(649017): The "soft fail" expectations of consumers are more broad than
+ // this, and may also entail parsing failures and parsed non-success OCSP
+ // responses.
+ bool allow_network_failure : 1;
+};
+
+// Checks the revocation status of |certs| according to |policy|, and adds
+// any failures to |errors|. On failure errors are added to |errors|. On success
+// no errors are added.
+//
+// |net_fetcher| may be null, however this may lead to failed revocation checks
+// depending on |policy|.
+NET_EXPORT_PRIVATE void CheckCertChainRevocation(
+ const ParsedCertificateList& certs,
+ const CertificateTrust& last_cert_trust,
+ const RevocationPolicy& policy,
+ base::StringPiece stapled_leaf_ocsp_response,
+ CertNetFetcher* net_fetcher,
+ CertPathErrors* errors);
// Checks the revocation status of a certificate chain using the CRLSet and adds
// revocation errors to |errors|.
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index e28bf04..def50757 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -10436,6 +10436,8 @@
#if defined(OS_WIN) || defined(OS_MACOSX)
// Windows can return CERT_STATUS_UNABLE_TO_CHECK_REVOCATION but we don't
// have that ability on other platforms.
+ // TODO(eroman): Should this also be the return value for
+ // CertVerifyProcBuiltin?
return CERT_STATUS_UNABLE_TO_CHECK_REVOCATION;
#else
return 0;
@@ -10450,8 +10452,8 @@
// If it does not, then tests which rely on 'hard fail' behaviour should be
// skipped.
static bool SystemSupportsHardFailRevocationChecking() {
-#if defined(OS_WIN) || defined(USE_NSS_CERTS)
- // TODO(crbug.com/762380): Enable on Fuchsia once it's implemented.
+#if defined(OS_WIN) || defined(USE_NSS_CERTS) || \
+ defined(USE_BUILTIN_CERT_VERIFIER)
return true;
#else
return false;
@@ -10484,9 +10486,8 @@
}
static bool SystemSupportsOCSP() {
-#if defined(OS_ANDROID) || defined(USE_BUILTIN_CERT_VERIFIER)
+#if defined(OS_ANDROID)
// TODO(jnd): http://crbug.com/117478 - EV verification is not yet supported.
- // TODO(crbug.com/762380): Enable on Fuchsia once it's implemented.
return false;
#else
return true;
@@ -10494,8 +10495,8 @@
}
static bool SystemSupportsOCSPStapling() {
-#if defined(USE_NSS_CERTS) || defined(OS_WIN)
- // TODO(crbug.com/762380): Enable on Fuchsia once it's implemented.
+#if defined(USE_NSS_CERTS) || defined(OS_WIN) || \
+ defined(USE_BUILTIN_CERT_VERIFIER)
return true;
#else
return false;
@@ -10555,8 +10556,17 @@
CertStatus cert_status;
DoConnection(ssl_options, &cert_status);
+#if defined(USE_BUILTIN_CERT_VERIFIER)
+ // TODO(649017): This test uses soft-fail revocation checking, but returns an
+ // invalid OCSP response (can't parse). CertVerifyProcBuiltin currently
+ // doesn't consider this a candidate for soft-fail (only considers
+ // network-level failures as skippable).
+ EXPECT_EQ(CERT_STATUS_UNABLE_TO_CHECK_REVOCATION,
+ cert_status & CERT_STATUS_UNABLE_TO_CHECK_REVOCATION);
+#else
EXPECT_EQ(ExpectedCertStatusForFailedOnlineRevocationCheck(),
cert_status & CERT_STATUS_ALL_ERRORS);
+#endif
// Without a positive OCSP response, we shouldn't show the EV status.
EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
@@ -11216,10 +11226,17 @@
CertStatus cert_status;
DoConnection(ssl_options, &cert_status);
- EXPECT_EQ(CERT_STATUS_REVOKED,
- cert_status & CERT_STATUS_REVOKED);
+#if defined(USE_BUILTIN_CERT_VERIFIER)
+ // TODO(crbug.com/649017): Should we consider invalid response as
+ // affirmatively revoked?
+ EXPECT_EQ(CERT_STATUS_UNABLE_TO_CHECK_REVOCATION,
+ cert_status & CERT_STATUS_UNABLE_TO_CHECK_REVOCATION);
+#else
+ EXPECT_EQ(CERT_STATUS_REVOKED, cert_status & CERT_STATUS_REVOKED);
+#endif
// Without a positive OCSP response, we shouldn't show the EV status.
+ EXPECT_FALSE(cert_status & CERT_STATUS_IS_EV);
EXPECT_TRUE(cert_status & CERT_STATUS_REV_CHECKING_ENABLED);
}