Use LOAD_VERIFY_EV_CERT to verify EV-ness in Verify().

If LOAD_VERIFY_EV_CERT is requested on load_flags
and revokation checking is performed, Verify() peforms
EV certificate verification as well, and sets
CERT_STATUS_IS_EV in verify_result.

Eliminate X509Certificate::IsEV()

BUG=3592
TEST=net_unittests with ALLOW_EXTERNAL_ACCESS=1, \
 visit https://www.thawte.com/ and shows EV info.

Review URL: http://codereview.chromium.org/125120

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19011 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 94 insertions, 50 deletions

diff --git a/net/base/cert_verifier.cc b/net/base/cert_verifier.cc
index e907e30..70be229 100644
--- a/net/base/cert_verifier.cc
+++ b/net/base/cert_verifier.cc
@@ -22,12 +22,12 @@ class CertVerifier::Request :
   Request(CertVerifier* verifier,
           X509Certificate* cert,
           const std::string& hostname,
-          bool rev_checking_enabled,
+          int flags,
           CertVerifyResult* verify_result,
           CompletionCallback* callback)
       : cert_(cert),
         hostname_(hostname),
-        rev_checking_enabled_(rev_checking_enabled),
+        flags_(flags),
         verifier_(verifier),
         verify_result_(verify_result),
         callback_(callback),
@@ -39,7 +39,7 @@ class CertVerifier::Request :
 
   void DoVerify() {
     // Running on the worker thread
-    error_ = cert_->Verify(hostname_, rev_checking_enabled_, &result_);
+    error_ = cert_->Verify(hostname_, flags_, &result_);
 #if defined(OS_LINUX)
     // Detach the thread from NSPR.
     // Calling NSS functions attaches the thread to NSPR, which stores
@@ -95,7 +95,8 @@ class CertVerifier::Request :
   // Set on the origin thread, read on the worker thread.
   scoped_refptr<X509Certificate> cert_;
   std::string hostname_;
-  bool rev_checking_enabled_;
+  // bitwise OR'd of X509Certificate::VerifyFlags.
+  int flags_;
 
   // Only used on the origin thread (where Verify was called).
   CertVerifier* verifier_;
@@ -123,7 +124,7 @@ CertVerifier::~CertVerifier() {
 
 int CertVerifier::Verify(X509Certificate* cert,
                          const std::string& hostname,
-                         bool rev_checking_enabled,
+                         int flags,
                          CertVerifyResult* verify_result,
                          CompletionCallback* callback) {
   DCHECK(!request_) << "verifier already in use";
@@ -131,13 +132,12 @@ int CertVerifier::Verify(X509Certificate* cert,
   // Do a synchronous verification.
   if (!callback) {
     CertVerifyResult result;
-    int rv = cert->Verify(hostname, rev_checking_enabled, &result);
+    int rv = cert->Verify(hostname, flags, &result);
     *verify_result = result;
     return rv;
   }
 
-  request_ = new Request(this, cert, hostname, rev_checking_enabled,
-                         verify_result, callback);
+  request_ = new Request(this, cert, hostname, flags, verify_result, callback);
 
   // Dispatch to worker thread...
   if (!WorkerPool::PostTask(FROM_HERE,
diff --git a/net/base/cert_verifier.h b/net/base/cert_verifier.h
index 051bc25..6a946d2 100644
--- a/net/base/cert_verifier.h
+++ b/net/base/cert_verifier.h
@@ -42,8 +42,14 @@ class CertVerifier {
   // |verify_result->cert_status|, and the error code for the most serious
   // error is returned.
   //
-  // If |rev_checking_enabled| is true, certificate revocation checking is
-  // performed.
+  // |flags| is bitwise OR'd of X509Certificate::VerifyFlags.
+  // If VERIFY_REV_CHECKING_ENABLED is set in |flags|, certificate revocation
+  // checking is performed.
+  //
+  // If VERIFY_EV_CERT is set in |flags| too, EV certificate verification is
+  // performed.  If |flags| is VERIFY_EV_CERT (that is,
+  // VERIFY_REV_CHECKING_ENABLED is not set), EV certificate verification will
+  // not be performed.
   //
   // When callback is null, the operation completes synchronously.
   //
@@ -52,7 +58,7 @@ class CertVerifier {
   // be passed to the callback when available.
   //
   int Verify(X509Certificate* cert, const std::string& hostname,
-             bool rev_checking_enabled, CertVerifyResult* verify_result,
+             int flags, CertVerifyResult* verify_result,
              CompletionCallback* callback);
 
  private:
diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h
index cf2c9bb..388b255 100644
--- a/net/base/ssl_config_service.h
+++ b/net/base/ssl_config_service.h
@@ -17,8 +17,8 @@ struct SSLConfig {
   // Default to no revocation checking.
   // Default to SSL 2.0 off, SSL 3.0 on, and TLS 1.0 on.
   SSLConfig()
-      : rev_checking_enabled(false), ssl2_enabled(false),
-        ssl3_enabled(true), tls1_enabled(true), send_client_cert(false) {
+      : rev_checking_enabled(false),  ssl2_enabled(false), ssl3_enabled(true),
+        tls1_enabled(true), send_client_cert(false), verify_ev_cert(false) {
   }
 
   bool rev_checking_enabled;  // True if server certificate revocation
@@ -39,6 +39,8 @@ struct SSLConfig {
   // True if we should send client_cert to the server.
   bool send_client_cert;
 
+  bool verify_ev_cert;  // True if we should verify the certificate for EV.
+
   scoped_refptr<X509Certificate> client_cert;
 };
 
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index a8bdfe9..cdadf54 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -133,6 +133,11 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
     SOURCE_FROM_NETWORK = 2,      // From the network.
   };
 
+  enum VerifyFlags {
+    VERIFY_REV_CHECKING_ENABLED = 1 << 0,
+    VERIFY_EV_CERT = 1 << 1,
+  };
+
   // Create an X509Certificate from a handle to the certificate object
   // in the underlying crypto library. This is a transfer of ownership;
   // X509Certificate will properly dispose of |cert_handle| for you.
@@ -207,16 +212,14 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // |verify_result->cert_status|, and the error code for the most serious
   // error is returned.
   //
-  // If |rev_checking_enabled| is true, certificate revocation checking is
-  // performed.
+  // |flags| is bitwise OR'd of VerifyFlags.
+  // If VERIFY_REV_CHECKING_ENABLED is set in |flags|, certificate revocation
+  // checking is performed.  If VERIFY_EV_CERT is set in |flags| too,
+  // EV certificate verification is performed.
   int Verify(const std::string& hostname,
-             bool rev_checking_enabled,
+             int flags,
              CertVerifyResult* verify_result) const;
 
-  // Returns true if the certificate is an extended-validation (EV)
-  // certificate.
-  bool IsEV(int cert_status) const;
-
   OSCertHandle os_cert_handle() const { return cert_handle_; }
 
  private:
@@ -258,6 +261,8 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   // Common object initialization code.  Called by the constructors only.
   void Initialize();
 
+  bool VerifyEV() const;
+
   // Creates an OS certificate handle from the BER-encoded representation.
   // Returns NULL on failure.
   static OSCertHandle CreateOSCertHandleFromBytes(const char* data,
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index edbc654..2e75489 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -263,8 +263,7 @@ void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
 }
 
 int X509Certificate::Verify(const std::string& hostname,
-                            bool rev_checking_enabled,
-                            CertVerifyResult* verify_result) const {
+                            int flags, CertVerifyResult* verify_result) const {
   NOTIMPLEMENTED();
   return ERR_NOT_IMPLEMENTED;
 }
@@ -277,7 +276,7 @@ int X509Certificate::Verify(const std::string& hostname,
 // in the certificate chain according to Section 7 (pp. 11-12) of the EV
 // Certificate Guidelines Version 1.0 at
 // http://cabforum.org/EV_Certificate_Guidelines.pdf.
-bool X509Certificate::IsEV(int cert_status) const {
+bool X509Certificate::VerifyEV() const {
   // TODO(avi): implement this
   NOTIMPLEMENTED();
   return false;
diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc
index 4954cfd..a33caa1 100644
--- a/net/base/x509_certificate_nss.cc
+++ b/net/base/x509_certificate_nss.cc
@@ -371,7 +371,7 @@ void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
 // The problem is that we get segfault when unit tests is going to terminate
 // if PR_Cleanup is called in NSSInitSingleton destructor.
 int X509Certificate::Verify(const std::string& hostname,
-                            bool rev_checking_enabled,
+                            int flags,
                             CertVerifyResult* verify_result) const {
   verify_result->Reset();
 
@@ -391,6 +391,9 @@ int X509Certificate::Verify(const std::string& hostname,
   // OCSP mode would fail with SEC_ERROR_UNKNOWN_ISSUER.
   // We need to set up OCSP and install an HTTP client for NSS.
   bool use_ocsp = false;
+  // EV requires revocation checking.
+  if (!(flags & VERIFY_REV_CHECKING_ENABLED))
+    flags &= ~VERIFY_EV_CERT;
 
   // TODO(wtc): Use CERT_REV_M_REQUIRE_INFO_ON_MISSING_SOURCE and
   // CERT_REV_MI_REQUIRE_SOME_FRESH_INFO_AVAILABLE for EV certificate
@@ -477,11 +480,13 @@ int X509Certificate::Verify(const std::string& hostname,
                    verify_result);
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);
+  if ((flags & VERIFY_EV_CERT) && VerifyEV())
+    verify_result->cert_status |= CERT_STATUS_IS_EV;
   return OK;
 }
 
 // TODO(port): Implement properly on Linux.
-bool X509Certificate::IsEV(int status) const {
+bool X509Certificate::VerifyEV() const {
   NOTIMPLEMENTED();
   return false;
 }
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index d18f118..b0faaf5 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "base/pickle.h"
 #include "net/base/cert_status_flags.h"
+#include "net/base/cert_verify_result.h"
+#include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -15,7 +17,7 @@
 #define ALLOW_EXTERNAL_ACCESS 0
 
 #if ALLOW_EXTERNAL_ACCESS && defined(OS_WIN)
-#define TEST_EV 1  // Test IsEV()
+#define TEST_EV 1  // Test CERT_STATUS_IS_EV
 #endif
 
 using base::Time;
@@ -392,7 +394,11 @@ TEST(X509CertificateTest, GoogleCertParsing) {
 
 #if TEST_EV
   // TODO(avi): turn this on for the Mac once EV checking is implemented.
-  EXPECT_EQ(false, google_cert->IsEV(net::CERT_STATUS_REV_CHECKING_ENABLED));
+  CertVerifyResult verify_result;
+  int flags = X509Certificate::VERIFY_REV_CHECKING_ENABLED |
+                X509Certificate::VERIFY_EV_CERT;
+  EXPECT_EQ(OK, google_cert->Verify("www.google.com", flags, &verify_result));
+  EXPECT_EQ(0, verify_result.cert_status & CERT_STATUS_IS_EV);
 #endif
 }
 
@@ -444,7 +450,11 @@ TEST(X509CertificateTest, WebkitCertParsing) {
   EXPECT_EQ("webkit.org", dns_names[1]);
 
 #if TEST_EV
-  EXPECT_EQ(false, webkit_cert->IsEV(net::CERT_STATUS_REV_CHECKING_ENABLED));
+  int flags = X509Certificate::VERIFY_REV_CHECKING_ENABLED |
+                X509Certificate::VERIFY_EV_CERT;
+  CertVerifyResult verify_result;
+  EXPECT_EQ(OK, webkit_cert->Verify("webkit.org", flags, &verify_result));
+  EXPECT_EQ(0, verify_result.cert_status & CERT_STATUS_IS_EV);
 #endif
 }
 
@@ -495,11 +505,17 @@ TEST(X509CertificateTest, ThawteCertParsing) {
   EXPECT_EQ("www.thawte.com", dns_names[0]);
 
 #if TEST_EV
+  int flags = X509Certificate::VERIFY_REV_CHECKING_ENABLED |
+                X509Certificate::VERIFY_EV_CERT;
+  CertVerifyResult verify_result;
   // EV cert verification requires revocation checking.
-  EXPECT_EQ(true, thawte_cert->IsEV(net::CERT_STATUS_REV_CHECKING_ENABLED));
+  EXPECT_EQ(OK, thawte_cert->Verify("www.thawte.com", flags, &verify_result));
+  EXPECT_NE(0, verify_result.cert_status & CERT_STATUS_IS_EV);
   // Consequently, if we don't have revocation checking enabled, we can't claim
   // any cert is EV.
-  EXPECT_EQ(false, thawte_cert->IsEV(0));
+  flags = X509Certificate::VERIFY_EV_CERT;
+  EXPECT_EQ(OK, thawte_cert->Verify("www.thawte.com", flags, &verify_result));
+  EXPECT_EQ(0, verify_result.cert_status & CERT_STATUS_IS_EV);
 #endif
 }
 
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index bae5607..ece65be 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -428,7 +428,7 @@ void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
 }
 
 int X509Certificate::Verify(const std::string& hostname,
-                            bool rev_checking_enabled,
+                            int flags,
                             CertVerifyResult* verify_result) const {
   verify_result->Reset();
 
@@ -443,12 +443,14 @@ int X509Certificate::Verify(const std::string& hostname,
   chain_para.RequestedUsage.Usage.cUsageIdentifier = 0;
   chain_para.RequestedUsage.Usage.rgpszUsageIdentifier = NULL;  // LPSTR*
   // We can set CERT_CHAIN_RETURN_LOWER_QUALITY_CONTEXTS to get more chains.
-  DWORD flags = CERT_CHAIN_CACHE_END_CERT;
-  if (rev_checking_enabled) {
+  DWORD chain_flags = CERT_CHAIN_CACHE_END_CERT;
+  if (flags & VERIFY_REV_CHECKING_ENABLED) {
     verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
-    flags |= CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT;
+    chain_flags |= CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT;
   } else {
-    flags |= CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY;
+    chain_flags |= CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY;
+    // EV requires revocation checking.
+    flags &= ~VERIFY_EV_CERT;
   }
   PCCERT_CHAIN_CONTEXT chain_context;
   if (!CertGetCertificateChain(
@@ -457,7 +459,7 @@ int X509Certificate::Verify(const std::string& hostname,
            NULL,  // current system time
            cert_handle_->hCertStore,  // search this store
            &chain_para,
-           flags,
+           chain_flags,
            NULL,  // reserved
            &chain_context)) {
     return MapSecurityError(GetLastError());
@@ -553,22 +555,20 @@ int X509Certificate::Verify(const std::string& hostname,
 
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);
+
+  // TODO(ukai): combine regular cert verification and EV cert verification.
+  if ((flags & VERIFY_EV_CERT) && VerifyEV())
+    verify_result->cert_status |= CERT_STATUS_IS_EV;
   return OK;
 }
 
 // Returns true if the certificate is an extended-validation certificate.
 //
-// The certificate has already been verified by the HTTP library.  cert_status
-// represents the result of that verification.  This function performs
-// additional checks of the certificatePolicies extensions of the certificates
-// in the certificate chain according to Section 7 (pp. 11-12) of the EV
-// Certificate Guidelines Version 1.0 at
+// This function checks the certificatePolicies extensions of the
+// certificates in the certificate chain according to Section 7 (pp. 11-12)
+// of the EV Certificate Guidelines Version 1.0 at
 // http://cabforum.org/EV_Certificate_Guidelines.pdf.
-bool X509Certificate::IsEV(int cert_status) const {
-  if (net::IsCertStatusError(cert_status) ||
-      (cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED) == 0)
-    return false;
-
+bool X509Certificate::VerifyEV() const {
   net::EVRootCAMetadata* metadata = net::EVRootCAMetadata::GetInstance();
 
   PCCERT_CHAIN_CONTEXT chain_context = ConstructCertChain(cert_handle_,
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index a6b5fe3..3d7b700 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -672,6 +672,9 @@ int HttpNetworkTransaction::DoSOCKSConnectComplete(int result) {
 int HttpNetworkTransaction::DoSSLConnect() {
   next_state_ = STATE_SSL_CONNECT_COMPLETE;
 
+  if (request_->load_flags & LOAD_VERIFY_EV_CERT)
+    ssl_config_.verify_ev_cert = true;
+
   // Add a SSL socket on top of our existing transport socket.
   ClientSocket* s = connection_.release_socket();
   s = socket_factory_->CreateSSLClientSocket(
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 786f897..fc2fed6 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -716,8 +716,12 @@ int SSLClientSocketNSS::DoHandshakeRead() {
 int SSLClientSocketNSS::DoVerifyCert(int result) {
   DCHECK(server_cert_);
   GotoState(STATE_VERIFY_CERT_COMPLETE);
-  return verifier_.Verify(server_cert_, hostname_,
-                          ssl_config_.rev_checking_enabled,
+  int flags = 0;
+  if (ssl_config_.rev_checking_enabled)
+    flags |= X509Certificate::VERIFY_REV_CHECKING_ENABLED;
+  if (ssl_config_.verify_ev_cert)
+    flags |= X509Certificate::VERIFY_EV_CERT;
+  return verifier_.Verify(server_cert_, hostname_, flags,
                           &server_cert_verify_result_, &io_callback_);
 }
 
diff --git a/net/socket/ssl_client_socket_win.cc b/net/socket/ssl_client_socket_win.cc
index bd2c594..f461094 100644
--- a/net/socket/ssl_client_socket_win.cc
+++ b/net/socket/ssl_client_socket_win.cc
@@ -848,8 +848,12 @@ int SSLClientSocketWin::DoVerifyCert() {
 
   DCHECK(server_cert_);
 
-  return verifier_.Verify(server_cert_, hostname_,
-                          ssl_config_.rev_checking_enabled,
+  int flags = 0;
+  if (ssl_config_.rev_checking_enabled)
+    flags |= X509Certificate::VERIFY_REV_CHECKING_ENABLED;
+  if (ssl_config_.verify_ev_cert)
+    flags |= X509Certificate::VERIFY_EV_CERT;
+  return verifier_.Verify(server_cert_, hostname_, flags,
                           &server_cert_verify_result_, &io_callback_);
 }