bug 1231203 - ensure OCSP responses to requests from private contexts aren't cached on disk r=jcj draft
authorDavid Keeler <dkeeler@mozilla.com>
Thu, 08 Feb 2018 17:16:03 -0800
changeset 754535 d4ea45f52a297e9bf6ba08e824169d993e4935c0
parent 754399 38b3c1d03a594664c6b32c35533734283c258f43
push id98909
push userbmo:dkeeler@mozilla.com
push dateTue, 13 Feb 2018 18:37:10 +0000
reviewersjcj
bugs1231203
milestone60.0a1
bug 1231203 - ensure OCSP responses to requests from private contexts aren't cached on disk r=jcj MozReview-Commit-ID: 374f7hERLee
security/manager/ssl/nsNSSCallbacks.cpp
security/manager/ssl/tests/unit/head_psm.js
security/manager/ssl/tests/unit/test_ocsp_private_caching.js
security/manager/ssl/tests/unit/xpcshell.ini
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -114,23 +114,29 @@ nsHTTPDownloadEvent::Run()
   // high priority to accommodate real time OCSP transactions.
   nsCOMPtr<nsISupportsPriority> priorityChannel = do_QueryInterface(chan);
   if (priorityChannel)
     priorityChannel->AdjustPriority(nsISupportsPriority::PRIORITY_HIGHEST);
 
   chan->SetLoadFlags(nsIRequest::LOAD_ANONYMOUS |
                      nsIChannel::LOAD_BYPASS_SERVICE_WORKER);
 
-  // For OCSP requests, only the first party domain aspect of origin attributes
-  // is used. This means that OCSP requests are shared across different
-  // containers.
+  // For OCSP requests, only the first party domain and private browsing id
+  // aspects of origin attributes are used. This means that:
+  // a) if first party isolation is enabled, OCSP requests will be isolated
+  // according to the first party domain of the original https request
+  // b) OCSP requests are shared across different containers as long as first
+  // party isolation is not enabled and none of the containers are in private
+  // browsing mode.
   if (mRequestSession->mOriginAttributes != OriginAttributes()) {
     OriginAttributes attrs;
     attrs.mFirstPartyDomain =
       mRequestSession->mOriginAttributes.mFirstPartyDomain;
+    attrs.mPrivateBrowsingId =
+      mRequestSession->mOriginAttributes.mPrivateBrowsingId;
 
     nsCOMPtr<nsILoadInfo> loadInfo = chan->GetLoadInfo();
     if (loadInfo) {
       rv = loadInfo->SetOriginAttributes(attrs);
       NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
--- a/security/manager/ssl/tests/unit/head_psm.js
+++ b/security/manager/ssl/tests/unit/head_psm.js
@@ -602,19 +602,22 @@ function getFailingHttpServer(serverPort
 //   responses will be generated (assumes appropiate keys are present)
 // expectedCertNames is an array of nicks of the certs to be responsed
 // expectedBasePaths is an optional array that is used to indicate
 //   what is the expected base path of the OCSP request.
 // expectedMethods is an optional array of methods ("GET" or "POST") indicating
 //   by which HTTP method the server is expected to be queried.
 // expectedResponseTypes is an optional array of OCSP response types to use (see
 //   GenerateOCSPResponse.cpp).
+// responseHeaderPairs is an optional array of HTTP header (name, value) pairs
+//   to set in each response.
 function startOCSPResponder(serverPort, identity, nssDBLocation,
                             expectedCertNames, expectedBasePaths,
-                            expectedMethods, expectedResponseTypes) {
+                            expectedMethods, expectedResponseTypes,
+                            responseHeaderPairs = []) {
   let ocspResponseGenerationArgs = expectedCertNames.map(
     function(expectedNick) {
       let responseType = "good";
       if (expectedResponseTypes && expectedResponseTypes.length >= 1) {
         responseType = expectedResponseTypes.shift();
       }
       return [responseType, expectedNick, "unused", 0];
     }
@@ -633,16 +636,19 @@ function startOCSPResponder(serverPort, 
       Assert.ok(expectedCertNames.length >= 1,
                 "expectedCertNames should contain >= 1 entries");
       if (expectedMethods && expectedMethods.length >= 1) {
         Assert.equal(aRequest.method, expectedMethods.shift(),
                      "Actual and expected fetch method should match");
       }
       aResponse.setStatusLine(aRequest.httpVersion, 200, "OK");
       aResponse.setHeader("Content-Type", "application/ocsp-response");
+      for (let headerPair of responseHeaderPairs) {
+        aResponse.setHeader(headerPair[0], headerPair[1]);
+      }
       aResponse.write(ocspResponses.shift());
     });
   httpServer.identity.setPrimary("http", identity, serverPort);
   httpServer.start(serverPort);
   return {
     stop(callback) {
       // make sure we consumed each expected response
       Assert.equal(ocspResponses.length, 0,
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_ocsp_private_caching.js
@@ -0,0 +1,112 @@
+// -*- 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";
+
+// In which we connect to a host and encounter OCSP responses with the
+// Cache-Control header set, which Necko will normally cache. We need to ensure
+// that these responses aren't cached to disk when the original https request
+// was in a private context.
+
+do_get_profile(); // must be called before getting nsIX509CertDB
+const certdb = Cc["@mozilla.org/security/x509certdb;1"]
+                 .getService(Ci.nsIX509CertDB);
+
+const SERVER_PORT = 8888;
+
+function start_ocsp_responder(expectedCertNames, expectedPaths,
+                              expectedMethods) {
+  return startOCSPResponder(SERVER_PORT, "www.example.com",
+                            "test_ocsp_fetch_method", expectedCertNames,
+                            expectedPaths, expectedMethods);
+}
+
+function check_cert_err(cert_name, expected_error) {
+  let cert = constructCertFromFile("test_ocsp_fetch_method/" + cert_name + ".pem");
+  return checkCertErrorGeneric(certdb, cert, expected_error,
+                               certificateUsageSSLServer);
+}
+
+function add_flush_cache() {
+  add_test(() => {
+    // This appears to either fire multiple times or fire once for every
+    // observer that has ever been passed to flush. To prevent multiple calls to
+    // run_next_test, keep track of if this observer has already called it.
+    let observed = false;
+    let observer = { observe: () => {
+        if (!observed) {
+          observed = true;
+          run_next_test();
+        }
+      }
+    };
+    Services.cache2.QueryInterface(Ci.nsICacheTesting).flush(observer);
+  });
+}
+
+function add_ocsp_necko_cache_test(loadContext, shouldFindEntry) {
+  // Pre-testcase cleanup/setup.
+  add_test(() => {
+    Services.cache2.clear();
+    run_next_test();
+  });
+  add_flush_cache();
+
+  let responder;
+  add_test(() => {
+    clearOCSPCache();
+    clearSessionCache();
+    responder = startOCSPResponder(SERVER_PORT, "localhost", "ocsp_certs",
+                                   ["default-ee"], [], [], [],
+                                   [["Cache-Control", "max-age: 1000"]]);
+    run_next_test();
+  });
+
+  // Prepare a connection that will cause an OCSP request.
+  add_connection_test("ocsp-stapling-none.example.com", PRErrorCodeSuccess,
+                      null, null, null, loadContext.originAttributes);
+
+  add_flush_cache();
+
+  // Traverse the cache and ensure the response made it into the cache with the
+  // appropriate properties (private or not private).
+  add_test(() => {
+    let foundEntry = false;
+    let visitor = {
+      onCacheStorageInfo() {},
+      onCacheEntryInfo(aURI, aIdEnhance, aDataSize, aFetchCount,
+                       aLastModifiedTime, aExpirationTime, aPinned, aInfo) {
+        Assert.equal(aURI.spec, "http://localhost:8888/",
+                     "expected OCSP request URI should match");
+        foundEntry = true;
+      },
+      onCacheEntryVisitCompleted() {
+        Assert.equal(foundEntry, shouldFindEntry,
+                     "should only find a cached entry if we're expecting one");
+        run_next_test();
+      },
+      QueryInterface(iid) {
+        if (iid.equals(Ci.nsICacheStorageVisitor)) {
+          return this;
+        }
+        throw Cr.NS_ERROR_NO_INTERFACE;
+      },
+    };
+    Services.cache2.asyncVisitAllStorages(visitor, true);
+  });
+
+  // Clean up (stop the responder).
+  add_test(() => {
+    responder.stop(run_next_test);
+  });
+}
+
+function run_test() {
+  Services.prefs.setIntPref("security.OCSP.enabled", 1);
+  add_tls_server_setup("OCSPStaplingServer", "ocsp_certs");
+  add_ocsp_necko_cache_test(Services.loadContextInfo.private, false);
+  add_ocsp_necko_cache_test(Services.loadContextInfo.default, true);
+  run_next_test();
+}
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -117,16 +117,18 @@ run-sequentially = hardcoded ports
 [test_ocsp_caching.js]
 run-sequentially = hardcoded ports
 [test_ocsp_enabled_pref.js]
 run-sequentially = hardcoded ports
 [test_ocsp_fetch_method.js]
 run-sequentially = hardcoded ports
 [test_ocsp_must_staple.js]
 run-sequentially = hardcoded ports
+[test_ocsp_private_caching.js]
+run-sequentially = hardcoded ports
 [test_ocsp_no_hsts_upgrade.js]
 run-sequentially = hardcoded ports
 [test_ocsp_required.js]
 run-sequentially = hardcoded ports
 [test_ocsp_stapling.js]
 run-sequentially = hardcoded ports
 [test_ocsp_stapling_expired.js]
 run-sequentially = hardcoded ports