net: rework the NPN patch.

net/socket/ssl_client_socket_nss.cc

 // Copyright (c) 2011 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.
 
 // This file includes code SSLClientSocketNSS::DoVerifyCertComplete() derived
 // from AuthCertificateCallback() in
 // mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp.
 
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ skipping 447 matching lines @@
       completed_handshake_(false),
       eset_mitm_detected_(false),
       kaspersky_mitm_detected_(false),
       predicted_cert_chain_correct_(false),
       next_handshake_state_(STATE_NONE),
       nss_fd_(NULL),
       nss_bufs_(NULL),
       net_log_(transport_socket->socket()->NetLog()),
       ssl_host_info_(ssl_host_info),
       dns_cert_checker_(context.dns_cert_checker),
+      next_proto_status_(kNextProtoUnsupported),
       valid_thread_id_(base::kInvalidThreadId) {
   EnterFunction("");
 }
 
 SSLClientSocketNSS::~SSLClientSocketNSS() {
   EnterFunction("");
   Disconnect();
   LeaveFunction("");
 }
 
@@ skipping 67 matching lines @@
       context.length(), out, outlen);
   if (result != SECSuccess) {
     LogFailedNSSFunction(net_log_, "SSL_ExportKeyingMaterial", "");
     return MapNSSError(PORT_GetError());
   }
   return OK;
 }
 
 SSLClientSocket::NextProtoStatus
 SSLClientSocketNSS::GetNextProto(std::string* proto) {
-#if defined(SSL_NEXT_PROTO_NEGOTIATED)
-  unsigned char buf[255];
-  int state;
-  unsigned len;
-  SECStatus rv = SSL_GetNextProto(nss_fd_, &state, buf, &len, sizeof(buf));
-  if (rv != SECSuccess) {
-    NOTREACHED() << "Error return from SSL_GetNextProto: " << rv;
-    proto->clear();
-    return kNextProtoUnsupported;
-  }
-  // We don't check for truncation because sizeof(buf) is large enough to hold
-  // the maximum protocol size.
-  switch (state) {
-    case SSL_NEXT_PROTO_NO_SUPPORT:
-      proto->clear();
-      return kNextProtoUnsupported;
-    case SSL_NEXT_PROTO_NEGOTIATED:
-      *proto = std::string(reinterpret_cast<char*>(buf), len);
-      return kNextProtoNegotiated;
-    case SSL_NEXT_PROTO_NO_OVERLAP:
-      *proto = std::string(reinterpret_cast<char*>(buf), len);
-      return kNextProtoNoOverlap;
-    default:
-      NOTREACHED() << "Unknown status from SSL_GetNextProto: " << state;
-      proto->clear();
-      return kNextProtoUnsupported;
-  }
-#else
-  // No NPN support in the libssl that we are building with.
-  proto->clear();
-  return kNextProtoUnsupported;
-#endif
+  *proto = next_proto_;
+  return next_proto_status_;
 }
 
 int SSLClientSocketNSS::Connect(OldCompletionCallback* callback) {
   EnterFunction("");
   DCHECK(transport_.get());
   DCHECK(next_handshake_state_ == STATE_NONE);
   DCHECK(!user_read_callback_);
   DCHECK(!user_write_callback_);
   DCHECK(!user_connect_callback_);
   DCHECK(!user_read_buf_);
@@ skipping 359 matching lines @@
   // http://extendedsubset.com/?p=8
 
   rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION,
                      SSL_RENEGOTIATE_TRANSITIONAL);
   if (rv != SECSuccess) {
     LogFailedNSSFunction(
         net_log_, "SSL_OptionSet", "SSL_ENABLE_RENEGOTIATION");
   }
 #endif  // SSL_ENABLE_RENEGOTIATION
 
-#ifdef SSL_NEXT_PROTO_NEGOTIATED
+#ifdef SSL_NEXT_PROTO_NEGOTIATION_SUPPORTED
   if (!ssl_config_.next_protos.empty()) {
-    rv = SSL_SetNextProtoNego(
-       nss_fd_,
-       reinterpret_cast<const unsigned char *>(ssl_config_.next_protos.data()),
-       ssl_config_.next_protos.size());
+    rv = SSL_SetNextProtoCallback(
+        nss_fd_, SSLClientSocketNSS::NextProtoCallback, this);
     if (rv != SECSuccess)
-      LogFailedNSSFunction(net_log_, "SSL_SetNextProtoNego", "");
+      LogFailedNSSFunction(net_log_, "SSL_SetNextProtoCallback", "");
   }
 #endif
 
 #ifdef SSL_ENABLE_OCSP_STAPLING
   if (IsOCSPStaplingSupported()) {
     rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
     if (rv != SECSuccess) {
       LogFailedNSSFunction(net_log_, "SSL_OptionSet",
                            "SSL_ENABLE_OCSP_STAPLING");
     }
@@ skipping 1526 matching lines @@
 void SSLClientSocketNSS::HandshakeCallback(PRFileDesc* socket,
                                            void* arg) {
   SSLClientSocketNSS* that = reinterpret_cast<SSLClientSocketNSS*>(arg);
 
   that->handshake_callback_called_ = true;
 
   that->UpdateServerCert();
   that->UpdateConnectionStatus();
 }
 
+// NextProtoCallback is called by NSS during the handshake, if the server
+// supports NPN, to select a protocol from the list that the server offered.
+// See the comment in net/third_party/nss/ssl/ssl.h for the meanings of the
+// arguments.
+// static
+SECStatus
+SSLClientSocketNSS::NextProtoCallback(void* arg,
+                                      PRFileDesc* nss_fd,
+                                      const unsigned char* protos,
+                                      unsigned short protos_len,
+                                      unsigned char* proto_out,
+                                      unsigned char* proto_out_len) {
+  SSLClientSocketNSS* that = reinterpret_cast<SSLClientSocketNSS*>(arg);
+
+  // For each protocol in server preference, see if we support it.
+  for (unsigned int i = 0; i < protos_len; ) {
+    const size_t len = protos[i];
+    for (std::vector<std::string>::const_iterator
+         j = that->ssl_config_.next_protos.begin();
+         j != that->ssl_config_.next_protos.end(); j++) {
+      // Having very long elements in the |next_protos| vector isn't a disaster
+      // because they'll never be selected, but it does indicate an error
+      // somewhere.
+      DCHECK_LT(j->size(), 256u);

[wtc, 2011/10/11 23:43:04]

If a DCHECK failure here indicates an NSS bug, then using a
DCHECK is fine.

Otherwise, we should replace the DCHECK with a LOG(ERROR).

[agl, 2011/10/17 17:37:24]

It represents a major programming error. I think that DCHECK is correct.

+
+      if (j->size() == len &&
+          memcmp(&protos[i+1], j->data(), len) == 0) {

[wtc, 2011/10/11 23:43:04]

Nit: our Style Guide recommend spaces around '+'.

[agl, 2011/10/17 17:37:24]

Done.

+        that->next_proto_status_ = kNextProtoNegotiated;
+        that->next_proto_ = *j;
+        break;
+      }
+    }
+
+    if (that->next_proto_status_ == kNextProtoNegotiated)
+      break;
+
+    // NSS checks that the data in |protos| is well formed, so we know that
+    // this doesn't cause us to jump off the end of the buffer.
+    i += len + 1;
+  }
+
+  // If we didn't find a protocol, we select the first one from our list.
+  if (that->next_proto_status_ != kNextProtoNegotiated) {
+    that->next_proto_status_ = kNextProtoNoOverlap;
+    that->next_proto_ = that->ssl_config_.next_protos[0];
+  }
+
+  memcpy(proto_out, that->next_proto_.data(), that->next_proto_.size());
+  *proto_out_len = that->next_proto_.size();
+  return SECSuccess;
+}
+
 void SSLClientSocketNSS::EnsureThreadIdAssigned() const {
   base::AutoLock auto_lock(lock_);
   if (valid_thread_id_ != base::kInvalidThreadId)
     return;
   valid_thread_id_ = base::PlatformThread::CurrentId();
 }
 
 bool SSLClientSocketNSS::CalledOnValidThread() const {
   EnsureThreadIdAssigned();
   base::AutoLock auto_lock(lock_);
   return valid_thread_id_ == base::PlatformThread::CurrentId();
 }
 
 }  // namespace net