From 2407845ec187fc0bddcf061f41a5791c7041d9ff Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 31 Mar 2020 09:44:30 +0200 Subject: Issue #1280 - Un-bust certerror pages and ForgetAboutSite --- docshell/base/nsDocShell.cpp | 17 ++++------------ netwerk/protocol/http/nsHttpChannel.cpp | 13 +----------- security/manager/ssl/SSLServerCertVerification.cpp | 23 +++++----------------- toolkit/forgetaboutsite/ForgetAboutSite.jsm | 7 +++---- 4 files changed, 13 insertions(+), 47 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 6104ebfa7..f53d89e81 100644 --- a/docshell/base/nsDocShell.cpp +++ b/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 - // and the certificate is bad, don't allow overrides (RFC 6797 section - // 12.1, HPKP draft spec section 2.6). + // If this is an HTTP Strict Transport Security host, don't allow + // overrides (RFC 6797 section 12.1). uint32_t flags = UsePrivateBrowsing() ? nsISocketProvider::NO_PERMANENT_STORAGE : 0; bool isStsHost = false; - bool isPinnedHost = false; if (XRE_IsParentProcess()) { nsCOMPtr 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 - // pinned host and display a more informative message to the user. - if (isStsHost || isPinnedHost) { + if (isStsHost) { cssClass.AssignLiteral("badStsCert"); } diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 05383916f..950b1a7ab 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/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; } diff --git a/security/manager/ssl/SSLServerCertVerification.cpp b/security/manager/ssl/SSLServerCertVerification.cpp index af985eb92..37a3b809f 100644 --- a/security/manager/ssl/SSLServerCertVerification.cpp +++ b/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 - // certificate is bad, don't allow overrides (RFC 6797 section 12.1, - // HPKP draft spec section 2.6). + // If this is an HTTP Strict Transport Security host, don't allow overrides + // RFC 6797 section 12.1. bool strictTransportSecurityEnabled = false; - bool hasPinningInformation = false; nsCOMPtr 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 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)); } diff --git a/toolkit/forgetaboutsite/ForgetAboutSite.jsm b/toolkit/forgetaboutsite/ForgetAboutSite.jsm index 8c7825392..9d7e512a8 100644 --- a/toolkit/forgetaboutsite/ForgetAboutSite.jsm +++ b/toolkit/forgetaboutsite/ForgetAboutSite.jsm @@ -216,8 +216,8 @@ this.ForgetAboutSite = { }); })); - // HSTS and HPKP - // TODO (bug 1290529): also remove HSTS/HPKP information for subdomains. + // HSTS + // 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; -- cgit v1.2.3