fixup! Bug 23247: Communicating security expectations for .onion

acatarineu · Oct 3, 2019 · d32c2e6 · d32c2e6
1 parent f970a6e
commit d32c2e6
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 18 deletions.
diff --git 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
@@ -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<nsIURI> 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
@@ -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();