Bug 634697 - Add permanent storage to user's client certificate selection r=keeler,baku,fluent-reviewers,Gijs

Differential Revision: https://phabricator.services.mozilla.com/D58820

build/pgo/server-locations.txt

@@ -117,6 +117,7 @@ https://untrusted.example.com:443      privileged,cert=untrusted
 https://expired.example.com:443        privileged,cert=expired
 https://requestclientcert.example.com:443         privileged,clientauth=request
 https://requireclientcert.example.com:443         privileged,clientauth=require
+https://requireclientcert-2.example.com:443         privileged,clientauth=require
 https://mismatch.expired.example.com:443	privileged,cert=expired
 https://mismatch.untrusted.example.com:443	privileged,cert=untrusted
 https://untrusted-expired.example.com:443	privileged,cert=untrustedandexpired

security/manager/locales/en-US/security/certificates/certManager.ftl

@@ -184,6 +184,9 @@ delete-email-cert-impact = If you delete a person’s e-mail certificate, you wi
 cert-with-serial =
     .value = Certificate with serial number: { $serialNumber }
 
+# Used to indicate that the user chose not to send a client authentication certificate to a server that requested one in a TLS handshake.
+send-no-client-certificate = Send no client certificate
+
 ## Add Security Exception dialog
 add-exception-branded-warning = You are about to override how { -brand-short-name } identifies this site.
 add-exception-invalid-header = This site attempts to identify itself with invalid information.

security/manager/pki/resources/content/certManager.js

@@ -49,7 +49,7 @@ var clientAuthRememberService;
 var richlist;
 
 var rememberedDecisionsRichList = {
-  buildRichList() {
+  async buildRichList() {
     let rememberedDecisions = clientAuthRememberService.getDecisions();
 
     let oldItems = richlist.querySelectorAll("richlistitem");
@@ -59,10 +59,12 @@ var rememberedDecisionsRichList = {
 
     let frag = document.createDocumentFragment();
     for (let decision of rememberedDecisions) {
-      let richlistitem = this._richBoxAddItem(decision);
+      let richlistitem = await this._richBoxAddItem(decision);
       frag.appendChild(richlistitem);
     }
     richlist.appendChild(frag);
+
+    richlist.addEventListener("select", () => this.setButtonState());
   },
 
   _createItem(item) {
@@ -81,7 +83,7 @@ var rememberedDecisionsRichList = {
     return innerHbox;
   },
 
-  _richBoxAddItem(item) {
+  async _richBoxAddItem(item) {
     let richlistitem = document.createXULElement("richlistitem");
 
     richlistitem.setAttribute("entryKey", item.entryKey);
@@ -91,20 +93,29 @@ var rememberedDecisionsRichList = {
     hbox.setAttribute("flex", "1");
     hbox.setAttribute("equalsize", "always");
 
-    let tmpCert = certdb.findCertByDBKey(item.dbKey);
-
     hbox.appendChild(this._createItem(item.asciiHost));
+    if (item.dbKey == "") {
+      let noCertSpecified = await document.l10n.formatValue(
+        "send-no-client-certificate"
+      );
 
-    hbox.appendChild(this._createItem(tmpCert.commonName));
+      hbox.appendChild(this._createItem(noCertSpecified));
 
-    hbox.appendChild(this._createItem(tmpCert.serialNumber));
+      hbox.appendChild(this._createItem(""));
+    } else {
+      let tmpCert = certdb.findCertByDBKey(item.dbKey);
+
+      hbox.appendChild(this._createItem(tmpCert.commonName));
+
+      hbox.appendChild(this._createItem(tmpCert.serialNumber));
+    }
 
     richlistitem.appendChild(hbox);
 
     return richlistitem;
   },
 
-  deleteSelectedRichListItem() {
+  async deleteSelectedRichListItem() {
     let selectedItem = richlist.selectedItem;
     let index = richlist.selectedIndex;
     if (index < 0) {
@@ -115,7 +126,8 @@ var rememberedDecisionsRichList = {
       selectedItem.attributes.entryKey.value
     );
 
-    this.buildRichList();
+    await this.buildRichList();
+    this.setButtonState();
   },
 
   viewSelectedRichListItem() {
@@ -125,12 +137,27 @@ var rememberedDecisionsRichList = {
       return;
     }
 
-    let cert = certdb.findCertByDBKey(selectedItem.attributes.dbKey.value);
-    viewCertHelper(window, cert);
+    if (selectedItem.attributes.dbKey.value != "") {
+      let cert = certdb.findCertByDBKey(selectedItem.attributes.dbKey.value);
+      viewCertHelper(window, cert);
+    }
+  },
+
+  setButtonState() {
+    let rememberedDeleteButton = document.getElementById(
+      "remembered_deleteButton"
+    );
+    let rememberedViewButton = document.getElementById("remembered_viewButton");
+
+    rememberedDeleteButton.disabled = richlist.selectedIndex < 0;
+    rememberedViewButton.disabled =
+      richlist.selectedItem == null
+        ? true
+        : richlist.selectedItem.attributes.dbKey.value == "";
   },
 };
 
-function LoadCerts() {
+async function LoadCerts() {
   certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(
     Ci.nsIX509CertDB
   );
@@ -166,7 +193,9 @@ function LoadCerts() {
 
   richlist = document.getElementById("rememberedList");
 
-  rememberedDecisionsRichList.buildRichList();
+  await rememberedDecisionsRichList.buildRichList();
+
+  rememberedDecisionsRichList.setButtonState();
 
   enableBackupAllButton();
 }

security/manager/ssl/DataStorageList.h

@@ -14,6 +14,7 @@
 // to something faster.
 
 DATA_STORAGE(AlternateServices)
+DATA_STORAGE(ClientAuthRememberList)
 DATA_STORAGE(SecurityPreloadState)
 DATA_STORAGE(SiteSecurityServiceState)
 DATA_STORAGE(TRRBlacklist)

security/manager/ssl/nsClientAuthRemember.cpp

@@ -7,6 +7,7 @@
 #include "nsClientAuthRemember.h"
 
 #include "mozilla/BasePrincipal.h"
+#include "mozilla/DataStorage.h"
 #include "mozilla/RefPtr.h"
 #include "nsCRT.h"
 #include "nsNSSCertHelper.h"
@@ -22,13 +23,17 @@
 #include "sechash.h"
 #include "SharedSSLState.h"
 
+#include "nsJSUtils.h"
+
 using namespace mozilla;
 using namespace mozilla::psm;
 
-NS_IMPL_ISUPPORTS(nsClientAuthRememberService, nsIClientAuthRememberService,
-                  nsIObserver)
+NS_IMPL_ISUPPORTS(nsClientAuthRememberService, nsIClientAuthRememberService)
 NS_IMPL_ISUPPORTS(nsClientAuthRemember, nsIClientAuthRememberRecord)
 
+const nsCString nsClientAuthRemember::SentinelValue =
+    "no client certificate"_ns;
+
 NS_IMETHODIMP
 nsClientAuthRemember::GetAsciiHost(/*out*/ nsACString& aAsciiHost) {
   aAsciiHost = mAsciiHost;
@@ -53,24 +58,18 @@ nsClientAuthRemember::GetEntryKey(/*out*/ nsACString& aEntryKey) {
   return NS_OK;
 }
 
-nsClientAuthRememberService::nsClientAuthRememberService()
-    : monitor("nsClientAuthRememberService.monitor") {}
-
-nsClientAuthRememberService::~nsClientAuthRememberService() {
-  RemoveAllFromMemory();
-}
-
 nsresult nsClientAuthRememberService::Init() {
   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", false);
-    observerService->AddObserver(this, "last-pb-context-exited", false);
+  mClientAuthRememberList =
+      mozilla::DataStorage::Get(DataStorageClass::ClientAuthRememberList);
+  nsresult rv = mClientAuthRememberList->Init(nullptr);
+
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
   }
 
   return NS_OK;
@@ -78,10 +77,8 @@ nsresult nsClientAuthRememberService::Init() {
 
 NS_IMETHODIMP
 nsClientAuthRememberService::ForgetRememberedDecision(const nsACString& key) {
-  {
-    ReentrantMonitorAutoEnter lock(monitor);
-    mSettingsTable.RemoveEntry(PromiseFlatCString(key).get());
-  }
+  mClientAuthRememberList->Remove(PromiseFlatCString(key),
+                                  mozilla::DataStorage_Persistent);
 
   nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
   return NS_OK;
@@ -90,57 +87,56 @@ nsClientAuthRememberService::ForgetRememberedDecision(const nsACString& key) {
 NS_IMETHODIMP
 nsClientAuthRememberService::GetDecisions(
     nsTArray<RefPtr<nsIClientAuthRememberRecord>>& results) {
-  ReentrantMonitorAutoEnter lock(monitor);
-  for (auto iter = mSettingsTable.Iter(); !iter.Done(); iter.Next()) {
-    if (!nsClientAuthRememberService::IsPrivateBrowsingKey(
-            iter.Get()->mEntryKey)) {
-      results.AppendElement(iter.Get()->mSettings);
+  nsTArray<mozilla::psm::DataStorageItem> decisions;
+  mClientAuthRememberList->GetAll(&decisions);
+
+  for (const mozilla::psm::DataStorageItem& decision : decisions) {
+    if (decision.type() == DataStorageType::DataStorage_Persistent) {
+      RefPtr<nsIClientAuthRememberRecord> tmp =
+          new nsClientAuthRemember(decision.key(), decision.value());
+
+      results.AppendElement(tmp);
     }
   }
 
   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;
-}
-
 NS_IMETHODIMP
 nsClientAuthRememberService::ClearRememberedDecisions() {
-  ReentrantMonitorAutoEnter lock(monitor);
-  RemoveAllFromMemory();
+  mClientAuthRememberList->Clear();
+  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
   return NS_OK;
 }
 
-nsresult nsClientAuthRememberService::ClearPrivateDecisions() {
-  ReentrantMonitorAutoEnter lock(monitor);
-  for (auto iter = mSettingsTable.Iter(); !iter.Done(); iter.Next()) {
-    if (nsClientAuthRememberService::IsPrivateBrowsingKey(
-            iter.Get()->mEntryKey)) {
-      iter.Remove();
+NS_IMETHODIMP
+nsClientAuthRememberService::DeleteDecisionsByHost(
+    const nsACString& aHostName, JS::Handle<JS::Value> aOriginAttributes,
+    JSContext* aCx) {
+  OriginAttributes attrs;
+  if (!aOriginAttributes.isObject() || !attrs.Init(aCx, aOriginAttributes)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  DataStorageType storageType = GetDataStorageType(attrs);
+
+  nsTArray<mozilla::psm::DataStorageItem> decisions;
+  mClientAuthRememberList->GetAll(&decisions);
+
+  for (const mozilla::psm::DataStorageItem& decision : decisions) {
+    if (decision.type() == storageType) {
+      RefPtr<nsIClientAuthRememberRecord> tmp =
+          new nsClientAuthRemember(decision.key(), decision.value());
+      nsAutoCString asciiHost;
+      tmp->GetAsciiHost(asciiHost);
+      if (asciiHost.Equals(aHostName)) {
+        mClientAuthRememberList->Remove(decision.key(), decision.type());
+      }
     }
   }
+  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
   return NS_OK;
 }
 
-void nsClientAuthRememberService::RemoveAllFromMemory() {
-  mSettingsTable.Clear();
-}
-
 NS_IMETHODIMP
 nsClientAuthRememberService::RememberDecision(
     const nsACString& aHostName, const OriginAttributes& aOriginAttributes,
@@ -158,19 +154,16 @@ nsClientAuthRememberService::RememberDecision(
     return rv;
   }
 
-  {
-    ReentrantMonitorAutoEnter lock(monitor);
-    if (aClientCert) {
-      RefPtr<nsNSSCertificate> pipCert(new nsNSSCertificate(aClientCert));
-      nsAutoCString dbkey;
-      rv = pipCert->GetDbKey(dbkey);
-      if (NS_SUCCEEDED(rv)) {
-        AddEntryToList(aHostName, aOriginAttributes, fpStr, dbkey);
-      }
-    } else {
-      nsCString empty;
-      AddEntryToList(aHostName, aOriginAttributes, fpStr, empty);
+  if (aClientCert) {
+    RefPtr<nsNSSCertificate> pipCert(new nsNSSCertificate(aClientCert));
+    nsAutoCString dbkey;
+    rv = pipCert->GetDbKey(dbkey);
+    if (NS_SUCCEEDED(rv)) {
+      AddEntryToList(aHostName, aOriginAttributes, fpStr, dbkey);
     }
+  } else {
+    AddEntryToList(aHostName, aOriginAttributes, fpStr,
+                   nsClientAuthRemember::SentinelValue);
   }
 
   return NS_OK;
@@ -185,6 +178,7 @@ nsClientAuthRememberService::HasRememberedDecision(
   NS_ENSURE_ARG_POINTER(aCert);
   NS_ENSURE_ARG_POINTER(aRetVal);
   *aRetVal = false;
+  aCertDBKey.Truncate();
 
   nsresult rv;
   nsAutoCString fpStr;
@@ -193,13 +187,18 @@ nsClientAuthRememberService::HasRememberedDecision(
 
   nsAutoCString entryKey;
   GetEntryKey(aHostName, aOriginAttributes, fpStr, entryKey);
-  {
-    ReentrantMonitorAutoEnter lock(monitor);
-    nsClientAuthRememberEntry* entry = mSettingsTable.GetEntry(entryKey.get());
-    if (!entry) return NS_OK;
-    entry->mSettings->GetDbKey(aCertDBKey);
-    *aRetVal = true;
+  DataStorageType storageType = GetDataStorageType(aOriginAttributes);
+
+  nsCString listEntry = mClientAuthRememberList->Get(entryKey, storageType);
+  if (listEntry.IsEmpty()) {
+    return NS_OK;
   }
+
+  if (!listEntry.Equals(nsClientAuthRemember::SentinelValue)) {
+    aCertDBKey = listEntry;
+  }
+  *aRetVal = true;
+
   return NS_OK;
 }
 
@@ -208,20 +207,12 @@ nsresult nsClientAuthRememberService::AddEntryToList(
     const nsACString& aFingerprint, const nsACString& aDBKey) {
   nsAutoCString entryKey;
   GetEntryKey(aHostName, aOriginAttributes, aFingerprint, entryKey);
+  DataStorageType storageType = GetDataStorageType(aOriginAttributes);
 
-  {
-    ReentrantMonitorAutoEnter lock(monitor);
-    nsClientAuthRememberEntry* entry = mSettingsTable.PutEntry(entryKey.get());
-
-    if (!entry) {
-      NS_ERROR("can't insert a null entry!");
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    entry->mEntryKey = entryKey;
-
-    entry->mSettings =
-        new nsClientAuthRemember(aHostName, aFingerprint, aDBKey, entryKey);
+  nsCString tmpDbKey(aDBKey);
+  nsresult rv = mClientAuthRememberList->Put(entryKey, tmpDbKey, storageType);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
 
   return NS_OK;
@@ -231,11 +222,13 @@ void nsClientAuthRememberService::GetEntryKey(
     const nsACString& aHostName, const OriginAttributes& aOriginAttributes,
     const nsACString& aFingerprint, nsACString& aEntryKey) {
   nsAutoCString hostCert(aHostName);
+  hostCert.Append(',');
+  hostCert.Append(aFingerprint);
+  hostCert.Append(',');
+
   nsAutoCString suffix;
   aOriginAttributes.CreateSuffix(suffix);
   hostCert.Append(suffix);
-  hostCert.Append(':');
-  hostCert.Append(aFingerprint);
 
   aEntryKey.Assign(hostCert);
 }
@@ -251,3 +244,11 @@ bool nsClientAuthRememberService::IsPrivateBrowsingKey(
   }
   return OriginAttributes::IsPrivateBrowsing(suffix);
 }
+
+DataStorageType nsClientAuthRememberService::GetDataStorageType(
+    const OriginAttributes& aOriginAttributes) {
+  if (aOriginAttributes.mPrivateBrowsingId > 0) {
+    return DataStorage_Private;
+  }
+  return DataStorage_Persistent;
+}

security/manager/ssl/nsClientAuthRemember.h

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "mozilla/Attributes.h"
+#include "mozilla/DataStorage.h"
 #include "mozilla/HashFunctions.h"
 #include "mozilla/ReentrantMonitor.h"
 #include "nsIClientAuthRememberService.h"
@@ -30,74 +31,39 @@ class nsClientAuthRemember final : public nsIClientAuthRememberRecord {
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSICLIENTAUTHREMEMBERRECORD
 
-  nsClientAuthRemember(const nsACString& aAsciiHost,
-                       const nsACString& aFingerprint, const nsACString& aDBKey,
-                       const nsACString& aEntryKey) {
-    mAsciiHost = aAsciiHost;
-    mFingerprint = aFingerprint;
-    mDBKey = aDBKey;
+  nsClientAuthRemember(const nsCString& aEntryKey, const nsCString& aDBKey) {
     mEntryKey = aEntryKey;
+    if (!aDBKey.Equals(nsClientAuthRemember::SentinelValue)) {
+      mDBKey = aDBKey;
+    }
+
+    nsTArray<nsCString*> fields = {&mAsciiHost, &mFingerprint};
+
+    auto fieldsIter = fields.begin();
+    auto splitter = aEntryKey.Split(',');
+    auto splitterIter = splitter.begin();
+    for (; fieldsIter != fields.end() && splitterIter != splitter.end();
+         ++fieldsIter, ++splitterIter) {
+      (*fieldsIter)->Assign(*splitterIter);
+    }
   }
 
   nsCString mAsciiHost;
   nsCString mFingerprint;
   nsCString mDBKey;
   nsCString mEntryKey;
+  static const nsCString SentinelValue;
 
  protected:
   ~nsClientAuthRemember() = default;
 };
 
-// hash entry class
-class nsClientAuthRememberEntry final : public PLDHashEntryHdr {
- public:
-  // Hash methods
-  typedef const char* KeyType;
-  typedef const char* KeyTypePointer;
-
-  // do nothing with aHost - we require mHead to be set before we're live!
-  explicit nsClientAuthRememberEntry(KeyTypePointer aHostWithCertUTF8) {}
-
-  nsClientAuthRememberEntry(nsClientAuthRememberEntry&& aToMove)
-      : PLDHashEntryHdr(std::move(aToMove)),
-        mSettings(std::move(aToMove.mSettings)),
-        mEntryKey(std::move(aToMove.mEntryKey)) {}
-
-  ~nsClientAuthRememberEntry() = default;
-
-  KeyType GetKey() const { return EntryKeyPtr(); }
-
-  KeyTypePointer GetKeyPointer() const { return EntryKeyPtr(); }
-
-  bool KeyEquals(KeyTypePointer aKey) const {
-    return !strcmp(EntryKeyPtr(), aKey);
-  }
-
-  static KeyTypePointer KeyToPointer(KeyType aKey) { return aKey; }
-
-  static PLDHashNumber HashKey(KeyTypePointer aKey) {
-    return mozilla::HashString(aKey);
-  }
-
-  enum { ALLOW_MEMMOVE = false };
-
-  // get methods
-  inline const nsCString& GetEntryKey() const { return mEntryKey; }
-
-  inline KeyTypePointer EntryKeyPtr() const { return mEntryKey.get(); }
-
-  nsCOMPtr<nsIClientAuthRememberRecord> mSettings;
-  nsCString mEntryKey;
-};
-
-class nsClientAuthRememberService final : public nsIObserver,
-                                          public nsIClientAuthRememberService {
+class nsClientAuthRememberService final : public nsIClientAuthRememberService {
  public:
   NS_DECL_THREADSAFE_ISUPPORTS
-  NS_DECL_NSIOBSERVER
   NS_DECL_NSICLIENTAUTHREMEMBERSERVICE
 
-  nsClientAuthRememberService();
+  nsClientAuthRememberService() = default;
 
   nsresult Init();
 
@@ -109,14 +75,12 @@ class nsClientAuthRememberService final : public nsIObserver,
   static bool IsPrivateBrowsingKey(const nsCString& entryKey);
 
  protected:
-  ~nsClientAuthRememberService();
+  ~nsClientAuthRememberService() = default;
 
-  mozilla::ReentrantMonitor monitor;
-  nsTHashtable<nsClientAuthRememberEntry> mSettingsTable;
+  static mozilla::DataStorageType GetDataStorageType(
+      const OriginAttributes& aOriginAttributes);
 
-  void RemoveAllFromMemory();
-
-  nsresult ClearPrivateDecisions();
+  RefPtr<mozilla::DataStorage> mClientAuthRememberList;
 
   nsresult AddEntryToList(const nsACString& aHost,
                           const OriginAttributes& aOriginAttributes,

security/manager/ssl/nsIClientAuthRememberService.idl

@@ -54,4 +54,9 @@ interface nsIClientAuthRememberService : nsISupports
 
   [must_use]
   void clearRememberedDecisions();
+
+  [implicit_jscontext]
+  void deleteDecisionsByHost(in ACString aHostName,
+                             in jsval aOriginAttributes);
+
 };

security/manager/ssl/nsNSSComponent.cpp

@@ -2488,14 +2488,7 @@ nsresult nsNSSComponent::LogoutAuthenticatedPK11() {
     icos->ClearValidityOverride("all:temporary-certificates"_ns, 0);
   }
 
-  nsCOMPtr<nsIClientAuthRememberService> svc =
-      do_GetService(NS_CLIENTAUTHREMEMBERSERVICE_CONTRACTID);
-
-  if (svc) {
-    nsresult rv = svc->ClearRememberedDecisions();
-
-    Unused << NS_WARN_IF(NS_FAILED(rv));
-  }
+  nsNSSComponent::ClearSSLExternalAndInternalSessionCacheNative();
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {

security/manager/ssl/nsNSSIOLayer.cpp

@@ -2342,7 +2342,12 @@ void ClientAuthDataRunnable::RunOnTargetThread() {
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return;
     }
-    if (found && !rememberedDBKey.IsEmpty()) {
+    if (found) {
+      // An empty dbKey indicates that the user chose not to use a certificate
+      // and chose to remember this decision
+      if (rememberedDBKey.IsEmpty()) {
+        return;
+      }
       nsCOMPtr<nsIX509CertDB> certdb = do_GetService(NS_X509CERTDB_CONTRACTID);
       if (NS_WARN_IF(!certdb)) {
         return;

security/manager/ssl/tests/mochitest/browser/browser_clientAuthRememberService.js

@@ -11,6 +11,7 @@ var cert;
 var cert2;
 var cert3;
 
+var sdr = Cc["@mozilla.org/security/sdr;1"].getService(Ci.nsISecretDecoderRing);
 var certDB = Cc["@mozilla.org/security/x509certdb;1"].getService(
   Ci.nsIX509CertDB
 );
@@ -30,6 +31,51 @@ function findCertByCommonName(commonName) {
   return null;
 }
 
+async function testHelper(connectURL, expectedURL) {
+  let win = await BrowserTestUtils.openNewBrowserWindow();
+
+  await SpecialPowers.pushPrefEnv({
+    set: [["security.default_personal_cert", "Ask Every Time"]],
+  });
+
+  await BrowserTestUtils.loadURI(win.gBrowser.selectedBrowser, connectURL);
+
+  await BrowserTestUtils.browserLoaded(
+    win.gBrowser.selectedBrowser,
+    false,
+    expectedURL,
+    true
+  );
+  let loadedURL = win.gBrowser.selectedBrowser.documentURI.spec;
+  Assert.ok(
+    loadedURL.startsWith(expectedURL),
+    `Expected and actual URLs should match (got '${loadedURL}', expected '${expectedURL}')`
+  );
+
+  await win.close();
+
+  // This clears the TLS session cache so we don't use a previously-established
+  // ticket to connect and bypass selecting a client auth certificate in
+  // subsequent tests.
+  sdr.logout();
+}
+
+async function openRequireClientCert() {
+  gClientAuthDialogs.chooseCertificateCalled = false;
+  await testHelper(
+    "https://requireclientcert.example.com:443",
+    "https://requireclientcert.example.com/"
+  );
+}
+
+async function openRequireClientCert2() {
+  gClientAuthDialogs.chooseCertificateCalled = false;
+  await testHelper(
+    "https://requireclientcert-2.example.com:443",
+    "https://requireclientcert-2.example.com/"
+  );
+}
+
 // Mock implementation of nsIClientAuthRememberService
 const gClientAuthRememberService = {
   forgetRememberedDecision(key) {
@@ -64,6 +110,35 @@ const gClientAuthRememberService = {
   QueryInterface: ChromeUtils.generateQI(["nsIClientAuthRememberService"]),
 };
 
+const gClientAuthDialogs = {
+  _chooseCertificateCalled: false,
+
+  get chooseCertificateCalled() {
+    return this._chooseCertificateCalled;
+  },
+
+  set chooseCertificateCalled(value) {
+    this._chooseCertificateCalled = value;
+  },
+
+  chooseCertificate(
+    hostname,
+    port,
+    organization,
+    issuerOrg,
+    certList,
+    selectedIndex,
+    rememberClientAuthCertificate
+  ) {
+    rememberClientAuthCertificate.value = true;
+    this.chooseCertificateCalled = true;
+    selectedIndex.value = 0;
+    return true;
+  },
+
+  QueryInterface: ChromeUtils.generateQI([Ci.nsIClientAuthDialogs]),
+};
+
 add_task(async function testRememberedDecisionsUI() {
   cert = findCertByCommonName("Mochitest client");
   cert2 = await readCertificate("pgo-ca-all-usages.pem", ",,");
@@ -77,10 +152,6 @@ add_task(async function testRememberedDecisionsUI() {
     gClientAuthRememberService
   );
 
-  registerCleanupFunction(() => {
-    MockRegistrar.unregister(clientAuthRememberServiceCID);
-  });
-
   let win = await openCertManager();
 
   let listItems = win.document
@@ -136,4 +207,51 @@ add_task(async function testRememberedDecisionsUI() {
 
   win.document.getElementById("certmanager").acceptDialog();
   await BrowserTestUtils.windowClosed(win);
+
+  MockRegistrar.unregister(clientAuthRememberServiceCID);
+});
+
+add_task(async function testDeletingRememberedDecisions() {
+  let clientAuthDialogsCID = MockRegistrar.register(
+    "@mozilla.org/nsClientAuthDialogs;1",
+    gClientAuthDialogs
+  );
+  let cars = Cc["@mozilla.org/security/clientAuthRememberService;1"].getService(
+    Ci.nsIClientAuthRememberService
+  );
+
+  await openRequireClientCert();
+  Assert.ok(
+    gClientAuthDialogs.chooseCertificateCalled,
+    "chooseCertificate should have been called if visiting 'requireclientcert.example.com' for the first time"
+  );
+
+  await openRequireClientCert();
+  Assert.ok(
+    !gClientAuthDialogs.chooseCertificateCalled,
+    "chooseCertificate should not have been called if visiting 'requireclientcert.example.com' for the second time"
+  );
+
+  await openRequireClientCert2();
+  Assert.ok(
+    gClientAuthDialogs.chooseCertificateCalled,
+    "chooseCertificate should have been called if visiting'requireclientcert-2.example.com' for the first time"
+  );
+
+  let originAttributes = { privateBrowsingId: 0 };
+  cars.deleteDecisionsByHost("requireclientcert.example.com", originAttributes);
+
+  await openRequireClientCert();
+  Assert.ok(
+    gClientAuthDialogs.chooseCertificateCalled,
+    "chooseCertificate should have been called after removing all remembered decisions for 'requireclientcert.example.com'"
+  );
+
+  await openRequireClientCert2();
+  Assert.ok(
+    !gClientAuthDialogs.chooseCertificateCalled,
+    "chooseCertificate should not have been called if visiting 'requireclientcert-2.example.com' for the second time"
+  );
+
+  MockRegistrar.unregister(clientAuthDialogsCID);
 });

security/manager/ssl/tests/mochitest/browser/browser_clientAuth_connection.js

@@ -21,6 +21,9 @@ const DialogState = {
 };
 
 var sdr = Cc["@mozilla.org/security/sdr;1"].getService(Ci.nsISecretDecoderRing);
+let cars = Cc["@mozilla.org/security/clientAuthRememberService;1"].getService(
+  Ci.nsIClientAuthRememberService
+);
 
 var gExpectedClientCertificateChoices;
 
@@ -160,10 +163,17 @@ add_task(async function setup() {
  *        If the connection is expected to load successfully, the URL that
  *        should load. If the connection is expected to fail and result in an
  *        error page, |undefined|.
+ * @param {Boolean} expectCallingChooseCertificate
+ *        Determines whether we expect chooseCertificate to be called.
  * @param {Object} options
  *        Optional options object to pass on to the window that gets opened.
  */
-async function testHelper(prefValue, expectedURL, options = undefined) {
+async function testHelper(
+  prefValue,
+  expectedURL,
+  expectCallingChooseCertificate,
+  options = undefined
+) {
   gClientAuthDialogs.chooseCertificateCalled = false;
   await SpecialPowers.pushPrefEnv({
     set: [["security.default_personal_cert", prefValue]],
@@ -189,7 +199,7 @@ async function testHelper(prefValue, expectedURL, options = undefined) {
   );
   Assert.equal(
     gClientAuthDialogs.chooseCertificateCalled,
-    prefValue == "Ask Every Time",
+    expectCallingChooseCertificate,
     "chooseCertificate should have been called if we were expecting it to be called"
   );
 
@@ -207,11 +217,12 @@ add_task(async function testCertChosenAutomatically() {
   gClientAuthDialogs.state = DialogState.ASSERT_NOT_CALLED;
   await testHelper(
     "Select Automatically",
-    "https://requireclientcert.example.com/"
+    "https://requireclientcert.example.com/",
+    false
   );
   // This clears all saved client auth certificate state so we don't influence
   // subsequent tests.
-  sdr.logoutAndTeardown();
+  cars.clearRememberedDecisions();
 });
 
 // Test that if the user doesn't choose a certificate, the connection fails and
@@ -220,16 +231,38 @@ add_task(async function testCertNotChosenByUser() {
   gClientAuthDialogs.state = DialogState.RETURN_CERT_NOT_SELECTED;
   await testHelper(
     "Ask Every Time",
-    "about:neterror?e=nssFailure2&u=https%3A//requireclientcert.example.com/"
+    "about:neterror?e=nssFailure2&u=https%3A//requireclientcert.example.com/",
+    true
   );
-  sdr.logoutAndTeardown();
+  cars.clearRememberedDecisions();
 });
 
 // Test that if the user chooses a certificate the connection suceeeds.
 add_task(async function testCertChosenByUser() {
   gClientAuthDialogs.state = DialogState.RETURN_CERT_SELECTED;
-  await testHelper("Ask Every Time", "https://requireclientcert.example.com/");
-  sdr.logoutAndTeardown();
+  await testHelper(
+    "Ask Every Time",
+    "https://requireclientcert.example.com/",
+    true
+  );
+  cars.clearRememberedDecisions();
+});
+
+// Test that the cancel decision is remembered correctly
+add_task(async function testEmptyCertChosenByUser() {
+  gClientAuthDialogs.state = DialogState.RETURN_CERT_NOT_SELECTED;
+  gClientAuthDialogs.rememberClientAuthCertificate = true;
+  await testHelper(
+    "Ask Every Time",
+    "about:neterror?e=nssFailure2&u=https%3A//requireclientcert.example.com/",
+    true
+  );
+  await testHelper(
+    "Ask Every Time",
+    "about:neterror?e=nssFailure2&u=https%3A//requireclientcert.example.com/",
+    false
+  );
+  cars.clearRememberedDecisions();
 });
 
 // Test that if the user chooses a certificate in a private browsing window,
@@ -243,18 +276,32 @@ add_task(async function testCertChosenByUser() {
 add_task(async function testClearPrivateBrowsingState() {
   gClientAuthDialogs.rememberClientAuthCertificate = true;
   gClientAuthDialogs.state = DialogState.RETURN_CERT_SELECTED;
-  await testHelper("Ask Every Time", "https://requireclientcert.example.com/", {
-    private: true,
-  });
-  await testHelper("Ask Every Time", "https://requireclientcert.example.com/", {
-    private: true,
-  });
-  await testHelper("Ask Every Time", "https://requireclientcert.example.com/");
-  // NB: we don't `sdr.logoutAndTeardown()` in between the two calls to
+  await testHelper(
+    "Ask Every Time",
+    "https://requireclientcert.example.com/",
+    true,
+    {
+      private: true,
+    }
+  );
+  await testHelper(
+    "Ask Every Time",
+    "https://requireclientcert.example.com/",
+    true,
+    {
+      private: true,
+    }
+  );
+  await testHelper(
+    "Ask Every Time",
+    "https://requireclientcert.example.com/",
+    true
+  );
+  // NB: we don't `cars.clearRememberedDecisions()` in between the two calls to
   // `testHelper` because that would clear all client auth certificate state and
   // obscure what we're testing (that Firefox properly clears the relevant state
   // when the last private window closes).
-  sdr.logoutAndTeardown();
+  cars.clearRememberedDecisions();
 });
 
 // Test that 3rd party certificates are taken into account when filtering client
@@ -282,8 +329,12 @@ add_task(async function testCertFilteringWithIntermediate() {
   nssComponent.addEnterpriseIntermediate(intermediateBytes);
   gExpectedClientCertificateChoices = 4;
   gClientAuthDialogs.state = DialogState.RETURN_CERT_SELECTED;
-  await testHelper("Ask Every Time", "https://requireclientcert.example.com/");
-  sdr.logoutAndTeardown();
+  await testHelper(
+    "Ask Every Time",
+    "https://requireclientcert.example.com/",
+    true
+  );
+  cars.clearRememberedDecisions();
   // This will reset the added intermediate.
   await SpecialPowers.pushPrefEnv({
     set: [["security.enterprise_roots.enabled", true]],

security/manager/ssl/tests/unit/head_psm.js

@@ -30,6 +30,7 @@ const isDebugBuild = Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2)
 // The test EV roots are only enabled in debug builds as a security measure.
 const gEVExpected = isDebugBuild;
 
+const CLIENT_AUTH_FILE_NAME = "ClientAuthRememberList.txt";
 const SSS_STATE_FILE_NAME = "SiteSecurityServiceState.txt";
 const PRELOAD_STATE_FILE_NAME = "SecurityPreloadState.txt";
 

security/manager/ssl/tests/unit/test_sss_eviction.js

@@ -10,7 +10,7 @@ var gSSService = null;
 var gProfileDir = null;
 
 function do_state_written(aSubject, aTopic, aData) {
-  if (aData == PRELOAD_STATE_FILE_NAME) {
+  if (aData == PRELOAD_STATE_FILE_NAME || aData == CLIENT_AUTH_FILE_NAME) {
     return;
   }
 
@@ -45,7 +45,7 @@ function do_state_written(aSubject, aTopic, aData) {
 }
 
 function do_state_read(aSubject, aTopic, aData) {
-  if (aData == PRELOAD_STATE_FILE_NAME) {
+  if (aData == PRELOAD_STATE_FILE_NAME || aData == CLIENT_AUTH_FILE_NAME) {
     return;
   }
 

security/manager/ssl/tests/unit/test_sss_readstate.js

@@ -9,7 +9,7 @@
 var gSSService = null;
 
 function checkStateRead(aSubject, aTopic, aData) {
-  if (aData == PRELOAD_STATE_FILE_NAME) {
+  if (aData == PRELOAD_STATE_FILE_NAME || aData == CLIENT_AUTH_FILE_NAME) {
     return;
   }
 

security/manager/ssl/tests/unit/test_sss_readstate_garbage.js

@@ -9,7 +9,7 @@
 var gSSService = null;
 
 function checkStateRead(aSubject, aTopic, aData) {
-  if (aData == PRELOAD_STATE_FILE_NAME) {
+  if (aData == PRELOAD_STATE_FILE_NAME || aData == CLIENT_AUTH_FILE_NAME) {
     return;
   }
 

security/manager/ssl/tests/unit/test_sss_readstate_huge.js

@@ -10,7 +10,7 @@
 var gSSService = null;
 
 function checkStateRead(aSubject, aTopic, aData) {
-  if (aData == PRELOAD_STATE_FILE_NAME) {
+  if (aData == PRELOAD_STATE_FILE_NAME || aData == CLIENT_AUTH_FILE_NAME) {
     return;
   }
 

security/manager/ssl/tests/unit/test_sss_savestate.js

@@ -17,7 +17,7 @@ const NON_ISSUED_KEY_HASH = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
 // separated by newlines ('\n')
 
 function checkStateWritten(aSubject, aTopic, aData) {
-  if (aData == PRELOAD_STATE_FILE_NAME) {
+  if (aData == PRELOAD_STATE_FILE_NAME || aData == CLIENT_AUTH_FILE_NAME) {
     return;
   }
 

toolkit/components/cleardata/ClearDataService.jsm

@@ -928,6 +928,11 @@ const SecuritySettingsCleaner = {
           );
         }
       }
+      let cars = Cc[
+        "@mozilla.org/security/clientAuthRememberService;1"
+      ].getService(Ci.nsIClientAuthRememberService);
+
+      cars.deleteDecisionsByHost(aHost, aOriginAttributes);
 
       aResolve();
     });
@@ -941,6 +946,10 @@ const SecuritySettingsCleaner = {
         Ci.nsISiteSecurityService
       );
       sss.clearAll();
+      let cars = Cc[
+        "@mozilla.org/security/clientAuthRememberService;1"
+      ].getService(Ci.nsIClientAuthRememberService);
+      cars.clearRememberedDecisions();
       aResolve();
     });
   },