Add an explicit function to init NSS for SSL server sockets

BUG=131622
TEST=tsan goes green for existing tests


Review URL: https://chromiumcodereview.appspot.com/10543106

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

net/base/run_all_unittests.cc

@@ -7,6 +7,7 @@
 #include "crypto/nss_util.h"
 #include "net/base/net_test_suite.h"
 #include "net/socket/client_socket_pool_base.h"
+#include "net/socket/ssl_server_socket.h"
 #include "net/spdy/spdy_session.h"
 
 using net::internal::ClientSocketPoolBaseHelper;
@@ -23,5 +24,9 @@ int main(int argc, char** argv) {
   crypto::EnsureNSPRInit();
 #endif
 
+  // Enable support for SSL server sockets, which must be done while
+  // single-threaded.
+  net::EnableSSLServerSockets();
+
   return test_suite.Run();
 }

net/socket/ssl_server_socket.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -31,6 +31,16 @@ class SSLServerSocket : public SSLSocket {
   virtual int Handshake(const CompletionCallback& callback) = 0;
 };
 
+// Configures the underlying SSL library for the use of SSL server sockets.
+//
+// Due to the requirements of the underlying libraries, this should be called
+// early in process initialization, before any SSL socket, client or server,
+// has been used.
+//
+// Note: If a process does not use SSL server sockets, this call may be
+// omitted.
+NET_EXPORT void EnableSSLServerSockets();
+
 // Creates an SSL server socket over an already-connected transport socket.
 // The caller must provide the server certificate and private key to use.
 //

net/socket/ssl_server_socket_nss.cc

@@ -29,6 +29,7 @@
 
 #include <limits>
 
+#include "base/lazy_instance.h"
 #include "base/memory/ref_counted.h"
 #include "crypto/rsa_private_key.h"
 #include "crypto/nss_util_internal.h"
@@ -45,11 +46,42 @@ static const int kRecvBufferSize = 4096;
 
 namespace net {
 
+namespace {
+
+bool g_nss_server_sockets_init = false;
+
+class NSSSSLServerInitSingleton {
+ public:
+  NSSSSLServerInitSingleton() {
+    EnsureNSSSSLInit();
+
+    SSL_ConfigServerSessionIDCache(1024, 5, 5, NULL);
+    g_nss_server_sockets_init = true;
+  }
+
+  ~NSSSSLServerInitSingleton() {
+    SSL_ShutdownServerSessionIDCache();
+    g_nss_server_sockets_init = false;
+  }
+};
+
+static base::LazyInstance<NSSSSLServerInitSingleton>
+    g_nss_ssl_server_init_singleton = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+void EnableSSLServerSockets() {
+  g_nss_ssl_server_init_singleton.Get();
+}
+
 SSLServerSocket* CreateSSLServerSocket(
     StreamSocket* socket,
     X509Certificate* cert,
     crypto::RSAPrivateKey* key,
     const SSLConfig& ssl_config) {
+  DCHECK(g_nss_server_sockets_init) << "EnableSSLServerSockets() has not been"
+                                    << "called yet!";
+
   return new SSLServerSocketNSS(socket, cert, key, ssl_config);
 }
 
@@ -335,12 +367,6 @@ int SSLServerSocketNSS::InitializeSSLOptions() {
     return ERR_UNEXPECTED;
   }
 
-  rv = SSL_ConfigServerSessionIDCache(1024, 5, 5, NULL);
-  if (rv != SECSuccess) {
-    LogFailedNSSFunction(net_log_, "SSL_ConfigureServerSessionIDCache", "");
-    return ERR_UNEXPECTED;
-  }
-
   rv = SSL_AuthCertificateHook(nss_fd_, OwnAuthCertHandler, this);
   if (rv != SECSuccess) {
     LogFailedNSSFunction(net_log_, "SSL_AuthCertificateHook", "");
@@ -771,6 +797,7 @@ int SSLServerSocketNSS::Init() {
   if (!NSS_IsInitialized())
     return ERR_UNEXPECTED;
 
+  EnableSSLServerSockets();
   return OK;
 }
 

net/socket/ssl_server_socket_openssl.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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 "base/logging.h"
 #include "net/socket/ssl_server_socket.h"
 
+// TODO(bulach): Provide simple stubs for EnableSSLServerSockets and
+// CreateSSLServerSocket so that when building for OpenSSL rather than NSS,
+// so that the code using SSL server sockets can be compiled and disabled
+// programatically rather than requiring to be carved out from the compile.
+
 namespace net {
 
-// TODO(bulach): Rather than disable components which call
-// CreateSSLServerSocket when building for OpenSSL rather than NSS, just
-// provide a stub for it for now.
+void EnableSSLServerSockets() {
+  NOTIMPLEMENTED();
+}
+
 SSLServerSocket* CreateSSLServerSocket(StreamSocket* socket,
                                        X509Certificate* certificate,
                                        crypto::RSAPrivateKey* key,

remoting/DEPS

@@ -6,6 +6,11 @@ include_rules = [
   "+crypto",
   "+media/base",
 
+  # Note: Only for net::EnableSSLServerSockets(), which must be called by
+  # unit tests at process start.
+  "-net",
+  "+net/socket",
+
   "-remoting",
   "+remoting/base",
   "+remoting/proto",

remoting/client/plugin/chromoting_instance.cc

@@ -21,6 +21,7 @@
 #include "base/values.h"
 #include "jingle/glue/thread_wrapper.h"
 #include "media/base/media.h"
+#include "net/socket/ssl_server_socket.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/input_event.h"
 #include "ppapi/cpp/mouse_cursor.h"
@@ -187,6 +188,13 @@ bool ChromotingInstance::Init(uint32_t argc,
     return false;
   }
 
+  // Enable support for SSL server sockets, which must be done as early as
+  // possible, preferably before any NSS SSL sockets (client or server) have
+  // been created.
+  // It's possible that the hosting process has already made use of SSL, in
+  // which case, there may be a slight race.
+  net::EnableSSLServerSockets();
+
   // Start all the threads.
   context_.Start();
 

remoting/host/DEPS

@@ -8,6 +8,10 @@ include_rules = [
   "+net/test",
   "+net/url_request",
 
+  # Note: Only for net::EnableSSLServerSockets(), which must be called by
+  # at process start.
+  "+net/socket",
+
   "+remoting/protocol",
   "+remoting/jingle_glue",
   "+third_party/jsoncpp",

remoting/host/plugin/host_plugin.cc

@@ -12,6 +12,7 @@
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/stringize_macros.h"
+#include "net/socket/ssl_server_socket.h"
 #include "remoting/base/plugin_message_loop_proxy.h"
 #include "remoting/host/plugin/constants.h"
 #include "remoting/host/plugin/host_log_handler.h"
@@ -114,6 +115,7 @@ class HostNPPlugin : public remoting::PluginMessageLoopProxy::Delegate {
       return false;
     }
 #endif  // OS_MACOSX
+    net::EnableSSLServerSockets();
     return true;
   }
 

remoting/host/remoting_me2me_host.cc

@@ -21,6 +21,7 @@
 #include "build/build_config.h"
 #include "crypto/nss_util.h"
 #include "net/base/network_change_notifier.h"
+#include "net/socket/ssl_server_socket.h"
 #include "remoting/base/breakpad.h"
 #include "remoting/base/constants.h"
 #include "remoting/host/branding.h"
@@ -546,6 +547,10 @@ int main(int argc, char** argv) {
   gfx::GtkInitFromCommandLine(*cmd_line);
 #endif  // TOOLKIT_GTK
 
+  // Enable support for SSL server sockets, which must be done while still
+  // single-threaded.
+  net::EnableSSLServerSockets();
+
   remoting::HostProcess me2me_host;
   me2me_host.InitWithCommandLine(cmd_line);
 

remoting/host/simple_host_process.cc

@@ -31,6 +31,7 @@
 #include "base/threading/thread.h"
 #include "crypto/nss_util.h"
 #include "net/base/network_change_notifier.h"
+#include "net/socket/ssl_server_socket.h"
 #include "remoting/base/constants.h"
 #include "remoting/host/capturer_fake.h"
 #include "remoting/host/chromoting_host.h"
@@ -348,6 +349,10 @@ int main(int argc, char** argv) {
   gfx::GtkInitFromCommandLine(*cmd_line);
 #endif  // TOOLKIT_GTK
 
+  // Enable support for SSL server sockets, which must be done while still
+  // single-threaded.
+  net::EnableSSLServerSockets();
+
   remoting::SimpleHost simple_host;
 
   if (cmd_line->HasSwitch(kConfigSwitchName)) {

remoting/remoting.gyp

@@ -811,6 +811,7 @@
         'remoting_client',
         'remoting_jingle_glue',
         '../media/media.gyp:media',
+        '../net/net.gyp:net',
         '../ppapi/ppapi.gyp:ppapi_cpp_objects',
         '../skia/skia.gyp:skia',
       ],
@@ -850,6 +851,7 @@
         'remoting_base',
         'remoting_host',
         'remoting_jingle_glue',
+        '../net/net.gyp:net',
         '../third_party/npapi/npapi.gyp:npapi',
       ],
       'sources': [
@@ -1301,6 +1303,7 @@
         '../base/base.gyp:base',
         '../base/base.gyp:base_i18n',
         '../media/media.gyp:media',
+        '../net/net.gyp:net',
       ],
       'sources': [
         'host/simple_host_process.cc',
@@ -1326,6 +1329,7 @@
         '../base/base.gyp:base',
         '../base/base.gyp:base_i18n',
         '../media/media.gyp:media',
+        '../net/net.gyp:net',
       ],
       'sources': [
         'host/branding.cc',

remoting/run_all_unittests.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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 "base/test/test_suite.h"
+#include "net/socket/ssl_server_socket.h"
 
 int main(int argc, char** argv) {
   base::TestSuite test_suite(argc, argv);
+
+  // Enable support for SSL server sockets, which must be done while
+  // single-threaded.
+  net::EnableSSLServerSockets();
+
   return test_suite.Run();
 }

tools/valgrind/tsan/suppressions.txt

@@ -948,33 +948,6 @@
   fun:media::AudioOutputController::DoClose
   fun:base::internal::RunnableAdapter::Run
 }
-{
-   bug_131622a
-   ThreadSanitizer:Race
-   fun:ssl_InitSessionCacheLocks
-   fun:lock_cache
-   fun:ssl_LookupSID
-   fun:ssl2_BeginClientHandshake
-   fun:ssl_Do1stHandshake
-   fun:SSL_ForceHandshake
-   fun:net::SSLClientSocketNSS::Core::DoHandshake
-   fun:net::SSLClientSocketNSS::Core::DoHandshakeLoop
-   fun:net::SSLClientSocketNSS::Core::Connect
-   fun:base::internal::RunnableAdapter::Run
-}
-{
-   bug_131622b
-   ThreadSanitizer:Race
-   fun:lock_cache
-   fun:ssl_LookupSID
-   fun:ssl2_BeginClientHandshake
-   fun:ssl_Do1stHandshake
-   fun:SSL_ForceHandshake
-   fun:net::SSLClientSocketNSS::Core::DoHandshake
-   fun:net::SSLClientSocketNSS::Core::DoHandshakeLoop
-   fun:net::SSLClientSocketNSS::Core::Connect
-   fun:base::internal::RunnableAdapter::Run
-}
 {
    bug_132230a
    ThreadSanitizer:Race