SSLConfig - Added GetCertVerifyFlags to SSLConfig and shared the common

code with HttpStreamFactoryImpl::Job and SSLClientSocket*.*. R=rch@chromium.org, rsleevi@chromium.org Review URL: https://codereview.chromium.org/1202313004 Cr-Commit-Position: refs/heads/master@{#335934}
Denger-Network · Jun 24, 2015 · 807f9ea · 807f9ea
1 parent 073cea5
commit 807f9ea
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 32 deletions.
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
@@ -840,19 +840,10 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
       secure_quic = using_ssl_;
       ssl_config = &server_ssl_config_;
     }
-    // TODO(rtenneti): Move the cert_verify_flags code into SSLConfig class.
-    int flags = 0;
-    if (ssl_config->rev_checking_enabled)
-      flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED;
-    if (ssl_config->verify_ev_cert)
-      flags |= CertVerifier::VERIFY_EV_CERT;
-    if (ssl_config->cert_io_enabled)
-      flags |= CertVerifier::VERIFY_CERT_IO_ENABLED;
-    if (ssl_config->rev_checking_required_local_anchors)
-      flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS;
     int rv = quic_request_.Request(
-        destination, secure_quic, request_info_.privacy_mode, flags,
-        origin_host, request_info_.method, net_log_, io_callback_);
+        destination, secure_quic, request_info_.privacy_mode,
+        ssl_config->GetCertVerifyFlags(), origin_host, request_info_.method,
+        net_log_, io_callback_);
     if (rv == OK) {
       using_existing_quic_session_ = true;
     } else {

diff --git a/net/net.gypi b/net/net.gypi
@@ -1721,6 +1721,7 @@
       'ssl/ssl_client_auth_cache_unittest.cc',
       'ssl/ssl_client_session_cache_openssl_unittest.cc',
       'ssl/ssl_config_service_unittest.cc',
+      'ssl/ssl_config_unittest.cc',
       'ssl/ssl_connection_status_flags_unittest.cc',
       'test/embedded_test_server/embedded_test_server_unittest.cc',
       'test/embedded_test_server/http_request_unittest.cc',

diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
@@ -3052,18 +3052,9 @@ int SSLClientSocketNSS::DoVerifyCert(int result) {
 
   start_cert_verification_time_ = base::TimeTicks::Now();
 
-  int flags = 0;
-  if (ssl_config_.rev_checking_enabled)
-    flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED;
-  if (ssl_config_.verify_ev_cert)
-    flags |= CertVerifier::VERIFY_EV_CERT;
-  if (ssl_config_.cert_io_enabled)
-    flags |= CertVerifier::VERIFY_CERT_IO_ENABLED;
-  if (ssl_config_.rev_checking_required_local_anchors)
-    flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS;
   return cert_verifier_->Verify(
       core_->state().server_cert.get(), host_and_port_.host(),
-      core_->state().stapled_ocsp_response, flags,
+      core_->state().stapled_ocsp_response, ssl_config_.GetCertVerifyFlags(),
       SSLConfigService::GetCRLSet().get(), &server_cert_verify_result_,
       base::Bind(&SSLClientSocketNSS::OnHandshakeIOComplete,
                  base::Unretained(this)),

diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
@@ -1111,17 +1111,9 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) {
 
   start_cert_verification_time_ = base::TimeTicks::Now();
 
-  int flags = 0;
-  if (ssl_config_.rev_checking_enabled)
-    flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED;
-  if (ssl_config_.verify_ev_cert)
-    flags |= CertVerifier::VERIFY_EV_CERT;
-  if (ssl_config_.cert_io_enabled)
-    flags |= CertVerifier::VERIFY_CERT_IO_ENABLED;
-  if (ssl_config_.rev_checking_required_local_anchors)
-    flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS;
   return cert_verifier_->Verify(
-      server_cert_.get(), host_and_port_.host(), ocsp_response, flags,
+      server_cert_.get(), host_and_port_.host(), ocsp_response,
+      ssl_config_.GetCertVerifyFlags(),
       // TODO(davidben): Route the CRLSet through SSLConfig so
       // SSLClientSocket doesn't depend on SSLConfigService.
       SSLConfigService::GetCRLSet().get(), &server_cert_verify_result_,

diff --git a/net/ssl/ssl_config.cc b/net/ssl/ssl_config.cc
@@ -4,6 +4,7 @@
 
 #include "net/ssl/ssl_config.h"
 
+#include "net/cert/cert_verifier.h"
 #include "net/socket/ssl_client_socket.h"
 
 namespace net {
@@ -58,4 +59,17 @@ bool SSLConfig::IsAllowedBadCert(const base::StringPiece& der_cert,
   return false;
 }
 
+int SSLConfig::GetCertVerifyFlags() const {
+  int flags = 0;
+  if (rev_checking_enabled)
+    flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED;
+  if (verify_ev_cert)
+    flags |= CertVerifier::VERIFY_EV_CERT;
+  if (cert_io_enabled)
+    flags |= CertVerifier::VERIFY_CERT_IO_ENABLED;
+  if (rev_checking_required_local_anchors)
+    flags |= CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS;
+  return flags;
+}
+
 }  // namespace net
diff --git a/net/ssl/ssl_config.h b/net/ssl/ssl_config.h
@@ -51,6 +51,11 @@ struct NET_EXPORT SSLConfig {
   bool IsAllowedBadCert(const base::StringPiece& der_cert,
                         CertStatus* cert_status) const;
 
+  // Returns the set of flags to use for certificate verification, which is a
+  // bitwise OR of CertVerifier::VerifyFlags that represent this SSLConfig's
+  // configuration.
+  int GetCertVerifyFlags() const;
+
   // rev_checking_enabled is true if online certificate revocation checking is
   // enabled (i.e. OCSP and CRL fetching).
   //

diff --git a/net/ssl/ssl_config_unittest.cc b/net/ssl/ssl_config_unittest.cc
@@ -0,0 +1,87 @@
+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/ssl/ssl_config.h"
+
+#include "net/cert/cert_verifier.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+void CheckCertVerifyFlags(SSLConfig& ssl_config,
+                          bool rev_checking_enabled,
+                          bool verify_ev_cert,
+                          bool cert_io_enabled,
+                          bool rev_checking_required_local_anchors) {
+  ssl_config.rev_checking_enabled = rev_checking_enabled;
+  ssl_config.verify_ev_cert = verify_ev_cert;
+  ssl_config.cert_io_enabled = cert_io_enabled;
+  ssl_config.rev_checking_required_local_anchors =
+      rev_checking_required_local_anchors;
+  int flags = ssl_config.GetCertVerifyFlags();
+  if (rev_checking_enabled)
+    EXPECT_TRUE(flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED);
+  else
+    EXPECT_FALSE(flags & CertVerifier::VERIFY_REV_CHECKING_ENABLED);
+  if (verify_ev_cert)
+    EXPECT_TRUE(flags & CertVerifier::VERIFY_EV_CERT);
+  else
+    EXPECT_FALSE(flags & CertVerifier::VERIFY_EV_CERT);
+  if (cert_io_enabled)
+    EXPECT_TRUE(flags & CertVerifier::VERIFY_CERT_IO_ENABLED);
+  else
+    EXPECT_FALSE(flags & CertVerifier::VERIFY_CERT_IO_ENABLED);
+  if (rev_checking_required_local_anchors) {
+    EXPECT_TRUE(flags &
+                CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS);
+  } else {
+    EXPECT_FALSE(flags &
+                 CertVerifier::VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS);
+  }
+}
+
+}  // namespace
+
+TEST(SSLConfigTest, GetCertVerifyFlags) {
+  SSLConfig ssl_config;
+  CheckCertVerifyFlags(ssl_config,
+                       /*rev_checking_enabled=*/true,
+                       /*verify_ev_cert=*/true,
+                       /*cert_io_enabled=*/true,
+                       /*rev_checking_required_local_anchors=*/true);
+
+  CheckCertVerifyFlags(ssl_config,
+                       /*rev_checking_enabled=*/false,
+                       /*verify_ev_cert=*/false,
+                       /*cert_io_enabled=*/false,
+                       /*rev_checking_required_local_anchors=*/false);
+
+  CheckCertVerifyFlags(ssl_config,
+                       /*rev_checking_enabled=*/true,
+                       /*verify_ev_cert=*/false,
+                       /*cert_io_enabled=*/false,
+                       /*rev_checking_required_local_anchors=*/false);
+
+  CheckCertVerifyFlags(ssl_config,
+                       /*rev_checking_enabled=*/false,
+                       /*verify_ev_cert=*/true,
+                       /*cert_io_enabled=*/false,
+                       /*rev_checking_required_local_anchors=*/false);
+
+  CheckCertVerifyFlags(ssl_config,
+                       /*rev_checking_enabled=*/false,
+                       /*verify_ev_cert=*/false,
+                       /*cert_io_enabled=*/true,
+                       /*rev_checking_required_local_anchors=*/false);
+
+  CheckCertVerifyFlags(ssl_config,
+                       /*rev_checking_enabled=*/false,
+                       /*verify_ev_cert=*/false,
+                       /*cert_io_enabled=*/false,
+                       /*rev_checking_required_local_anchors=*/true);
+}
+
+}  // namespace net