Bug 1622028 - Log failed upgrades for HTTPS Only Mode. r=ckerschb,dra…

…gana Differential Revision: https://phabricator.services.mozilla.com/D68026 --HG-- extra : moz-landing-system : lando
mozilla · Mar 27, 2020 · b9a66ff · b9a66ff
1 parent 40755c3
commit b9a66ff
Show file tree

Hide file tree

Showing 18 changed files with 200 additions and 77 deletions.
diff --git a/dom/locales/en-US/chrome/security/security.properties b/dom/locales/en-US/chrome/security/security.properties
@@ -147,4 +147,6 @@ XFOSameOrigin = Load denied by X-Frame-Options: “%1$S” from “%2$S”, site
 # LOCALIZATION NOTE: %1$S is the URL of the upgraded request; %2$S is the upgraded scheme.
 HTTPSOnlyUpgradeRequest = Upgrading insecure request “%1$S” to use “%2$S”.
 # LOCALIZATION NOTE: %1$S is the URL of request.
-HTTPSOnlyNoUpgrade = Request for “%1$S” was not upgraded because it had the NoUpgrade-flag.
+HTTPSOnlyNoUpgradeException = Not upgrading insecure request “%1$S” because it is exempt.
+# LOCALIZATION NOTE: %1$S is the URL of the failed request; %2$S is an error-code.
+HTTPSOnlyFailedRequest = Upgrading insecure request “%1$S” failed. (%2$S)
diff --git a/dom/security/moz.build b/dom/security/moz.build
@@ -21,6 +21,7 @@ EXPORTS.mozilla.dom += [
     'nsCSPContext.h',
     'nsCSPService.h',
     'nsCSPUtils.h',
+    'nsHTTPSOnlyStreamListener.h',
     'nsHTTPSOnlyUtils.h',
     'nsMixedContentBlocker.h',
     'PolicyTokenizer.h',
@@ -49,6 +50,7 @@ UNIFIED_SOURCES += [
     'nsCSPParser.cpp',
     'nsCSPService.cpp',
     'nsCSPUtils.cpp',
+    'nsHTTPSOnlyStreamListener.cpp',
     'nsHTTPSOnlyUtils.cpp',
     'nsMixedContentBlocker.cpp',
     'PolicyTokenizer.cpp',

diff --git a/dom/security/nsHTTPSOnlyStreamListener.cpp b/dom/security/nsHTTPSOnlyStreamListener.cpp
@@ -0,0 +1,66 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsHTTPSOnlyStreamListener.h"
+#include "nsHTTPSOnlyUtils.h"
+#include "nsCOMPtr.h"
+#include "nsIChannel.h"
+#include "nsIRequest.h"
+#include "nsITransportSecurityInfo.h"
+#include "nsIURI.h"
+#include "nsPrintfCString.h"
+
+NS_IMPL_ISUPPORTS(nsHTTPSOnlyStreamListener, nsIStreamListener,
+                  nsIRequestObserver)
+
+nsHTTPSOnlyStreamListener::nsHTTPSOnlyStreamListener(
+    nsIStreamListener* aListener) {
+  mListener = aListener;
+}
+
+NS_IMETHODIMP
+nsHTTPSOnlyStreamListener::OnDataAvailable(nsIRequest* aRequest,
+                                           nsIInputStream* aInputStream,
+                                           uint64_t aOffset, uint32_t aCount) {
+  return mListener->OnDataAvailable(aRequest, aInputStream, aOffset, aCount);
+}
+
+NS_IMETHODIMP
+nsHTTPSOnlyStreamListener::OnStartRequest(nsIRequest* request) {
+  return mListener->OnStartRequest(request);
+}
+
+NS_IMETHODIMP
+nsHTTPSOnlyStreamListener::OnStopRequest(nsIRequest* request,
+                                         nsresult aStatus) {
+  // If the request failed we'll log it to the console with the error-code
+  if (NS_FAILED(aStatus)) {
+    nsresult rv;
+    // Try to query for the channel-object
+    nsCOMPtr<nsIChannel> channel = do_QueryInterface(request, &rv);
+    if (NS_SUCCEEDED(rv)) {
+      nsCOMPtr<nsILoadInfo> loadInfo = channel->LoadInfo();
+      uint32_t innerWindowId = loadInfo->GetInnerWindowID();
+
+      nsCOMPtr<nsIURI> uri;
+      rv = channel->GetURI(getter_AddRefs(uri));
+      if (NS_SUCCEEDED(rv)) {
+        // Logging URI as well as Module- and Error-Code
+        AutoTArray<nsString, 2> params = {
+            NS_ConvertUTF8toUTF16(uri->GetSpecOrDefault()),
+            NS_ConvertUTF8toUTF16(nsPrintfCString("M%u-C%u",
+                                                  NS_ERROR_GET_MODULE(aStatus),
+                                                  NS_ERROR_GET_CODE(aStatus)))};
+        nsHTTPSOnlyUtils::LogLocalizedString(
+            "HTTPSOnlyFailedRequest", params, nsIScriptError::errorFlag,
+            innerWindowId, !!loadInfo->GetOriginAttributes().mPrivateBrowsingId,
+            uri);
+      }
+    }
+  }
+
+  return mListener->OnStopRequest(request, aStatus);
+}
diff --git a/dom/security/nsHTTPSOnlyStreamListener.h b/dom/security/nsHTTPSOnlyStreamListener.h
@@ -0,0 +1,32 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef nsHTTPSOnlyStreamListener_h___
+#define nsHTTPSOnlyStreamListener_h___
+
+#include "nsIStreamListener.h"
+#include "nsCOMPtr.h"
+
+/**
+ * This event listener gets registered for requests that have been upgraded
+ * using the HTTPS-only mode to log failed upgrades to the console.
+ */
+class nsHTTPSOnlyStreamListener : public nsIStreamListener {
+ public:
+  // nsISupports methods
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIREQUESTOBSERVER
+  NS_DECL_NSISTREAMLISTENER
+
+  explicit nsHTTPSOnlyStreamListener(nsIStreamListener* aListener);
+
+ private:
+  virtual ~nsHTTPSOnlyStreamListener() = default;
+
+  nsCOMPtr<nsIStreamListener> mListener;
+};
+
+#endif /* nsHTTPSOnlyStreamListener_h___ */
diff --git a/dom/security/nsHTTPSOnlyUtils.cpp b/dom/security/nsHTTPSOnlyUtils.cpp
@@ -18,14 +18,16 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeRequest(nsIURI* aURI,
     return false;
   }
   // 2. Check if NoUpgrade-flag is set in LoadInfo
-  if (aLoadInfo->GetHttpsOnlyNoUpgrade()) {
+  uint32_t httpsOnlyStatus = aLoadInfo->GetHttpsOnlyStatus();
+  if (httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_EXEMPT) {
     // Let's log to the console, that we didn't upgrade this request
     uint32_t innerWindowId = aLoadInfo->GetInnerWindowID();
-    AutoTArray<nsString, 2> params = {
+    AutoTArray<nsString, 1> params = {
         NS_ConvertUTF8toUTF16(aURI->GetSpecOrDefault())};
     nsHTTPSOnlyUtils::LogLocalizedString(
-        "HTTPSOnlyNoUpgrade", params, nsIScriptError::infoFlag, innerWindowId,
-        !!aLoadInfo->GetOriginAttributes().mPrivateBrowsingId, aURI);
+        "HTTPSOnlyNoUpgradeException", params, nsIScriptError::infoFlag,
+        innerWindowId, !!aLoadInfo->GetOriginAttributes().mPrivateBrowsingId,
+        aURI);
     return false;
   }
 
@@ -46,6 +48,14 @@ bool nsHTTPSOnlyUtils::ShouldUpgradeRequest(nsIURI* aURI,
       innerWindowId, !!aLoadInfo->GetOriginAttributes().mPrivateBrowsingId,
       aURI);
 
+  // If the status was not determined before, we now indicate that the request
+  // will get upgraded, but no event-listener has been registered yet.
+  if (httpsOnlyStatus & nsILoadInfo::HTTPS_ONLY_UNINITIALIZED) {
+    httpsOnlyStatus ^= nsILoadInfo::HTTPS_ONLY_UNINITIALIZED;
+    httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_UPGRADED_LISTENER_NOT_REGISTERED;
+    aLoadInfo->SetHttpsOnlyStatus(httpsOnlyStatus);
+  }
+
   return true;
 }
 
@@ -79,32 +89,8 @@ void nsHTTPSOnlyUtils::LogMessage(const nsAString& aMessage, uint32_t aFlags,
                                               aInnerWindowID, aURI);
   } else {
     // Send to browser console
-    LogSimpleConsoleError(message, category.get(), aFromPrivateWindow,
-                          true /* from chrome context */, aFlags);
-  }
-}
-
-/* static */
-void nsHTTPSOnlyUtils::LogSimpleConsoleError(const nsAString& aErrorText,
-                                             const char* aCategory,
-                                             bool aFromPrivateWindow,
-                                             bool aFromChromeContext,
-                                             uint32_t aErrorFlags) {
-  nsCOMPtr<nsIScriptError> scriptError =
-      do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
-  if (!scriptError) {
-    return;
-  }
-  nsCOMPtr<nsIConsoleService> console =
-      do_GetService(NS_CONSOLESERVICE_CONTRACTID);
-  if (!console) {
-    return;
-  }
-  nsresult rv = scriptError->Init(aErrorText, EmptyString(), EmptyString(), 0,
-                                  0, aErrorFlags, aCategory, aFromPrivateWindow,
-                                  aFromChromeContext);
-  if (NS_FAILED(rv)) {
-    return;
+    nsContentUtils::LogSimpleConsoleError(
+        message, category.get(), aFromPrivateWindow,
+        true /* from chrome context */, aFlags);
   }
-  console->LogMessage(scriptError);
 }
diff --git a/dom/security/nsHTTPSOnlyUtils.h b/dom/security/nsHTTPSOnlyUtils.h
@@ -46,19 +46,6 @@ class nsHTTPSOnlyUtils {
   static void LogMessage(const nsAString& aMessage, uint32_t aFlags,
                          uint64_t aInnerWindowID, bool aFromPrivateWindow,
                          nsIURI* aURI = nullptr);
-
-  /**
-   * Report simple error message to the browser console
-   *   @param aErrorText the error message
-   *   @param aCategory Name of the module reporting error
-   *   @param aFromPrivateWindow Whether from private window or not
-   *   @param aFromChromeContext Whether from chrome context or not
-   *   @param [aErrorFlags] See nsIScriptError.
-   */
-  static void LogSimpleConsoleError(
-      const nsAString& aErrorText, const char* aCategory,
-      bool aFromPrivateWindow, bool aFromChromeContext,
-      uint32_t aErrorFlags = nsIScriptError::errorFlag);
 };
 
 #endif /* nsHTTPSOnlyUtils_h___ */
diff --git a/ipc/glue/BackgroundUtils.cpp b/ipc/glue/BackgroundUtils.cpp
@@ -580,7 +580,7 @@ nsresult LoadInfoToLoadInfoArgs(nsILoadInfo* aLoadInfo,
       aLoadInfo->GetDocumentHasLoaded(),
       aLoadInfo->GetAllowListFutureDocumentsCreatedFromThisRedirectChain(),
       cspNonce, aLoadInfo->GetSkipContentSniffing(),
-      aLoadInfo->GetHttpsOnlyNoUpgrade(),
+      aLoadInfo->GetHttpsOnlyStatus(),
       aLoadInfo->GetIsFromProcessingFrameAttributes(), cookieJarSettingsArgs,
       aLoadInfo->GetRequestBlockingReason(), maybeCspToInheritInfo));
 
@@ -778,7 +778,7 @@ nsresult LoadInfoArgsToLoadInfo(
       loadInfoArgs.documentHasLoaded(),
       loadInfoArgs.allowListFutureDocumentsCreatedFromThisRedirectChain(),
       loadInfoArgs.cspNonce(), loadInfoArgs.skipContentSniffing(),
-      loadInfoArgs.httpsOnlyNoUpgrade(), loadInfoArgs.requestBlockingReason(),
+      loadInfoArgs.httpsOnlyStatus(), loadInfoArgs.requestBlockingReason(),
       loadingContext);
 
   if (loadInfoArgs.isFromProcessingFrameAttributes()) {
@@ -794,8 +794,8 @@ void LoadInfoToParentLoadInfoForwarder(
   if (!aLoadInfo) {
     *aForwarderArgsOut = ParentLoadInfoForwarderArgs(
         false, false, Nothing(), nsILoadInfo::TAINTING_BASIC,
-        false,  // SkipContentSniffing
-        false,  // HttpsOnlyNoUpgrade
+        false,                                  // SkipContentSniffing
+        nsILoadInfo::HTTPS_ONLY_UNINITIALIZED,  // httpsOnlyStatus
         false,  // serviceWorkerTaintingSynthesized
         false,  // documentHasUserInteracted
         false,  // documentHasLoaded
@@ -830,7 +830,7 @@ void LoadInfoToParentLoadInfoForwarder(
   *aForwarderArgsOut = ParentLoadInfoForwarderArgs(
       aLoadInfo->GetAllowInsecureRedirectToDataURI(),
       aLoadInfo->GetBypassCORSChecks(), ipcController, tainting,
-      aLoadInfo->GetSkipContentSniffing(), aLoadInfo->GetHttpsOnlyNoUpgrade(),
+      aLoadInfo->GetSkipContentSniffing(), aLoadInfo->GetHttpsOnlyStatus(),
       aLoadInfo->GetServiceWorkerTaintingSynthesized(),
       aLoadInfo->GetDocumentHasUserInteracted(),
       aLoadInfo->GetDocumentHasLoaded(),
@@ -869,7 +869,7 @@ nsresult MergeParentLoadInfoForwarder(
   rv = aLoadInfo->SetSkipContentSniffing(aForwarderArgs.skipContentSniffing());
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = aLoadInfo->SetHttpsOnlyNoUpgrade(aForwarderArgs.httpsOnlyNoUpgrade());
+  rv = aLoadInfo->SetHttpsOnlyStatus(aForwarderArgs.httpsOnlyStatus());
   NS_ENSURE_SUCCESS(rv, rv);
 
   MOZ_ALWAYS_SUCCEEDS(aLoadInfo->SetDocumentHasUserInteracted(

diff --git a/netwerk/base/LoadInfo.cpp b/netwerk/base/LoadInfo.cpp
@@ -103,7 +103,7 @@ LoadInfo::LoadInfo(
       mDocumentHasLoaded(false),
       mAllowListFutureDocumentsCreatedFromThisRedirectChain(false),
       mSkipContentSniffing(false),
-      mHttpsOnlyNoUpgrade(false),
+      mHttpsOnlyStatus(nsILoadInfo::HTTPS_ONLY_UNINITIALIZED),
       mIsFromProcessingFrameAttributes(false) {
   MOZ_ASSERT(mLoadingPrincipal);
   MOZ_ASSERT(mTriggeringPrincipal);
@@ -366,7 +366,7 @@ LoadInfo::LoadInfo(nsPIDOMWindowOuter* aOuterWindow,
       mDocumentHasLoaded(false),
       mAllowListFutureDocumentsCreatedFromThisRedirectChain(false),
       mSkipContentSniffing(false),
-      mHttpsOnlyNoUpgrade(false),
+      mHttpsOnlyStatus(nsILoadInfo::HTTPS_ONLY_UNINITIALIZED),
       mIsFromProcessingFrameAttributes(false) {
   // Top-level loads are never third-party
   // Grab the information we can out of the window.
@@ -467,7 +467,7 @@ LoadInfo::LoadInfo(dom::CanonicalBrowsingContext* aBrowsingContext,
       mDocumentHasLoaded(false),
       mAllowListFutureDocumentsCreatedFromThisRedirectChain(false),
       mSkipContentSniffing(false),
-      mHttpsOnlyNoUpgrade(false),
+      mHttpsOnlyStatus(nsILoadInfo::HTTPS_ONLY_UNINITIALIZED),
       mIsFromProcessingFrameAttributes(false) {
   // Top-level loads are never third-party
   // Grab the information we can out of the window.
@@ -568,7 +568,7 @@ LoadInfo::LoadInfo(const LoadInfo& rhs)
           rhs.mAllowListFutureDocumentsCreatedFromThisRedirectChain),
       mCspNonce(rhs.mCspNonce),
       mSkipContentSniffing(rhs.mSkipContentSniffing),
-      mHttpsOnlyNoUpgrade(rhs.mHttpsOnlyNoUpgrade),
+      mHttpsOnlyStatus(rhs.mHttpsOnlyStatus),
       mIsFromProcessingFrameAttributes(rhs.mIsFromProcessingFrameAttributes) {}
 
 LoadInfo::LoadInfo(
@@ -606,7 +606,7 @@ LoadInfo::LoadInfo(
     bool aDocumentHasLoaded,
     bool aAllowListFutureDocumentsCreatedFromThisRedirectChain,
     const nsAString& aCspNonce, bool aSkipContentSniffing,
-    bool aHttpsOnlyNoUpgrade, uint32_t aRequestBlockingReason,
+    uint32_t aHttpsOnlyStatus, uint32_t aRequestBlockingReason,
     nsINode* aLoadingContext)
     : mLoadingPrincipal(aLoadingPrincipal),
       mTriggeringPrincipal(aTriggeringPrincipal),
@@ -663,7 +663,7 @@ LoadInfo::LoadInfo(
           aAllowListFutureDocumentsCreatedFromThisRedirectChain),
       mCspNonce(aCspNonce),
       mSkipContentSniffing(aSkipContentSniffing),
-      mHttpsOnlyNoUpgrade(aHttpsOnlyNoUpgrade),
+      mHttpsOnlyStatus(aHttpsOnlyStatus),
       mIsFromProcessingFrameAttributes(false) {
   // Only top level TYPE_DOCUMENT loads can have a null loadingPrincipal
   MOZ_ASSERT(mLoadingPrincipal ||
@@ -1470,14 +1470,14 @@ LoadInfo::SetSkipContentSniffing(bool aSkipContentSniffing) {
 }
 
 NS_IMETHODIMP
-LoadInfo::GetHttpsOnlyNoUpgrade(bool* aHttpsOnlyNoUpgrade) {
-  *aHttpsOnlyNoUpgrade = mHttpsOnlyNoUpgrade;
+LoadInfo::GetHttpsOnlyStatus(uint32_t* aHttpsOnlyStatus) {
+  *aHttpsOnlyStatus = mHttpsOnlyStatus;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-LoadInfo::SetHttpsOnlyNoUpgrade(bool aHttpsOnlyNoUpgrade) {
-  mHttpsOnlyNoUpgrade = aHttpsOnlyNoUpgrade;
+LoadInfo::SetHttpsOnlyStatus(uint32_t aHttpsOnlyStatus) {
+  mHttpsOnlyStatus = aHttpsOnlyStatus;
   return NS_OK;
 }
 

diff --git a/netwerk/base/LoadInfo.h b/netwerk/base/LoadInfo.h
@@ -163,7 +163,7 @@ class LoadInfo final : public nsILoadInfo {
            bool aDocumentHasUserInteracted, bool aDocumentHasLoaded,
            bool aAllowListFutureDocumentsCreatedFromThisRedirectChain,
            const nsAString& aCspNonce, bool aSkipContentSniffing,
-           bool aHttpsOnlyNoUpgrade, uint32_t aRequestBlockingReason,
+           uint32_t aHttpsOnlyStatus, uint32_t aRequestBlockingReason,
            nsINode* aLoadingContext);
   LoadInfo(const LoadInfo& rhs);
 
@@ -259,7 +259,7 @@ class LoadInfo final : public nsILoadInfo {
   bool mAllowListFutureDocumentsCreatedFromThisRedirectChain;
   nsString mCspNonce;
   bool mSkipContentSniffing;
-  bool mHttpsOnlyNoUpgrade;
+  uint32_t mHttpsOnlyStatus;
 
   // Is true if this load was triggered by processing the attributes of the
   // browsing context container.

diff --git a/netwerk/base/NetworkConnectivityService.cpp b/netwerk/base/NetworkConnectivityService.cpp
@@ -209,7 +209,9 @@ static inline already_AddRefed<nsIChannel> SetupIPCheckChannel(bool ipv4) {
   {
     // Prevent HTTPS-Only Mode from upgrading the OCSP request.
     nsCOMPtr<nsILoadInfo> loadInfo = channel->LoadInfo();
-    loadInfo->SetHttpsOnlyNoUpgrade(true);
+    uint32_t httpsOnlyStatus = loadInfo->GetHttpsOnlyStatus();
+    httpsOnlyStatus |= nsILoadInfo::HTTPS_ONLY_EXEMPT;
+    loadInfo->SetHttpsOnlyStatus(httpsOnlyStatus);
   }
 
   NS_ENSURE_SUCCESS(rv, nullptr);

diff --git a/netwerk/base/TRRLoadInfo.cpp b/netwerk/base/TRRLoadInfo.cpp
@@ -587,12 +587,12 @@ already_AddRefed<nsIContentSecurityPolicy> TRRLoadInfo::GetCspToInherit() {
 }
 
 NS_IMETHODIMP
-TRRLoadInfo::GetHttpsOnlyNoUpgrade(bool* aHttpsOnlyNoUpgrade) {
+TRRLoadInfo::GetHttpsOnlyStatus(uint32_t* aHttpsOnlyStatus) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
-TRRLoadInfo::SetHttpsOnlyNoUpgrade(bool aHttpsOnlyNoUpgrade) {
+TRRLoadInfo::SetHttpsOnlyStatus(uint32_t aHttpsOnlyStatus) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }