Bug 1427726 - remove nsIJARChannel::isUnsafe and nsIDocShell::channelIsUnsafe, r=bz
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 07 Feb 2018 14:25:33 +0000
changeset 412733 064ca3f3d42b04fd3fc8fe9edd670cd0adf5db22
parent 412732 a9185d7a30d832b0f93919f8522bd465e96a9b45
child 412734 964dbe5e2afe0956a3f1b08ec1c0248dfdb33820
push id33817
push userapavel@mozilla.com
push dateWed, 11 Apr 2018 14:35:14 +0000
treeherdermozilla-central@14b2d3f79612 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1427726
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1427726 - remove nsIJARChannel::isUnsafe and nsIDocShell::channelIsUnsafe, r=bz MozReview-Commit-ID: 7XyRqzKDbsq
docshell/base/nsDocShell.cpp
docshell/base/nsIDocShell.idl
dom/base/nsGlobalWindowOuter.cpp
modules/libjar/nsIJARChannel.idl
modules/libjar/nsJARChannel.cpp
modules/libjar/nsJARChannel.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -1605,34 +1605,16 @@ nsDocShell::GetParentCharset(const Encod
                              nsIPrincipal** aPrincipal)
 {
   aCharset = mParentCharset;
   *aCharsetSource = mParentCharsetSource;
   NS_IF_ADDREF(*aPrincipal = mParentCharsetPrincipal);
 }
 
 NS_IMETHODIMP
-nsDocShell::GetChannelIsUnsafe(bool* aUnsafe)
-{
-  *aUnsafe = false;
-
-  nsIChannel* channel = GetCurrentDocChannel();
-  if (!channel) {
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIJARChannel> jarChannel = do_QueryInterface(channel);
-  if (!jarChannel) {
-    return NS_OK;
-  }
-
-  return jarChannel->GetIsUnsafe(aUnsafe);
-}
-
-NS_IMETHODIMP
 nsDocShell::GetHasMixedActiveContentLoaded(bool* aHasMixedActiveContentLoaded)
 {
   nsCOMPtr<nsIDocument> doc(GetDocument());
   *aHasMixedActiveContentLoaded = doc && doc->GetHasMixedActiveContentLoaded();
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -1680,22 +1662,16 @@ nsDocShell::GetHasTrackingContentLoaded(
 }
 
 NS_IMETHODIMP
 nsDocShell::GetAllowPlugins(bool* aAllowPlugins)
 {
   NS_ENSURE_ARG_POINTER(aAllowPlugins);
 
   *aAllowPlugins = mAllowPlugins;
-  if (!mAllowPlugins) {
-    return NS_OK;
-  }
-
-  bool unsafe;
-  *aAllowPlugins = NS_SUCCEEDED(GetChannelIsUnsafe(&unsafe)) && !unsafe;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::SetAllowPlugins(bool aAllowPlugins)
 {
   mAllowPlugins = aAllowPlugins;
   // XXX should enable or disable a plugin host
@@ -1899,22 +1875,16 @@ nsDocShell::NotifyReflowObservers(bool a
 }
 
 NS_IMETHODIMP
 nsDocShell::GetAllowMetaRedirects(bool* aReturn)
 {
   NS_ENSURE_ARG_POINTER(aReturn);
 
   *aReturn = mAllowMetaRedirects;
-  if (!mAllowMetaRedirects) {
-    return NS_OK;
-  }
-
-  bool unsafe;
-  *aReturn = NS_SUCCEEDED(GetChannelIsUnsafe(&unsafe)) && !unsafe;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::SetAllowMetaRedirects(bool aValue)
 {
   mAllowMetaRedirects = aValue;
   return NS_OK;
@@ -9576,44 +9546,16 @@ nsDocShell::InternalLoad(nsIURI* aURI,
         (aFlags & INTERNAL_LOAD_FLAGS_INHERIT_PRINCIPAL) &&
          NS_SUCCEEDED(nsContentUtils::URIInheritsSecurityContext(aURI,
                                                                  &inherits)) &&
          inherits) {
       principalToInherit = GetInheritedPrincipal(true);
     }
   }
 
-  // Don't allow loads that would inherit our security context
-  // if this document came from an unsafe channel.
-  {
-    bool willInherit;
-    // This condition needs to match the one in
-    // nsContentUtils::ChannelShouldInheritPrincipal.
-    // Except we reverse the rv check to be safe in case
-    // nsContentUtils::URIInheritsSecurityContext fails here and
-    // succeeds there.
-    rv = nsContentUtils::URIInheritsSecurityContext(aURI, &willInherit);
-    if (NS_FAILED(rv) || willInherit || NS_IsAboutBlank(aURI)) {
-      nsCOMPtr<nsIDocShellTreeItem> treeItem = this;
-      do {
-        nsCOMPtr<nsIDocShell> itemDocShell = do_QueryInterface(treeItem);
-        bool isUnsafe;
-        if (itemDocShell &&
-            NS_SUCCEEDED(itemDocShell->GetChannelIsUnsafe(&isUnsafe)) &&
-            isUnsafe) {
-          return NS_ERROR_DOM_SECURITY_ERR;
-        }
-
-        nsCOMPtr<nsIDocShellTreeItem> parent;
-        treeItem->GetSameTypeParent(getter_AddRefs(parent));
-        parent.swap(treeItem);
-      } while (treeItem);
-    }
-  }
-
   nsIDocument* doc = mContentViewer ? mContentViewer->GetDocument()
                                     : nullptr;
 
   const bool isDocumentAuxSandboxed = doc &&
     (doc->GetSandboxFlags() & SANDBOXED_AUXILIARY_NAVIGATION);
 
   if (aURI && mLoadURIDelegate &&
       (!targetDocShell || targetDocShell == static_cast<nsIDocShell*>(this))) {
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -578,23 +578,16 @@ interface nsIDocShell : nsIDocShellTreeI
 
   /**
    * Find out whether the docshell is currently in the middle of a page
    * transition. This is set just before the pagehide/unload events fire.
    */
   readonly attribute boolean isInUnload;
 
   /**
-   * Find out if the currently loaded document came from a suspicious channel
-   * (such as a JAR channel where the server-returned content type isn't a
-   * known JAR type).
-   */
-  readonly attribute boolean channelIsUnsafe;
-
-  /**
    * This attribute determines whether Mixed Active Content is loaded on the
    * document. When it is true, mixed active content was not blocked and has
    * loaded (or is about to load) on the page. When it is false, mixed active content
    * has not loaded on the page, either because there was no mixed active content
    * requests on the page or such requests were blocked by nsMixedContentBlocker.
    * This boolean is set to true in nsMixedContentBlocker if Mixed Active Content
    * is allowed (either explicitly on the page by the user or when the about:config
    * setting security.mixed_content.block_active_content is set to false).
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -2000,24 +2000,16 @@ nsGlobalWindowOuter::SetNewDocument(nsID
       JS::Rooted<JSObject*> obj(cx, newInnerGlobal);
       rv = kungFuDeathGrip->InitClasses(obj);
       NS_ENSURE_SUCCESS(rv, rv);
 
       rv = newInnerWindow->ExecutionReady();
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
-    // If the document comes from a JAR, check if the channel was determined
-    // to be unsafe. If so, permanently disable script on the compartment by
-    // calling Block() and throwing away the key.
-    nsCOMPtr<nsIJARChannel> jarChannel = do_QueryInterface(aDocument->GetChannel());
-    if (jarChannel && jarChannel->GetIsUnsafe()) {
-      xpc::Scriptability::Get(newInnerGlobal).Block();
-    }
-
     if (mArguments) {
       newInnerWindow->DefineArgumentsProperty(mArguments);
       mArguments = nullptr;
     }
 
     // Give the new inner window our chrome event handler (since it
     // doesn't have one).
     newInnerWindow->mChromeEventHandler = mChromeEventHandler;
--- a/modules/libjar/nsIJARChannel.idl
+++ b/modules/libjar/nsIJARChannel.idl
@@ -7,24 +7,16 @@
 
 interface nsIFile;
 interface nsIZipEntry;
 
 [scriptable, builtinclass, uuid(e72b179b-d5df-4d87-b5de-fd73a65c60f6)]
 interface nsIJARChannel : nsIChannel
 {
     /**
-     * Returns TRUE if the JAR file is not safe (if the content type reported
-     * by the server for a remote JAR is not of an expected type).  Scripting,
-     * redirects, and plugins should be disabled when loading from this
-     * channel.
-     */
-    [infallible] readonly attribute boolean isUnsafe;
-
-    /**
      * Returns the JAR file.  May be null if the jar is remote.
      * Setting the JAR file is optional and overrides the JAR
      * file used for local file JARs. Setting the JAR file after
      * the channel has been opened is not permitted.
      */
     attribute nsIFile jarFile;
 
     /**
--- a/modules/libjar/nsJARChannel.cpp
+++ b/modules/libjar/nsJARChannel.cpp
@@ -195,17 +195,16 @@ nsJARInputThunk::IsNonBlocking(bool *non
 nsJARChannel::nsJARChannel()
     : mOpened(false)
     , mContentLength(-1)
     , mLoadFlags(LOAD_NORMAL)
     , mStatus(NS_OK)
     , mIsPending(false)
     , mEnableOMT(true)
     , mPendingEvent()
-    , mIsUnsafe(true)
 {
     LOG(("nsJARChannel::nsJARChannel [this=%p]\n", this));
     // hold an owning reference to the jar handler
     mJarHandler = gJarHandler;
 }
 
 nsJARChannel::~nsJARChannel()
 {
@@ -429,19 +428,16 @@ nsJARChannel::OpenLocalFile()
     LOG(("nsJARChannel::OpenLocalFile [this=%p]\n", this));
 
     MOZ_ASSERT(NS_IsMainThread());
 
     MOZ_ASSERT(mWorker);
     MOZ_ASSERT(mIsPending);
     MOZ_ASSERT(mJarFile);
 
-    // Local files are always considered safe.
-    mIsUnsafe = false;
-
     nsresult rv;
 
     // Set mLoadGroup and mOpened before AsyncOpen return, and set back if
     // if failed when callback.
     if (mLoadGroup) {
         mLoadGroup->AddRequest(this, nullptr);
     }
     mOpened = true;
@@ -935,17 +931,16 @@ NS_IMETHODIMP
 nsJARChannel::Open(nsIInputStream **stream)
 {
     LOG(("nsJARChannel::Open [this=%p]\n", this));
 
     NS_ENSURE_TRUE(!mOpened, NS_ERROR_IN_PROGRESS);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
 
     mJarFile = nullptr;
-    mIsUnsafe = true;
 
     nsresult rv = LookupFile();
     if (NS_FAILED(rv))
         return rv;
 
     // If mJarFile was not set by LookupFile, we can't open a channel.
     if (!mJarFile) {
         NS_NOTREACHED("only file-backed jars are supported");
@@ -954,18 +949,16 @@ nsJARChannel::Open(nsIInputStream **stre
 
     RefPtr<nsJARInputThunk> input;
     rv = CreateJarInput(gJarHandler->JarCache(), getter_AddRefs(input));
     if (NS_FAILED(rv))
         return rv;
 
     input.forget(stream);
     mOpened = true;
-    // local files are always considered safe
-    mIsUnsafe = false;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsJARChannel::Open2(nsIInputStream** aStream)
 {
     LOG(("nsJARChannel::Open2 [this=%p]\n", this));
     nsCOMPtr<nsIStreamListener> listener;
@@ -985,17 +978,16 @@ nsJARChannel::AsyncOpen(nsIStreamListene
                 nsContentUtils::IsSystemPrincipal(mLoadInfo->LoadingPrincipal())),
                "security flags in loadInfo but asyncOpen2() not called");
 
     NS_ENSURE_ARG_POINTER(listener);
     NS_ENSURE_TRUE(!mOpened, NS_ERROR_IN_PROGRESS);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
 
     mJarFile = nullptr;
-    mIsUnsafe = true;
 
     // Initialize mProgressSink
     NS_QueryNotificationCallbacks(mCallbacks, mLoadGroup, mProgressSink);
 
     mListener = listener;
     mListenerContext = ctx;
     mIsPending = true;
 
@@ -1040,23 +1032,16 @@ nsJARChannel::AsyncOpen2(nsIStreamListen
 
     return AsyncOpen(listener, nullptr);
 }
 
 //-----------------------------------------------------------------------------
 // nsIJARChannel
 //-----------------------------------------------------------------------------
 NS_IMETHODIMP
-nsJARChannel::GetIsUnsafe(bool *isUnsafe)
-{
-    *isUnsafe = mIsUnsafe;
-    return NS_OK;
-}
-
-NS_IMETHODIMP
 nsJARChannel::GetJarFile(nsIFile **aFile)
 {
     NS_IF_ADDREF(*aFile = mJarFile);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsJARChannel::SetJarFile(nsIFile *aFile)
--- a/modules/libjar/nsJARChannel.h
+++ b/modules/libjar/nsJARChannel.h
@@ -90,18 +90,16 @@ private:
 
     bool                            mEnableOMT;
     // |Cancel()|, |Suspend()|, and |Resume()| might be called during AsyncOpen.
     struct {
         bool isCanceled;
         uint32_t suspendCount;
     }                               mPendingEvent;
 
-    bool                            mIsUnsafe;
-
     nsCOMPtr<nsIInputStreamPump>    mPump;
     // mRequest is only non-null during OnStartRequest, so we'll have a pointer
     // to the request if we get called back via RetargetDeliveryTo.
     nsCOMPtr<nsIRequest>            mRequest;
     nsCOMPtr<nsIFile>               mJarFile;
     nsCOMPtr<nsIFile>               mJarFileOverride;
     nsCOMPtr<nsIZipReader>          mPreCachedJarReader;
     nsCOMPtr<nsIURI>                mJarBaseURI;