bug 1564481 - reset HSTS/HPKP state to factory settings rather than storing knockout entries for preloaded sites r=jcj r=KevinJacobs a=RyanVM
authorDana Keeler <dkeeler@mozilla.com>
Thu, 11 Jul 2019 13:48:28 -0700
changeset 544681 fea8c86cd0d3c087575f393fa2b793eca3976126
parent 544680 80b47b3c406e9e8bffb31d45c9e11bfb1547dfc3
child 544682 441998ce7a5e32dca5f00505c575d4b75706256b
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj, KevinJacobs, RyanVM
bugs1564481, 775370
milestone69.0
bug 1564481 - reset HSTS/HPKP state to factory settings rather than storing knockout entries for preloaded sites r=jcj r=KevinJacobs a=RyanVM As originally implemented, nsISiteSecurityService.removeState allowed direct access to remove HSTS state. It also provided the implementation for when the browser encountered an HSTS header with "max-age=0". In bug 775370, it was updated to store an entry that would override preloaded information when processing such headers. However, this meant that the semantics of the direct access API had changed. Preloaded information could be overridden if a user invoked the "forget about this site" feature. This change fixes the public API (and renames it to "resetState") so it actually behaves as its consumers expect. Reviewers: jcj!, KevinJacobs! Tags: #secure-revision Bug #: 1564481 Differential Revision: https://phabricator.services.mozilla.com/D38108
browser/base/content/test/general/browser_blockHPKP.js
browser/base/content/test/general/browser_star_hsts.js
devtools/shared/webconsole/test/test_network_security-hpkp.html
devtools/shared/webconsole/test/test_network_security-hsts.html
security/manager/ssl/nsISiteSecurityService.idl
security/manager/ssl/nsSiteSecurityService.cpp
security/manager/ssl/nsSiteSecurityService.h
security/manager/ssl/tests/mochitest/browser/browser_bug627234_perwindowpb.js
security/manager/ssl/tests/unit/test_ocsp_must_staple.js
security/manager/ssl/tests/unit/test_pinning_header_parsing.js
security/manager/ssl/tests/unit/test_sss_originAttributes.js
security/manager/ssl/tests/unit/test_sss_resetState.js
security/manager/ssl/tests/unit/test_sts_fqdn.js
security/manager/ssl/tests/unit/xpcshell.ini
testing/specialpowers/content/SpecialPowersAPIParent.jsm
toolkit/components/cleardata/ClearDataService.jsm
--- a/browser/base/content/test/general/browser_blockHPKP.js
+++ b/browser/base/content/test/general/browser_blockHPKP.js
@@ -38,17 +38,17 @@ function test() {
   // Enable enforcing strict pinning and processing headers from
   // non-builtin roots.
   Services.prefs.setIntPref(kpkpEnforcementPref, 2);
   Services.prefs.setBoolPref(khpkpPinninEnablePref, true);
   registerCleanupFunction(function() {
     Services.prefs.clearUserPref(kpkpEnforcementPref);
     Services.prefs.clearUserPref(khpkpPinninEnablePref);
     let uri = Services.io.newURI("https://" + kPinningDomain);
-    gSSService.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
+    gSSService.resetState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
   });
   whenNewTabLoaded(window, loadPinningPage);
 }
 
 // Start by making a successful connection to a domain that will pin a site
 function loadPinningPage() {
   BrowserTestUtils.loadURI(
     gBrowser.selectedBrowser,
--- a/browser/base/content/test/general/browser_star_hsts.js
+++ b/browser/base/content/test/general/browser_star_hsts.js
@@ -9,17 +9,17 @@ var unsecureURL =
   "http://example.com/browser/browser/base/content/test/general/browser_star_hsts.sjs";
 
 add_task(async function test_star_redirect() {
   registerCleanupFunction(async () => {
     // Ensure to remove example.com from the HSTS list.
     let sss = Cc["@mozilla.org/ssservice;1"].getService(
       Ci.nsISiteSecurityService
     );
-    sss.removeState(
+    sss.resetState(
       Ci.nsISiteSecurityService.HEADER_HSTS,
       NetUtil.newURI("http://example.com/"),
       0
     );
     await PlacesUtils.bookmarks.eraseEverything();
     gBrowser.removeCurrentTab();
   });
 
--- a/devtools/shared/webconsole/test/test_network_security-hpkp.html
+++ b/devtools/shared/webconsole/test/test_network_security-hpkp.html
@@ -49,17 +49,17 @@ function startTest() {
     // Reset pinning state.
     let gSSService = Cc["@mozilla.org/ssservice;1"]
                        .getService(Ci.nsISiteSecurityService);
 
     let gIOService = Cc["@mozilla.org/network/io-service;1"]
                        .getService(Ci.nsIIOService);
     for (let {url} of TEST_CASES) {
       let uri = gIOService.newURI(url);
-      gSSService.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
+      gSSService.resetState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
     }
   });
 
   info("Test detection of Public Key Pinning.");
   removeEventListener("load", startTest);
   attachConsoleToTab(["NetworkActivity"], onAttach);
 }
 
--- a/devtools/shared/webconsole/test/test_network_security-hsts.html
+++ b/devtools/shared/webconsole/test/test_network_security-hsts.html
@@ -43,17 +43,17 @@ function startTest()
     // Reset HSTS state.
     let gSSService = Cc["@mozilla.org/ssservice;1"]
                        .getService(Ci.nsISiteSecurityService);
 
     let gIOService = Cc["@mozilla.org/network/io-service;1"]
                        .getService(Ci.nsIIOService);
 
     let uri = gIOService.newURI(TEST_CASES[0].url);
-    gSSService.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
+    gSSService.resetState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
   });
 
   info("Test detection of HTTP Strict Transport Security.");
   removeEventListener("load", startTest);
   attachConsoleToTab(["NetworkActivity"], onAttach);
 }
 
 function onAttach(aState, aResponse)
--- a/security/manager/ssl/nsISiteSecurityService.idl
+++ b/security/manager/ssl/nsISiteSecurityService.idl
@@ -155,35 +155,38 @@ interface nsISiteSecurityService : nsISu
                        in uint32_t aFlags,
                        in uint32_t aSource,
                        [optional] in jsval aOriginAttributes,
                        [optional] out unsigned long long aMaxAge,
                        [optional] out boolean aIncludeSubdomains,
                        [optional] out uint32_t aFailureResult);
 
     /**
-     * Given a header type, removes state relating to that header of a host,
+     * Given a header type, resets state relating to that header of a host,
      * including the includeSubdomains state that would affect subdomains.
      * This essentially removes the state for the domain tree rooted at this
-     * host.
+     * host. If any preloaded information is present for that host, that
+     * information will then be used instead of any other previously existing
+     * state.
+     *
      * @param aType   the type of security state in question
      * @param aURI    the URI of the target host
      * @param aFlags  options for this request as defined in nsISocketProvider:
      *                  NO_PERMANENT_STORAGE
      * @param aOriginAttributes the origin attributes that isolate this origin,
      *                          (note that this implementation does not isolate
      *                          by userContextId because of the risk of man-in-
      *                          the-middle attacks before trust-on-second-use
      *                          happens).
      */
     [implicit_jscontext, optional_argc, must_use]
-    void removeState(in uint32_t aType,
-                     in nsIURI aURI,
-                     in uint32_t aFlags,
-                     [optional] in jsval aOriginAttributes);
+    void resetState(in uint32_t aType,
+                    in nsIURI aURI,
+                    in uint32_t aFlags,
+                    [optional] in jsval aOriginAttributes);
 
     /**
      * Checks whether or not the URI's hostname has a given security state set.
      * For example, for HSTS:
      * The URI is an HSTS URI if either the host has the HSTS state set, or one
      * of its super-domains has the HSTS "includeSubdomains" flag set.
      * NOTE: this function makes decisions based only on the
      * host contained in the URI, and disregards other portions of the URI
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -552,21 +552,22 @@ static int64_t ExpireTimeFromMaxAge(uint
 }
 
 nsresult nsSiteSecurityService::SetHSTSState(
     uint32_t aType, const char* aHost, int64_t maxage, bool includeSubdomains,
     uint32_t flags, SecurityPropertyState aHSTSState,
     SecurityPropertySource aSource, const OriginAttributes& aOriginAttributes) {
   nsAutoCString hostname(aHost);
   bool isPreload = (aSource == SourcePreload);
-  // If max-age is zero, that's an indication to immediately remove the
-  // security state, so here's a shortcut.
-  if (!maxage) {
-    return RemoveStateInternal(aType, hostname, flags, isPreload,
-                               aOriginAttributes);
+  // If max-age is zero, the host is no longer considered HSTS. If the host was
+  // preloaded, we store an entry indicating that this host is not HSTS, causing
+  // the preloaded information to be ignored.
+  if (maxage == 0) {
+    return MarkHostAsNotHSTS(aType, hostname, flags, isPreload,
+                             aOriginAttributes);
   }
 
   MOZ_ASSERT(
       (aHSTSState == SecurityPropertySet ||
        aHSTSState == SecurityPropertyNegative),
       "HSTS State must be SecurityPropertySet or SecurityPropertyNegative");
   if (isPreload && aOriginAttributes != OriginAttributes()) {
     return NS_ERROR_INVALID_ARG;
@@ -603,47 +604,35 @@ nsresult nsSiteSecurityService::SetHSTSS
     }
     rv = mSiteStateStorage->Put(storageKey, stateString, storageType);
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
-nsresult nsSiteSecurityService::RemoveStateInternal(
-    uint32_t aType, nsIURI* aURI, uint32_t aFlags,
-    const OriginAttributes& aOriginAttributes) {
-  nsAutoCString hostname;
-  GetHost(aURI, hostname);
-  return RemoveStateInternal(aType, hostname, aFlags, false, aOriginAttributes);
-}
-
-nsresult nsSiteSecurityService::RemoveStateInternal(
+// Helper function to mark a host as not HSTS. In the general case, we can just
+// remove the HSTS state. However, for preloaded entries, we have to store an
+// entry that indicates this host is not HSTS to prevent the implementation
+// using the preloaded information.
+nsresult nsSiteSecurityService::MarkHostAsNotHSTS(
     uint32_t aType, const nsAutoCString& aHost, uint32_t aFlags,
     bool aIsPreload, const OriginAttributes& aOriginAttributes) {
-  // Child processes are not allowed direct access to this.
-  if (!XRE_IsParentProcess()) {
-    MOZ_CRASH(
-        "Child process: no direct access to "
-        "nsISiteSecurityService::RemoveStateInternal");
+  // This only applies to HSTS.
+  if (aType != nsISiteSecurityService::HEADER_HSTS) {
+    return NS_ERROR_INVALID_ARG;
   }
-
-  // Only HSTS is supported at the moment.
-  NS_ENSURE_TRUE(aType == nsISiteSecurityService::HEADER_HSTS ||
-                     aType == nsISiteSecurityService::HEADER_HPKP,
-                 NS_ERROR_NOT_IMPLEMENTED);
   if (aIsPreload && aOriginAttributes != OriginAttributes()) {
     return NS_ERROR_INVALID_ARG;
   }
 
   bool isPrivate = aFlags & nsISocketProvider::NO_PERMANENT_STORAGE;
   mozilla::DataStorageType storageType = isPrivate
                                              ? mozilla::DataStorage_Private
                                              : mozilla::DataStorage_Persistent;
-  // If this host is in the preload list, we have to store a knockout entry.
   nsAutoCString storageKey;
   SetStorageKey(aHost, aType, aOriginAttributes, storageKey);
 
   nsCString value =
       mPreloadStateStorage->Get(storageKey, mozilla::DataStorage_Persistent);
   RefPtr<SiteHSTSState> dynamicState =
       new SiteHSTSState(aHost, aOriginAttributes, value);
   if (GetPreloadStatus(aHost) ||
@@ -670,29 +659,69 @@ nsresult nsSiteSecurityService::RemoveSt
       mSiteStateStorage->Remove(storageKey, storageType);
     }
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsSiteSecurityService::RemoveState(uint32_t aType, nsIURI* aURI,
-                                   uint32_t aFlags,
-                                   JS::HandleValue aOriginAttributes,
-                                   JSContext* aCx, uint8_t aArgc) {
+nsSiteSecurityService::ResetState(uint32_t aType, nsIURI* aURI, uint32_t aFlags,
+                                  JS::HandleValue aOriginAttributes,
+                                  JSContext* aCx, uint8_t aArgc) {
+  if (!XRE_IsParentProcess()) {
+    MOZ_CRASH(
+        "Child process: no direct access to "
+        "nsISiteSecurityService::ResetState");
+  }
+  if (!aURI) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   OriginAttributes originAttributes;
   if (aArgc > 0) {
     // OriginAttributes were passed in.
     if (!aOriginAttributes.isObject() ||
         !originAttributes.Init(aCx, aOriginAttributes)) {
       return NS_ERROR_INVALID_ARG;
     }
   }
-  return RemoveStateInternal(aType, aURI, aFlags, originAttributes);
+
+  return ResetStateInternal(aType, aURI, aFlags, originAttributes);
+}
+
+// Helper function to reset stored state of the given type for the host
+// identified by the given URI. If there is preloaded information for the host,
+// that information will be used for future queries. C.f. MarkHostAsNotHSTS,
+// which will store a knockout entry for preloaded HSTS hosts that have sent a
+// header with max-age=0 (meaning preloaded information will then not be used
+// for that host).
+nsresult nsSiteSecurityService::ResetStateInternal(
+    uint32_t aType, nsIURI* aURI, uint32_t aFlags,
+    const OriginAttributes& aOriginAttributes) {
+  if (!aURI) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  if (aType != nsISiteSecurityService::HEADER_HSTS &&
+      aType != nsISiteSecurityService::HEADER_HPKP) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  nsAutoCString hostname;
+  nsresult rv = GetHost(aURI, hostname);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  nsAutoCString storageKey;
+  SetStorageKey(hostname, aType, aOriginAttributes, storageKey);
+  bool isPrivate = aFlags & nsISocketProvider::NO_PERMANENT_STORAGE;
+  mozilla::DataStorageType storageType = isPrivate
+                                             ? mozilla::DataStorage_Private
+                                             : mozilla::DataStorage_Persistent;
+  mSiteStateStorage->Remove(storageKey, storageType);
+  return NS_OK;
 }
 
 static bool HostIsIPAddress(const nsCString& hostname) {
   PRNetAddr hostAddr;
   PRErrorCode prv = PR_StringToNetAddr(hostname.get(), &hostAddr);
   return (prv == PR_SUCCESS);
 }
 
@@ -1057,19 +1086,22 @@ nsresult nsSiteSecurityService::ProcessP
 
   if (!isBuiltIn && !mProcessPKPHeadersFromNonBuiltInRoots) {
     if (aFailureResult) {
       *aFailureResult = nsISiteSecurityService::ERROR_ROOT_NOT_BUILT_IN;
     }
     return NS_ERROR_FAILURE;
   }
 
-  // if maxAge == 0 we must delete all state, for now no hole-punching
+  // If maxAge == 0, we remove dynamic HPKP state for this host. Due to
+  // architectural constraints, if this host was preloaded, any future lookups
+  // will use the preloaded state (i.e. we can't store a "this host is not HPKP"
+  // entry like we can for HSTS).
   if (maxAge == 0) {
-    return RemoveStateInternal(aType, aSourceURI, aFlags, aOriginAttributes);
+    return ResetStateInternal(aType, aSourceURI, aFlags, aOriginAttributes);
   }
 
   // clamp maxAge to the maximum set by pref
   if (maxAge > mMaxMaxAge) {
     maxAge = mMaxMaxAge;
   }
 
   bool chainMatchesPinset;
--- a/security/manager/ssl/nsSiteSecurityService.h
+++ b/security/manager/ssl/nsSiteSecurityService.h
@@ -186,21 +186,21 @@ class nsSiteSecurityService : public nsI
   nsresult ProcessPKPHeader(nsIURI* aSourceURI, const nsCString& aHeader,
                             nsITransportSecurityInfo* aSecInfo, uint32_t flags,
                             const OriginAttributes& aOriginAttributes,
                             uint64_t* aMaxAge, bool* aIncludeSubdomains,
                             uint32_t* aFailureResult);
   nsresult SetHPKPState(const char* aHost, SiteHPKPState& entry, uint32_t flags,
                         bool aIsPreload,
                         const OriginAttributes& aOriginAttributes);
-  nsresult RemoveStateInternal(uint32_t aType, nsIURI* aURI, uint32_t aFlags,
-                               const OriginAttributes& aOriginAttributes);
-  nsresult RemoveStateInternal(uint32_t aType, const nsAutoCString& aHost,
-                               uint32_t aFlags, bool aIsPreload,
-                               const OriginAttributes& aOriginAttributes);
+  nsresult MarkHostAsNotHSTS(uint32_t aType, const nsAutoCString& aHost,
+                             uint32_t aFlags, bool aIsPreload,
+                             const OriginAttributes& aOriginAttributes);
+  nsresult ResetStateInternal(uint32_t aType, nsIURI* aURI, uint32_t aFlags,
+                              const OriginAttributes& aOriginAttributes);
   bool HostHasHSTSEntry(const nsAutoCString& aHost,
                         bool aRequireIncludeSubdomains, uint32_t aFlags,
                         const OriginAttributes& aOriginAttributes,
                         bool* aResult, bool* aCached,
                         SecurityPropertySource* aSource);
   bool GetPreloadStatus(
       const nsACString& aHost,
       /*optional out*/ bool* aIncludeSubdomains = nullptr) const;
--- a/security/manager/ssl/tests/mochitest/browser/browser_bug627234_perwindowpb.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_bug627234_perwindowpb.js
@@ -88,17 +88,17 @@ function test() {
   }
 
   // this function is called after calling finish() on the test.
   registerCleanupFunction(function() {
     windowsToClose.forEach(function(aWin) {
       aWin.close();
     });
     uri = Services.io.newURI("http://localhost");
-    gSSService.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
+    gSSService.resetState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
   });
 
   // test first when on private mode
   testOnWindow({ private: true }, function(aWin) {
     doTest(true, aWin, function() {
       // test when not on private mode
       testOnWindow({}, function(aWin) {
         doTest(false, aWin, function() {
--- a/security/manager/ssl/tests/unit/test_ocsp_must_staple.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_must_staple.js
@@ -64,17 +64,17 @@ function add_tests() {
       Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
     );
     ok(
       ssservice.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
       "ocsp-stapling-must-staple-ee-with-must-staple-int.example.com should have HPKP set"
     );
 
     // Clear accumulated state.
-    ssservice.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
+    ssservice.resetState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
     Services.prefs.clearUserPref(
       "security.cert_pinning.process_headers_from_non_builtin_roots"
     );
     Services.prefs.clearUserPref("security.cert_pinning.enforcement_level");
     run_next_test();
   });
 
   // Next, a case where it's present in the intermediate, not the ee
--- a/security/manager/ssl/tests/unit/test_pinning_header_parsing.js
+++ b/security/manager/ssl/tests/unit/test_pinning_header_parsing.js
@@ -50,17 +50,17 @@ function checkPassValidPin(pinValue, set
     certFromFile("a.pinning2.example.com-pinningroot")
   );
   let uri = Services.io.newURI("https://a.pinning2.example.com");
   let maxAge = {};
 
   // setup preconditions for the test, if setting ensure there is no previous
   // state, if removing ensure there is a valid pin in place.
   if (settingPin) {
-    gSSService.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
+    gSSService.resetState(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0);
   } else {
     // add a known valid pin!
     let validPinValue = "max-age=5000;" + VALID_PIN1 + BACKUP_PIN1;
     gSSService.processHeader(
       Ci.nsISiteSecurityService.HEADER_HPKP,
       uri,
       validPinValue,
       secInfo,
--- a/security/manager/ssl/tests/unit/test_sss_originAttributes.js
+++ b/security/manager/ssl/tests/unit/test_sss_originAttributes.js
@@ -78,25 +78,25 @@ function doTest(originAttributes1, origi
       sss.isSecureURI(type, uri, 0, originAttributes2),
       shouldShare,
       "URI should be secure given different origin attributes if and " +
         "only if shouldShare is true"
     );
 
     if (!shouldShare) {
       // Remove originAttributes2 from the storage.
-      sss.removeState(type, uri, 0, originAttributes2);
+      sss.resetState(type, uri, 0, originAttributes2);
       ok(
         sss.isSecureURI(type, uri, 0, originAttributes1),
         "URI should still be secure given original origin attributes"
       );
     }
 
     // Remove originAttributes1 from the storage.
-    sss.removeState(type, uri, 0, originAttributes1);
+    sss.resetState(type, uri, 0, originAttributes1);
     ok(
       !sss.isSecureURI(type, uri, 0, originAttributes1),
       "URI should be not be secure after removeState"
     );
   }
   // Set HPKP for originAttributes1.
   sss.setKeyPins(
     host,
@@ -156,17 +156,17 @@ function testInvalidOriginAttributes(ori
           uri,
           header,
           secInfo,
           0,
           Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
           originAttributes
         ),
       () => sss.isSecureURI(type, uri, 0, originAttributes),
-      () => sss.removeState(type, uri, 0, originAttributes),
+      () => sss.resetState(type, uri, 0, originAttributes),
     ];
 
     for (let callback of callbacks) {
       throws(
         callback,
         /NS_ERROR_ILLEGAL_VALUE/,
         "Should get an error with invalid origin attributes"
       );
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_sss_resetState.js
@@ -0,0 +1,126 @@
+// -*- indent-tabs-mode: nil; js-indent-level: 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/.
+
+"use strict";
+
+// Tests that resetting HSTS/HPKP state in the way the "forget about this site"
+// functionality does works as expected for preloaded and non-preloaded sites.
+
+do_get_profile();
+
+var gCertDB = Cc["@mozilla.org/security/x509certdb;1"].getService(
+  Ci.nsIX509CertDB
+);
+const ROOT_CERT = addCertFromFile(gCertDB, "bad_certs/test-ca.pem", "CTu,,");
+
+var gSSService = Cc["@mozilla.org/ssservice;1"].getService(
+  Ci.nsISiteSecurityService
+);
+
+function run_test() {
+  Services.prefs.setBoolPref(
+    "security.cert_pinning.process_headers_from_non_builtin_roots",
+    true
+  );
+  test_removeState(Ci.nsISiteSecurityService.HEADER_HSTS, 0);
+  test_removeState(
+    Ci.nsISiteSecurityService.HEADER_HSTS,
+    Ci.nsISocketProvider.NO_PERMANENT_STORAGE
+  );
+  test_removeState(Ci.nsISiteSecurityService.HEADER_HPKP, 0);
+  test_removeState(
+    Ci.nsISiteSecurityService.HEADER_HPKP,
+    Ci.nsISocketProvider.NO_PERMANENT_STORAGE
+  );
+}
+
+function test_removeState(type, flags) {
+  info(`running test_removeState(type=${type}, flags=${flags})`);
+  const NON_ISSUED_KEY_HASH = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
+  const PINNING_ROOT_KEY_HASH = "VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=";
+  const PINNING_HEADERS = `pin-sha256="${NON_ISSUED_KEY_HASH}"; pin-sha256="${PINNING_ROOT_KEY_HASH}"`;
+  let headerAddendum =
+    type == Ci.nsISiteSecurityService.HEADER_HPKP ? PINNING_HEADERS : "";
+  let secInfo = new FakeTransportSecurityInfo(
+    constructCertFromFile("bad_certs/default-ee.pem")
+  );
+  // Simulate visiting a non-preloaded site by processing an HSTS or HPKP header
+  // (depending on which type we were given), check that the HSTS/HPKP bit gets
+  // set, simulate "forget about this site" (call removeState), and then check
+  // that the HSTS/HPKP bit isn't set.
+  let notPreloadedURI = Services.io.newURI("https://not-preloaded.example.com");
+  ok(!gSSService.isSecureURI(type, notPreloadedURI, flags));
+  gSSService.processHeader(
+    type,
+    notPreloadedURI,
+    "max-age=1000;" + headerAddendum,
+    secInfo,
+    flags,
+    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+  );
+  ok(gSSService.isSecureURI(type, notPreloadedURI, flags));
+  gSSService.resetState(type, notPreloadedURI, flags);
+  ok(!gSSService.isSecureURI(type, notPreloadedURI, flags));
+
+  // Simulate visiting a non-preloaded site that unsets HSTS/HPKP by processing
+  // an HSTS/HPKP header with "max-age=0", check that the HSTS/HPKP bit isn't
+  // set, simulate "forget about this site" (call removeState), and then check
+  // that the HSTS/HPKP bit isn't set.
+  gSSService.processHeader(
+    type,
+    notPreloadedURI,
+    "max-age=0;" + headerAddendum,
+    secInfo,
+    flags,
+    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+  );
+  ok(!gSSService.isSecureURI(type, notPreloadedURI, flags));
+  gSSService.resetState(type, notPreloadedURI, flags);
+  ok(!gSSService.isSecureURI(type, notPreloadedURI, flags));
+
+  // Simulate visiting a preloaded site by processing an HSTS/HPKP header, check
+  // that the HSTS/HPKP bit is still set, simulate "forget about this site"
+  // (call removeState), and then check that the HSTS/HPKP bit is still set.
+  let preloadedHost =
+    type == Ci.nsISiteSecurityService.HEADER_HPKP
+      ? "include-subdomains.pinning.example.com"
+      : "includesubdomains.preloaded.test";
+  let preloadedURI = Services.io.newURI(`https://${preloadedHost}`);
+  ok(gSSService.isSecureURI(type, preloadedURI, flags));
+  gSSService.processHeader(
+    type,
+    preloadedURI,
+    "max-age=1000;" + headerAddendum,
+    secInfo,
+    flags,
+    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+  );
+  ok(gSSService.isSecureURI(type, preloadedURI, flags));
+  gSSService.resetState(type, preloadedURI, flags);
+  ok(gSSService.isSecureURI(type, preloadedURI, flags));
+
+  // Simulate visiting a preloaded site that unsets HSTS/HPKP by processing an
+  // HSTS/HPKP header with "max-age=0", check that the HSTS/HPKP bit is what we
+  // expect (see below), simulate "forget about this site" (call removeState),
+  // and then check that the HSTS/HPKP bit is set.
+  gSSService.processHeader(
+    type,
+    preloadedURI,
+    "max-age=0;" + headerAddendum,
+    secInfo,
+    flags,
+    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+  );
+  // Due to architectural constraints, encountering a "max-age=0" header for a
+  // preloaded HPKP site does not mark that site as not HPKP (whereas with HSTS,
+  // it does).
+  if (type == Ci.nsISiteSecurityService.HEADER_HPKP) {
+    ok(gSSService.isSecureURI(type, preloadedURI, flags));
+  } else {
+    ok(!gSSService.isSecureURI(type, preloadedURI, flags));
+  }
+  gSSService.resetState(type, preloadedURI, flags);
+  ok(gSSService.isSecureURI(type, preloadedURI, flags));
+}
--- a/security/manager/ssl/tests/unit/test_sts_fqdn.js
+++ b/security/manager/ssl/tests/unit/test_sts_fqdn.js
@@ -24,17 +24,17 @@ function run_test() {
     secInfo,
     0,
     Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
   );
   ok(SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0));
   ok(SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri1, 0));
   ok(SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri2, 0));
 
-  SSService.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
+  SSService.resetState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
   ok(!SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0));
   ok(!SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri1, 0));
   ok(!SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri2, 0));
 
   // Somehow creating this malformed URI succeeds - we need to handle it
   // gracefully.
   uri = Services.io.newURI("https://../foo");
   equal(uri.host, "..");
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -193,16 +193,17 @@ run-sequentially = hardcoded ports
 [test_sss_readstate.js]
 [test_sss_readstate_child.js]
 support-files = sss_readstate_child_worker.js
 # bug 1124289 - run_test_in_child violates the sandbox on android
 skip-if = toolkit == 'android'
 [test_sss_readstate_empty.js]
 [test_sss_readstate_garbage.js]
 [test_sss_readstate_huge.js]
+[test_sss_resetState.js]
 [test_sss_savestate.js]
 [test_sss_sanitizeOnShutdown.js]
 firefox-appdir = browser
 # Sanitization works differently on Android - this doesn't apply.
 # browser/modules/Sanitizer.jsm used by the test isn't available in Thunderbird.
 skip-if = toolkit == 'android' || appname == 'thunderbird'
 [test_startcom_wosign.js]
 [test_sts_fqdn.js]
--- a/testing/specialpowers/content/SpecialPowersAPIParent.jsm
+++ b/testing/specialpowers/content/SpecialPowersAPIParent.jsm
@@ -700,17 +700,17 @@ class SpecialPowersAPIParent extends JSW
 
       case "SPCleanUpSTSData": {
         let origin = aMessage.data.origin;
         let flags = aMessage.data.flags;
         let uri = Services.io.newURI(origin);
         let sss = Cc["@mozilla.org/ssservice;1"].getService(
           Ci.nsISiteSecurityService
         );
-        sss.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, flags);
+        sss.resetState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, flags);
         return undefined;
       }
 
       case "SPRequestDumpCoverageCounters": {
         return PerTestCoverageUtils.afterTest();
       }
 
       case "SPRequestResetCoverageCounters": {
--- a/toolkit/components/cleardata/ClearDataService.jsm
+++ b/toolkit/components/cleardata/ClearDataService.jsm
@@ -915,24 +915,24 @@ const SecuritySettingsCleaner = {
     return new Promise(aResolve => {
       let sss = Cc["@mozilla.org/ssservice;1"].getService(
         Ci.nsISiteSecurityService
       );
       for (let type of [
         Ci.nsISiteSecurityService.HEADER_HSTS,
         Ci.nsISiteSecurityService.HEADER_HPKP,
       ]) {
-        // Also remove HSTS/HPKP/OMS information for subdomains by enumerating
+        // Also remove HSTS/HPKP information for subdomains by enumerating
         // the information in the site security service.
         for (let entry of sss.enumerate(type)) {
           let hostname = entry.hostname;
           if (Services.eTLD.hasRootDomain(hostname, aHost)) {
-            // This uri is used as a key to remove the state.
+            // This uri is used as a key to reset the state.
             let uri = Services.io.newURI("https://" + hostname);
-            sss.removeState(type, uri, 0, entry.originAttributes);
+            sss.resetState(type, uri, 0, entry.originAttributes);
           }
         }
       }
 
       aResolve();
     });
   },