From d32c2e6795ddb9a4dcff982ccc6d48b54dfaddcd Mon Sep 17 00:00:00 2001 From: Alex Catarineu Date: Fri, 20 Sep 2019 12:48:40 +0200 Subject: [PATCH] fixup! Bug 23247: Communicating security expectations for .onion --- browser/base/content/browser-siteIdentity.js | 4 ++ docshell/base/nsDocShell.cpp | 4 +- .../manager/ssl/nsSecureBrowserUIImpl.cpp | 46 ++++++++++++------- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/browser/base/content/browser-siteIdentity.js b/browser/base/content/browser-siteIdentity.js index 12d9a29bf5122..9da2e289b86d8 100644 --- a/browser/base/content/browser-siteIdentity.js +++ b/browser/base/content/browser-siteIdentity.js @@ -612,6 +612,10 @@ var gIdentityHandler = { * built-in (returns false) or imported (returns true). */ _hasCustomRoot() { + if (!this._secInfo) { + return false; + } + let issuerCert = null; // Walk the whole chain to get the last cert. // eslint-disable-next-line no-empty diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 578a06d61a83b..7f3d8a3e05427 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -63,6 +63,7 @@ #include "mozilla/dom/ChildSHistory.h" #include "mozilla/dom/nsCSPContext.h" #include "mozilla/dom/LoadURIOptionsBinding.h" +#include "mozilla/dom/nsMixedContentBlocker.h" #include "mozilla/net/ReferrerPolicy.h" #include "mozilla/net/UrlClassifierFeatureFactory.h" @@ -5583,7 +5584,8 @@ nsDocShell::GetAllowMixedContentAndConnectionData( // aRootHasSecureConnection should be false. nsCOMPtr rootUri = rootPrincipal->GetURI(); if (nsContentUtils::IsSystemPrincipal(rootPrincipal) || !rootUri || - !SchemeIsHTTPS(rootUri)) { + (!SchemeIsHTTPS(rootUri) && + !nsMixedContentBlocker::IsPotentiallyTrustworthyOnion(rootUri))) { *aRootHasSecureConnection = false; } diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.cpp b/security/manager/ssl/nsSecureBrowserUIImpl.cpp index 2aba013650f05..ea1dfca20b6b5 100644 --- a/security/manager/ssl/nsSecureBrowserUIImpl.cpp +++ b/security/manager/ssl/nsSecureBrowserUIImpl.cpp @@ -9,6 +9,7 @@ #include "mozilla/Logging.h" #include "mozilla/Unused.h" #include "mozilla/dom/Document.h" +#include "mozilla/dom/nsMixedContentBlocker.h" #include "nsContentUtils.h" #include "nsIChannel.h" #include "nsDocShell.h" @@ -246,8 +247,8 @@ static nsresult URICanBeConsideredSecure( return rv; } - nsAutoCString host; - bool isOnion = NS_SUCCEEDED(innermostURI->GetHost(host)) && StringEndsWith(host, NS_LITERAL_CSTRING(".onion")); + bool isOnion = + nsMixedContentBlocker::IsPotentiallyTrustworthyOnion(innermostURI); canBeConsideredSecure = isHttps || isOnion; @@ -314,24 +315,35 @@ nsresult nsSecureBrowserUIImpl::UpdateStateAndSecurityInfo(nsIChannel* channel, if (NS_FAILED(rv)) { return rv; } - // If the security state is STATE_IS_INSECURE, the TLS handshake never - // completed. Don't set any further state. - if (mState == STATE_IS_INSECURE) { - return NS_OK; + // Skip setting some state if mState == STATE_IS_INSECURE (TLS handshake + // never completed). But do not return in that case, since a + // STATE_IS_INSECURE can still be changed later to STATE_IS_SECURE if it's + // routed over tor (.onion). + if (mState != STATE_IS_INSECURE) { + mTopLevelSecurityInfo = securityInfo; + MOZ_LOG(gSecureBrowserUILog, LogLevel::Debug, + (" set mTopLevelSecurityInfo")); + bool isEV; + rv = mTopLevelSecurityInfo->GetIsExtendedValidation(&isEV); + if (NS_FAILED(rv)) { + return rv; + } + if (isEV) { + MOZ_LOG(gSecureBrowserUILog, LogLevel::Debug, (" is EV")); + mState |= STATE_IDENTITY_EV_TOPLEVEL; + } } + } - mTopLevelSecurityInfo = securityInfo; - MOZ_LOG(gSecureBrowserUILog, LogLevel::Debug, - (" set mTopLevelSecurityInfo")); - bool isEV; - rv = mTopLevelSecurityInfo->GetIsExtendedValidation(&isEV); - if (NS_FAILED(rv)) { - return rv; - } - if (isEV) { - MOZ_LOG(gSecureBrowserUILog, LogLevel::Debug, (" is EV")); - mState |= STATE_IDENTITY_EV_TOPLEVEL; + // any protocol routed over tor is secure + if ((mState & STATE_IS_SECURE) == 0) { + if (nsMixedContentBlocker::IsPotentiallyTrustworthyOnion(uri)) { + MOZ_LOG(gSecureBrowserUILog, LogLevel::Debug, (" URI is onion")); + mState = STATE_IS_SECURE; } + } + + if (mState != STATE_IS_INSECURE) { // Proactively check for mixed content in case GetState() is never called // (this can happen when loading from the BF cache). CheckForMixedContent();