Bug 1564221 - Do not use FakeTransportSecurityInfo in test_forget_about_site_security_headers.js r=keeler

As part of making `nsITranportSecurityInfo` builtinclass, we can no longer use JS-implemented `nsITransportSecurityInfo` instances in test cases. This patch migrates `test_forget_about_site_security_headers.js to use `add_connection_test()` to get a valid `nsITransportSecurityInfo` instance for the unit tests. Differential Revision: https://phabricator.services.mozilla.com/D40347 --HG-- extra : moz-landing-system : lando
2019-08-20 21:37:38 +00:00 · 2019-08-20 21:37:38 +00:00 · 4fee6b8f31
--- a/security/manager/ssl/tests/unit/test_forget_about_site_security_headers.js
+++ b/security/manager/ssl/tests/unit/test_forget_about_site_security_headers.js
@ -9,10 +9,12 @@
 // Ensures that HSTS (HTTP Strict Transport Security) and HPKP (HTTP Public key
 // pinning) are cleared when using "Forget About This Site".

-var { ForgetAboutSite } = ChromeUtils.import(
+const { ForgetAboutSite } = ChromeUtils.import(
  "resource://gre/modules/ForgetAboutSite.jsm"
 );

+do_get_profile(); // must be done before instantiating nsIX509CertDB
+
 registerCleanupFunction(() => {
  Services.prefs.clearUserPref("security.cert_pinning.enforcement_level");
  Services.prefs.clearUserPref(
@ -27,162 +29,35 @@ const VALID_PIN = `pin-sha256="${PINNING_ROOT_KEY_HASH}";`;
 const BACKUP_PIN = `pin-sha256="${NON_ISSUED_KEY_HASH}";`;
 const GOOD_MAX_AGE = `max-age=${GOOD_MAX_AGE_SECONDS};`;

-do_get_profile(); // must be done before instantiating nsIX509CertDB
-
-Services.prefs.setIntPref("security.cert_pinning.enforcement_level", 2);
-Services.prefs.setBoolPref(
-  "security.cert_pinning.process_headers_from_non_builtin_roots",
-  true
+const sss = Cc["@mozilla.org/ssservice;1"].getService(
+  Ci.nsISiteSecurityService
 );
+const uri = Services.io.newURI("https://a.pinning.example.com");

-var certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(
-  Ci.nsIX509CertDB
-);
-addCertFromFile(certdb, "test_pinning_dynamic/pinningroot.pem", "CTu,CTu,CTu");
+function add_tests() {
+  let secInfo = null;

-var sss = Cc["@mozilla.org/ssservice;1"].getService(Ci.nsISiteSecurityService);
-var uri = Services.io.newURI("https://a.pinning2.example.com");
-
-// This test re-uses certificates from pinning tests because that's easier and
-// simpler than recreating new certificates, hence the slightly longer than
-// necessary domain name.
-var secInfo = new FakeTransportSecurityInfo(
-  constructCertFromFile(
-    "test_pinning_dynamic/a.pinning2.example.com-pinningroot.pem"
-  )
-);
-
-// Test the normal case of processing HSTS and HPKP headers for
-// a.pinning2.example.com, using "Forget About Site" on a.pinning2.example.com,
-// and then checking that the platform doesn't consider a.pinning2.example.com
-// to be HSTS or HPKP any longer.
-add_task(async function() {
-  sss.processHeader(
-    Ci.nsISiteSecurityService.HEADER_HSTS,
-    uri,
-    GOOD_MAX_AGE,
-    secInfo,
-    0,
-    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
-  );
-  sss.processHeader(
-    Ci.nsISiteSecurityService.HEADER_HPKP,
-    uri,
-    GOOD_MAX_AGE + VALID_PIN + BACKUP_PIN,
-    secInfo,
-    0,
-    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+  add_connection_test(
+    "a.pinning.example.com",
+    PRErrorCodeSuccess,
+    undefined,
+    aSecInfo => {
+      secInfo = aSecInfo;
+    }
  );

-  Assert.ok(
-    sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
-    "a.pinning2.example.com should be HSTS"
-  );
-  Assert.ok(
-    sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
-    "a.pinning2.example.com should be HPKP"
-  );
-
-  await ForgetAboutSite.removeDataFromDomain("a.pinning2.example.com");
-
-  Assert.ok(
-    !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
-    "a.pinning2.example.com should not be HSTS now"
-  );
-  Assert.ok(
-    !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
-    "a.pinning2.example.com should not be HPKP now"
-  );
-});
-
-// Test the case of processing HSTS and HPKP headers for a.pinning2.example.com,
-// using "Forget About Site" on example.com, and then checking that the platform
-// doesn't consider the subdomain to be HSTS or HPKP any longer. Also test that
-// unrelated sites don't also get removed.
-add_task(async function() {
-  sss.processHeader(
-    Ci.nsISiteSecurityService.HEADER_HSTS,
-    uri,
-    GOOD_MAX_AGE,
-    secInfo,
-    0,
-    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
-  );
-  sss.processHeader(
-    Ci.nsISiteSecurityService.HEADER_HPKP,
-    uri,
-    GOOD_MAX_AGE + VALID_PIN + BACKUP_PIN,
-    secInfo,
-    0,
-    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
-  );
-
-  Assert.ok(
-    sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
-    "a.pinning2.example.com should be HSTS (subdomain case)"
-  );
-  Assert.ok(
-    sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
-    "a.pinning2.example.com should be HPKP (subdomain case)"
-  );
-
-  // Add an unrelated site to HSTS.  Not HPKP because we have no valid keys for
-  // example.org.
-  let unrelatedURI = Services.io.newURI("https://example.org");
-  sss.processHeader(
-    Ci.nsISiteSecurityService.HEADER_HSTS,
-    unrelatedURI,
-    GOOD_MAX_AGE,
-    secInfo,
-    0,
-    Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
-  );
-  Assert.ok(
-    sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, unrelatedURI, 0),
-    "example.org should be HSTS"
-  );
-
-  await ForgetAboutSite.removeDataFromDomain("example.com");
-
-  Assert.ok(
-    !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
-    "a.pinning2.example.com should not be HSTS now (subdomain case)"
-  );
-  Assert.ok(
-    !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
-    "a.pinning2.example.com should not be HPKP now (subdomain case)"
-  );
-
-  Assert.ok(
-    sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, unrelatedURI, 0),
-    "example.org should still be HSTS"
-  );
-});
-
-// Test the case of processing HSTS and HPKP headers for a.pinning2.example.com
-// with various originAttributes, using "Forget About Site" on example.com, and
-// then checking that the platform doesn't consider the subdomain to be HSTS or
-// HPKP for any originAttributes any longer. Also test that unrelated sites
-// don't also get removed.
-add_task(async function() {
-  let originAttributesList = [
-    {},
-    { userContextId: 1 },
-    { firstPartyDomain: "foo.com" },
-    { userContextId: 1, firstPartyDomain: "foo.com" },
-  ];
-
-  let unrelatedURI = Services.io.newURI("https://example.org");
-
-  for (let originAttributes of originAttributesList) {
+  // Test the normal case of processing HSTS and HPKP headers for
+  // a.pinning.example.com, using "Forget About Site" on a.pinning2.example.com,
+  // and then checking that the platform doesn't consider a.pinning.example.com
+  // to be HSTS or HPKP any longer.
+  add_task(async function() {
    sss.processHeader(
      Ci.nsISiteSecurityService.HEADER_HSTS,
      uri,
      GOOD_MAX_AGE,
      secInfo,
      0,
-      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
-      originAttributes
+      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
    );
    sss.processHeader(
      Ci.nsISiteSecurityService.HEADER_HPKP,
@ -190,82 +65,216 @@ add_task(async function() {
      GOOD_MAX_AGE + VALID_PIN + BACKUP_PIN,
      secInfo,
      0,
-      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
-      originAttributes
+      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
    );

    Assert.ok(
-      sss.isSecureURI(
-        Ci.nsISiteSecurityService.HEADER_HSTS,
-        uri,
-        0,
-        originAttributes
-      ),
-      "a.pinning2.example.com should be HSTS (originAttributes case)"
+      sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
+      "a.pinning.example.com should be HSTS"
    );
    Assert.ok(
-      sss.isSecureURI(
-        Ci.nsISiteSecurityService.HEADER_HPKP,
-        uri,
-        0,
-        originAttributes
-      ),
-      "a.pinning2.example.com should be HPKP (originAttributes case)"
+      sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
+      "a.pinning.example.com should be HPKP"
    );

-    // Add an unrelated site to HSTS.  Not HPKP because we have no valid keys.
+    await ForgetAboutSite.removeDataFromDomain("a.pinning.example.com");
+
+    Assert.ok(
+      !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
+      "a.pinning.example.com should not be HSTS now"
+    );
+    Assert.ok(
+      !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
+      "a.pinning.example.com should not be HPKP now"
+    );
+  });
+
+  // Test the case of processing HSTS and HPKP headers for a.pinning.example.com,
+  // using "Forget About Site" on example.com, and then checking that the platform
+  // doesn't consider the subdomain to be HSTS or HPKP any longer. Also test that
+  // unrelated sites don't also get removed.
+  add_task(async function() {
+    sss.processHeader(
+      Ci.nsISiteSecurityService.HEADER_HSTS,
+      uri,
+      GOOD_MAX_AGE,
+      secInfo,
+      0,
+      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+    );
+    sss.processHeader(
+      Ci.nsISiteSecurityService.HEADER_HPKP,
+      uri,
+      GOOD_MAX_AGE + VALID_PIN + BACKUP_PIN,
+      secInfo,
+      0,
+      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
+    );
+
+    Assert.ok(
+      sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
+      "a.pinning.example.com should be HSTS (subdomain case)"
+    );
+    Assert.ok(
+      sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
+      "a.pinning.example.com should be HPKP (subdomain case)"
+    );
+
+    // Add an unrelated site to HSTS.  Not HPKP because we have no valid keys for
+    // example.org.
+    let unrelatedURI = Services.io.newURI("https://example.org");
    sss.processHeader(
      Ci.nsISiteSecurityService.HEADER_HSTS,
      unrelatedURI,
      GOOD_MAX_AGE,
      secInfo,
      0,
-      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
-      originAttributes
+      Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST
    );
    Assert.ok(
-      sss.isSecureURI(
-        Ci.nsISiteSecurityService.HEADER_HSTS,
-        unrelatedURI,
-        0,
-        originAttributes
-      ),
-      "example.org should be HSTS (originAttributes case)"
+      sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, unrelatedURI, 0),
+      "example.org should be HSTS"
    );
-  }

-  await ForgetAboutSite.removeDataFromDomain("example.com");
+    await ForgetAboutSite.removeDataFromDomain("example.com");

-  for (let originAttributes of originAttributesList) {
    Assert.ok(
-      !sss.isSecureURI(
+      !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0),
+      "a.pinning.example.com should not be HSTS now (subdomain case)"
+    );
+    Assert.ok(
+      !sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0),
+      "a.pinning.example.com should not be HPKP now (subdomain case)"
+    );
+
+    Assert.ok(
+      sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, unrelatedURI, 0),
+      "example.org should still be HSTS"
+    );
+  });
+
+  // Test the case of processing HSTS and HPKP headers for a.pinning.example.com
+  // with various originAttributes, using "Forget About Site" on example.com, and
+  // then checking that the platform doesn't consider the subdomain to be HSTS or
+  // HPKP for any originAttributes any longer. Also test that unrelated sites
+  // don't also get removed.
+  add_task(async function() {
+    let originAttributesList = [
+      {},
+      { userContextId: 1 },
+      { firstPartyDomain: "foo.com" },
+      { userContextId: 1, firstPartyDomain: "foo.com" },
+    ];
+
+    let unrelatedURI = Services.io.newURI("https://example.org");
+
+    for (let originAttributes of originAttributesList) {
+      sss.processHeader(
        Ci.nsISiteSecurityService.HEADER_HSTS,
        uri,
+        GOOD_MAX_AGE,
+        secInfo,
        0,
+        Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
        originAttributes
-      ),
-      "a.pinning2.example.com should not be HSTS now " +
-        "(originAttributes case)"
-    );
-    Assert.ok(
-      !sss.isSecureURI(
+      );
+      sss.processHeader(
        Ci.nsISiteSecurityService.HEADER_HPKP,
        uri,
+        GOOD_MAX_AGE + VALID_PIN + BACKUP_PIN,
+        secInfo,
        0,
+        Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
        originAttributes
-      ),
-      "a.pinning2.example.com should not be HPKP now " +
-        "(originAttributes case)"
-    );
+      );

-    Assert.ok(
-      sss.isSecureURI(
+      Assert.ok(
+        sss.isSecureURI(
+          Ci.nsISiteSecurityService.HEADER_HSTS,
+          uri,
+          0,
+          originAttributes
+        ),
+        "a.pinning.example.com should be HSTS (originAttributes case)"
+      );
+      Assert.ok(
+        sss.isSecureURI(
+          Ci.nsISiteSecurityService.HEADER_HPKP,
+          uri,
+          0,
+          originAttributes
+        ),
+        "a.pinning.example.com should be HPKP (originAttributes case)"
+      );
+
+      // Add an unrelated site to HSTS.  Not HPKP because we have no valid keys.
+      sss.processHeader(
        Ci.nsISiteSecurityService.HEADER_HSTS,
        unrelatedURI,
+        GOOD_MAX_AGE,
+        secInfo,
        0,
+        Ci.nsISiteSecurityService.SOURCE_ORGANIC_REQUEST,
        originAttributes
-      ),
-      "example.org should still be HSTS (originAttributes case)"
-    );
-  }
-});
+      );
+      Assert.ok(
+        sss.isSecureURI(
+          Ci.nsISiteSecurityService.HEADER_HSTS,
+          unrelatedURI,
+          0,
+          originAttributes
+        ),
+        "example.org should be HSTS (originAttributes case)"
+      );
+    }
+
+    await ForgetAboutSite.removeDataFromDomain("example.com");
+
+    for (let originAttributes of originAttributesList) {
+      Assert.ok(
+        !sss.isSecureURI(
+          Ci.nsISiteSecurityService.HEADER_HSTS,
+          uri,
+          0,
+          originAttributes
+        ),
+        "a.pinning.example.com should not be HSTS now " +
+          "(originAttributes case)"
+      );
+      Assert.ok(
+        !sss.isSecureURI(
+          Ci.nsISiteSecurityService.HEADER_HPKP,
+          uri,
+          0,
+          originAttributes
+        ),
+        "a.pinning.example.com should not be HPKP now " +
+          "(originAttributes case)"
+      );
+
+      Assert.ok(
+        sss.isSecureURI(
+          Ci.nsISiteSecurityService.HEADER_HSTS,
+          unrelatedURI,
+          0,
+          originAttributes
+        ),
+        "example.org should still be HSTS (originAttributes case)"
+      );
+    }
+  });
+}
+
+function run_test() {
+  Services.prefs.setIntPref("security.cert_pinning.enforcement_level", 2);
+  Services.prefs.setBoolPref(
+    "security.cert_pinning.process_headers_from_non_builtin_roots",
+    true
+  );
+
+  add_tls_server_setup("BadCertAndPinningServer", "bad_certs");
+
+  add_tests();
+
+  run_next_test();
+}
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertAndPinningServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertAndPinningServer.cpp
@ -83,6 +83,7 @@ const BadCertAndPinningHost sBadCertAndPinningHosts[] = {
     "ee-from-missing-intermediate"},
    {"imminently-distrusted.example.com", "ee-imminently-distrusted"},
    {"localhost", "unknownissuer"},
+    {"a.pinning.example.com", "default-ee"},
    {nullptr, nullptr}};

 int32_t DoSNISocketConfigBySubjectCN(PRFileDesc* aFd,
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@ -112,6 +112,7 @@ tags = blocklist psm
 run-sequentially = hardcoded ports
 [test_forget_about_site_security_headers.js]
 skip-if = toolkit == 'android'
+run-sequentially = hardcoded ports
 [test_hash_algorithms.js]
 [test_hash_algorithms_wrap.js]
 # bug 1124289 - run_test_in_child violates the sandbox on android