bug 1368868 - give up on ocsp stapling strictness because we can't have nice things r=jcj
authorDavid Keeler <dkeeler@mozilla.com>
Wed, 08 Nov 2017 15:50:26 -0800
changeset 391622 3c298625e9678cdab2479638c0b1340b34a48746
parent 391621 24cdbe1d8af0f36f072111f71261acfd437726f4
child 391623 d1cc8eaa72fc18ce5de7448589affd3d537d5777
push id32896
push userccoroiu@mozilla.com
push dateTue, 14 Nov 2017 10:22:27 +0000
treeherdermozilla-central@f9717fd3e226 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj
bugs1368868
milestone59.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 1368868 - give up on ocsp stapling strictness because we can't have nice things r=jcj MozReview-Commit-ID: nbX0c251oC
CLOBBER
security/certverifier/NSSCertDBTrustDomain.cpp
security/manager/ssl/tests/unit/ocsp_certs/must-staple-ee.pem
security/manager/ssl/tests/unit/ocsp_certs/must-staple-ee.pem.certspec
security/manager/ssl/tests/unit/test_ocsp_must_staple.js
security/manager/ssl/tests/unit/test_ocsp_stapling.js
security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js
security/manager/ssl/tests/unit/tlsserver/cmd/OCSPStaplingServer.cpp
--- a/CLOBBER
+++ b/CLOBBER
@@ -17,9 +17,9 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Merge day clobber
+bug 1368868 - if PSM xpcshell tests have been run locally, these changes will break the tests without a clobber (see bug 1416332)
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -407,16 +407,27 @@ NSSCertDBTrustDomain::CheckRevocation(En
       return Success;
     }
     if (stapledOCSPResponseResult == Result::ERROR_OCSP_OLD_RESPONSE ||
         expired) {
       // stapled OCSP response present but expired
       mOCSPStaplingStatus = CertVerifier::OCSP_STAPLING_EXPIRED;
       MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
              ("NSSCertDBTrustDomain: expired stapled OCSP response"));
+    } else if (stapledOCSPResponseResult ==
+                 Result::ERROR_OCSP_TRY_SERVER_LATER ||
+               stapledOCSPResponseResult ==
+                 Result::ERROR_OCSP_INVALID_SIGNING_CERT) {
+      // Stapled OCSP response present but invalid for a small number of reasons
+      // CAs/servers commonly get wrong. This will be treated similarly to an
+      // expired stapled response.
+      mOCSPStaplingStatus = CertVerifier::OCSP_STAPLING_INVALID;
+      MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
+              ("NSSCertDBTrustDomain: stapled OCSP response: "
+               "failure (whitelisted for compatibility)"));
     } else {
       // stapled OCSP response present but invalid for some reason
       mOCSPStaplingStatus = CertVerifier::OCSP_STAPLING_INVALID;
       MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
              ("NSSCertDBTrustDomain: stapled OCSP response: failure"));
       return stapledOCSPResponseResult;
     }
   } else if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) {
@@ -475,23 +486,23 @@ NSSCertDBTrustDomain::CheckRevocation(En
 
   // If we have a fresh OneCRL Blocklist we can skip OCSP for CA certs
   bool blocklistIsFresh;
   nsresult nsrv = mCertBlocklist->IsBlocklistFresh(&blocklistIsFresh);
   if (NS_FAILED(nsrv)) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
-  // TODO: We still need to handle the fallback for expired responses. But,
-  // if/when we disable OCSP fetching by default, it would be ambiguous whether
-  // security.OCSP.enable==0 means "I want the default" or "I really never want
-  // you to ever fetch OCSP."
-
+  // TODO: We still need to handle the fallback for invalid stapled responses.
+  // But, if/when we disable OCSP fetching by default, it would be ambiguous
+  // whether security.OCSP.enable==0 means "I want the default" or "I really
+  // never want you to ever fetch OCSP."
+  // Additionally, this doesn't properly handle OCSP-must-staple when OCSP
+  // fetching is disabled.
   Duration shortLifetime(mCertShortLifetimeInDays * Time::ONE_DAY_IN_SECONDS);
-
   if ((mOCSPFetching == NeverFetchOCSP) ||
       (validityDuration < shortLifetime) ||
       (endEntityOrCA == EndEntityOrCA::MustBeCA &&
        (mOCSPFetching == FetchOCSPForDVHardFail ||
         mOCSPFetching == FetchOCSPForDVSoftFail ||
         blocklistIsFresh))) {
     // We're not going to be doing any fetching, so if there was a cached
     // "unknown" response, say so.
@@ -609,17 +620,17 @@ NSSCertDBTrustDomain::CheckRevocation(En
     if (cachedResponseResult == Result::ERROR_OCSP_UNKNOWN_CERT) {
       MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
              ("NSSCertDBTrustDomain: returning SECFailure from cached "
               "response after OCSP request failure"));
       return cachedResponseResult;
     }
     if (stapledOCSPResponseResult != Success) {
       MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
-             ("NSSCertDBTrustDomain: returning SECFailure from expired "
+             ("NSSCertDBTrustDomain: returning SECFailure from expired/invalid "
               "stapled response after OCSP request failure"));
       return stapledOCSPResponseResult;
     }
 
     MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
            ("NSSCertDBTrustDomain: returning SECSuccess after "
             "OCSP request failure"));
     return Success; // Soft fail -> success :(
@@ -640,18 +651,18 @@ NSSCertDBTrustDomain::CheckRevocation(En
   }
 
   if (rv == Result::ERROR_OCSP_UNKNOWN_CERT ||
       rv == Result::ERROR_REVOKED_CERTIFICATE) {
     return rv;
   }
   if (stapledOCSPResponseResult != Success) {
     MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
-           ("NSSCertDBTrustDomain: returning SECFailure from expired stapled "
-            "response after OCSP request verification failure"));
+           ("NSSCertDBTrustDomain: returning SECFailure from expired/invalid "
+            "stapled response after OCSP request verification failure"));
     return stapledOCSPResponseResult;
   }
 
   MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
          ("NSSCertDBTrustDomain: end of CheckRevocation"));
 
   return Success; // Soft fail -> success :(
 }
--- a/security/manager/ssl/tests/unit/ocsp_certs/must-staple-ee.pem
+++ b/security/manager/ssl/tests/unit/ocsp_certs/must-staple-ee.pem
@@ -1,18 +1,19 @@
 -----BEGIN CERTIFICATE-----
-MIIC6TCCAdOgAwIBAgIUV0syO5TM/Hi/n/3k2+YYxrJdoUkwCwYJKoZIhvcNAQEL
+MIIDHTCCAgegAwIBAgIUdflJMoCHd1udkN6qKgRwlEmLYzEwCwYJKoZIhvcNAQEL
 MBIxEDAOBgNVBAMMB1Rlc3QgQ0EwIhgPMjAxNTExMjgwMDAwMDBaGA8yMDE4MDIw
 NTAwMDAwMFowGjEYMBYGA1UEAwwPVGVzdCBFbmQtZW50aXR5MIIBIjANBgkqhkiG
 9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuohRqESOFtZB/W62iAY2ED08E9nq5DVKtOz1
 aFdsJHvBxyWo4NgfvbGcBptuGobya+KvWnVramRxCHqlWqdFh/cc1SScAn7NQ/we
 adA4ICmTqyDDSeTbuUzCa2wO7RWCD/F+rWkasdMCOosqQe6ncOAPDY39ZgsrsCSS
 pH25iGF5kLFXkD3SO8XguEgfqDfTiEPvJxbYVbdmWqp+ApAvOnsQgAYkzBxsl62W
 YVu34pYSwHUxowyR3bTK9/ytHSXTCe+5Fw6naOGzey8ib2njtIqVYR3uJtYlnauR
-CE42yxwkBCy/Fosv5fGPmRcxuLP+SSP6clHEMdUDrNoYCjXtjQIDAQABoy8wLTAY
-BgNVHREEETAPgg0qLmV4YW1wbGUuY29tMBEGCCsGAQUFBwEYBAUwAwIBBTALBgkq
-hkiG9w0BAQsDggEBAIUrl9TwMdyfzqh7QH65lQ84r/+fdhYZmTAQC+Rb43aZBwCR
-WlhmJdcNSq6vIGorap0dAPC9+COLcOA6L0YcqaGxIf1fJdiCPuOR1r6Gnns2DnLi
-NkxEF8BfKi3POeGK5oEgAbvdvnZgL/GMMST9508WHMY6StjbyGZCE9nQh1dcVme1
-sJ6681Q3CnPYN6D2C/D5mJi8sgdjCPRvntHjfE8uHfvJ7r5/FjsXmY0r056QFPb6
-pi7YhaJP179c5jxQEfakz8zwu4vxkdCwyuj+WK0AjmVTp0vghiX9mHXyXvqTEQuz
-tO9XdoIyDDbG7Ki0CFkAObPBnSyf1kWaAHrjmVs=
+CE42yxwkBCy/Fosv5fGPmRcxuLP+SSP6clHEMdUDrNoYCjXtjQIDAQABo2MwYTAY
+BgNVHREEETAPgg0qLmV4YW1wbGUuY29tMBEGCCsGAQUFBwEYBAUwAwIBBTAyBggr
+BgEFBQcBAQQmMCQwIgYIKwYBBQUHMAGGFmh0dHA6Ly9sb2NhbGhvc3Q6ODg4OC8w
+CwYJKoZIhvcNAQELA4IBAQBW/HKJxH3x/06cLthboB7H5OiJczd6k4nsrC0iqeAE
+kalAw2MUo16cIkFtMS2Fl39yJ2w/3UZ7l6tidvnWUF//lDs/C9sXQweyCRBEnR/x
+Wfg/H94jhPtJXzY4r/r2jx4ePF0ar84A07gS2neLfVEVv+/jPf8/UyGPi1JVuOxY
+eDCHVKmrjFlsofLAFwPaFyameWWJdxgrRBe+SmtWLFOSJ1i8jmYi9H8HwWpX/03c
+hHVUsLLH+r6q4NZwZuv4VqGNCLGd+Lzjhy2aHrLlQA20KpKUKzzSQpgXIXMJ0P25
+VKCn+IClnRYCpZILaogomy5nSgnfBlhDraxcxpX9HcyW
 -----END CERTIFICATE-----
\ No newline at end of file
--- a/security/manager/ssl/tests/unit/ocsp_certs/must-staple-ee.pem.certspec
+++ b/security/manager/ssl/tests/unit/ocsp_certs/must-staple-ee.pem.certspec
@@ -1,4 +1,5 @@
 issuer:Test CA
 subject:Test End-entity
 extension:subjectAlternativeName:*.example.com
 extension:TLSFeature:OCSPMustStaple
+extension:authorityInformationAccess:http://localhost:8888/
--- a/security/manager/ssl/tests/unit/test_ocsp_must_staple.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_must_staple.js
@@ -5,20 +5,21 @@
 "use strict";
 
 // Tests OCSP Must Staple handling by connecting to various domains (as faked by
 // a server running locally) that correspond to combinations of whether the
 // extension is present in intermediate and end-entity certificates.
 
 var gExpectOCSPRequest;
 
-function add_ocsp_test(aHost, aExpectedResult, aStaplingEnabled) {
+function add_ocsp_test(aHost, aExpectedResult, aStaplingEnabled,
+                       aExpectOCSPRequest = false) {
   add_connection_test(aHost, aExpectedResult,
     function() {
-      gExpectOCSPRequest = !aStaplingEnabled;
+      gExpectOCSPRequest = aExpectOCSPRequest;
       clearOCSPCache();
       clearSessionCache();
       Services.prefs.setBoolPref("security.ssl.enable_ocsp_stapling",
                                  aStaplingEnabled);
     });
 }
 
 function add_tests() {
@@ -68,38 +69,54 @@ function add_tests() {
   // Now a bunch of operations with only a must-staple ee
   add_ocsp_test("ocsp-stapling-must-staple.example.com",
                 PRErrorCodeSuccess, true);
 
   add_ocsp_test("ocsp-stapling-must-staple-revoked.example.com",
                 SEC_ERROR_REVOKED_CERTIFICATE, true);
 
   add_ocsp_test("ocsp-stapling-must-staple-missing.example.com",
-                MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING, true);
+                MOZILLA_PKIX_ERROR_REQUIRED_TLS_FEATURE_MISSING, true, true);
 
   add_ocsp_test("ocsp-stapling-must-staple-empty.example.com",
                 SEC_ERROR_OCSP_MALFORMED_RESPONSE, true);
 
   add_ocsp_test("ocsp-stapling-must-staple-missing.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
+
+  // If the stapled response is expired, we will try to fetch a new one.
+  // If that fails, we should report the original error.
+  add_ocsp_test("ocsp-stapling-must-staple-expired.example.com",
+                SEC_ERROR_OCSP_OLD_RESPONSE, true, true);
+  // Similarly with a "try server later" response.
+  add_ocsp_test("ocsp-stapling-must-staple-try-later.example.com",
+                SEC_ERROR_OCSP_TRY_SERVER_LATER, true, true);
+  // And again with an invalid OCSP response signing certificate.
+  add_ocsp_test("ocsp-stapling-must-staple-invalid-signer.example.com",
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
 
   // check that disabling must-staple works
   add_test(function() {
     clearSessionCache();
     Services.prefs.setBoolPref("security.ssl.enable_ocsp_must_staple", false);
     run_next_test();
   });
 
   add_ocsp_test("ocsp-stapling-must-staple-missing.example.com",
-                PRErrorCodeSuccess, true);
+                PRErrorCodeSuccess, true, true);
 }
 
 function run_test() {
   do_get_profile();
   Services.prefs.setBoolPref("security.ssl.enable_ocsp_must_staple", true);
+  Services.prefs.setIntPref("security.OCSP.enabled", 1);
+  // This test may sometimes fail on android due to an OCSP request timing out.
+  // That aspect of OCSP requests is not what we're testing here, so we can just
+  // bump the timeout and hopefully avoid these failures.
+  Services.prefs.setIntPref("security.OCSP.timeoutMilliseconds.soft", 5000);
 
   let fakeOCSPResponder = new HttpServer();
   fakeOCSPResponder.registerPrefixHandler("/", function (request, response) {
     response.setStatusLine(request.httpVersion, 500, "Internal Server Error");
     ok(gExpectOCSPRequest,
        "Should be getting an OCSP request only when expected");
   });
   fakeOCSPResponder.start(8888);
--- a/security/manager/ssl/tests/unit/test_ocsp_stapling.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_stapling.js
@@ -5,111 +5,107 @@
 "use strict";
 
 // In which we connect to a number of domains (as faked by a server running
 // locally) with and without OCSP stapling enabled to determine that good
 // things happen and bad things don't.
 
 var gExpectOCSPRequest;
 
-function add_ocsp_test(aHost, aExpectedResult, aStaplingEnabled) {
+function add_ocsp_test(aHost, aExpectedResult, aStaplingEnabled,
+                       aExpectOCSPRequest = false) {
   add_connection_test(aHost, aExpectedResult,
     function() {
-      gExpectOCSPRequest = !aStaplingEnabled;
+      gExpectOCSPRequest = aExpectOCSPRequest;
       clearOCSPCache();
       clearSessionCache();
       Services.prefs.setBoolPref("security.ssl.enable_ocsp_stapling",
                                  aStaplingEnabled);
     });
 }
 
 function add_tests() {
   // In the absence of OCSP stapling, these should actually all work.
   add_ocsp_test("ocsp-stapling-good.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-revoked.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-good-other-ca.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-malformed.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-srverr.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-trylater.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-needssig.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-unauthorized.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-unknown.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-good-other.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-none.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-expired.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-expired-fresh-ca.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-skip-responseBytes.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-critical-extension.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-noncritical-extension.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
   add_ocsp_test("ocsp-stapling-empty-extensions.example.com",
-                PRErrorCodeSuccess, false);
+                PRErrorCodeSuccess, false, true);
 
   // Now test OCSP stapling
   // The following error codes are defined in security/nss/lib/util/SECerrs.h
 
   add_ocsp_test("ocsp-stapling-good.example.com", PRErrorCodeSuccess, true);
 
   add_ocsp_test("ocsp-stapling-revoked.example.com",
                 SEC_ERROR_REVOKED_CERTIFICATE, true);
 
-  // SEC_ERROR_OCSP_INVALID_SIGNING_CERT vs SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE
-  // depends on whether the CA that signed the response is a trusted CA
-  // (but only with the classic implementation - mozilla::pkix always
-  // results in the error SEC_ERROR_OCSP_INVALID_SIGNING_CERT).
-
   // This stapled response is from a CA that is untrusted and did not issue
   // the server's certificate.
   let certDB = Cc["@mozilla.org/security/x509certdb;1"]
                   .getService(Ci.nsIX509CertDB);
   let otherTestCA = constructCertFromFile("ocsp_certs/other-test-ca.pem");
   add_test(function() {
     certDB.setCertTrust(otherTestCA, Ci.nsIX509Cert.CA_CERT,
                         Ci.nsIX509CertDB.UNTRUSTED);
     run_next_test();
   });
   add_ocsp_test("ocsp-stapling-good-other-ca.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
 
   // The stapled response is from a CA that is trusted but did not issue the
   // server's certificate.
   add_test(function() {
     certDB.setCertTrust(otherTestCA, Ci.nsIX509Cert.CA_CERT,
                         Ci.nsIX509CertDB.TRUSTED_SSL);
     run_next_test();
   });
   // TODO(bug 979055): When using ByName instead of ByKey, the error here is
   // SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE. We should be testing both cases.
   add_ocsp_test("ocsp-stapling-good-other-ca.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
 
   // TODO: Test the case where the signing cert can't be found at all, which
   // will result in SEC_ERROR_BAD_DATABASE in the NSS classic case.
 
   add_ocsp_test("ocsp-stapling-malformed.example.com",
                 SEC_ERROR_OCSP_MALFORMED_REQUEST, true);
   add_ocsp_test("ocsp-stapling-srverr.example.com",
                 SEC_ERROR_OCSP_SERVER_ERROR, true);
   add_ocsp_test("ocsp-stapling-trylater.example.com",
-                SEC_ERROR_OCSP_TRY_SERVER_LATER, true);
+                SEC_ERROR_OCSP_TRY_SERVER_LATER, true, true);
   add_ocsp_test("ocsp-stapling-needssig.example.com",
                 SEC_ERROR_OCSP_REQUEST_NEEDS_SIG, true);
   add_ocsp_test("ocsp-stapling-unauthorized.example.com",
                 SEC_ERROR_OCSP_UNAUTHORIZED_REQUEST, true);
   add_ocsp_test("ocsp-stapling-unknown.example.com",
                 SEC_ERROR_OCSP_UNKNOWN_CERT, true);
   add_ocsp_test("ocsp-stapling-good-other.example.com",
                 MOZILLA_PKIX_ERROR_OCSP_RESPONSE_FOR_CERT_MISSING, true);
@@ -138,36 +134,36 @@ function add_tests() {
   add_ocsp_test("ocsp-stapling-empty-extensions.example.com",
                 PRErrorCodeSuccess, true);
 
   add_ocsp_test("ocsp-stapling-delegated-included.example.com",
                 PRErrorCodeSuccess, true);
   add_ocsp_test("ocsp-stapling-delegated-included-last.example.com",
                 PRErrorCodeSuccess, true);
   add_ocsp_test("ocsp-stapling-delegated-missing.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
   add_ocsp_test("ocsp-stapling-delegated-missing-multiple.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
   add_ocsp_test("ocsp-stapling-delegated-no-extKeyUsage.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
   add_ocsp_test("ocsp-stapling-delegated-from-intermediate.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
   add_ocsp_test("ocsp-stapling-delegated-keyUsage-crlSigning.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
   add_ocsp_test("ocsp-stapling-delegated-wrong-extKeyUsage.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
 
   // ocsp-stapling-expired.example.com and
   // ocsp-stapling-expired-fresh-ca.example.com are handled in
   // test_ocsp_stapling_expired.js
 
   // Check that OCSP responder certificates with key sizes below 1024 bits are
   // rejected, even when the main certificate chain keys are at least 1024 bits.
   add_ocsp_test("keysize-ocsp-delegated.example.com",
-                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true);
+                SEC_ERROR_OCSP_INVALID_SIGNING_CERT, true, true);
 
   add_ocsp_test("revoked-ca-cert-used-as-end-entity.example.com",
                 SEC_ERROR_REVOKED_CERTIFICATE, true);
 }
 
 function check_ocsp_stapling_telemetry() {
   let histogram = Cc["@mozilla.org/base/telemetry;1"]
                     .getService(Ci.nsITelemetry)
@@ -183,16 +179,21 @@ function check_ocsp_stapling_telemetry()
         "Actual and expected connections with an expired response should match");
   equal(histogram.counts[4], 21,
         "Actual and expected connections with bad responses should match");
   run_next_test();
 }
 
 function run_test() {
   do_get_profile();
+  Services.prefs.setIntPref("security.OCSP.enabled", 1);
+  // This test may sometimes fail on android due to an OCSP request timing out.
+  // That aspect of OCSP requests is not what we're testing here, so we can just
+  // bump the timeout and hopefully avoid these failures.
+  Services.prefs.setIntPref("security.OCSP.timeoutMilliseconds.soft", 5000);
 
   let fakeOCSPResponder = new HttpServer();
   fakeOCSPResponder.registerPrefixHandler("/", function (request, response) {
     response.setStatusLine(request.httpVersion, 500, "Internal Server Error");
     ok(gExpectOCSPRequest,
        "Should be getting an OCSP request only when expected");
   });
   fakeOCSPResponder.start(8888);
--- a/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js
+++ b/security/manager/ssl/tests/unit/test_ocsp_stapling_expired.js
@@ -3,16 +3,24 @@
 // 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 number of domains (as faked by a server running
 // locally) with OCSP stapling enabled to determine that good things happen
 // and bad things don't, specifically with respect to various expired OCSP
 // responses (stapled and otherwise).
+// According to RFC 6066, if a stapled OCSP response can't be satisfactorilly
+// verified, the client should terminate the connection. Unfortunately, due to
+// some bugs where servers will staple any old garbage without verifying it, we
+// can't be this strict in practice. Originally this caveat only applied to
+// expired responses, but recent high-profile failures have caused us to expand
+// this to "try later" responses and responses where the signing certificate
+// doesn't verify successfully.
+
 
 var gCurrentOCSPResponse = null;
 var gOCSPRequestCount = 0;
 
 function add_ocsp_test(aHost, aExpectedResult, aOCSPResponseToServe,
                        aExpectedRequestCount) {
   add_connection_test(aHost, aExpectedResult,
     function() {
@@ -36,28 +44,30 @@ Services.prefs.setIntPref("security.OCSP
 // bump the timeout and hopefully avoid these failures.
 Services.prefs.setIntPref("security.OCSP.timeoutMilliseconds.soft", 5000);
 Services.prefs.setIntPref("security.pki.sha1_enforcement_level", 4);
 var args = [["good", "default-ee", "unused", 0],
              ["expiredresponse", "default-ee", "unused", 0],
              ["oldvalidperiod", "default-ee", "unused", 0],
              ["revoked", "default-ee", "unused", 0],
              ["unknown", "default-ee", "unused", 0],
+             ["good", "must-staple-ee", "unused", 0],
             ];
 var ocspResponses = generateOCSPResponses(args, "ocsp_certs");
 // Fresh response, certificate is good.
 var ocspResponseGood = ocspResponses[0];
 // Expired response, certificate is good.
 var expiredOCSPResponseGood = ocspResponses[1];
 // Fresh signature, old validity period, certificate is good.
 var oldValidityPeriodOCSPResponseGood = ocspResponses[2];
 // Fresh signature, certificate is revoked.
 var ocspResponseRevoked = ocspResponses[3];
 // Fresh signature, certificate is unknown.
 var ocspResponseUnknown = ocspResponses[4];
+var ocspResponseGoodMustStaple = ocspResponses[5];
 
 // sometimes we expect a result without re-fetch
 var willNotRetry = 1;
 // but sometimes, since a bad response is in the cache, OCSP fetch will be
 // attempted for each validation - in practice, for these test certs, this
 // means 6 requests because various hash algorithm and key size combinations
 // are tried.
 var willRetry = 6;
@@ -159,30 +169,43 @@ function run_test() {
                 ocspResponseGood, willNotRetry);
   add_ocsp_test("ocsp-stapling-ancient-valid.example.com",
                 SEC_ERROR_REVOKED_CERTIFICATE,
                 ocspResponseRevoked, willNotRetry);
   add_ocsp_test("ocsp-stapling-ancient-valid.example.com",
                 SEC_ERROR_OCSP_UNKNOWN_CERT,
                 ocspResponseUnknown, willRetry);
 
+  // Test how OCSP-must-staple (i.e. TLS feature) interacts with stapled OCSP
+  // responses that don't successfully verify.
+  // A strict reading of the relevant RFCs might say that these connections
+  // should all fail because a satisfactory stapled OCSP response is not
+  // present, but for compatibility reasons we fall back to active OCSP fetching
+  // in these situations. If the fetch succeeds, then connection succeeds.
+  add_ocsp_test("ocsp-stapling-must-staple-expired.example.com",
+                PRErrorCodeSuccess, ocspResponseGoodMustStaple, willNotRetry);
+  add_ocsp_test("ocsp-stapling-must-staple-try-later.example.com",
+                PRErrorCodeSuccess, ocspResponseGoodMustStaple, willNotRetry);
+  add_ocsp_test("ocsp-stapling-must-staple-invalid-signer.example.com",
+                PRErrorCodeSuccess, ocspResponseGoodMustStaple, willNotRetry);
+
   add_test(function () { ocspResponder.stop(run_next_test); });
   add_test(check_ocsp_stapling_telemetry);
   run_next_test();
 }
 
 function check_ocsp_stapling_telemetry() {
   let histogram = Cc["@mozilla.org/base/telemetry;1"]
                     .getService(Ci.nsITelemetry)
                     .getHistogramById("SSL_OCSP_STAPLING")
                     .snapshot();
   equal(histogram.counts[0], 0,
         "Should have 0 connections for unused histogram bucket 0");
   equal(histogram.counts[1], 0,
         "Actual and expected connections with a good response should match");
   equal(histogram.counts[2], 0,
         "Actual and expected connections with no stapled response should match");
-  equal(histogram.counts[3], 21,
+  equal(histogram.counts[3], 22,
         "Actual and expected connections with an expired response should match");
-  equal(histogram.counts[4], 0,
+  equal(histogram.counts[4], 2,
         "Actual and expected connections with bad responses should match");
   run_next_test();
 }
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/OCSPStaplingServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/OCSPStaplingServer.cpp
@@ -55,16 +55,19 @@ const OCSPHost sOCSPHosts[] =
   { "keysize-ocsp-delegated.example.com", ORTDelegatedIncluded, "rsa-1016-keysizeDelegatedSigner", nullptr },
   { "revoked-ca-cert-used-as-end-entity.example.com", ORTRevoked, "ca-used-as-end-entity", nullptr },
   { "ocsp-stapling-must-staple.example.com", ORTGood, nullptr, "must-staple-ee" },
   { "ocsp-stapling-must-staple-revoked.example.com", ORTRevoked, nullptr, "must-staple-ee" },
   { "ocsp-stapling-must-staple-missing.example.com", ORTNone, nullptr, "must-staple-ee" },
   { "ocsp-stapling-must-staple-empty.example.com", ORTEmpty, nullptr, "must-staple-ee" },
   { "ocsp-stapling-must-staple-ee-with-must-staple-int.example.com", ORTGood, nullptr, "must-staple-ee-with-must-staple-int" },
   { "ocsp-stapling-plain-ee-with-must-staple-int.example.com", ORTGood, nullptr, "must-staple-missing-ee" },
+  { "ocsp-stapling-must-staple-expired.example.com", ORTExpired, nullptr, "must-staple-ee" },
+  { "ocsp-stapling-must-staple-try-later.example.com", ORTTryLater, nullptr, "must-staple-ee" },
+  { "ocsp-stapling-must-staple-invalid-signer.example.com", ORTGoodOtherCA, "other-test-ca", "must-staple-ee" },
   { "multi-tls-feature-good.example.com", ORTNone, nullptr, "multi-tls-feature-good-ee" },
   { "multi-tls-feature-bad.example.com", ORTNone, nullptr, "multi-tls-feature-bad-ee" },
   { nullptr, ORTNull, nullptr, nullptr }
 };
 
 int32_t
 DoSNISocketConfig(PRFileDesc *aFd, const SECItem *aSrvNameArr,
                   uint32_t aSrvNameArrSize, void *aArg)