From 1425f020c47b3cbe134f71717299714aead28502 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sat, 1 Sep 2018 23:45:10 +0200 Subject: Remove support for TLS session caches in TLSServerSocket. This resolves #738 --- devtools/shared/security/socket.js | 1 - dom/presentation/provider/PresentationControlService.js | 1 - netwerk/base/TLSServerSocket.cpp | 16 ++-------------- netwerk/base/nsITLSServerSocket.idl | 9 --------- netwerk/test/unit/test_be_conservative.js | 1 - netwerk/test/unit/test_tls_server.js | 1 - netwerk/test/unit/test_tls_server_multiple_clients.js | 1 - security/manager/ssl/nsNSSComponent.cpp | 14 -------------- security/manager/ssl/tests/unit/test_weak_crypto.js | 1 - xpcom/build/XPCOMInit.cpp | 13 +++++++++++++ 10 files changed, 15 insertions(+), 43 deletions(-) diff --git a/devtools/shared/security/socket.js b/devtools/shared/security/socket.js index 068a8ea81..9c6f5750a 100644 --- a/devtools/shared/security/socket.js +++ b/devtools/shared/security/socket.js @@ -480,7 +480,6 @@ SocketListener.prototype = { _setAdditionalSocketOptions: Task.async(function* () { if (this.encryption) { this._socket.serverCert = yield cert.local.getOrCreate(); - this._socket.setSessionCache(false); this._socket.setSessionTickets(false); let requestCert = Ci.nsITLSServerSocket.REQUEST_NEVER; this._socket.setRequestClientCertificate(requestCert); diff --git a/dom/presentation/provider/PresentationControlService.js b/dom/presentation/provider/PresentationControlService.js index fe61d26d6..e9f92247f 100644 --- a/dom/presentation/provider/PresentationControlService.js +++ b/dom/presentation/provider/PresentationControlService.js @@ -100,7 +100,6 @@ PresentationControlService.prototype = { if (aCert) { this._serverSocket.serverCert = aCert; - this._serverSocket.setSessionCache(false); this._serverSocket.setSessionTickets(false); let requestCert = Ci.nsITLSServerSocket.REQUEST_NEVER; this._serverSocket.setRequestClientCertificate(requestCert); diff --git a/netwerk/base/TLSServerSocket.cpp b/netwerk/base/TLSServerSocket.cpp index 257a7f5da..97c7f5423 100644 --- a/netwerk/base/TLSServerSocket.cpp +++ b/netwerk/base/TLSServerSocket.cpp @@ -52,12 +52,12 @@ TLSServerSocket::SetSocketDefaults() SSL_OptionSet(mFD, SSL_SECURITY, true); SSL_OptionSet(mFD, SSL_HANDSHAKE_AS_CLIENT, false); SSL_OptionSet(mFD, SSL_HANDSHAKE_AS_SERVER, true); - + SSL_OptionSet(mFD, SSL_NO_CACHE, true); + // We don't currently notify the server API consumer of renegotiation events // (to revalidate peer certs, etc.), so disable it for now. SSL_OptionSet(mFD, SSL_ENABLE_RENEGOTIATION, SSL_RENEGOTIATE_NEVER); - SetSessionCache(true); SetSessionTickets(true); SetRequestClientCertificate(REQUEST_NEVER); @@ -171,18 +171,6 @@ TLSServerSocket::SetServerCert(nsIX509Cert* aCert) return NS_OK; } -NS_IMETHODIMP -TLSServerSocket::SetSessionCache(bool aEnabled) -{ - // If AsyncListen was already called (and set mListener), it's too late to set - // this. - if (NS_WARN_IF(mListener)) { - return NS_ERROR_IN_PROGRESS; - } - SSL_OptionSet(mFD, SSL_NO_CACHE, !aEnabled); - return NS_OK; -} - NS_IMETHODIMP TLSServerSocket::SetSessionTickets(bool aEnabled) { diff --git a/netwerk/base/nsITLSServerSocket.idl b/netwerk/base/nsITLSServerSocket.idl index 57485357f..dce54ffe7 100644 --- a/netwerk/base/nsITLSServerSocket.idl +++ b/netwerk/base/nsITLSServerSocket.idl @@ -19,15 +19,6 @@ interface nsITLSServerSocket : nsIServerSocket */ attribute nsIX509Cert serverCert; - /** - * setSessionCache - * - * Whether the server should use a session cache. Defaults to true. This - * should be set before calling |asyncListen| if you wish to change the - * default. - */ - void setSessionCache(in boolean aSessionCache); - /** * setSessionTickets * diff --git a/netwerk/test/unit/test_be_conservative.js b/netwerk/test/unit/test_be_conservative.js index 2c6ac46ad..36b6d3b90 100644 --- a/netwerk/test/unit/test_be_conservative.js +++ b/netwerk/test/unit/test_be_conservative.js @@ -140,7 +140,6 @@ function startServer(cert, minServerVersion, maxServerVersion) { tlsServer.init(-1, true, -1); tlsServer.serverCert = cert; tlsServer.setVersionRange(minServerVersion, maxServerVersion); - tlsServer.setSessionCache(false); tlsServer.setSessionTickets(false); tlsServer.asyncListen(new ServerSocketListener()); return tlsServer; diff --git a/netwerk/test/unit/test_tls_server.js b/netwerk/test/unit/test_tls_server.js index d805359c7..12154a27f 100644 --- a/netwerk/test/unit/test_tls_server.js +++ b/netwerk/test/unit/test_tls_server.js @@ -90,7 +90,6 @@ function startServer(cert, expectingPeerCert, clientCertificateConfig, onStopListening: function() {} }; - tlsServer.setSessionCache(false); tlsServer.setSessionTickets(false); tlsServer.setRequestClientCertificate(clientCertificateConfig); diff --git a/netwerk/test/unit/test_tls_server_multiple_clients.js b/netwerk/test/unit/test_tls_server_multiple_clients.js index b63c0189b..74b814e9c 100644 --- a/netwerk/test/unit/test_tls_server_multiple_clients.js +++ b/netwerk/test/unit/test_tls_server_multiple_clients.js @@ -67,7 +67,6 @@ function startServer(cert) { onStopListening: function() {} }; - tlsServer.setSessionCache(true); tlsServer.setSessionTickets(false); tlsServer.asyncListen(listener); diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index 71043a0e7..14b1312de 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -1938,20 +1938,6 @@ nsNSSComponent::InitializeNSS() return NS_ERROR_FAILURE; } - // TLSServerSocket may be run with the session cache enabled. It is necessary - // to call this once before that can happen. This specifies a maximum of 1000 - // cache entries (the default number of cache entries is 10000, which seems a - // little excessive as there probably won't be that many clients connecting to - // any TLSServerSockets the browser runs.) - // Note that this must occur before any calls to SSL_ClearSessionCache - // (otherwise memory will leak). - if (SSL_ConfigServerSessionIDCache(1000, 0, 0, nullptr) != SECSuccess) { -#ifdef ANDROID - MOZ_RELEASE_ASSERT(false); -#endif - return NS_ERROR_FAILURE; - } - // ensure the CertBlocklist is initialised nsCOMPtr certList = do_GetService(NS_CERTBLOCKLIST_CONTRACTID); #ifdef ANDROID diff --git a/security/manager/ssl/tests/unit/test_weak_crypto.js b/security/manager/ssl/tests/unit/test_weak_crypto.js index effedf8e3..3367e9067 100644 --- a/security/manager/ssl/tests/unit/test_weak_crypto.js +++ b/security/manager/ssl/tests/unit/test_weak_crypto.js @@ -77,7 +77,6 @@ function startServer(cert, rc4only) { onStopListening: function() {} }; - tlsServer.setSessionCache(false); tlsServer.setSessionTickets(false); tlsServer.setRequestClientCertificate(Ci.nsITLSServerSocket.REQUEST_NEVER); if (rc4only) { diff --git a/xpcom/build/XPCOMInit.cpp b/xpcom/build/XPCOMInit.cpp index e8ee5828a..b89f51a98 100644 --- a/xpcom/build/XPCOMInit.cpp +++ b/xpcom/build/XPCOMInit.cpp @@ -123,6 +123,8 @@ extern nsresult nsStringInputStreamConstructor(nsISupports*, REFNSIID, void**); #include "nsMemoryInfoDumper.h" #include "nsSecurityConsoleMessage.h" #include "nsMessageLoop.h" +#include "nss.h" +#include "ssl.h" #include "nsStatusReporterManager.h" @@ -1043,6 +1045,17 @@ ShutdownXPCOM(nsIServiceManager* aServMgr) sInitializedJS = false; } + // At this point all networking threads should have been joined and the + // component manager is shut down. Any remaining objects that hold NSS + // resources (should!) have been released, so we can safely shut down NSS. + if (NSS_IsInitialized()) { + SSL_ClearSessionCache(); + // XXX: It would be nice if we can enforce this shutdown. + if (NSS_Shutdown() != SECSuccess) { + NS_WARNING("NSS Shutdown failed - some resources are still in use"); + } + } + // Release our own singletons // Do this _after_ shutting down the component manager, because the // JS component loader will use XPConnect to call nsIModule::canUnload, -- cgit v1.2.3