A follow-up of r64178.

Check for TLS 1.1 and TLS 1.2 (not yet supported)
version numbers to be future-proof.

Avoid a null pointer dereference when SSL client
authentication is used with SSL 2.0.  This will
be removed when we remove SSL 2.0 support.

Define the SSL_CONNECTION_VERSION_xxx enum constants
separately so we can assert their values are in range
at compile time.

R=eroman
BUG=53659
TEST=none
Review URL: http://codereview.chromium.org/4211006

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

2 files changed, 24 insertions, 5 deletions

diff --git a/net/base/ssl_connection_status_flags.h b/net/base/ssl_connection_status_flags.h
index 51eb884..9596f00 100644
--- a/net/base/ssl_connection_status_flags.h
+++ b/net/base/ssl_connection_status_flags.h
@@ -30,16 +30,24 @@ enum {
   // The next three bits are reserved for the SSL version.
   SSL_CONNECTION_VERSION_SHIFT = 20,
   SSL_CONNECTION_VERSION_MASK = 7,
-  SSL_CONNECTION_VERSION_UNKNOWN = 0,  // Unknown SSL version or SSL not used.
+
+  // 1 << 31 (the sign bit) is reserved so that the SSL connection status will
+  // never be negative.
+};
+
+// NOTE: the SSL version enum constants must be between 0 and
+// SSL_CONNECTION_VERSION_MASK, inclusive.
+enum {
+  SSL_CONNECTION_VERSION_UNKNOWN = 0,  // Unknown SSL version.
   SSL_CONNECTION_VERSION_SSL2 = 1,
   SSL_CONNECTION_VERSION_SSL3 = 2,
   SSL_CONNECTION_VERSION_TLS1 = 3,
   SSL_CONNECTION_VERSION_TLS1_1 = 4,
   SSL_CONNECTION_VERSION_TLS1_2 = 5,
-
-  // 1 << 31 (the sign bit) is reserved so that the SSL connection status will
-  // never be negative.
+  SSL_CONNECTION_VERSION_MAX,
 };
+COMPILE_ASSERT(SSL_CONNECTION_VERSION_MAX - 1 <= SSL_CONNECTION_VERSION_MASK,
+               SSL_CONNECTION_VERSION_MASK_too_small);
 
 inline int SSLConnectionStatusToCipherSuite(int connection_status) {
   return (connection_status >> SSL_CONNECTION_CIPHERSUITE_SHIFT) &
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 3a1e3c78..73038e3 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -366,7 +366,7 @@ BOOL WINAPI ClientCertFindCallback(PCCERT_CONTEXT cert_context,
 // and the other elements are in the order given by the server.
 class PeerCertificateChain {
  public:
-  PeerCertificateChain(PRFileDesc* nss_fd)
+  explicit PeerCertificateChain(PRFileDesc* nss_fd)
       : num_certs_(0),
         certs_(NULL) {
     SECStatus rv = SSL_PeerCertificateChain(nss_fd, NULL, &num_certs_);
@@ -1105,6 +1105,8 @@ void SSLClientSocketNSS::UpdateConnectionStatus() {
          SSL_CONNECTION_COMPRESSION_MASK) <<
         SSL_CONNECTION_COMPRESSION_SHIFT;
 
+    // NSS 3.12.x doesn't have version macros for TLS 1.1 and 1.2 (because NSS
+    // doesn't support them yet), so we use 0x0302 and 0x0303 directly.
     int version = SSL_CONNECTION_VERSION_UNKNOWN;
     if (channel_info.protocolVersion < SSL_LIBRARY_VERSION_3_0) {
       // All versions less than SSL_LIBRARY_VERSION_3_0 are treated as SSL
@@ -1114,6 +1116,10 @@ void SSLClientSocketNSS::UpdateConnectionStatus() {
       version = SSL_CONNECTION_VERSION_SSL3;
     } else if (channel_info.protocolVersion == SSL_LIBRARY_VERSION_3_1_TLS) {
       version = SSL_CONNECTION_VERSION_TLS1;
+    } else if (channel_info.protocolVersion == 0x0302) {
+      version = SSL_CONNECTION_VERSION_TLS1_1;
+    } else if (channel_info.protocolVersion == 0x0303) {
+      version = SSL_CONNECTION_VERSION_TLS1_2;
     }
     ssl_connection_status_ |=
         (version & SSL_CONNECTION_VERSION_MASK) <<
@@ -1651,6 +1657,11 @@ SECStatus SSLClientSocketNSS::ClientAuthHandler(
     CERTDistNames* ca_names,
     CERTCertificate** result_certificate,
     SECKEYPrivateKey** result_private_key) {
+  // NSS passes a null ca_names if SSL 2.0 is used.  Just fail rather than
+  // trying to make this work, as we plan to remove SSL 2.0 support soon.
+  if (!ca_names)
+    return SECFailure;
+
   SSLClientSocketNSS* that = reinterpret_cast<SSLClientSocketNSS*>(arg);
 
   that->client_auth_cert_needed_ = !that->ssl_config_.send_client_cert;