Bug 1486551 - turn cert revocation error pages into (non-overridable)…

… certificate error pages r=jschanck,fluent-reviewers,webidl,bolsson,smaug SEC_ERROR_REVOKED_CERTIFICATE is a certificate error, not a TLS protocol error. This patch updates the categorization of this error while maintaining the property that it cannot be overridden. This has the benefit of making it possible to show more diagnostic information in the error page, which this patch also adds. Differential Revision: https://phabricator.services.mozilla.com/D239220
mozilla · Feb 27, 2025 · c0c870f · c0c870f
1 parent 20e0335
commit c0c870f
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 19 deletions.
diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
@@ -26,6 +26,7 @@
 #include "ExpandedPrincipal.h"
 #include "MainThreadUtils.h"
 #include "MobileViewportManager.h"
+#include "NSSErrorsService.h"
 #include "NodeUbiReporting.h"
 #include "PLDHashTable.h"
 #include "StorageAccessPermissionRequest.h"
@@ -1931,6 +1932,8 @@ void Document::GetFailedCertSecurityInfo(FailedCertSecurityInfo& aInfo,
     return;
   }
 
+  aInfo.mErrorIsOverridable = mozilla::psm::ErrorIsOverridable(errorCode);
+
   nsCOMPtr<nsINSSErrorsService> nsserr =
       do_GetService("@mozilla.org/nss_errors_service;1");
   if (NS_WARN_IF(!nsserr)) {

diff --git a/dom/webidl/FailedCertSecurityInfo.webidl b/dom/webidl/FailedCertSecurityInfo.webidl
@@ -18,6 +18,7 @@ enum OverridableErrorCategory {
 
 dictionary FailedCertSecurityInfo {
   DOMString errorCodeString = "";
+  boolean errorIsOverridable = false;
   unsigned long channelStatus = 0;
   OverridableErrorCategory overridableErrorCategory = "unset";
   DOMTimeStamp validNotBefore = 0;

diff --git a/security/manager/ssl/CommonSocketControl.cpp b/security/manager/ssl/CommonSocketControl.cpp
@@ -53,11 +53,9 @@ CommonSocketControl::CommonSocketControl(const nsCString& aHostName,
 }
 
 void CommonSocketControl::SetStatusErrorBits(
-    const nsCOMPtr<nsIX509Cert>& cert,
     nsITransportSecurityInfo::OverridableErrorCategory
         overridableErrorCategory) {
   COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
-  SetServerCert(cert, mozilla::psm::EVStatus::NotEV);
   mOverridableErrorCategory = Some(overridableErrorCategory);
 }
 
@@ -307,11 +305,10 @@ void CommonSocketControl::RebuildCertificateInfoFromSSLTokenCache() {
   mozilla::net::SessionCacheInfo& info = *mSessionCacheInfo;
   nsCOMPtr<nsIX509Cert> cert(
       new nsNSSCertificate(std::move(info.mServerCertBytes)));
-  if (info.mOverridableErrorCategory ==
+  SetServerCert(cert, info.mEVStatus);
+  if (info.mOverridableErrorCategory !=
       nsITransportSecurityInfo::OverridableErrorCategory::ERROR_UNSET) {
-    SetServerCert(cert, info.mEVStatus);
-  } else {
-    SetStatusErrorBits(cert, info.mOverridableErrorCategory);
+    SetStatusErrorBits(info.mOverridableErrorCategory);
   }
   SetCertificateTransparencyStatus(info.mCertificateTransparencyStatus);
   if (info.mSucceededCertChainBytes) {

diff --git a/security/manager/ssl/CommonSocketControl.h b/security/manager/ssl/CommonSocketControl.h
@@ -82,8 +82,7 @@ class CommonSocketControl : public nsITLSSocketControl {
     COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();
     return mServerCert != nullptr;
   }
-  void SetStatusErrorBits(const nsCOMPtr<nsIX509Cert>& cert,
-                          nsITransportSecurityInfo::OverridableErrorCategory
+  void SetStatusErrorBits(nsITransportSecurityInfo::OverridableErrorCategory
                               overridableErrorCategory);
   bool HasUserOverriddenCertificateError() {
     COMMON_SOCKET_CONTROL_ASSERT_ON_OWNING_THREAD();

diff --git a/security/manager/ssl/NSSErrorsService.cpp b/security/manager/ssl/NSSErrorsService.cpp
@@ -124,7 +124,10 @@ NSSErrorsService::GetErrorClass(nsresult aXPCOMErrorCode,
     return NS_ERROR_FAILURE;
   }
 
-  if (mozilla::psm::ErrorIsOverridable(aNSPRCode)) {
+  // All overridable errors are certificate errors, but not all certificate
+  // errors are overridable.
+  if (mozilla::psm::ErrorIsOverridable(aNSPRCode) ||
+      aNSPRCode == SEC_ERROR_REVOKED_CERTIFICATE) {
     *aErrorClass = ERROR_CLASS_BAD_CERT;
   } else {
     *aErrorClass = ERROR_CLASS_SSL_PROTOCOL;

diff --git a/security/manager/ssl/NSSErrorsService.h b/security/manager/ssl/NSSErrorsService.h
@@ -46,11 +46,7 @@ bool ErrorIsOverridable(PRErrorCode code);
 }  // namespace psm
 }  // namespace mozilla
 
-#define NS_NSSERRORSSERVICE_CID                      \
-  {                                                  \
-    0x9ef18451, 0xa157, 0x4d17, {                    \
-      0x81, 0x32, 0x47, 0xaf, 0xef, 0x21, 0x36, 0x89 \
-    }                                                \
-  }
+#define NS_NSSERRORSSERVICE_CID \
+  {0x9ef18451, 0xa157, 0x4d17, {0x81, 0x32, 0x47, 0xaf, 0xef, 0x21, 0x36, 0x89}}
 
 #endif  // NSSErrorsService_h
diff --git a/security/manager/ssl/SSLServerCertVerification.cpp b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -1156,12 +1156,11 @@ SSLServerCertVerificationResult::Run() {
   } else {
     nsTArray<uint8_t> certBytes(mPeerCertChain.ElementAt(0).Clone());
     nsCOMPtr<nsIX509Cert> cert(new nsNSSCertificate(std::move(certBytes)));
-    // Certificate validation failed; store the peer certificate chain on
-    // mSocketControl so it can be used for error reporting.
+    mSocketControl->SetServerCert(cert, EVStatus::NotEV);
     mSocketControl->SetFailedCertChain(std::move(mPeerCertChain));
     if (mOverridableErrorCategory !=
         nsITransportSecurityInfo::OverridableErrorCategory::ERROR_UNSET) {
-      mSocketControl->SetStatusErrorBits(cert, mOverridableErrorCategory);
+      mSocketControl->SetStatusErrorBits(mOverridableErrorCategory);
     }
   }
 

diff --git a/toolkit/content/aboutNetError.mjs b/toolkit/content/aboutNetError.mjs
@@ -213,7 +213,8 @@ function disallowCertOverridesIfNeeded() {
   // Disallow overrides if this is a Strict-Transport-Security
   // host and the cert is bad (STS Spec section 7.3) or if the
   // certerror is in a frame (bug 633691).
-  if (gHasSts || window != top) {
+  const failedCertInfo = document.getFailedCertSecurityInfo();
+  if (gHasSts || window != top || !failedCertInfo.errorIsOverridable) {
     document.getElementById("exceptionDialogButton").hidden = true;
   }
   if (gHasSts) {
@@ -1179,6 +1180,16 @@ function setCertErrorDetails() {
         ],
       ];
       break;
+    case "SEC_ERROR_REVOKED_CERTIFICATE":
+      whatToDoParts = [
+        [
+          "p",
+          // This string was added for the certificate transparency error case,
+          // but it applies in other cases as well, such as this one.
+          "cert-error-trust-certificate-transparency-what-can-you-do-about-it",
+        ],
+      ];
+      break;
   }
 
   if (whatToDoParts) {
@@ -1375,6 +1386,11 @@ function setTechnicalDetailsOnCertError(
       break;
   }
 
+  if (failedCertInfo.errorCodeString == "SEC_ERROR_REVOKED_CERTIFICATE") {
+    addLabel("cert-error-revoked", { hostname });
+    addErrorCodeLink();
+  }
+
   getFailedCertificatesAsPEMString().then(pemString => {
     const errorText = document.getElementById("certificateErrorText");
     errorText.textContent = pemString;

diff --git a/toolkit/locales/en-US/toolkit/neterror/certError.ftl b/toolkit/locales/en-US/toolkit/neterror/certError.ftl
@@ -34,6 +34,10 @@ cert-error-trust-symantec = Certificates issued by GeoTrust, RapidSSL, Symantec,
 #   $hostname (string) - Hostname of the website with cert error.
 cert-error-trust-certificate-transparency = { -brand-short-name } doesn’t trust { $hostname } because it couldn’t prove it meets public certificate transparency requirements.
 
+# Variables:
+#   $hostname (string) - Hostname of the website with cert error.
+cert-error-revoked = Websites prove their identity via certificates. { -brand-short-name } doesn’t trust { $hostname } because it uses a certificate that has been revoked.
+
 cert-error-untrusted-default = The certificate does not come from a trusted source.
 
 # Variables: