We don't support SSL renegotiation yet. Add the

ERR_SSL_RENEGOTIATION_REQUESTED error code for when we
received a renegotiation request from a server.

Support the completion of an SSL handshake after we write
something.  (This happens in a session resumption
handshake.)

Use the SSL configuration settings to turn on or turn off
various versions of the SSL protocol and server certificate
revocation checking.

Report all the errors of a certificate and whether revocation
checking was done in in the server_cert_status_ bitmask.

Create a new scoped_cert_chain_context.h header for the
ScopedCertChainContext class that used to be in
x509_certificate_win.cc, and use it to fix a leak of
chain_context on error paths in
SSLClientSocketWin::VerifyServerCert.

R=rvargas
BUG=3002,3003,3004
Review URL: http://codereview.chromium.org/7505

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

8 files changed, 222 insertions, 43 deletions

diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index c081bf3..35e0a24 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -84,6 +84,9 @@ NET_ERROR(NO_SSL_VERSIONS_ENABLED, -112)
 // cipher suite.
 NET_ERROR(SSL_VERSION_OR_CIPHER_MISMATCH, -113)
 
+// The server requested a renegotiation (rehandshake).
+NET_ERROR(SSL_RENEGOTIATION_REQUESTED, -114)
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
diff --git a/net/base/scoped_cert_chain_context.h b/net/base/scoped_cert_chain_context.h
new file mode 100644
index 0000000..61bb2f3
--- /dev/null
+++ b/net/base/scoped_cert_chain_context.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2008 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_BASE_SCOPED_CERT_CHAIN_CONTEXT_H_
+#define NET_BASE_SCOPED_CERT_CHAIN_CONTEXT_H_
+
+#include <windows.h>
+#include <wincrypt.h>
+
+#include "base/scoped_ptr.h"
+
+namespace net {
+
+// This class wraps the CertFreeCertificateChain function in a class that can
+// be passed as a template argument to scoped_ptr_malloc.
+class ScopedPtrMallocFreeCertChain {
+ public:
+  void operator()(const CERT_CHAIN_CONTEXT* x) const {
+    CertFreeCertificateChain(x);
+  }
+};
+
+typedef scoped_ptr_malloc<const CERT_CHAIN_CONTEXT,
+                          ScopedPtrMallocFreeCertChain> ScopedCertChainContext;
+
+}  // namespace net
+
+#endif  // NET_BASE_SCOPED_CERT_CHAIN_CONTEXT_H_
diff --git a/net/base/ssl_client_socket_win.cc b/net/base/ssl_client_socket_win.cc
index cd7f7cb..32d30a1 100644
--- a/net/base/ssl_client_socket_win.cc
+++ b/net/base/ssl_client_socket_win.cc
@@ -9,6 +9,7 @@
 #include "base/singleton.h"
 #include "base/string_util.h"
 #include "net/base/net_errors.h"
+#include "net/base/scoped_cert_chain_context.h"
 #include "net/base/ssl_info.h"
 
 #pragma comment(lib, "secur32.lib")
@@ -34,9 +35,12 @@ static int MapSecurityError(SECURITY_STATUS err) {
       return ERR_CERT_DATE_INVALID;
     case CRYPT_E_NO_REVOCATION_CHECK:
       return ERR_CERT_NO_REVOCATION_MECHANISM;
+    case CRYPT_E_REVOCATION_OFFLINE:
+      return ERR_CERT_UNABLE_TO_CHECK_REVOCATION;
     case CRYPT_E_REVOKED:  // Schannel and CryptoAPI
       return ERR_CERT_REVOKED;
     case SEC_E_CERT_UNKNOWN:
+    case CERT_E_ROLE:
       return ERR_CERT_INVALID;
     // We received an unexpected_message or illegal_parameter alert message
     // from the server.
@@ -78,6 +82,91 @@ static int MapNetErrorToCertStatus(int error) {
   }
 }
 
+static int MapCertStatusToNetError(int cert_status) {
+  // A certificate may have multiple errors.  We report the most
+  // serious error.
+
+  // Unrecoverable errors
+  if (cert_status & CERT_STATUS_INVALID)
+    return ERR_CERT_INVALID;
+  if (cert_status & CERT_STATUS_REVOKED)
+    return ERR_CERT_REVOKED;
+
+  // Recoverable errors
+  if (cert_status & CERT_STATUS_AUTHORITY_INVALID)
+    return ERR_CERT_AUTHORITY_INVALID;
+  if (cert_status & CERT_STATUS_COMMON_NAME_INVALID)
+    return ERR_CERT_COMMON_NAME_INVALID;
+  if (cert_status & CERT_STATUS_DATE_INVALID)
+    return ERR_CERT_DATE_INVALID;
+
+  // Unknown status.  Give it the benefit of the doubt.
+  if (cert_status & CERT_STATUS_UNABLE_TO_CHECK_REVOCATION)
+    return ERR_CERT_UNABLE_TO_CHECK_REVOCATION;
+  if (cert_status & CERT_STATUS_NO_REVOCATION_MECHANISM)
+    return ERR_CERT_NO_REVOCATION_MECHANISM;
+
+  NOTREACHED();
+  return ERR_UNEXPECTED;
+}
+
+// Map the errors in the chain_context->TrustStatus.dwErrorStatus returned by
+// CertGetCertificateChain to our certificate status flags.
+static int MapCertChainErrorStatusToCertStatus(DWORD error_status) {
+  int cert_status = 0;
+
+  // CERT_TRUST_IS_NOT_TIME_NESTED means a subject certificate's time validity
+  // does not nest correctly within its issuer's time validity.
+  const DWORD kDateInvalidErrors = CERT_TRUST_IS_NOT_TIME_VALID |
+                                   CERT_TRUST_IS_NOT_TIME_NESTED |
+                                   CERT_TRUST_CTL_IS_NOT_TIME_VALID;
+  if (error_status & kDateInvalidErrors)
+    cert_status |= CERT_STATUS_DATE_INVALID;
+
+  const DWORD kAuthorityInvalidErrors = CERT_TRUST_IS_UNTRUSTED_ROOT |
+                                        CERT_TRUST_IS_EXPLICIT_DISTRUST |
+                                        CERT_TRUST_IS_PARTIAL_CHAIN;
+  if (error_status & kAuthorityInvalidErrors)
+    cert_status |= CERT_STATUS_AUTHORITY_INVALID;
+
+  if ((error_status & CERT_TRUST_REVOCATION_STATUS_UNKNOWN) &&
+      !(error_status & CERT_TRUST_IS_OFFLINE_REVOCATION))
+    cert_status |= CERT_STATUS_NO_REVOCATION_MECHANISM;
+
+  if (error_status & CERT_TRUST_IS_OFFLINE_REVOCATION)
+    cert_status |= CERT_STATUS_UNABLE_TO_CHECK_REVOCATION;
+
+  if (error_status & CERT_TRUST_IS_REVOKED)
+    cert_status |= CERT_STATUS_REVOKED;
+
+  const DWORD kWrongUsageErrors = CERT_TRUST_IS_NOT_VALID_FOR_USAGE |
+                                  CERT_TRUST_CTL_IS_NOT_VALID_FOR_USAGE;
+  if (error_status & kWrongUsageErrors) {
+    // TODO(wtc): Handle these errors.
+    // cert_status = |= CERT_STATUS_WRONG_USAGE;
+  }
+
+  // The rest of the errors.
+  const DWORD kCertInvalidErrors =
+      CERT_TRUST_IS_NOT_SIGNATURE_VALID |
+      CERT_TRUST_IS_CYCLIC |
+      CERT_TRUST_INVALID_EXTENSION |
+      CERT_TRUST_INVALID_POLICY_CONSTRAINTS |
+      CERT_TRUST_INVALID_BASIC_CONSTRAINTS |
+      CERT_TRUST_INVALID_NAME_CONSTRAINTS |
+      CERT_TRUST_CTL_IS_NOT_SIGNATURE_VALID |
+      CERT_TRUST_HAS_NOT_SUPPORTED_NAME_CONSTRAINT |
+      CERT_TRUST_HAS_NOT_DEFINED_NAME_CONSTRAINT |
+      CERT_TRUST_HAS_NOT_PERMITTED_NAME_CONSTRAINT |
+      CERT_TRUST_HAS_EXCLUDED_NAME_CONSTRAINT |
+      CERT_TRUST_NO_ISSUANCE_CHAIN_POLICY |
+      CERT_TRUST_HAS_NOT_SUPPORTED_CRITICAL_EXT;
+  if (error_status & kCertInvalidErrors)
+    cert_status |= CERT_STATUS_INVALID;
+
+  return cert_status;
+}
+
 //-----------------------------------------------------------------------------
 
 // Size of recv_buffer_
@@ -111,6 +200,7 @@ SSLClientSocketWin::SSLClientSocketWin(ClientSocket* transport_socket,
       received_ptr_(NULL),
       bytes_received_(0),
       completed_handshake_(false),
+      complete_handshake_on_write_complete_(false),
       ignore_ok_result_(false),
       no_client_cert_(false) {
   memset(&stream_sizes_, 0, sizeof(stream_sizes_));
@@ -530,12 +620,12 @@ int SSLClientSocketWin::DoHandshakeReadComplete(int result) {
       (status == SEC_E_OK ||
        status == SEC_I_CONTINUE_NEEDED ||
        FAILED(status) && (out_flags & ISC_RET_EXTENDED_ERROR))) {
-    // TODO(wtc): if status is SEC_E_OK, we should finish the handshake
-    // successfully after sending send_buffer_.
     // If FAILED(status) is true, we should terminate the connection after
     // sending send_buffer_.
-    DCHECK(status == SEC_I_CONTINUE_NEEDED);  // We only handle this case
-                                              // correctly.
+    if (status == SEC_E_OK)
+      complete_handshake_on_write_complete_ = true;
+    // We only handle these cases correctly.
+    DCHECK(status == SEC_E_OK || status == SEC_I_CONTINUE_NEEDED);
     next_state_ = STATE_HANDSHAKE_WRITE;
     bytes_received_ = 0;
     return OK;
@@ -609,6 +699,8 @@ int SSLClientSocketWin::DoHandshakeWriteComplete(int result) {
     bytes_sent_ = 0;
     if (overflow)  // Bug!
       return ERR_UNEXPECTED;
+    if (complete_handshake_on_write_complete_)
+      return DidCompleteHandshake();
     next_state_ = STATE_HANDSHAKE_READ;
   } else {
     // Send the remaining bytes.
@@ -716,8 +808,13 @@ int SSLClientSocketWin::DoPayloadReadComplete(int result) {
       received_ptr_ = recv_buffer_.get();
     }
   }
-  // TODO(wtc): need to handle SEC_I_RENEGOTIATE.
-  DCHECK(status == SEC_E_OK);
+
+  if (status == SEC_I_RENEGOTIATE) {
+    // TODO(wtc): support renegotiation.
+    // Should ideally send a no_renegotiation alert to the server.
+    return ERR_SSL_RENEGOTIATION_REQUESTED;
+  }
+
   // If we decrypted 0 bytes, don't report 0 bytes read, which would be
   // mistaken for EOF.  Continue decrypting or read more.
   if (len == 0) {
@@ -834,16 +931,13 @@ int SSLClientSocketWin::DidCompleteHandshake() {
   }
 
   completed_handshake_ = true;
-  int rv = VerifyServerCert();
-  // TODO(wtc): for now, always check revocation.
-  server_cert_status_ = CERT_STATUS_REV_CHECKING_ENABLED;
-  if (rv)
-    server_cert_status_ |= MapNetErrorToCertStatus(rv);
-  return rv;
+  return VerifyServerCert();
 }
 
+// Set server_cert_status_ and return OK or a network error.
 int SSLClientSocketWin::VerifyServerCert() {
   DCHECK(server_cert_);
+  server_cert_status_ = 0;
 
   // Build and validate certificate chain.
 
@@ -855,21 +949,30 @@ int SSLClientSocketWin::VerifyServerCert() {
   chain_para.RequestedUsage.dwType = USAGE_MATCH_TYPE_AND;
   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 (ssl_config_.rev_checking_enabled) {
+    server_cert_status_ |= CERT_STATUS_REV_CHECKING_ENABLED;
+    flags |= CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT;
+  } else {
+    flags |= CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY;
+  }
   PCCERT_CHAIN_CONTEXT chain_context;
-  // TODO(wtc): for now, always check revocation.  If we don't want to check
-  // revocation, use the CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY flag.
   if (!CertGetCertificateChain(
            NULL,  // default chain engine, HCCE_CURRENT_USER
            server_cert_,
            NULL,  // current system time
            server_cert_->hCertStore,  // search this store
            &chain_para,
-           CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT |
-           CERT_CHAIN_CACHE_END_CERT,
+           flags,
            NULL,  // reserved
            &chain_context)) {
     return MapSecurityError(GetLastError());
   }
+  ScopedCertChainContext scoped_chain_context(chain_context);
+
+  server_cert_status_ |= MapCertChainErrorStatusToCertStatus(
+      chain_context->TrustStatus.dwErrorStatus);
 
   std::wstring wstr_hostname = ASCIIToWide(hostname_);
 
@@ -877,12 +980,6 @@ int SSLClientSocketWin::VerifyServerCert() {
   memset(&extra_policy_para, 0, sizeof(extra_policy_para));
   extra_policy_para.cbSize = sizeof(extra_policy_para);
   extra_policy_para.dwAuthType = AUTHTYPE_SERVER;
-  // TODO(wtc): Set these flags in fdwChecks to ignore cert errors.
-  //   SECURITY_FLAG_IGNORE_REVOCATION
-  //   SECURITY_FLAG_IGNORE_UNKNOWN_CA
-  //   SECURITY_FLAG_IGNORE_WRONG_USAGE
-  //   SECURITY_FLAG_IGNORE_CERT_CN_INVALID
-  //   SECURITY_FLAG_IGNORE_CERT_DATE_INVALID
   extra_policy_para.fdwChecks = 0;
   extra_policy_para.pwszServerName =
       const_cast<wchar_t*>(wstr_hostname.c_str());
@@ -890,8 +987,6 @@ int SSLClientSocketWin::VerifyServerCert() {
   CERT_CHAIN_POLICY_PARA policy_para;
   memset(&policy_para, 0, sizeof(policy_para));
   policy_para.cbSize = sizeof(policy_para);
-  // TODO(wtc): It seems that we can also ignore cert errors by setting
-  // dwFlags.
   policy_para.dwFlags = 0;
   policy_para.pvExtraPolicyPara = &extra_policy_para;
 
@@ -907,11 +1002,58 @@ int SSLClientSocketWin::VerifyServerCert() {
     return MapSecurityError(GetLastError());
   }
 
-  if (policy_status.dwError)
-    return MapSecurityError(policy_status.dwError);
-
-  CertFreeCertificateChain(chain_context);
+  if (policy_status.dwError) {
+    server_cert_status_ |= MapNetErrorToCertStatus(
+        MapSecurityError(policy_status.dwError));
+
+    // CertVerifyCertificateChainPolicy reports only one error (in
+    // policy_status.dwError) if the certificate has multiple errors.
+    // CertGetCertificateChain doesn't report certificate name mismatch, so
+    // CertVerifyCertificateChainPolicy is the only function that can report
+    // certificate name mismatch.
+    //
+    // To prevent a potential certificate name mismatch from being hidden by
+    // some other certificate error, if we get any other certificate error,
+    // we call CertVerifyCertificateChainPolicy again, ignoring all other
+    // certificate errors.  Both extra_policy_para.fdwChecks and
+    // policy_para.dwFlags allow us to ignore certificate errors, so we set
+    // them both.
+    if (policy_status.dwError != CERT_E_CN_NO_MATCH) {
+      const DWORD extra_ignore_flags =
+          0x00000080 |  // SECURITY_FLAG_IGNORE_REVOCATION
+          0x00000100 |  // SECURITY_FLAG_IGNORE_UNKNOWN_CA
+          0x00002000 |  // SECURITY_FLAG_IGNORE_CERT_DATE_INVALID
+          0x00000200;   // SECURITY_FLAG_IGNORE_WRONG_USAGE
+      extra_policy_para.fdwChecks = extra_ignore_flags;
+      const DWORD ignore_flags =
+          CERT_CHAIN_POLICY_IGNORE_ALL_NOT_TIME_VALID_FLAGS |
+          CERT_CHAIN_POLICY_IGNORE_INVALID_BASIC_CONSTRAINTS_FLAG |
+          CERT_CHAIN_POLICY_ALLOW_UNKNOWN_CA_FLAG |
+          CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG |
+          CERT_CHAIN_POLICY_IGNORE_INVALID_NAME_FLAG |
+          CERT_CHAIN_POLICY_IGNORE_INVALID_POLICY_FLAG |
+          CERT_CHAIN_POLICY_IGNORE_ALL_REV_UNKNOWN_FLAGS |
+          CERT_CHAIN_POLICY_ALLOW_TESTROOT_FLAG |
+          CERT_CHAIN_POLICY_TRUST_TESTROOT_FLAG |
+          CERT_CHAIN_POLICY_IGNORE_NOT_SUPPORTED_CRITICAL_EXT_FLAG |
+          CERT_CHAIN_POLICY_IGNORE_PEER_TRUST_FLAG;
+      policy_para.dwFlags = ignore_flags;
+      if (!CertVerifyCertificateChainPolicy(
+               CERT_CHAIN_POLICY_SSL,
+               chain_context,
+               &policy_para,
+               &policy_status)) {
+        return MapSecurityError(GetLastError());
+      }
+      if (policy_status.dwError) {
+        server_cert_status_ |= MapNetErrorToCertStatus(
+            MapSecurityError(policy_status.dwError));
+      }
+    }
+  }
 
+  if (IsCertStatusError(server_cert_status_))
+    return MapCertStatusToNetError(server_cert_status_);
   return OK;
 }
 
diff --git a/net/base/ssl_client_socket_win.h b/net/base/ssl_client_socket_win.h
index 403e7f3..1d85377 100644
--- a/net/base/ssl_client_socket_win.h
+++ b/net/base/ssl_client_socket_win.h
@@ -117,6 +117,7 @@ class SSLClientSocketWin : public SSLClientSocket {
   int bytes_received_;  // The number of bytes of received ciphertext.
 
   bool completed_handshake_;
+  bool complete_handshake_on_write_complete_;
 
   // Only used in the STATE_HANDSHAKE_READ_COMPLETE and
   // STATE_PAYLOAD_READ_COMPLETE states.  True if a 'result' argument of OK
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index c63b351..138f20d 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -11,6 +11,7 @@
 #include "base/string_util.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/ev_root_ca_metadata.h"
+#include "net/base/scoped_cert_chain_context.h"
 
 #pragma comment(lib, "crypt32.lib")
 
@@ -157,18 +158,6 @@ bool ContainsPolicy(const CERT_POLICIES_INFO* policies_info,
   return false;
 }
 
-// This class wraps the CertFreeCertificateChain function in a class that can
-// be passed as a template argument to scoped_ptr_malloc.
-class ScopedPtrMallocFreeCertChain {
- public:
-  void operator()(const CERT_CHAIN_CONTEXT* x) const {
-    CertFreeCertificateChain(x);
-  }
-};
-
-typedef scoped_ptr_malloc<const CERT_CHAIN_CONTEXT,
-                          ScopedPtrMallocFreeCertChain> ScopedCertChainContext;
-
 // Helper function to parse a principal from a WinInet description of that
 // principal.
 void ParsePrincipal(const std::string& description,
diff --git a/net/build/net.vcproj b/net/build/net.vcproj
index aa71faf..9bed56f 100644
--- a/net/build/net.vcproj
+++ b/net/build/net.vcproj
@@ -413,6 +413,10 @@
 				>
 			</File>
 			<File
+				RelativePath="..\base\scoped_cert_chain_context.h"
+				>
+			</File>
+			<File
 				RelativePath="..\base\sdch_filter.cc"
 				>
 			</File>
diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h
index 82acf5d..e808554 100644
--- a/net/http/http_network_session.h
+++ b/net/http/http_network_session.h
@@ -8,6 +8,7 @@
 #include "base/ref_counted.h"
 #include "net/base/auth_cache.h"
 #include "net/base/client_socket_pool.h"
+#include "net/base/ssl_config_service.h"
 #include "net/proxy/proxy_service.h"
 
 namespace net {
@@ -19,8 +20,8 @@ class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> {
   enum {
     MAX_SOCKETS_PER_GROUP = 6
   };
-  
-  HttpNetworkSession(ProxyResolver* proxy_resolver)
+
+  explicit HttpNetworkSession(ProxyResolver* proxy_resolver)
       : connection_pool_(new ClientSocketPool(MAX_SOCKETS_PER_GROUP)),
         proxy_resolver_(proxy_resolver),
         proxy_service_(proxy_resolver) {
@@ -29,12 +30,19 @@ class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> {
   AuthCache* auth_cache() { return &auth_cache_; }
   ClientSocketPool* connection_pool() { return connection_pool_; }
   ProxyService* proxy_service() { return &proxy_service_; }
+#if defined(OS_WIN)
+  SSLConfigService* ssl_config_service() { return &ssl_config_service_; }
+#endif
 
  private:
   AuthCache auth_cache_;
   scoped_refptr<ClientSocketPool> connection_pool_;
   scoped_ptr<ProxyResolver> proxy_resolver_;
   ProxyService proxy_service_;
+#if defined(OS_WIN)
+  // TODO(port): Port the SSLConfigService class to Linux and Mac OS X.
+  SSLConfigService ssl_config_service_;
+#endif
 };
 
 }  // namespace net
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 9d74c87..033d0d9 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -56,7 +56,10 @@ HttpNetworkTransaction::HttpNetworkTransaction(HttpNetworkSession* session,
       read_buf_(NULL),
       read_buf_len_(0),
       next_state_(STATE_NONE) {
-  // TODO(wtc): Initialize ssl_config_with SSL settings (bug 3003).
+#if defined(OS_WIN)
+  // TODO(port): Port the SSLConfigService class to Linux and Mac OS X.
+  session->ssl_config_service()->GetSSLConfig(&ssl_config_);
+#endif
 }
 
 void HttpNetworkTransaction::Destroy() {