Bug 1217456: Add a security flag for controlling redirects. Use this flag in fetch() implementation. r=bkelly,jduell

dom/fetch/FetchDriver.cpp

@@ -29,6 +29,7 @@
 #include "nsPrintfCString.h"
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
+#include "nsHttpChannel.h"
 
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/workers/Workers.h"
@@ -51,7 +52,6 @@ FetchDriver::FetchDriver(InternalRequest* aRequest, nsIPrincipal* aPrincipal,
   , mLoadGroup(aLoadGroup)
   , mRequest(aRequest)
   , mHasBeenCrossSite(false)
-  , mFoundOpaqueRedirect(false)
   , mResponseAvailableCalled(false)
   , mFetchCalled(false)
 {
@@ -236,6 +236,10 @@ FetchDriver::HttpFetch()
     return NS_ERROR_UNEXPECTED;
   }
 
+  if (mRequest->GetRedirectMode() != RequestRedirect::Follow) {
+    secFlags |= nsILoadInfo::SEC_DONT_FOLLOW_REDIRECTS;
+  }
+
   // From here on we create a channel and set its properties with the
   // information from the InternalRequest. This is an implementation detail.
   MOZ_ASSERT(mLoadGroup);
@@ -441,7 +445,9 @@ FetchDriver::IsUnsafeRequest()
 }
 
 already_AddRefed<InternalResponse>
-FetchDriver::BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aFinalURI)
+FetchDriver::BeginAndGetFilteredResponse(InternalResponse* aResponse,
+                                         nsIURI* aFinalURI,
+                                         bool aFoundOpaqueRedirect)
 {
   MOZ_ASSERT(aResponse);
   nsAutoCString reqURL;
@@ -454,7 +460,7 @@ FetchDriver::BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aF
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   RefPtr<InternalResponse> filteredResponse;
-  if (mFoundOpaqueRedirect) {
+  if (aFoundOpaqueRedirect) {
     filteredResponse = aResponse->OpaqueRedirectResponse();
   } else {
     switch (mRequest->GetResponseTainting()) {
@@ -551,10 +557,22 @@ FetchDriver::OnStartRequest(nsIRequest* aRequest,
   nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aRequest);
   nsCOMPtr<nsIJARChannel> jarChannel = do_QueryInterface(aRequest);
 
+  bool foundOpaqueRedirect = false;
+
   if (httpChannel) {
     uint32_t responseStatus;
     httpChannel->GetResponseStatus(&responseStatus);
 
+    if (mozilla::net::nsHttpChannel::IsRedirectStatus(responseStatus)) {
+      if (mRequest->GetRedirectMode() == RequestRedirect::Error) {
+        FailWithNetworkError();
+        return NS_BINDING_FAILED;
+      }
+      if (mRequest->GetRedirectMode() == RequestRedirect::Manual) {
+        foundOpaqueRedirect = true;
+      }
+    }
+
     nsAutoCString statusText;
     httpChannel->GetResponseStatusText(statusText);
 
@@ -660,7 +678,8 @@ FetchDriver::OnStartRequest(nsIRequest* aRequest,
 
   // Resolves fetch() promise which may trigger code running in a worker.  Make
   // sure the Response is fully initialized before calling this.
-  mResponse = BeginAndGetFilteredResponse(response, channelURI);
+  mResponse = BeginAndGetFilteredResponse(response, channelURI,
+                                          foundOpaqueRedirect);
 
   nsCOMPtr<nsIEventTarget> sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -743,6 +762,12 @@ FetchDriver::AsyncOnChannelRedirect(nsIChannel* aOldChannel,
     return NS_OK;
   }
 
+  // We should only ever get here if we use a "follow" redirect policy,
+  // or if if we set an "error" policy as a result of a CORS policy.
+  MOZ_ASSERT(mRequest->GetRedirectMode() == RequestRedirect::Follow ||
+             (mRequest->GetRedirectMode() == RequestRedirect::Error &&
+              IsUnsafeRequest()));
+
   // HTTP Fetch step 5, "redirect status", step 1
   if (NS_WARN_IF(mRequest->GetRedirectMode() == RequestRedirect::Error)) {
     aOldChannel->Cancel(NS_BINDING_FAILED);
@@ -763,33 +788,10 @@ FetchDriver::AsyncOnChannelRedirect(nsIChannel* aOldChannel,
   // HTTP Fetch step 5, "redirect status", step 10 requires us to halt the
   // redirect, but successfully return an opaqueredirect Response to the
   // initiating Fetch.
-  if (mRequest->GetRedirectMode() == RequestRedirect::Manual) {
-    // Ideally we would simply not cancel the old channel and allow it to
-    // be processed as normal.  Unfortunately this is quite fragile and
-    // other redirect handlers can easily break it for certain use cases.
-    //
-    // For example, nsCORSListenerProxy cancels vetoed redirect channels.
-    // The HTTP cache will also error on vetoed redirects when the
-    // redirect has been previously cached.
-    //
-    // Therefore simulate the completion of the channel to produce the
-    // opaqueredirect Response and then cancel the original channel.  This
-    // will result in OnStartRequest() getting called twice, but the second
-    // time will be with an error response (from the Cancel) which will
-    // be ignored.
-    MOZ_ASSERT(!mFoundOpaqueRedirect);
-    mFoundOpaqueRedirect = true;
-    Unused << OnStartRequest(aOldChannel, nullptr);
-    Unused << OnStopRequest(aOldChannel, nullptr, NS_OK);
-
-    aOldChannel->Cancel(NS_BINDING_FAILED);
-
-    return NS_BINDING_FAILED;
-  }
 
   // The following steps are from HTTP Fetch step 5, "redirect status", step 11
-  // which requires the RequestRedirect to be "follow".
-  MOZ_ASSERT(mRequest->GetRedirectMode() == RequestRedirect::Follow);
+  // which requires the RequestRedirect to be "follow". We asserted that we're
+  // in either "follow" or "error" mode here.
 
   // HTTP Fetch step 5, "redirect status", steps 11.1 and 11.2 block redirecting
   // to a URL with credentials in CORS mode.  This is implemented in

dom/fetch/FetchDriver.h

@@ -83,7 +83,6 @@ private:
   RefPtr<FetchDriverObserver> mObserver;
   nsCOMPtr<nsIDocument> mDocument;
   bool mHasBeenCrossSite;
-  bool mFoundOpaqueRedirect;
 
   DebugOnly<bool> mResponseAvailableCalled;
   DebugOnly<bool> mFetchCalled;
@@ -100,7 +99,8 @@ private:
   // Returns the filtered response sent to the observer.
   // Callers who don't have access to a channel can pass null for aFinalURI.
   already_AddRefed<InternalResponse>
-  BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aFinalURI);
+  BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aFinalURI,
+                              bool aFoundOpaqueRedirect);
   // Utility since not all cases need to do any post processing of the filtered
   // response.
   nsresult FailWithNetworkError();

netwerk/base/LoadInfo.cpp

@@ -277,6 +277,14 @@ LoadInfo::GetAllowChrome(bool* aResult)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+LoadInfo::GetDontFollowRedirects(bool* aResult)
+{
+  *aResult =
+    (mSecurityFlags & nsILoadInfo::SEC_DONT_FOLLOW_REDIRECTS);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 LoadInfo::GetExternalContentPolicyType(nsContentPolicyType* aResult)
 {

netwerk/base/nsAsyncRedirectVerifyHelper.cpp

@@ -12,6 +12,7 @@
 #include "nsIChannel.h"
 #include "nsIHttpChannelInternal.h"
 #include "nsIAsyncVerifyRedirectCallback.h"
+#include "nsILoadInfo.h"
 
 static mozilla::LazyLogModule gRedirectLog("nsRedirect");
 #undef LOG
@@ -65,6 +66,15 @@ nsAsyncRedirectVerifyHelper::Init(nsIChannel* oldChan, nsIChannel* newChan,
     mFlags             = flags;
     mCallbackThread    = do_GetCurrentThread();
 
+    if (!(flags & (nsIChannelEventSink::REDIRECT_INTERNAL |
+                   nsIChannelEventSink::REDIRECT_STS_UPGRADE))) {
+      nsCOMPtr<nsILoadInfo> loadInfo = oldChan->GetLoadInfo();
+      if (loadInfo && loadInfo->GetDontFollowRedirects()) {
+        ExplicitCallback(NS_BINDING_ABORTED);
+        return NS_OK;
+      }
+    }
+
     if (synchronize)
       mWaitingForRedirectCallback = true;
 

netwerk/base/nsILoadInfo.idl

@@ -26,7 +26,7 @@ typedef unsigned long nsSecurityFlags;
 /**
  * An nsILoadOwner represents per-load information about who started the load.
  */
-[scriptable, builtinclass, uuid(542b38c6-1c7b-4630-bd6b-9fee0d1b474d)]
+[scriptable, builtinclass, uuid(c21803eb-ad4f-46bd-b0a5-8081381253f2)]
 interface nsILoadInfo : nsISupports
 {
   /**
@@ -131,6 +131,19 @@ interface nsILoadInfo : nsISupports
    */
   const unsigned long SEC_ALLOW_CHROME = (1<<9);
 
+  /**
+   * Don't follow redirects. Instead the redirect response is returned
+   * as a successful response for the channel.
+   *
+   * Redirects not initiated by a server response, i.e. REDIRECT_INTERNAL and
+   * REDIRECT_STS_UPGRADE, are still followed.
+   *
+   * Note: If this flag is set and the channel response is a redirect, then
+   * the response body might not be available.
+   * This can happen if the redirect was cached.
+   */
+  const unsigned long SEC_DONT_FOLLOW_REDIRECTS = (1<<10);
+
   /**
    * The loadingPrincipal is the principal that is responsible for the load.
    * It is *NOT* the principal tied to the resource/URI that this
@@ -252,6 +265,11 @@ interface nsILoadInfo : nsISupports
    */
   [infallible] readonly attribute boolean allowChrome;
 
+  /**
+   * Returns true if SEC_DONT_FOLLOW_REDIRECTS is set.
+   */
+  [infallible] readonly attribute boolean dontFollowRedirects;
+
   /**
    * The external contentPolicyType of the channel, used for security checks
    * like Mixed Content Blocking and Content Security Policy.

netwerk/protocol/http/nsHttpChannel.cpp

@@ -161,13 +161,6 @@ Hash(const char *buf, nsACString &hash)
     return NS_OK;
 }
 
-bool IsRedirectStatus(uint32_t status)
-{
-    // 305 disabled as a security measure (see bug 187996).
-    return status == 300 || status == 301 || status == 302 || status == 303 ||
-           status == 307 || status == 308;
-}
-
 } // unnamed namespace
 
 // We only treat 3xx responses as redirects if they have a Location header and
@@ -175,7 +168,7 @@ bool IsRedirectStatus(uint32_t status)
 bool
 WillRedirect(const nsHttpResponseHead * response)
 {
-    return IsRedirectStatus(response->Status()) &&
+    return nsHttpChannel::IsRedirectStatus(response->Status()) &&
            response->PeekHeader(nsHttp::Location);
 }
 
@@ -611,16 +604,27 @@ nsHttpChannel::ContinueHandleAsyncRedirect(nsresult rv)
         // If AsyncProcessRedirection fails, then we have to send out the
         // OnStart/OnStop notifications.
         LOG(("ContinueHandleAsyncRedirect got failure result [rv=%x]\n", rv));
-        mStatus = rv;
-        DoNotifyListener();
+
+        bool redirectsEnabled =
+            !mLoadInfo || !mLoadInfo->GetDontFollowRedirects();
+
+        if (redirectsEnabled) {
+            // TODO: stop failing original channel if redirect vetoed?
+            mStatus = rv;
+
+            DoNotifyListener();
+
+            // Blow away cache entry if we couldn't process the redirect
+            // for some reason (the cache entry might be corrupt).
+            if (mCacheEntry) {
+                mCacheEntry->AsyncDoom(nullptr);
+            }
+        }
+        else {
+            DoNotifyListener();
+        }
     }
 
-    // close the cache entry.  Blow it away if we couldn't process the redirect
-    // for some reason (the cache entry might be corrupt).
-    if (mCacheEntry) {
-        if (NS_FAILED(rv))
-            mCacheEntry->AsyncDoom(nullptr);
-    }
     CloseCacheEntry(false);
 
     mIsPending = false;
@@ -7071,6 +7075,14 @@ nsHttpChannel::OnPush(const nsACString &url, Http2PushedStream *pushedStream)
     return rv;
 }
 
+// static
+bool nsHttpChannel::IsRedirectStatus(uint32_t status)
+{
+    // 305 disabled as a security measure (see bug 187996).
+    return status == 300 || status == 301 || status == 302 || status == 303 ||
+           status == 307 || status == 308;
+}
+
 void
 nsHttpChannel::SetCouldBeSynthesized()
 {

netwerk/protocol/http/nsHttpChannel.h

@@ -122,6 +122,9 @@ public:
 
     nsresult OnPush(const nsACString &uri, Http2PushedStream *pushedStream);
 
+    static bool IsRedirectStatus(uint32_t status);
+
+
     // Methods HttpBaseChannel didn't implement for us or that we override.
     //
     // nsIRequest