bug 1368868 - give up on ocsp stapling strictness because we can't have nice things r?jcj draft
authorDavid Keeler <dkeeler@mozilla.com>
Wed, 08 Nov 2017 15:50:26 -0800
changeset 697279 1dd4e422a3d21c449b5c7048459531316f18ae0c
parent 697096 fc194660762d1b92e1679d860a8bf41116d0f54f
child 740080 83dee5c9ea6e7a5acf04b0fb2aa68b207b4eaff8
push id88956
push userbmo:dkeeler@mozilla.com
push dateMon, 13 Nov 2017 20:49:17 +0000
reviewersjcj
bugs1368868
milestone59.0a1
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)