Bug 634697 - Refactoring nsClientAuthRememberService to work as a service r=keeler
☠☠ backed out by 4143bf08dfe6 ☠ ☠
authorMoritz Birghan <mbirghan@mozilla.com>
Thu, 27 Feb 2020 21:28:34 +0000
changeset 516028 a538b04973365a41ccd25ee4aa54b3d72e6091a1
parent 516027 3c89496f7998ae78032b40e9c793eb7c89aa6af8
child 516029 946b79ab705376c3fcaaa2fc24068cbb4061f6f8
push id37165
push useraiakab@mozilla.com
push dateFri, 28 Feb 2020 09:24:28 +0000
treeherdermozilla-central@fb4f281c1c54 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs634697
milestone75.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 634697 - Refactoring nsClientAuthRememberService to work as a service r=keeler Differential Revision: https://phabricator.services.mozilla.com/D62585
security/manager/ssl/SharedSSLState.cpp
security/manager/ssl/SharedSSLState.h
security/manager/ssl/components.conf
security/manager/ssl/moz.build
security/manager/ssl/nsClientAuthRemember.cpp
security/manager/ssl/nsClientAuthRemember.h
security/manager/ssl/nsIClientAuthRemember.idl
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSIOLayer.cpp
--- a/security/manager/ssl/SharedSSLState.cpp
+++ b/security/manager/ssl/SharedSSLState.cpp
@@ -119,37 +119,29 @@ PrivateBrowsingObserver::Observe(nsISupp
 SharedSSLState::SharedSSLState(uint32_t aTlsFlags)
     : mIOLayerHelpers(aTlsFlags),
       mMutex("SharedSSLState::mMutex"),
       mSocketCreated(false),
       mOCSPStaplingEnabled(false),
       mOCSPMustStapleEnabled(false),
       mSignedCertTimestampsEnabled(false) {
   mIOLayerHelpers.Init();
-  if (!aTlsFlags) {  // the per socket flags don't need memory
-    mClientAuthRemember = new nsClientAuthRememberService();
-    mClientAuthRemember->Init();
-  }
 }
 
 SharedSSLState::~SharedSSLState() {}
 
 void SharedSSLState::NotePrivateBrowsingStatus() {
   MOZ_ASSERT(NS_IsMainThread(), "Not on main thread");
   mObserver = new PrivateBrowsingObserver(this);
   nsCOMPtr<nsIObserverService> obsSvc = mozilla::services::GetObserverService();
   obsSvc->AddObserver(mObserver, "last-pb-context-exited", false);
 }
 
 void SharedSSLState::ResetStoredData() {
-  if (!mClientAuthRemember) {
-    return;
-  }
   MOZ_ASSERT(NS_IsMainThread(), "Not on main thread");
-  mClientAuthRemember->ClearRememberedDecisions();
   mIOLayerHelpers.clearStoredData();
 }
 
 void SharedSSLState::NoteSocketCreated() {
   MutexAutoLock lock(mMutex);
   mSocketCreated = true;
 }
 
--- a/security/manager/ssl/SharedSSLState.h
+++ b/security/manager/ssl/SharedSSLState.h
@@ -2,37 +2,31 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef SharedSSLState_h
 #define SharedSSLState_h
 
-#include "mozilla/RefPtr.h"
 #include "nsNSSIOLayer.h"
 
-class nsClientAuthRememberService;
 class nsIObserver;
 
 namespace mozilla {
 namespace psm {
 
 class SharedSSLState {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedSSLState)
   explicit SharedSSLState(uint32_t aTlsFlags = 0);
 
   static void GlobalInit();
   static void GlobalCleanup();
 
-  nsClientAuthRememberService* GetClientAuthRememberService() {
-    return mClientAuthRemember;
-  }
-
   nsSSLIOLayerHelpers& IOLayerHelpers() { return mIOLayerHelpers; }
 
   // Main-thread only
   void ResetStoredData();
   void NotePrivateBrowsingStatus();
   void SetOCSPStaplingEnabled(bool staplingEnabled) {
     mOCSPStaplingEnabled = staplingEnabled;
   }
@@ -54,17 +48,16 @@ class SharedSSLState {
   }
 
  private:
   ~SharedSSLState();
 
   void Cleanup();
 
   nsCOMPtr<nsIObserver> mObserver;
-  RefPtr<nsClientAuthRememberService> mClientAuthRemember;
   nsSSLIOLayerHelpers mIOLayerHelpers;
 
   // True if any sockets have been created that use this shared data.
   // Requires synchronization between the socket and main threads for
   // reading/writing.
   Mutex mMutex;
   bool mSocketCreated;
   bool mOCSPStaplingEnabled;
--- a/security/manager/ssl/components.conf
+++ b/security/manager/ssl/components.conf
@@ -85,16 +85,23 @@ Classes = [
     },
     {
         'cid': '{fb0bbc5c-452e-4783-b32c-80124693d871}',
         'contract_ids': ['@mozilla.org/security/x509certdb;1'],
         'type': 'nsNSSCertificateDB',
         'legacy_constructor': 'mozilla::psm::NSSConstructor<nsNSSCertificateDB>',
     },
     {
+        'cid': '{1dbc6eb6-0972-4bdb-9dc4-acd0abf72369}',
+        'contract_ids': ['@mozilla.org/security/clientAuthRemember;1'],
+        'type': 'nsClientAuthRememberService',
+        'headers': ['nsClientAuthRemember.h'],
+        'init_method': 'Init',
+    },
+    {
         'cid': '{36a1d3b3-d886-4317-96ff-87b0005cfef7}',
         'contract_ids': ['@mozilla.org/security/hash;1'],
         'type': 'nsCryptoHash',
         'legacy_constructor': 'mozilla::psm::NSSConstructor<nsCryptoHash>',
     },
     {
         'cid': '{a496d0a2-dff7-4e23-bd65-1ca742fa178a}',
         'contract_ids': ['@mozilla.org/security/hmac;1'],
--- a/security/manager/ssl/moz.build
+++ b/security/manager/ssl/moz.build
@@ -14,16 +14,17 @@ TEST_DIRS += [ 'tests' ]
 
 XPIDL_SOURCES += [
     'nsIASN1Object.idl',
     'nsIASN1PrintableItem.idl',
     'nsIASN1Sequence.idl',
     'nsICertificateDialogs.idl',
     'nsICertOverrideService.idl',
     'nsIClientAuthDialogs.idl',
+    'nsIClientAuthRemember.idl',
     'nsIContentSignatureVerifier.idl',
     'nsICryptoHash.idl',
     'nsICryptoHMAC.idl',
     'nsIKeyModule.idl',
     'nsILocalCertService.idl',
     'nsINSSComponent.idl',
     'nsINSSErrorsService.idl',
     'nsINSSVersion.idl',
--- a/security/manager/ssl/nsClientAuthRemember.cpp
+++ b/security/manager/ssl/nsClientAuthRemember.cpp
@@ -20,18 +20,18 @@
 #include "pk11pub.h"
 #include "certdb.h"
 #include "sechash.h"
 #include "SharedSSLState.h"
 
 using namespace mozilla;
 using namespace mozilla::psm;
 
-NS_IMPL_ISUPPORTS(nsClientAuthRememberService, nsIObserver,
-                  nsISupportsWeakReference)
+NS_IMPL_ISUPPORTS(nsClientAuthRememberService, nsIClientAuthRemember,
+                  nsIObserver)
 
 nsClientAuthRememberService::nsClientAuthRememberService()
     : monitor("nsClientAuthRememberService.monitor") {}
 
 nsClientAuthRememberService::~nsClientAuthRememberService() {
   RemoveAllFromMemory();
 }
 
@@ -39,62 +39,73 @@ nsresult nsClientAuthRememberService::In
   if (!NS_IsMainThread()) {
     NS_ERROR("nsClientAuthRememberService::Init called off the main thread");
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   nsCOMPtr<nsIObserverService> observerService =
       mozilla::services::GetObserverService();
   if (observerService) {
-    observerService->AddObserver(this, "profile-before-change", true);
+    observerService->AddObserver(this, "profile-before-change", false);
+    observerService->AddObserver(this, "last-pb-context-exited", false);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsClientAuthRememberService::Observe(nsISupports* aSubject, const char* aTopic,
                                      const char16_t* aData) {
   // check the topic
   if (!nsCRT::strcmp(aTopic, "profile-before-change")) {
     // The profile is about to change,
     // or is going away because the application is shutting down.
 
     ReentrantMonitorAutoEnter lock(monitor);
     RemoveAllFromMemory();
+  } else if (!nsCRT::strcmp(aTopic, "last-pb-context-exited")) {
+    ReentrantMonitorAutoEnter lock(monitor);
+    ClearPrivateDecisions();
   }
 
   return NS_OK;
 }
 
-void nsClientAuthRememberService::ClearRememberedDecisions() {
+NS_IMETHODIMP
+nsClientAuthRememberService::ClearRememberedDecisions() {
   ReentrantMonitorAutoEnter lock(monitor);
   RemoveAllFromMemory();
+  return NS_OK;
 }
 
-void nsClientAuthRememberService::ClearAllRememberedDecisions() {
-  RefPtr<nsClientAuthRememberService> svc =
-      PublicSSLState()->GetClientAuthRememberService();
-  MOZ_ASSERT(svc);
-  if (svc) {
-    svc->ClearRememberedDecisions();
+nsresult nsClientAuthRememberService::ClearPrivateDecisions() {
+  ReentrantMonitorAutoEnter lock(monitor);
+  for (auto iter = mSettingsTable.Iter(); !iter.Done(); iter.Next()) {
+    nsCString entryKey = iter.Get()->mEntryKey;
+    const int32_t separator = entryKey.Find(":", false, 0, -1);
+    nsCString suffix;
+    if (separator >= 0) {
+      entryKey.Left(suffix, separator);
+    } else {
+      suffix = entryKey;
+    }
+
+    if (OriginAttributes::IsPrivateBrowsing(suffix)) {
+      iter.Remove();
+    }
   }
-
-  svc = PrivateSSLState()->GetClientAuthRememberService();
-  MOZ_ASSERT(svc);
-  if (svc) {
-    svc->ClearRememberedDecisions();
-  }
+  return NS_OK;
 }
 
 void nsClientAuthRememberService::RemoveAllFromMemory() {
   mSettingsTable.Clear();
 }
 
-nsresult nsClientAuthRememberService::RememberDecision(
+NS_IMETHODIMP
+nsClientAuthRememberService::RememberDecision(
     const nsACString& aHostName, const OriginAttributes& aOriginAttributes,
     CERTCertificate* aServerCert, CERTCertificate* aClientCert) {
   // aClientCert == nullptr means: remember that user does not want to use a
   // cert
   NS_ENSURE_ARG_POINTER(aServerCert);
   if (aHostName.IsEmpty()) {
     return NS_ERROR_INVALID_ARG;
   }
@@ -118,17 +129,18 @@ nsresult nsClientAuthRememberService::Re
       nsCString empty;
       AddEntryToList(aHostName, aOriginAttributes, fpStr, empty);
     }
   }
 
   return NS_OK;
 }
 
-nsresult nsClientAuthRememberService::HasRememberedDecision(
+NS_IMETHODIMP
+nsClientAuthRememberService::HasRememberedDecision(
     const nsACString& aHostName, const OriginAttributes& aOriginAttributes,
     CERTCertificate* aCert, nsACString& aCertDBKey, bool* aRetVal) {
   if (aHostName.IsEmpty()) return NS_ERROR_INVALID_ARG;
 
   NS_ENSURE_ARG_POINTER(aCert);
   NS_ENSURE_ARG_POINTER(aRetVal);
   *aRetVal = false;
 
--- a/security/manager/ssl/nsClientAuthRemember.h
+++ b/security/manager/ssl/nsClientAuthRemember.h
@@ -7,16 +7,17 @@
 #ifndef __NSCLIENTAUTHREMEMBER_H__
 #define __NSCLIENTAUTHREMEMBER_H__
 
 #include <utility>
 
 #include "mozilla/Attributes.h"
 #include "mozilla/HashFunctions.h"
 #include "mozilla/ReentrantMonitor.h"
+#include "nsIClientAuthRemember.h"
 #include "nsIObserver.h"
 #include "nsNSSCertificate.h"
 #include "nsString.h"
 #include "nsTHashtable.h"
 #include "nsWeakReference.h"
 
 namespace mozilla {
 class OriginAttributes;
@@ -82,49 +83,47 @@ class nsClientAuthRememberEntry final : 
 
   inline KeyTypePointer EntryKeyPtr() const { return mEntryKey.get(); }
 
   nsClientAuthRemember mSettings;
   nsCString mEntryKey;
 };
 
 class nsClientAuthRememberService final : public nsIObserver,
-                                          public nsSupportsWeakReference {
+                                          public nsIClientAuthRemember {
  public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOBSERVER
+  NS_DECL_NSICLIENTAUTHREMEMBER
 
   nsClientAuthRememberService();
 
   nsresult Init();
 
   static void GetEntryKey(const nsACString& aHostName,
                           const OriginAttributes& aOriginAttributes,
                           const nsACString& aFingerprint,
                           /*out*/ nsACString& aEntryKey);
 
-  nsresult RememberDecision(const nsACString& aHostName,
-                            const OriginAttributes& aOriginAttributes,
-                            CERTCertificate* aServerCert,
-                            CERTCertificate* aClientCert);
-
-  nsresult HasRememberedDecision(const nsACString& aHostName,
-                                 const OriginAttributes& aOriginAttributes,
-                                 CERTCertificate* aServerCert,
-                                 nsACString& aCertDBKey, bool* aRetVal);
-
-  void ClearRememberedDecisions();
-  static void ClearAllRememberedDecisions();
-
  protected:
   ~nsClientAuthRememberService();
 
   mozilla::ReentrantMonitor monitor;
   nsTHashtable<nsClientAuthRememberEntry> mSettingsTable;
 
   void RemoveAllFromMemory();
+
+  nsresult ClearPrivateDecisions();
+
   nsresult AddEntryToList(const nsACString& aHost,
                           const OriginAttributes& aOriginAttributes,
                           const nsACString& aServerFingerprint,
                           const nsACString& aDBKey);
 };
 
+#define NS_CLIENTAUTHREMEMBER_CID                    \
+  { /* 1dbc6eb6-0972-4bdb-9dc4-acd0abf72369 */       \
+    0x1dbc6eb6, 0x0972, 0x4bdb, {                    \
+      0x9d, 0xc4, 0xac, 0xd0, 0xab, 0xf7, 0x23, 0x69 \
+    }                                                \
+  }
+
 #endif
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/nsIClientAuthRemember.idl
@@ -0,0 +1,35 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsISupports.idl"
+
+%{C++
+#include "cert.h"
+#define NS_CLIENTAUTHREMEMBER_CONTRACTID "@mozilla.org/security/clientAuthRemember;1"
+%}
+
+[ptr] native CERTCertificatePtr(CERTCertificate);
+[ref] native const_OriginAttributesRef(const mozilla::OriginAttributes);
+
+[scriptable, uuid(1dbc6eb6-0972-4bdb-9dc4-acd0abf72369)]
+interface nsIClientAuthRemember : nsISupports
+{
+
+  [must_use, noscript]
+  void rememberDecision(in ACString aHostName,
+                        in const_OriginAttributesRef aOriginAttributes,
+                        in CERTCertificatePtr aServerCert,
+                        in CERTCertificatePtr aClientCert);
+
+  [must_use, noscript]
+  bool hasRememberedDecision(in ACString aHostName,
+                             in const_OriginAttributesRef aOriginAttributes,
+                             in CERTCertificatePtr aServerCert,
+                             out ACString aCertDBKey);
+
+  [must_use]
+  void clearRememberedDecisions();
+};
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -1964,16 +1964,19 @@ nsresult nsNSSComponent::InitializeNSS()
 
   rv = CommonInit();
 
   MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
   if (NS_FAILED(rv)) {
     return NS_ERROR_UNEXPECTED;
   }
 
+  nsCOMPtr<nsIClientAuthRemember> cars =
+      do_GetService(NS_CLIENTAUTHREMEMBER_CONTRACTID);
+
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS Initialization done\n"));
 
   {
     MutexAutoLock lock(mMutex);
 
     // ensure we have initial values for various root hashes
 #ifdef DEBUG
     mTestBuiltInRootHash.Truncate();
@@ -2194,17 +2197,22 @@ nsresult nsNSSComponent::GetNewPrompter(
 nsresult nsNSSComponent::LogoutAuthenticatedPK11() {
   nsCOMPtr<nsICertOverrideService> icos =
       do_GetService("@mozilla.org/security/certoverride;1");
   if (icos) {
     icos->ClearValidityOverride(
         NS_LITERAL_CSTRING("all:temporary-certificates"), 0);
   }
 
-  nsClientAuthRememberService::ClearAllRememberedDecisions();
+  nsCOMPtr<nsIClientAuthRemember> svc =
+      do_GetService(NS_CLIENTAUTHREMEMBER_CONTRACTID);
+
+  if (svc) {
+    svc->ClearRememberedDecisions();
+  }
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
     os->NotifyObservers(nullptr, "net:cancel-all-connections", nullptr);
   }
 
   return NS_OK;
 }
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -1950,18 +1950,21 @@ void ClientAuthDataRunnable::RunOnTarget
     if (!cert) {
       goto loser;
     }
   } else {  // Not Auto => ask
     // Get the SSL Certificate
 
     const nsACString& hostname = mSocketInfo->GetHostName();
 
-    RefPtr<nsClientAuthRememberService> cars =
-        mSocketInfo->SharedState().GetClientAuthRememberService();
+    nsCOMPtr<nsIClientAuthRemember> cars = nullptr;
+
+    if (mSocketInfo->GetProviderTlsFlags() == 0) {
+      cars = do_GetService(NS_CLIENTAUTHREMEMBER_CONTRACTID);
+    }
 
     bool hasRemembered = false;
     nsCString rememberedDBKey;
     if (cars) {
       bool found;
       nsresult rv = cars->HasRememberedDecision(
           hostname, mSocketInfo->GetOriginAttributes(), mServerCert,
           rememberedDBKey, &found);