diff options
36 files changed, 369 insertions, 426 deletions
diff --git a/chrome/browser/chromeos/net/cert_verify_proc_chromeos.cc b/chrome/browser/chromeos/net/cert_verify_proc_chromeos.cc index be4621e..59f6a5b 100644 --- a/chrome/browser/chromeos/net/cert_verify_proc_chromeos.cc +++ b/chrome/browser/chromeos/net/cert_verify_proc_chromeos.cc @@ -40,6 +40,7 @@ CertVerifyProcChromeOS::~CertVerifyProcChromeOS() {} int CertVerifyProcChromeOS::VerifyInternal( net::X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, net::CRLSet* crl_set, const net::CertificateList& additional_trust_anchors, @@ -52,12 +53,8 @@ int CertVerifyProcChromeOS::VerifyInternal( chain_verify_callback.isChainValidArg = static_cast<void*>(&chain_verify_args); - return VerifyInternalImpl(cert, - hostname, - flags, - crl_set, - additional_trust_anchors, - &chain_verify_callback, + return VerifyInternalImpl(cert, hostname, ocsp_response, flags, crl_set, + additional_trust_anchors, &chain_verify_callback, verify_result); } diff --git a/chrome/browser/chromeos/net/cert_verify_proc_chromeos.h b/chrome/browser/chromeos/net/cert_verify_proc_chromeos.h index b79ac8a..346642f 100644 --- a/chrome/browser/chromeos/net/cert_verify_proc_chromeos.h +++ b/chrome/browser/chromeos/net/cert_verify_proc_chromeos.h @@ -34,6 +34,7 @@ class CertVerifyProcChromeOS : public net::CertVerifyProcNSS { // net::CertVerifyProcNSS implementation: int VerifyInternal(net::X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, net::CRLSet* crl_set, const net::CertificateList& additional_trust_anchors, diff --git a/chrome/browser/chromeos/net/cert_verify_proc_chromeos_unittest.cc b/chrome/browser/chromeos/net/cert_verify_proc_chromeos_unittest.cc index f7deba8..dc65814 100644 --- a/chrome/browser/chromeos/net/cert_verify_proc_chromeos_unittest.cc +++ b/chrome/browser/chromeos/net/cert_verify_proc_chromeos_unittest.cc @@ -80,12 +80,9 @@ class CertVerifyProcChromeOSTest : public testing::Test { std::string* root_subject_name) { int flags = 0; net::CertVerifyResult verify_result; - int error = verify_proc->Verify(cert, - "127.0.0.1", - flags, - NULL, - additional_trust_anchors, - &verify_result); + int error = + verify_proc->Verify(cert, "127.0.0.1", std::string(), flags, NULL, + additional_trust_anchors, &verify_result); if (verify_result.verified_cert.get() && !verify_result.verified_cert->GetIntermediateCertificates().empty()) { net::X509Certificate::OSCertHandle root = diff --git a/chrome/browser/chromeos/policy/policy_cert_verifier.cc b/chrome/browser/chromeos/policy/policy_cert_verifier.cc index cca86b06..d3abda8e 100644 --- a/chrome/browser/chromeos/policy/policy_cert_verifier.cc +++ b/chrome/browser/chromeos/policy/policy_cert_verifier.cc @@ -71,6 +71,7 @@ void PolicyCertVerifier::SetTrustAnchors( int PolicyCertVerifier::Verify( net::X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, net::CRLSet* crl_set, net::CertVerifyResult* verify_result, @@ -84,8 +85,9 @@ int PolicyCertVerifier::Verify( anchor_used_callback_, completion_callback, verify_result); - int error = delegate_->Verify(cert, hostname, flags, crl_set, verify_result, - wrapped_callback, out_req, net_log); + int error = + delegate_->Verify(cert, hostname, ocsp_response, flags, crl_set, + verify_result, wrapped_callback, out_req, net_log); MaybeSignalAnchorUse(error, anchor_used_callback_, *verify_result); return error; } @@ -95,6 +97,11 @@ void PolicyCertVerifier::CancelRequest(RequestHandle req) { delegate_->CancelRequest(req); } +bool PolicyCertVerifier::SupportsOCSPStapling() { + DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)); + return delegate_->SupportsOCSPStapling(); +} + const net::CertificateList& PolicyCertVerifier::GetAdditionalTrustAnchors() { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)); return trust_anchors_; diff --git a/chrome/browser/chromeos/policy/policy_cert_verifier.h b/chrome/browser/chromeos/policy/policy_cert_verifier.h index 11060cc..774b36b 100644 --- a/chrome/browser/chromeos/policy/policy_cert_verifier.h +++ b/chrome/browser/chromeos/policy/policy_cert_verifier.h @@ -49,6 +49,7 @@ class PolicyCertVerifier : public net::CertVerifier, // Note: |callback| can be null. int Verify(net::X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, net::CRLSet* crl_set, net::CertVerifyResult* verify_result, @@ -58,6 +59,8 @@ class PolicyCertVerifier : public net::CertVerifier, void CancelRequest(RequestHandle req) override; + bool SupportsOCSPStapling() override; + // CertTrustAnchorProvider: const net::CertificateList& GetAdditionalTrustAnchors() override; diff --git a/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc b/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc index 4d6bff1..aeb626e 100644 --- a/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc +++ b/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc @@ -67,13 +67,9 @@ class PolicyCertVerifierTest : public testing::Test { int VerifyTestServerCert(const net::TestCompletionCallback& test_callback, net::CertVerifyResult* verify_result, net::CertVerifier::RequestHandle* request_handle) { - return cert_verifier_->Verify(test_server_cert_.get(), - "127.0.0.1", - 0, - NULL, - verify_result, - test_callback.callback(), - request_handle, + return cert_verifier_->Verify(test_server_cert_.get(), "127.0.0.1", + std::string(), 0, NULL, verify_result, + test_callback.callback(), request_handle, net::BoundNetLog()); } diff --git a/google_apis/gcm/tools/mcs_probe.cc b/google_apis/gcm/tools/mcs_probe.cc index 0f464c5..bbff7ff 100644 --- a/google_apis/gcm/tools/mcs_probe.cc +++ b/google_apis/gcm/tools/mcs_probe.cc @@ -171,6 +171,7 @@ class MyTestCertVerifier : public net::CertVerifier { int Verify(net::X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, net::CRLSet* crl_set, net::CertVerifyResult* verify_result, diff --git a/net/cert/cert_verifier.cc b/net/cert/cert_verifier.cc index e4acec4..4152d55 100644 --- a/net/cert/cert_verifier.cc +++ b/net/cert/cert_verifier.cc @@ -4,13 +4,28 @@ #include "net/cert/cert_verifier.h" +#include "build/build_config.h" #include "net/cert/cert_verify_proc.h" + +#if defined(OS_NACL) +#include "base/logging.h" +#else #include "net/cert/multi_threaded_cert_verifier.h" +#endif namespace net { +bool CertVerifier::SupportsOCSPStapling() { + return false; +} + CertVerifier* CertVerifier::CreateDefault() { +#if defined(OS_NACL) + NOTIMPLEMENTED(); + return nullptr; +#else return new MultiThreadedCertVerifier(CertVerifyProc::CreateDefault()); +#endif } } // namespace net diff --git a/net/cert/cert_verifier.h b/net/cert/cert_verifier.h index 743c835..71a3478 100644 --- a/net/cert/cert_verifier.h +++ b/net/cert/cert_verifier.h @@ -81,6 +81,8 @@ class NET_EXPORT CertVerifier { // |verify_result->cert_status|, and the error code for the most serious // error is returned. // + // |ocsp_response|, if non-empty, is a stapled OCSP response to use. + // // |flags| is bitwise OR'd of VerifyFlags. // If VERIFY_REV_CHECKING_ENABLED is set in |flags|, certificate revocation // checking is performed. @@ -103,6 +105,7 @@ class NET_EXPORT CertVerifier { // TODO(rsleevi): Move CRLSet* out of the CertVerifier signature. virtual int Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, @@ -114,6 +117,9 @@ class NET_EXPORT CertVerifier { // After a request is canceled, its completion callback will not be called. virtual void CancelRequest(RequestHandle req) = 0; + // Returns true if this CertVerifier supports stapled OCSP responses. + virtual bool SupportsOCSPStapling(); + // Creates a CertVerifier implementation that verifies certificates using // the preferred underlying cryptographic libraries. static CertVerifier* CreateDefault(); diff --git a/net/cert/cert_verify_proc.cc b/net/cert/cert_verify_proc.cc index 1e3fc89..9ea19b4 100644 --- a/net/cert/cert_verify_proc.cc +++ b/net/cert/cert_verify_proc.cc @@ -190,6 +190,7 @@ CertVerifyProc::~CertVerifyProc() {} int CertVerifyProc::Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -209,7 +210,7 @@ int CertVerifyProc::Verify(X509Certificate* cert, if (flags & CertVerifier::VERIFY_EV_CERT) flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED_EV_ONLY; - int rv = VerifyInternal(cert, hostname, flags, crl_set, + int rv = VerifyInternal(cert, hostname, ocsp_response, flags, crl_set, additional_trust_anchors, verify_result); UMA_HISTOGRAM_BOOLEAN("Net.CertCommonNameFallback", diff --git a/net/cert/cert_verify_proc.h b/net/cert/cert_verify_proc.h index 3cea884..fdc1205 100644 --- a/net/cert/cert_verify_proc.h +++ b/net/cert/cert_verify_proc.h @@ -38,6 +38,8 @@ class NET_EXPORT CertVerifyProc // |verify_result->cert_status|, and the error code for the most serious // error is returned. // + // |ocsp_response|, if non-empty, is a stapled OCSP response to use. + // // |flags| is bitwise OR'd of VerifyFlags: // // If VERIFY_REV_CHECKING_ENABLED is set in |flags|, online certificate @@ -56,6 +58,7 @@ class NET_EXPORT CertVerifyProc // implementation. int Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -66,6 +69,11 @@ class NET_EXPORT CertVerifyProc // passed to Verify() is ignored when this returns false. virtual bool SupportsAdditionalTrustAnchors() const = 0; + // Returns true if the implementation supports passing a stapled OCSP response + // to the Verify() call. The |ocsp_response| parameter passed to Verify() is + // ignored when this returns false. + virtual bool SupportsOCSPStapling() const = 0; + protected: CertVerifyProc(); virtual ~CertVerifyProc(); @@ -81,6 +89,7 @@ class NET_EXPORT CertVerifyProc // value must be left untouched. virtual int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_android.cc b/net/cert/cert_verify_proc_android.cc index 91d0b55..6689ad8 100644 --- a/net/cert/cert_verify_proc_android.cc +++ b/net/cert/cert_verify_proc_android.cc @@ -155,9 +155,14 @@ bool CertVerifyProcAndroid::SupportsAdditionalTrustAnchors() const { return false; } +bool CertVerifyProcAndroid::SupportsOCSPStapling() const { + return false; +} + int CertVerifyProcAndroid::VerifyInternal( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_android.h b/net/cert/cert_verify_proc_android.h index 6830df6..755ce80 100644 --- a/net/cert/cert_verify_proc_android.h +++ b/net/cert/cert_verify_proc_android.h @@ -16,6 +16,7 @@ class CertVerifyProcAndroid : public CertVerifyProc { CertVerifyProcAndroid(); bool SupportsAdditionalTrustAnchors() const override; + bool SupportsOCSPStapling() const override; protected: ~CertVerifyProcAndroid() override; @@ -23,6 +24,7 @@ class CertVerifyProcAndroid : public CertVerifyProc { private: int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_mac.cc b/net/cert/cert_verify_proc_mac.cc index b84612b..d298fee 100644 --- a/net/cert/cert_verify_proc_mac.cc +++ b/net/cert/cert_verify_proc_mac.cc @@ -476,9 +476,16 @@ bool CertVerifyProcMac::SupportsAdditionalTrustAnchors() const { return false; } +bool CertVerifyProcMac::SupportsOCSPStapling() const { + // TODO(rsleevi): Plumb an OCSP response into the Mac system library. + // https://crbug.com/430714 + return false; +} + int CertVerifyProcMac::VerifyInternal( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_mac.h b/net/cert/cert_verify_proc_mac.h index 5cd7f4c..3c87edd 100644 --- a/net/cert/cert_verify_proc_mac.h +++ b/net/cert/cert_verify_proc_mac.h @@ -16,6 +16,7 @@ class CertVerifyProcMac : public CertVerifyProc { CertVerifyProcMac(); bool SupportsAdditionalTrustAnchors() const override; + bool SupportsOCSPStapling() const override; protected: ~CertVerifyProcMac() override; @@ -23,6 +24,7 @@ class CertVerifyProcMac : public CertVerifyProc { private: int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_nss.cc b/net/cert/cert_verify_proc_nss.cc index 1236f41..02a584f 100644 --- a/net/cert/cert_verify_proc_nss.cc +++ b/net/cert/cert_verify_proc_nss.cc @@ -33,6 +33,10 @@ #include "net/cert/x509_util_ios.h" #endif // defined(OS_IOS) +#if defined(USE_NSS_CERTS) +#include <dlfcn.h> +#endif + namespace net { namespace { @@ -756,7 +760,14 @@ CERTCertList* CertificateListToCERTCertList(const CertificateList& list) { } // namespace -CertVerifyProcNSS::CertVerifyProcNSS() {} +CertVerifyProcNSS::CertVerifyProcNSS() +#if defined(USE_NSS_CERTS) + : cache_ocsp_response_from_side_channel_( + reinterpret_cast<CacheOCSPResponseFromSideChannelFunction>( + dlsym(RTLD_DEFAULT, "CERT_CacheOCSPResponseFromSideChannel"))) +#endif +{ +} CertVerifyProcNSS::~CertVerifyProcNSS() {} @@ -764,9 +775,19 @@ bool CertVerifyProcNSS::SupportsAdditionalTrustAnchors() const { return true; } +bool CertVerifyProcNSS::SupportsOCSPStapling() const { +#if defined(USE_NSS_CERTS) + return cache_ocsp_response_from_side_channel_; +#else + // TODO(davidben): Support OCSP stapling on iOS. + return false; +#endif +} + int CertVerifyProcNSS::VerifyInternalImpl( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -781,6 +802,21 @@ int CertVerifyProcNSS::VerifyInternalImpl( CERTCertificate* cert_handle = cert->os_cert_handle(); #endif // defined(OS_IOS) +#if defined(USE_NSS_CERTS) + if (!ocsp_response.empty() && cache_ocsp_response_from_side_channel_) { + // Note: NSS uses a thread-safe global hash table, so this call will + // affect any concurrent verification operations on |cert| or copies of + // the same certificate. This is an unavoidable limitation of NSS's OCSP + // API. + SECItem ocsp_response_item; + ocsp_response_item.data = reinterpret_cast<unsigned char*>( + const_cast<char*>(ocsp_response.data())); + ocsp_response_item.len = ocsp_response.size(); + cache_ocsp_response_from_side_channel_(CERT_GetDefaultCertDB(), cert_handle, + PR_Now(), &ocsp_response_item, NULL); + } +#endif // defined(USE_NSS_CERTS) + if (!cert->VerifyNameMatch(hostname, &verify_result->common_name_fallback_used)) { verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID; @@ -928,14 +964,12 @@ int CertVerifyProcNSS::VerifyInternalImpl( int CertVerifyProcNSS::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) { - return VerifyInternalImpl(cert, - hostname, - flags, - crl_set, + return VerifyInternalImpl(cert, hostname, ocsp_response, flags, crl_set, additional_trust_anchors, NULL, // chain_verify_callback verify_result); diff --git a/net/cert/cert_verify_proc_nss.h b/net/cert/cert_verify_proc_nss.h index 395facf..5a4b361 100644 --- a/net/cert/cert_verify_proc_nss.h +++ b/net/cert/cert_verify_proc_nss.h @@ -18,6 +18,7 @@ class NET_EXPORT_PRIVATE CertVerifyProcNSS : public CertVerifyProc { CertVerifyProcNSS(); bool SupportsAdditionalTrustAnchors() const override; + bool SupportsOCSPStapling() const override; protected: ~CertVerifyProcNSS() override; @@ -27,6 +28,7 @@ class NET_EXPORT_PRIVATE CertVerifyProcNSS : public CertVerifyProc { // CERTChainVerifyCallbackFunc in NSS's lib/certdb/certt.h. int VerifyInternalImpl(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -36,10 +38,22 @@ class NET_EXPORT_PRIVATE CertVerifyProcNSS : public CertVerifyProc { 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; + +#if defined(USE_NSS_CERTS) + using CacheOCSPResponseFromSideChannelFunction = + SECStatus (*)(CERTCertDBHandle* handle, + CERTCertificate* cert, + PRTime time, + SECItem* encodedResponse, + void* pwArg); + const CacheOCSPResponseFromSideChannelFunction + cache_ocsp_response_from_side_channel_; +#endif }; } // namespace net diff --git a/net/cert/cert_verify_proc_openssl.cc b/net/cert/cert_verify_proc_openssl.cc index 890462e..5649245 100644 --- a/net/cert/cert_verify_proc_openssl.cc +++ b/net/cert/cert_verify_proc_openssl.cc @@ -190,9 +190,14 @@ bool CertVerifyProcOpenSSL::SupportsAdditionalTrustAnchors() const { return false; } +bool CertVerifyProcOpenSSL::SupportsOCSPStapling() const { + return false; +} + int CertVerifyProcOpenSSL::VerifyInternal( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_openssl.h b/net/cert/cert_verify_proc_openssl.h index 979d65a..5836386 100644 --- a/net/cert/cert_verify_proc_openssl.h +++ b/net/cert/cert_verify_proc_openssl.h @@ -15,6 +15,7 @@ class CertVerifyProcOpenSSL : public CertVerifyProc { CertVerifyProcOpenSSL(); bool SupportsAdditionalTrustAnchors() const override; + bool SupportsOCSPStapling() const override; protected: ~CertVerifyProcOpenSSL() override; @@ -22,6 +23,7 @@ class CertVerifyProcOpenSSL : public CertVerifyProc { private: int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/cert_verify_proc_unittest.cc b/net/cert/cert_verify_proc_unittest.cc index ed1c8fa..9208e86 100644 --- a/net/cert/cert_verify_proc_unittest.cc +++ b/net/cert/cert_verify_proc_unittest.cc @@ -57,6 +57,7 @@ class WellKnownCaCertVerifyProc : public CertVerifyProc { // CertVerifyProc implementation: bool SupportsAdditionalTrustAnchors() const override { return false; } + bool SupportsOCSPStapling() const override { return false; } protected: ~WellKnownCaCertVerifyProc() override {} @@ -64,6 +65,7 @@ class WellKnownCaCertVerifyProc : public CertVerifyProc { private: int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -77,6 +79,7 @@ class WellKnownCaCertVerifyProc : public CertVerifyProc { int WellKnownCaCertVerifyProc::VerifyInternal( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -125,7 +128,7 @@ class CertVerifyProcTest : public testing::Test { CRLSet* crl_set, const CertificateList& additional_trust_anchors, CertVerifyResult* verify_result) { - return verify_proc_->Verify(cert, hostname, flags, crl_set, + return verify_proc_->Verify(cert, hostname, std::string(), flags, crl_set, additional_trust_anchors, verify_result); } diff --git a/net/cert/cert_verify_proc_win.cc b/net/cert/cert_verify_proc_win.cc index 13a337b..21572b4 100644 --- a/net/cert/cert_verify_proc_win.cc +++ b/net/cert/cert_verify_proc_win.cc @@ -559,9 +559,19 @@ bool CertVerifyProcWin::SupportsAdditionalTrustAnchors() const { return false; } +bool CertVerifyProcWin::SupportsOCSPStapling() const { + // CERT_OCSP_RESPONSE_PROP_ID is only implemented on Vista+, but it can be + // set on Windows XP without error. There is some overhead from the server + // sending the OCSP response if it supports the extension, for the subset of + // XP clients who will request it but be unable to use it, but this is an + // acceptable trade-off for simplicity of implementation. + return true; +} + int CertVerifyProcWin::VerifyInternal( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -633,6 +643,18 @@ int CertVerifyProcWin::VerifyInternal( chain_engine.reset(TestRootCerts::GetInstance()->GetChainEngine()); ScopedPCCERT_CONTEXT cert_list(cert->CreateOSCertChainForCert()); + + if (!ocsp_response.empty()) { + // Attach the OCSP response to the chain. + CRYPT_DATA_BLOB ocsp_response_blob; + ocsp_response_blob.cbData = ocsp_response.size(); + ocsp_response_blob.pbData = + reinterpret_cast<BYTE*>(const_cast<char*>(ocsp_response.data())); + CertSetCertificateContextProperty( + cert_list.get(), CERT_OCSP_RESPONSE_PROP_ID, + CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, &ocsp_response_blob); + } + PCCERT_CHAIN_CONTEXT chain_context; // IE passes a non-NULL pTime argument that specifies the current system // time. IE passes CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT as the diff --git a/net/cert/cert_verify_proc_win.h b/net/cert/cert_verify_proc_win.h index 9c2d26a..c56c821 100644 --- a/net/cert/cert_verify_proc_win.h +++ b/net/cert/cert_verify_proc_win.h @@ -16,6 +16,7 @@ class CertVerifyProcWin : public CertVerifyProc { CertVerifyProcWin(); bool SupportsAdditionalTrustAnchors() const override; + bool SupportsOCSPStapling() const override; protected: ~CertVerifyProcWin() override; @@ -23,6 +24,7 @@ class CertVerifyProcWin : public CertVerifyProc { private: int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, diff --git a/net/cert/mock_cert_verifier.cc b/net/cert/mock_cert_verifier.cc index ea5538e..faffacf 100644 --- a/net/cert/mock_cert_verifier.cc +++ b/net/cert/mock_cert_verifier.cc @@ -38,6 +38,7 @@ MockCertVerifier::~MockCertVerifier() {} int MockCertVerifier::Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, diff --git a/net/cert/mock_cert_verifier.h b/net/cert/mock_cert_verifier.h index 000da11..1ccfe62 100644 --- a/net/cert/mock_cert_verifier.h +++ b/net/cert/mock_cert_verifier.h @@ -27,6 +27,7 @@ class MockCertVerifier : public CertVerifier { // CertVerifier implementation int Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, diff --git a/net/cert/multi_threaded_cert_verifier.cc b/net/cert/multi_threaded_cert_verifier.cc index 6012a35..44eff8e 100644 --- a/net/cert/multi_threaded_cert_verifier.cc +++ b/net/cert/multi_threaded_cert_verifier.cc @@ -12,6 +12,7 @@ #include "base/message_loop/message_loop.h" #include "base/metrics/histogram.h" #include "base/profiler/scoped_tracker.h" +#include "base/sha1.h" #include "base/stl_util.h" #include "base/synchronization/lock.h" #include "base/threading/worker_pool.h" @@ -218,6 +219,7 @@ class CertVerifierWorker { CertVerifierWorker(CertVerifyProc* verify_proc, X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -225,14 +227,14 @@ class CertVerifierWorker { : verify_proc_(verify_proc), cert_(cert), hostname_(hostname), + ocsp_response_(ocsp_response), flags_(flags), crl_set_(crl_set), additional_trust_anchors_(additional_trust_anchors), origin_loop_(base::MessageLoop::current()), cert_verifier_(cert_verifier), canceled_(false), - error_(ERR_FAILED) { - } + error_(ERR_FAILED) {} // Returns the certificate being verified. May only be called /before/ // Start() is called. @@ -257,12 +259,9 @@ class CertVerifierWorker { private: void Run() { // Runs on a worker thread. - error_ = verify_proc_->Verify(cert_.get(), - hostname_, - flags_, - crl_set_.get(), - additional_trust_anchors_, - &verify_result_); + error_ = verify_proc_->Verify(cert_.get(), hostname_, ocsp_response_, + flags_, crl_set_.get(), + additional_trust_anchors_, &verify_result_); #if defined(USE_NSS_CERTS) || defined(OS_IOS) // Detach the thread from NSPR. // Calling NSS functions attaches the thread to NSPR, which stores @@ -290,11 +289,8 @@ class CertVerifierWorker { // memory leaks or worse errors. base::AutoLock locked(lock_); if (!canceled_) { - cert_verifier_->HandleResult(cert_.get(), - hostname_, - flags_, - additional_trust_anchors_, - error_, + cert_verifier_->HandleResult(cert_.get(), hostname_, ocsp_response_, + flags_, additional_trust_anchors_, error_, verify_result_); } } @@ -331,6 +327,7 @@ class CertVerifierWorker { scoped_refptr<CertVerifyProc> verify_proc_; scoped_refptr<X509Certificate> cert_; const std::string hostname_; + const std::string ocsp_response_; const int flags_; scoped_refptr<CRLSet> crl_set_; const CertificateList additional_trust_anchors_; @@ -460,6 +457,7 @@ void MultiThreadedCertVerifier::SetCertTrustAnchorProvider( int MultiThreadedCertVerifier::Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, @@ -480,8 +478,8 @@ int MultiThreadedCertVerifier::Verify(X509Certificate* cert, trust_anchor_provider_ ? trust_anchor_provider_->GetAdditionalTrustAnchors() : empty_cert_list; - const RequestParams key(cert->fingerprint(), cert->ca_fingerprint(), - hostname, flags, additional_trust_anchors); + const RequestParams key(cert->fingerprint(), cert->ca_fingerprint(), hostname, + ocsp_response, flags, additional_trust_anchors); const CertVerifierCache::value_type* cached_entry = cache_.Get(key, CacheValidityPeriod(base::Time::Now())); if (cached_entry) { @@ -502,14 +500,9 @@ int MultiThreadedCertVerifier::Verify(X509Certificate* cert, job = j->second; } else { // Need to make a new request. - CertVerifierWorker* worker = - new CertVerifierWorker(verify_proc_.get(), - cert, - hostname, - flags, - crl_set, - additional_trust_anchors, - this); + CertVerifierWorker* worker = new CertVerifierWorker( + verify_proc_.get(), cert, hostname, ocsp_response, flags, crl_set, + additional_trust_anchors, this); job = new CertVerifierJob( worker, BoundNetLog::Make(net_log.net_log(), NetLog::SOURCE_CERT_VERIFIER_JOB)); @@ -541,15 +534,24 @@ void MultiThreadedCertVerifier::CancelRequest(RequestHandle req) { request->Cancel(); } +bool MultiThreadedCertVerifier::SupportsOCSPStapling() { + return verify_proc_->SupportsOCSPStapling(); +} + MultiThreadedCertVerifier::RequestParams::RequestParams( const SHA1HashValue& cert_fingerprint_arg, const SHA1HashValue& ca_fingerprint_arg, const std::string& hostname_arg, + const std::string& ocsp_response_arg, int flags_arg, const CertificateList& additional_trust_anchors) - : hostname(hostname_arg), - flags(flags_arg) { - hash_values.reserve(2 + additional_trust_anchors.size()); + : hostname(hostname_arg), flags(flags_arg) { + hash_values.reserve(3 + additional_trust_anchors.size()); + SHA1HashValue ocsp_hash; + base::SHA1HashBytes( + reinterpret_cast<const unsigned char*>(ocsp_response_arg.data()), + ocsp_response_arg.size(), ocsp_hash.data); + hash_values.push_back(ocsp_hash); hash_values.push_back(cert_fingerprint_arg); hash_values.push_back(ca_fingerprint_arg); for (size_t i = 0; i < additional_trust_anchors.size(); ++i) @@ -560,9 +562,9 @@ MultiThreadedCertVerifier::RequestParams::~RequestParams() {} bool MultiThreadedCertVerifier::RequestParams::operator<( const RequestParams& other) const { - // |flags| is compared before |cert_fingerprint|, |ca_fingerprint|, and - // |hostname| under assumption that integer comparisons are faster than - // memory and string comparisons. + // |flags| is compared before |cert_fingerprint|, |ca_fingerprint|, + // |hostname|, and |ocsp_response|, under assumption that integer comparisons + // are faster than memory and string comparisons. if (flags != other.flags) return flags < other.flags; if (hostname != other.hostname) @@ -577,14 +579,15 @@ bool MultiThreadedCertVerifier::RequestParams::operator<( void MultiThreadedCertVerifier::HandleResult( X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, const CertificateList& additional_trust_anchors, int error, const CertVerifyResult& verify_result) { DCHECK(CalledOnValidThread()); - const RequestParams key(cert->fingerprint(), cert->ca_fingerprint(), - hostname, flags, additional_trust_anchors); + const RequestParams key(cert->fingerprint(), cert->ca_fingerprint(), hostname, + ocsp_response, flags, additional_trust_anchors); CachedResult cached_result; cached_result.error = error; diff --git a/net/cert/multi_threaded_cert_verifier.h b/net/cert/multi_threaded_cert_verifier.h index 6880960..bfcab38 100644 --- a/net/cert/multi_threaded_cert_verifier.h +++ b/net/cert/multi_threaded_cert_verifier.h @@ -56,6 +56,7 @@ class NET_EXPORT_PRIVATE MultiThreadedCertVerifier // CertVerifier implementation int Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, @@ -65,6 +66,8 @@ class NET_EXPORT_PRIVATE MultiThreadedCertVerifier void CancelRequest(CertVerifier::RequestHandle req) override; + bool SupportsOCSPStapling() override; + private: friend class CertVerifierWorker; // Calls HandleResult. friend class CertVerifierRequest; @@ -84,6 +87,7 @@ class NET_EXPORT_PRIVATE MultiThreadedCertVerifier RequestParams(const SHA1HashValue& cert_fingerprint_arg, const SHA1HashValue& ca_fingerprint_arg, const std::string& hostname_arg, + const std::string& ocsp_response_arg, int flags_arg, const CertificateList& additional_trust_anchors); ~RequestParams(); @@ -131,6 +135,7 @@ class NET_EXPORT_PRIVATE MultiThreadedCertVerifier void HandleResult(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, const CertificateList& additional_trust_anchors, int error, diff --git a/net/cert/multi_threaded_cert_verifier_unittest.cc b/net/cert/multi_threaded_cert_verifier_unittest.cc index c7c143f..142e65a 100644 --- a/net/cert/multi_threaded_cert_verifier_unittest.cc +++ b/net/cert/multi_threaded_cert_verifier_unittest.cc @@ -41,9 +41,11 @@ class MockCertVerifyProc : public CertVerifyProc { // CertVerifyProc implementation bool SupportsAdditionalTrustAnchors() const override { return false; } + bool SupportsOCSPStapling() const override { return false; } int VerifyInternal(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, const CertificateList& additional_trust_anchors, @@ -85,14 +87,9 @@ TEST_F(MultiThreadedCertVerifierTest, CacheHit) { TestCompletionCallback callback; CertVerifier::RequestHandle request_handle; - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); error = callback.WaitForResult(); @@ -102,14 +99,9 @@ TEST_F(MultiThreadedCertVerifierTest, CacheHit) { ASSERT_EQ(0u, verifier_.inflight_joins()); ASSERT_EQ(1u, verifier_.GetCacheSize()); - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); // Synchronous completion. ASSERT_NE(ERR_IO_PENDING, error); ASSERT_TRUE(IsCertificateError(error)); @@ -155,14 +147,9 @@ TEST_F(MultiThreadedCertVerifierTest, DifferentCACerts) { TestCompletionCallback callback; CertVerifier::RequestHandle request_handle; - error = verifier_.Verify(cert_chain1.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(cert_chain1.get(), "www.example.com", std::string(), + 0, NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); error = callback.WaitForResult(); @@ -172,14 +159,9 @@ TEST_F(MultiThreadedCertVerifierTest, DifferentCACerts) { ASSERT_EQ(0u, verifier_.inflight_joins()); ASSERT_EQ(1u, verifier_.GetCacheSize()); - error = verifier_.Verify(cert_chain2.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(cert_chain2.get(), "www.example.com", std::string(), + 0, NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); error = callback.WaitForResult(); @@ -205,24 +187,14 @@ TEST_F(MultiThreadedCertVerifierTest, InflightJoin) { TestCompletionCallback callback2; CertVerifier::RequestHandle request_handle2; - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result2, - callback2.callback(), - &request_handle2, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result2, callback2.callback(), + &request_handle2, BoundNetLog()); EXPECT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle2 != NULL); error = callback.WaitForResult(); @@ -245,14 +217,9 @@ TEST_F(MultiThreadedCertVerifierTest, CancelRequest) { CertVerifyResult verify_result; CertVerifier::RequestHandle request_handle; - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - base::Bind(&FailTest), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, base::Bind(&FailTest), + &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); ASSERT_TRUE(request_handle != NULL); verifier_.CancelRequest(request_handle); @@ -262,14 +229,9 @@ TEST_F(MultiThreadedCertVerifierTest, CancelRequest) { // worker thread) is likely to complete by the end of this test. TestCompletionCallback callback; for (int i = 0; i < 5; ++i) { - error = verifier_.Verify(test_cert.get(), - "www2.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www2.example.com", std::string(), + 0, NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); error = callback.WaitForResult(); @@ -294,8 +256,8 @@ TEST_F(MultiThreadedCertVerifierTest, CancelRequestThenQuit) { // CertVerifyWorker may be leaked if the main thread shuts down before the // worker thread. ANNOTATE_SCOPED_MEMORY_LEAK; - error = verifier_.Verify(test_cert.get(), "www.example.com", 0, NULL, - &verify_result, callback.callback(), + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), + 0, NULL, &verify_result, callback.callback(), &request_handle, BoundNetLog()); } ASSERT_EQ(ERR_IO_PENDING, error); @@ -327,55 +289,67 @@ TEST_F(MultiThreadedCertVerifierTest, RequestParamsComparators) { // 1 means key1 is greater than key2 int expected_result; } tests[] = { - { // Test for basic equivalence. - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, test_list), - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, test_list), - 0, - }, - { // Test that different certificates but with the same CA and for + { + // Test for basic equivalence. + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + 0, + }, + { + // Test that different certificates but with the same CA and for // the same host are different validation keys. - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, test_list), - MultiThreadedCertVerifier::RequestParams(z_key, a_key, "www.example.test", - 0, test_list), - -1, - }, - { // Test that the same EE certificate for the same host, but with + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + MultiThreadedCertVerifier::RequestParams( + z_key, a_key, "www.example.test", std::string(), 0, test_list), + -1, + }, + { + // Test that the same EE certificate for the same host, but with // different chains are different validation keys. - MultiThreadedCertVerifier::RequestParams(a_key, z_key, "www.example.test", - 0, test_list), - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, test_list), - 1, - }, - { // The same certificate, with the same chain, but for different + MultiThreadedCertVerifier::RequestParams( + a_key, z_key, "www.example.test", std::string(), 0, test_list), + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + 1, + }, + { + // The same certificate, with the same chain, but for different // hosts are different validation keys. - MultiThreadedCertVerifier::RequestParams(a_key, a_key, - "www1.example.test", 0, - test_list), - MultiThreadedCertVerifier::RequestParams(a_key, a_key, - "www2.example.test", 0, - test_list), - -1, - }, - { // The same certificate, chain, and host, but with different flags + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www1.example.test", std::string(), 0, test_list), + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www2.example.test", std::string(), 0, test_list), + -1, + }, + { + // The same certificate, chain, and host, but with different flags // are different validation keys. - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - CertVerifier::VERIFY_EV_CERT, - test_list), - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, test_list), - 1, - }, - { // Different additional_trust_anchors. - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, empty_list), - MultiThreadedCertVerifier::RequestParams(a_key, a_key, "www.example.test", - 0, test_list), - -1, - }, + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), + CertVerifier::VERIFY_EV_CERT, test_list), + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + 1, + }, + { + // Different additional_trust_anchors. + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, empty_list), + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + -1, + }, + { + // Different OCSP responses. + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", "ocsp response", 0, test_list), + MultiThreadedCertVerifier::RequestParams( + a_key, a_key, "www.example.test", std::string(), 0, test_list), + -1, + }, }; for (size_t i = 0; i < arraysize(tests); ++i) { SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]", i)); @@ -422,14 +396,9 @@ TEST_F(MultiThreadedCertVerifierTest, CertTrustAnchorProvider) { CertVerifier::RequestHandle request_handle; EXPECT_CALL(trust_provider, GetAdditionalTrustAnchors()) .WillOnce(ReturnRef(empty_cert_list)); - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); Mock::VerifyAndClearExpectations(&trust_provider); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); @@ -441,14 +410,9 @@ TEST_F(MultiThreadedCertVerifierTest, CertTrustAnchorProvider) { // The next Verify() uses the cached result. EXPECT_CALL(trust_provider, GetAdditionalTrustAnchors()) .WillOnce(ReturnRef(empty_cert_list)); - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); Mock::VerifyAndClearExpectations(&trust_provider); EXPECT_EQ(ERR_CERT_COMMON_NAME_INVALID, error); EXPECT_FALSE(request_handle); @@ -459,14 +423,9 @@ TEST_F(MultiThreadedCertVerifierTest, CertTrustAnchorProvider) { // trust anchors will not reuse the cache. EXPECT_CALL(trust_provider, GetAdditionalTrustAnchors()) .WillOnce(ReturnRef(cert_list)); - error = verifier_.Verify(test_cert.get(), - "www.example.com", - 0, - NULL, - &verify_result, - callback.callback(), - &request_handle, - BoundNetLog()); + error = verifier_.Verify(test_cert.get(), "www.example.com", std::string(), 0, + NULL, &verify_result, callback.callback(), + &request_handle, BoundNetLog()); Mock::VerifyAndClearExpectations(&trust_provider); ASSERT_EQ(ERR_IO_PENDING, error); EXPECT_TRUE(request_handle); diff --git a/net/cert/nss_cert_database_unittest.cc b/net/cert/nss_cert_database_unittest.cc index 9b026cc..7c97f11 100644 --- a/net/cert/nss_cert_database_unittest.cc +++ b/net/cert/nss_cert_database_unittest.cc @@ -554,12 +554,9 @@ TEST_F(CertDatabaseNSSTest, DISABLED_ImportServerCert) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(goog_cert.get(), - "www.google.com", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(goog_cert.get(), "www.google.com", std::string(), + flags, NULL, empty_cert_list_, &verify_result); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result.cert_status); } @@ -585,12 +582,9 @@ TEST_F(CertDatabaseNSSTest, ImportServerCert_SelfSigned) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(puny_cert.get(), - "xn--wgv71a119e.com", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(puny_cert.get(), "xn--wgv71a119e.com", std::string(), + flags, NULL, empty_cert_list_, &verify_result); EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error); EXPECT_EQ(CERT_STATUS_AUTHORITY_INVALID, verify_result.cert_status); } @@ -617,12 +611,9 @@ TEST_F(CertDatabaseNSSTest, ImportServerCert_SelfSigned_Trusted) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(puny_cert.get(), - "xn--wgv71a119e.com", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(puny_cert.get(), "xn--wgv71a119e.com", std::string(), + flags, NULL, empty_cert_list_, &verify_result); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result.cert_status); } @@ -653,12 +644,9 @@ TEST_F(CertDatabaseNSSTest, ImportCaAndServerCert) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result.cert_status); } @@ -695,12 +683,9 @@ TEST_F(CertDatabaseNSSTest, ImportCaAndServerCert_DistrustServer) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result); EXPECT_EQ(ERR_CERT_REVOKED, error); EXPECT_EQ(CERT_STATUS_REVOKED, verify_result.cert_status); } @@ -743,12 +728,9 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result.cert_status); @@ -774,12 +756,8 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa) { // Server cert should fail to verify. CertVerifyResult verify_result2; - error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result2); + error = verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result2); EXPECT_EQ(ERR_CERT_REVOKED, error); EXPECT_EQ(CERT_STATUS_REVOKED, verify_result2.cert_status); } @@ -819,12 +797,9 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa2) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result.cert_status); @@ -834,12 +809,8 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa2) { // Server cert should fail to verify. CertVerifyResult verify_result2; - error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result2); + error = verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result2); EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error); EXPECT_EQ(CERT_STATUS_AUTHORITY_INVALID, verify_result2.cert_status); } @@ -889,12 +860,9 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa3) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result.cert_status); @@ -904,12 +872,8 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa3) { // Server cert should fail to verify. CertVerifyResult verify_result2; - error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result2); + error = verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result2); EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error); EXPECT_EQ(CERT_STATUS_AUTHORITY_INVALID, verify_result2.cert_status); } @@ -953,12 +917,9 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa4) { scoped_refptr<CertVerifyProc> verify_proc(new CertVerifyProcNSS()); int flags = 0; CertVerifyResult verify_result; - int error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result); + int error = + verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result); EXPECT_EQ(ERR_CERT_REVOKED, error); EXPECT_EQ(CERT_STATUS_REVOKED, verify_result.cert_status); @@ -968,12 +929,8 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa4) { // Server cert should verify. CertVerifyResult verify_result2; - error = verify_proc->Verify(certs[0].get(), - "127.0.0.1", - flags, - NULL, - empty_cert_list_, - &verify_result2); + error = verify_proc->Verify(certs[0].get(), "127.0.0.1", std::string(), flags, + NULL, empty_cert_list_, &verify_result2); EXPECT_EQ(OK, error); EXPECT_EQ(0U, verify_result2.cert_status); } diff --git a/net/cert/single_request_cert_verifier.cc b/net/cert/single_request_cert_verifier.cc index 909af07..afd1ac2 100644 --- a/net/cert/single_request_cert_verifier.cc +++ b/net/cert/single_request_cert_verifier.cc @@ -27,6 +27,7 @@ SingleRequestCertVerifier::~SingleRequestCertVerifier() { int SingleRequestCertVerifier::Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, @@ -40,7 +41,7 @@ int SingleRequestCertVerifier::Verify(X509Certificate* cert, // We need to be notified of completion before |callback| is called, so that // we can clear out |cur_request_*|. int rv = cert_verifier_->Verify( - cert, hostname, flags, crl_set, verify_result, + cert, hostname, ocsp_response, flags, crl_set, verify_result, base::Bind(&SingleRequestCertVerifier::OnVerifyCompletion, base::Unretained(this)), &request, net_log); diff --git a/net/cert/single_request_cert_verifier.h b/net/cert/single_request_cert_verifier.h index 6b19281..67bb088 100644 --- a/net/cert/single_request_cert_verifier.h +++ b/net/cert/single_request_cert_verifier.h @@ -26,6 +26,7 @@ class SingleRequestCertVerifier { // upon success. See CertVerifier::Verify() for details. int Verify(X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, CRLSet* crl_set, CertVerifyResult* verify_result, diff --git a/net/cert/test_root_certs_unittest.cc b/net/cert/test_root_certs_unittest.cc index 1bbb8fc..e55c147 100644 --- a/net/cert/test_root_certs_unittest.cc +++ b/net/cert/test_root_certs_unittest.cc @@ -89,12 +89,9 @@ TEST(TestRootCertsTest, OverrideTrust) { int flags = 0; CertVerifyResult bad_verify_result; scoped_refptr<CertVerifyProc> verify_proc(CertVerifyProc::CreateDefault()); - int bad_status = verify_proc->Verify(test_cert.get(), - "127.0.0.1", - flags, - NULL, - CertificateList(), - &bad_verify_result); + int bad_status = + verify_proc->Verify(test_cert.get(), "127.0.0.1", std::string(), flags, + NULL, CertificateList(), &bad_verify_result); EXPECT_NE(OK, bad_status); EXPECT_NE(0u, bad_verify_result.cert_status & CERT_STATUS_AUTHORITY_INVALID); @@ -106,12 +103,9 @@ TEST(TestRootCertsTest, OverrideTrust) { // Test that the certificate verification now succeeds, because the // TestRootCerts is successfully imbuing trust. CertVerifyResult good_verify_result; - int good_status = verify_proc->Verify(test_cert.get(), - "127.0.0.1", - flags, - NULL, - CertificateList(), - &good_verify_result); + int good_status = + verify_proc->Verify(test_cert.get(), "127.0.0.1", std::string(), flags, + NULL, CertificateList(), &good_verify_result); EXPECT_EQ(OK, good_status); EXPECT_EQ(0u, good_verify_result.cert_status); @@ -122,12 +116,9 @@ TEST(TestRootCertsTest, OverrideTrust) { // revert to their original state, and don't linger. If trust status // lingers, it will likely break other tests in net_unittests. CertVerifyResult restored_verify_result; - int restored_status = verify_proc->Verify(test_cert.get(), - "127.0.0.1", - flags, - NULL, - CertificateList(), - &restored_verify_result); + int restored_status = + verify_proc->Verify(test_cert.get(), "127.0.0.1", std::string(), flags, + NULL, CertificateList(), &restored_verify_result); EXPECT_NE(OK, restored_status); EXPECT_NE(0u, restored_verify_result.cert_status & CERT_STATUS_AUTHORITY_INVALID); diff --git a/net/cert_net/nss_ocsp_unittest.cc b/net/cert_net/nss_ocsp_unittest.cc index 33bb919..be72e82 100644 --- a/net/cert_net/nss_ocsp_unittest.cc +++ b/net/cert_net/nss_ocsp_unittest.cc @@ -141,14 +141,9 @@ TEST_F(NssHttpTest, TestAia) { CertVerifier::RequestHandle request_handle; int flags = CertVerifier::VERIFY_CERT_IO_ENABLED; - int error = verifier()->Verify(test_cert.get(), - "aia-host.invalid", - flags, - NULL, - &verify_result, - test_callback.callback(), - &request_handle, - BoundNetLog()); + int error = verifier()->Verify( + test_cert.get(), "aia-host.invalid", std::string(), flags, NULL, + &verify_result, test_callback.callback(), &request_handle, BoundNetLog()); ASSERT_EQ(ERR_IO_PENDING, error); error = test_callback.WaitForResult(); diff --git a/net/quic/crypto/proof_verifier_chromium.cc b/net/quic/crypto/proof_verifier_chromium.cc index c20463d..386a52c 100644 --- a/net/quic/crypto/proof_verifier_chromium.cc +++ b/net/quic/crypto/proof_verifier_chromium.cc @@ -223,15 +223,12 @@ int ProofVerifierChromium::Job::DoVerifyCert(int result) { next_state_ = STATE_VERIFY_CERT_COMPLETE; int flags = 0; - return verifier_->Verify( - cert_.get(), - hostname_, - flags, - SSLConfigService::GetCRLSet().get(), - &verify_details_->cert_verify_result, - base::Bind(&ProofVerifierChromium::Job::OnIOComplete, - base::Unretained(this)), - net_log_); + return verifier_->Verify(cert_.get(), hostname_, std::string(), flags, + SSLConfigService::GetCRLSet().get(), + &verify_details_->cert_verify_result, + base::Bind(&ProofVerifierChromium::Job::OnIOComplete, + base::Unretained(this)), + net_log_); } int ProofVerifierChromium::Job::DoVerifyCertComplete(int result) { diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index ae1c427..03bb4e5 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -69,7 +69,6 @@ #include "base/callback_helpers.h" #include "base/compiler_specific.h" #include "base/logging.h" -#include "base/memory/singleton.h" #include "base/metrics/histogram.h" #include "base/single_thread_task_runner.h" #include "base/stl_util.h" @@ -159,57 +158,6 @@ const int kSendBufferSize = 17 * 1024; // overlap with any value of the net::Error range, including net::OK). const int kNoPendingReadResult = 1; -#if defined(USE_NSS_CERTS) -typedef SECStatus -(*CacheOCSPResponseFromSideChannelFunction)( - CERTCertDBHandle *handle, CERTCertificate *cert, PRTime time, - SECItem *encodedResponse, void *pwArg); - -// On Linux, we dynamically link against the system version of libnss3.so. In -// order to continue working on systems without up-to-date versions of NSS we -// lookup CERT_CacheOCSPResponseFromSideChannel with dlsym. - -// RuntimeLibNSSFunctionPointers is a singleton which caches the results of any -// runtime symbol resolution that we need. -class RuntimeLibNSSFunctionPointers { - public: - CacheOCSPResponseFromSideChannelFunction - GetCacheOCSPResponseFromSideChannelFunction() { - return cache_ocsp_response_from_side_channel_; - } - - static RuntimeLibNSSFunctionPointers* GetInstance() { - return Singleton<RuntimeLibNSSFunctionPointers>::get(); - } - - private: - friend struct DefaultSingletonTraits<RuntimeLibNSSFunctionPointers>; - - RuntimeLibNSSFunctionPointers() { - cache_ocsp_response_from_side_channel_ = - (CacheOCSPResponseFromSideChannelFunction) - dlsym(RTLD_DEFAULT, "CERT_CacheOCSPResponseFromSideChannel"); - } - - CacheOCSPResponseFromSideChannelFunction - cache_ocsp_response_from_side_channel_; -}; - -CacheOCSPResponseFromSideChannelFunction -GetCacheOCSPResponseFromSideChannelFunction() { - return RuntimeLibNSSFunctionPointers::GetInstance() - ->GetCacheOCSPResponseFromSideChannelFunction(); -} - -bool IsOCSPStaplingSupported() { - return GetCacheOCSPResponseFromSideChannelFunction() != NULL; -} -#else -bool IsOCSPStaplingSupported() { - return false; -} -#endif - // Helper functions to make it possible to log events from within the // SSLClientSocketNSS::Core. void AddLogEvent(const base::WeakPtr<BoundNetLog>& net_log, @@ -2069,18 +2017,6 @@ void SSLClientSocketNSS::Core::UpdateStapledOCSPResponse() { nss_handshake_state_.stapled_ocsp_response = std::string( reinterpret_cast<char*>(ocsp_responses->items[0].data), ocsp_responses->items[0].len); - - if (IsOCSPStaplingSupported()) { -#if defined(USE_NSS_CERTS) - CacheOCSPResponseFromSideChannelFunction cache_ocsp_response = - GetCacheOCSPResponseFromSideChannelFunction(); - - cache_ocsp_response( - CERT_GetDefaultCertDB(), - nss_handshake_state_.server_cert_chain[0], PR_Now(), - &ocsp_responses->items[0], NULL); -#endif - } } void SSLClientSocketNSS::Core::UpdateConnectionStatus() { @@ -2877,8 +2813,8 @@ int SSLClientSocketNSS::InitializeSSLOptions() { // Request OCSP stapling even on platforms that don't support it, in // order to extract Certificate Transparency information. rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_OCSP_STAPLING, - (IsOCSPStaplingSupported() || - ssl_config_.signed_cert_timestamps_enabled)); + cert_verifier_->SupportsOCSPStapling() || + ssl_config_.signed_cert_timestamps_enabled); if (rv != SECSuccess) { LogFailedNSSFunction(net_log_, "SSL_OptionSet", "SSL_ENABLE_OCSP_STAPLING"); @@ -3108,11 +3044,9 @@ int SSLClientSocketNSS::DoVerifyCert(int result) { flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS; verifier_.reset(new SingleRequestCertVerifier(cert_verifier_)); return verifier_->Verify( - core_->state().server_cert.get(), - host_and_port_.host(), - flags, - SSLConfigService::GetCRLSet().get(), - &server_cert_verify_result_, + core_->state().server_cert.get(), host_and_port_.host(), + core_->state().stapled_ocsp_response, flags, + SSLConfigService::GetCRLSet().get(), &server_cert_verify_result_, base::Bind(&SSLClientSocketNSS::OnHandshakeIOComplete, base::Unretained(this)), net_log_); diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc index 0b3e1c2..cf9cec0 100644 --- a/net/socket/ssl_client_socket_openssl.cc +++ b/net/socket/ssl_client_socket_openssl.cc @@ -145,19 +145,6 @@ int LogErrorCallback(const char* str, size_t len, void* context) { return 1; } -bool IsOCSPStaplingSupported() { -#if defined(OS_WIN) - // CERT_OCSP_RESPONSE_PROP_ID is only implemented on Vista+, but it can be - // set on Windows XP without error. There is some overhead from the server - // sending the OCSP response if it supports the extension, for the subset of - // XP clients who will request it but be unable to use it, but this is an - // acceptable trade-off for simplicity of implementation. - return true; -#else - return false; -#endif -} - } // namespace class SSLClientSocketOpenSSL::SSLContext { @@ -847,7 +834,7 @@ int SSLClientSocketOpenSSL::Init() { SSL_enable_ocsp_stapling(ssl_); } - if (IsOCSPStaplingSupported()) + if (cert_verifier_->SupportsOCSPStapling()) SSL_enable_ocsp_stapling(ssl_); // Enable fastradio padding. @@ -948,7 +935,7 @@ int SSLClientSocketOpenSSL::DoHandshake() { // Only record OCSP histograms if OCSP was requested. if (ssl_config_.signed_cert_timestamps_enabled || - IsOCSPStaplingSupported()) { + cert_verifier_->SupportsOCSPStapling()) { const uint8_t* ocsp_response; size_t ocsp_response_len; SSL_get0_ocsp_response(ssl_, &ocsp_response, &ocsp_response_len); @@ -1081,6 +1068,15 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) { return ERR_CERT_INVALID; } + std::string ocsp_response; + if (cert_verifier_->SupportsOCSPStapling()) { + const uint8_t* ocsp_response_raw; + size_t ocsp_response_len; + SSL_get0_ocsp_response(ssl_, &ocsp_response_raw, &ocsp_response_len); + ocsp_response.assign(reinterpret_cast<const char*>(ocsp_response_raw), + ocsp_response_len); + } + start_cert_verification_time_ = base::TimeTicks::Now(); int flags = 0; @@ -1094,13 +1090,10 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) { flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS; verifier_.reset(new SingleRequestCertVerifier(cert_verifier_)); return verifier_->Verify( - server_cert_.get(), - host_and_port_.host(), - flags, + server_cert_.get(), host_and_port_.host(), ocsp_response, flags, // TODO(davidben): Route the CRLSet through SSLConfig so // SSLClientSocket doesn't depend on SSLConfigService. - SSLConfigService::GetCRLSet().get(), - &server_cert_verify_result_, + SSLConfigService::GetCRLSet().get(), &server_cert_verify_result_, base::Bind(&SSLClientSocketOpenSSL::OnHandshakeIOComplete, base::Unretained(this)), net_log_); @@ -1176,33 +1169,6 @@ void SSLClientSocketOpenSSL::UpdateServerCert() { NetLog::TYPE_SSL_CERTIFICATES_RECEIVED, base::Bind(&NetLogX509CertificateCallback, base::Unretained(server_cert_.get()))); - - // TODO(rsleevi): Plumb an OCSP response into the Mac system library and - // update IsOCSPStaplingSupported for Mac. https://crbug.com/430714 - if (IsOCSPStaplingSupported()) { -#if defined(OS_WIN) - const uint8_t* ocsp_response_raw; - size_t ocsp_response_len; - SSL_get0_ocsp_response(ssl_, &ocsp_response_raw, &ocsp_response_len); - - CRYPT_DATA_BLOB ocsp_response_blob; - ocsp_response_blob.cbData = ocsp_response_len; - ocsp_response_blob.pbData = const_cast<BYTE*>(ocsp_response_raw); - BOOL ok = CertSetCertificateContextProperty( - server_cert_->os_cert_handle(), - CERT_OCSP_RESPONSE_PROP_ID, - CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, - &ocsp_response_blob); - if (!ok) { - VLOG(1) << "Failed to set OCSP response property: " - << GetLastError(); - } -#else - // TODO(davidben): Support OCSP stapling when NSS is the system - // certificate verifier. https://crbug.com/479034. - NOTREACHED(); -#endif - } } } diff --git a/remoting/protocol/ssl_hmac_channel_authenticator.cc b/remoting/protocol/ssl_hmac_channel_authenticator.cc index 23158d2..d8385c3 100644 --- a/remoting/protocol/ssl_hmac_channel_authenticator.cc +++ b/remoting/protocol/ssl_hmac_channel_authenticator.cc @@ -38,6 +38,7 @@ class FailingCertVerifier : public net::CertVerifier { int Verify(net::X509Certificate* cert, const std::string& hostname, + const std::string& ocsp_response, int flags, net::CRLSet* crl_set, net::CertVerifyResult* verify_result, |