Issue #1280 - Un-bust certerror pages and ForgetAboutSite

docshell/base/nsDocShell.cpp

@@ -4943,13 +4943,11 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
       if (errorClass == nsINSSErrorsService::ERROR_CLASS_BAD_CERT) {
         error.AssignLiteral("nssBadCert");
 
-        // If this is an HTTP Strict Transport Security host or a pinned host
+        // If this is an HTTP Strict Transport Security host, don't allow
-        // and the certificate is bad, don't allow overrides (RFC 6797 section
+        // overrides (RFC 6797 section 12.1).
-        // 12.1, HPKP draft spec section 2.6).
         uint32_t flags =
           UsePrivateBrowsing() ? nsISocketProvider::NO_PERMANENT_STORAGE : 0;
         bool isStsHost = false;
-        bool isPinnedHost = false;
         if (XRE_IsParentProcess()) {
           nsCOMPtr<nsISiteSecurityService> sss =
             do_GetService(NS_SSSERVICE_CONTRACTID, &rv);
@@ -4957,9 +4955,6 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
           rv = sss->IsSecureURI(nsISiteSecurityService::HEADER_HSTS, aURI,
                                 flags, nullptr, &isStsHost);
           NS_ENSURE_SUCCESS(rv, rv);
-          rv = sss->IsSecureURI(nsISiteSecurityService::HEADER_HPKP, aURI,
-                                flags, nullptr, &isPinnedHost);
-          NS_ENSURE_SUCCESS(rv, rv);
         } else {
           mozilla::dom::ContentChild* cc =
             mozilla::dom::ContentChild::GetSingleton();
@@ -4967,8 +4962,6 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
           SerializeURI(aURI, uri);
           cc->SendIsSecureURI(nsISiteSecurityService::HEADER_HSTS, uri, flags,
                               &isStsHost);
-          cc->SendIsSecureURI(nsISiteSecurityService::HEADER_HPKP, uri, flags,
-                              &isPinnedHost);
         }
 
         if (Preferences::GetBool(
@@ -4976,11 +4969,9 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
           cssClass.AssignLiteral("expertBadCert");
         }
 
-        // HSTS/pinning takes precedence over the expert bad cert pref. We
+        // HSTS takes precedence over the expert bad cert pref. We
         // never want to show the "Add Exception" button for these sites.
-        // In the future we should differentiate between an HSTS host and a
+        if (isStsHost) {
-        // pinned host and display a more informative message to the user.
-        if (isStsHost || isPinnedHost) {
           cssClass.AssignLiteral("badStsCert");
         }
 

netwerk/protocol/http/nsHttpChannel.cpp

@@ -1530,7 +1530,7 @@ GetPKPConsoleErrorTag(uint32_t failureResult, nsAString& consoleErrorTag)
 }
 
 /**
- * Process a single security header. Only two types are supported: HSTS and HPKP.
+ * Process a single security header. Only one type is supported: HSTS.
  */
 nsresult
 nsHttpChannel::ProcessSingleSecurityHeader(uint32_t aType,
@@ -1542,9 +1542,6 @@ nsHttpChannel::ProcessSingleSecurityHeader(uint32_t aType,
         case nsISiteSecurityService::HEADER_HSTS:
             atom = nsHttp::ResolveAtom("Strict-Transport-Security");
             break;
-        case nsISiteSecurityService::HEADER_HPKP:
-            atom = nsHttp::ResolveAtom("Public-Key-Pins");
-            break;
         default:
             NS_NOTREACHED("Invalid security header type");
             return NS_ERROR_FAILURE;
@@ -1568,10 +1565,6 @@ nsHttpChannel::ProcessSingleSecurityHeader(uint32_t aType,
                     GetSTSConsoleErrorTag(failureResult, consoleErrorTag);
                     consoleErrorCategory = NS_LITERAL_STRING("Invalid HSTS Headers");
                     break;
-                case nsISiteSecurityService::HEADER_HPKP:
-                    GetPKPConsoleErrorTag(failureResult, consoleErrorTag);
-                    consoleErrorCategory = NS_LITERAL_STRING("Invalid HPKP Headers");
-                    break;
                 default:
                     return NS_ERROR_FAILURE;
             }
@@ -1641,10 +1634,6 @@ nsHttpChannel::ProcessSecurityHeaders()
                                      sslStatus, flags);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = ProcessSingleSecurityHeader(nsISiteSecurityService::HEADER_HPKP,
-                                     sslStatus, flags);
-    NS_ENSURE_SUCCESS(rv, rv);
-
     return NS_OK;
 }
 

security/manager/ssl/SSLServerCertVerification.cpp

@@ -425,11 +425,9 @@ CertErrorRunnable::CheckCertOverrides()
   uint32_t remaining_display_errors = mCollectedErrors;
 
 
-  // If this is an HTTP Strict Transport Security host or a pinned host and the
+  // If this is an HTTP Strict Transport Security host, don't allow overrides
-  // certificate is bad, don't allow overrides (RFC 6797 section 12.1,
+  // RFC 6797 section 12.1.
-  // HPKP draft spec section 2.6).
   bool strictTransportSecurityEnabled = false;
-  bool hasPinningInformation = false;
   nsCOMPtr<nsISiteSecurityService> sss(do_GetService(NS_SSSERVICE_CONTRACTID));
   if (!sss) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
@@ -449,21 +447,10 @@ CertErrorRunnable::CheckCertOverrides()
     return new SSLServerCertVerificationResult(mInfoObject,
                                                mDefaultErrorCodeToReport);
   }
-  nsrv = sss->IsSecureHost(nsISiteSecurityService::HEADER_HPKP,
-                           mInfoObject->GetHostNameRaw(),
-                           mProviderFlags,
-                           nullptr,
-                           &hasPinningInformation);
-  if (NS_FAILED(nsrv)) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-           ("[%p][%p] checking for HPKP failed\n", mFdForLogging, this));
-    return new SSLServerCertVerificationResult(mInfoObject,
-                                               mDefaultErrorCodeToReport);
-  }
 
-  if (!strictTransportSecurityEnabled && !hasPinningInformation) {
+  if (!strictTransportSecurityEnabled) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-           ("[%p][%p] no HSTS or HPKP - overrides allowed\n",
+           ("[%p][%p] no HSTS - overrides allowed\n",
             mFdForLogging, this));
     nsCOMPtr<nsICertOverrideService> overrideService =
       do_GetService(NS_CERTOVERRIDE_CONTRACTID);
@@ -497,7 +484,7 @@ CertErrorRunnable::CheckCertOverrides()
     }
   } else {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-           ("[%p][%p] HSTS or HPKP - no overrides allowed\n",
+           ("[%p][%p] HSTS - no overrides allowed\n",
             mFdForLogging, this));
   }
 

toolkit/forgetaboutsite/ForgetAboutSite.jsm

@@ -216,8 +216,8 @@ this.ForgetAboutSite = {
       });
     }));
 
-    // HSTS and HPKP
+    // HSTS
-    // TODO (bug 1290529): also remove HSTS/HPKP information for subdomains.
+    // TODO (bug 1290529): also remove HSTS information for subdomains.
     // Since we can't enumerate the information in the site security service
     // (bug 1115712), we can't implement this right now.
     promises.push(Task.spawn(function*() {
@@ -225,9 +225,8 @@ this.ForgetAboutSite = {
                 getService(Ci.nsISiteSecurityService);
       let httpsURI = NetUtil.newURI("https://" + aDomain);
       sss.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, httpsURI, 0);
-      sss.removeState(Ci.nsISiteSecurityService.HEADER_HPKP, httpsURI, 0);
     }).catch(ex => {
-      throw new Error("Exception thrown while clearing HSTS/HPKP: " + ex);
+      throw new Error("Exception thrown while clearing HSTS: " + ex);
     }));
 
     let ErrorCount = 0;