Bug 1656296 - Pass Blocked Downloads to the DownloadsUI r=ckerschb,Gijs

***

***
Added Better Testing

Differential Revision: https://phabricator.services.mozilla.com/D85439
This commit is contained in:
Sebastian Streich 2020-08-24 15:29:23 +00:00
parent e8197c08de
commit 27e2164c57
13 changed files with 134 additions and 38 deletions

View File

@ -539,6 +539,7 @@ DownloadsViewUI.DownloadElementShell.prototype = {
case Downloads.Error.BLOCK_VERDICT_UNCOMMON:
this.showButton("askOpenOrRemoveFile");
break;
case Downloads.Error.BLOCK_VERDICT_INSECURE:
case Downloads.Error.BLOCK_VERDICT_POTENTIALLY_UNWANTED:
this.showButton("askRemoveFileOrAllow");
break;
@ -626,6 +627,8 @@ DownloadsViewUI.DownloadElementShell.prototype = {
switch (this.download.error.reputationCheckVerdict) {
case Downloads.Error.BLOCK_VERDICT_UNCOMMON:
return [s.blockedUncommon2, [s.unblockTypeUncommon2, s.unblockTip2]];
case Downloads.Error.BLOCK_VERDICT_INSECURE:
return [s.blockedInsecure, [s.blockedInsecure, s.unblockTip2]];
case Downloads.Error.BLOCK_VERDICT_POTENTIALLY_UNWANTED:
return [
s.blockedPotentiallyUnwanted,

View File

@ -8,6 +8,7 @@ add_task(async function mainTest() {
Downloads.Error.BLOCK_VERDICT_UNCOMMON,
Downloads.Error.BLOCK_VERDICT_MALWARE,
Downloads.Error.BLOCK_VERDICT_POTENTIALLY_UNWANTED,
Downloads.Error.BLOCK_VERDICT_INSECURE,
];
await task_addDownloads(verdicts.map(v => makeDownload(v)));

View File

@ -32,6 +32,7 @@ stateBlockedParentalControls=Blocked by Parental Controls
# be longer than the other existing status strings.
blockedMalware=This file contains a virus or malware.
blockedPotentiallyUnwanted=This file may harm your computer.
blockedInsecure = This file could not be downloaded securely.
blockedUncommon2=This file is not commonly downloaded.
# LOCALIZATION NOTE (fileMovedOrMissing):

View File

@ -13,6 +13,7 @@
#include "nsIHttpChannel.h"
#include "nsIMultiPartChannel.h"
#include "nsIURI.h"
#include "nsITransfer.h"
#if defined(XP_WIN)
# include "WinUtils.h"
# include <wininet.h>
@ -1115,7 +1116,7 @@ void nsContentSecurityUtils::LogMessageToConsole(nsIHttpChannel* aChannel,
}
/* static */
bool nsContentSecurityUtils::IsDownloadAllowed(
long nsContentSecurityUtils::ClassifyDownload(
nsIChannel* aChannel, const nsAutoCString& aMimeTypeGuess) {
MOZ_ASSERT(aChannel, "IsDownloadAllowed without channel?");
@ -1151,15 +1152,15 @@ bool nsContentSecurityUtils::IsDownloadAllowed(
if (httpChannel) {
LogMessageToConsole(httpChannel, "MixedContentBlockedDownload");
}
return false;
return nsITransfer::DOWNLOAD_POTENTIALLY_UNSAFE;
}
if (loadInfo->TriggeringPrincipal()->IsSystemPrincipal()) {
return true;
return nsITransfer::DOWNLOAD_ACCEPTABLE;
}
if (!StaticPrefs::dom_block_download_in_sandboxed_iframes()) {
return true;
return nsITransfer::DOWNLOAD_ACCEPTABLE;
}
uint32_t triggeringFlags = loadInfo->GetTriggeringSandboxFlags();
@ -1171,8 +1172,8 @@ bool nsContentSecurityUtils::IsDownloadAllowed(
if (httpChannel) {
LogMessageToConsole(httpChannel, "IframeSandboxBlockedDownload");
}
return false;
return nsITransfer::DOWNLOAD_FORBIDDEN;
}
return true;
}
return nsITransfer::DOWNLOAD_ACCEPTABLE;
}

View File

@ -51,8 +51,8 @@ class nsContentSecurityUtils {
static void PerformCSPFrameAncestorAndXFOCheck(nsIChannel* aChannel);
// Helper function to Check if a Download is allowed;
static bool IsDownloadAllowed(nsIChannel* aChannel,
const nsAutoCString& aMimeTypeGuess);
static long ClassifyDownload(nsIChannel* aChannel,
const nsAutoCString& aMimeTypeGuess);
#if defined(DEBUG)
static void AssertAboutPageHasCSP(mozilla::dom::Document* aDocument);
@ -60,10 +60,6 @@ class nsContentSecurityUtils {
static bool ValidateScriptFilename(const char* aFilename,
bool aIsSystemRealm);
/*
* Checks if a Channel should be able to proceed a download.
*/
static bool IsDownloadAllowed(nsIChannel* aChannel);
// Helper Function to Post a message to the corresponding JS-Console
static void LogMessageToConsole(nsIHttpChannel* aChannel, const char* aMsg);
};

View File

@ -1,3 +1,9 @@
ChromeUtils.defineModuleGetter(
this,
"Downloads",
"resource://gre/modules/Downloads.jsm"
);
let INSECURE_BASE_URL =
getRootDirectory(gTestPath).replace(
"chrome://mochitests/content/",
@ -49,10 +55,41 @@ function shouldConsoleError() {
});
}
async function resetDownloads() {
// Removes all downloads from the download List
let publicList = await Downloads.getList(Downloads.PUBLIC);
let downloads = await publicList.getAll();
for (let download of downloads) {
publicList.remove(download);
await download.finalize(true);
}
}
async function shouldNotifyDownloadUI() {
// Waits until a Blocked download was added to the Download List
let list = await Downloads.getList(Downloads.ALL);
await new Promise(res => {
const view = {
onDownloadAdded: aDownload => {
let { error } = aDownload;
if (
error.becauseBlockedByReputationCheck &&
error.reputationCheckVerdict == Downloads.Error.BLOCK_VERDICT_INSECURE
) {
res(true);
list.removeView(view);
}
},
};
list.addView(view);
});
}
async function runTest(url, link, checkFunction, decscription) {
await SpecialPowers.pushPrefEnv({
set: [["dom.block_download_insecure", true]],
});
await resetDownloads();
let tab = BrowserTestUtils.addTab(gBrowser, url);
gBrowser.selectedTab = tab;
@ -92,7 +129,7 @@ add_task(async function() {
await runTest(
SECURE_BASE_URL,
"insecure",
shouldConsoleError,
() => Promise.all([shouldNotifyDownloadUI(), shouldConsoleError()]),
"Secure -> Insecure should Error"
);
await runTest(

View File

@ -1690,6 +1690,7 @@ var DownloadError = function(aProperties) {
*/
DownloadError.BLOCK_VERDICT_MALWARE = "Malware";
DownloadError.BLOCK_VERDICT_POTENTIALLY_UNWANTED = "PotentiallyUnwanted";
DownloadError.BLOCK_VERDICT_INSECURE = "Insecure";
DownloadError.BLOCK_VERDICT_UNCOMMON = "Uncommon";
DownloadError.prototype = {

View File

@ -19,6 +19,11 @@ ChromeUtils.defineModuleGetter(
"Downloads",
"resource://gre/modules/Downloads.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"DownloadError",
"resource://gre/modules/DownloadCore.jsm"
);
/**
* nsITransfer implementation that provides a bridge to a Download object.
@ -269,7 +274,8 @@ DownloadLegacyTransfer.prototype = {
aStartTime,
aTempFile,
aCancelable,
aIsPrivate
aIsPrivate,
aDownloadClassification
) {
return this._nsITransferInitInternal(
aSource,
@ -279,7 +285,8 @@ DownloadLegacyTransfer.prototype = {
aStartTime,
aTempFile,
aCancelable,
aIsPrivate
aIsPrivate,
aDownloadClassification
);
},
@ -293,6 +300,7 @@ DownloadLegacyTransfer.prototype = {
aTempFile,
aCancelable,
aIsPrivate,
aDownloadClassification,
aBrowsingContext,
aHandleInternally
) {
@ -313,6 +321,7 @@ DownloadLegacyTransfer.prototype = {
aTempFile,
aCancelable,
aIsPrivate,
aDownloadClassification,
userContextId,
browsingContextId,
aHandleInternally
@ -328,11 +337,16 @@ DownloadLegacyTransfer.prototype = {
aTempFile,
aCancelable,
isPrivate,
aDownloadClassification,
userContextId = 0,
browsingContextId = 0,
handleInternally = false
) {
this._cancelable = aCancelable;
if (aDownloadClassification == Ci.nsITransfer.DOWNLOAD_ACCEPTABLE) {
// Only keep a refrence if it acceptable, as the download was canceled
// already if it isn't.
this._cancelable = aCancelable;
}
let launchWhenSucceeded = false,
contentType = null,
@ -351,11 +365,10 @@ DownloadLegacyTransfer.prototype = {
launcherPath = appHandler.executable.path;
}
}
// Create a new Download object associated to a DownloadLegacySaver, and
// wait for it to be available. This operation may cause the entire
// download system to initialize before the object is created.
Downloads.createDownload({
let serialisedDownload = {
source: {
url: aSource.spec,
isPrivate,
@ -371,8 +384,20 @@ DownloadLegacyTransfer.prototype = {
contentType,
launcherPath,
handleInternally,
})
.then(aDownload => {
};
// In case the Download was classified as insecure/dangerous
// it is already canceled, so we need to generate and attach the
// corresponding error to the download.
if (aDownloadClassification == Ci.nsITransfer.DOWNLOAD_POTENTIALLY_UNSAFE) {
serialisedDownload.errorObj = {
becauseBlockedByReputationCheck: true,
reputationCheckVerdict: DownloadError.BLOCK_VERDICT_INSECURE,
};
}
Downloads.createDownload(serialisedDownload)
.then(async aDownload => {
// Legacy components keep partial data when they use a ".part" file.
if (aTempFile) {
aDownload.tryToKeepPartialData = true;
@ -386,9 +411,14 @@ DownloadLegacyTransfer.prototype = {
this._resolveDownload(aDownload);
// Add the download to the list, allowing it to be seen and canceled.
return Downloads.getList(Downloads.ALL).then(list =>
list.add(aDownload)
);
await (await Downloads.getList(Downloads.ALL)).add(aDownload);
if (serialisedDownload.errorObj) {
// In case we added an already canceled dummy download
// we need to manually trigger a change event
// as all the animations for finishing downloads are
// listening on onChange.
aDownload._notifyChange();
}
})
.catch(Cu.reportError);
},

View File

@ -407,7 +407,8 @@ function promiseStartLegacyDownload(aSourceUrl, aOptions) {
null,
null,
persist,
isPrivate
isPrivate,
Ci.nsITransfer.DOWNLOAD_ACCEPTABLE
);
persist.progressListener = transfer;

View File

@ -425,7 +425,8 @@ function internalPersist(persistArgs) {
null,
null,
persist,
persistArgs.isPrivate
persistArgs.isPrivate,
Ci.nsITransfer.DOWNLOAD_ACCEPTABLE
);
persist.progressListener = new DownloadListener(window, tr);

View File

@ -15,6 +15,10 @@ webidl BrowsingContext;
[scriptable, uuid(37ec75d3-97ad-4da8-afaa-eabe5b4afd73)]
interface nsITransfer : nsIWebProgressListener2 {
const unsigned long DOWNLOAD_ACCEPTABLE = 0;
const unsigned long DOWNLOAD_FORBIDDEN = 1;
const unsigned long DOWNLOAD_POTENTIALLY_UNSAFE = 2;
/**
* Initializes the transfer with certain properties. This function must
* be called prior to accessing any properties on this interface.
@ -51,6 +55,8 @@ interface nsITransfer : nsIWebProgressListener2 {
* If true, indicates that the transfer was initiated from
* a source that desires privacy.
*
* @param aDownloadClassification Indicates wheter the dowload is unwanted,
* should be considered dangerous or insecure.
*/
void init(in nsIURI aSource,
in nsIURI aTarget,
@ -59,7 +65,8 @@ interface nsITransfer : nsIWebProgressListener2 {
in PRTime startTime,
in nsIFile aTempFile,
in nsICancelable aCancelable,
in boolean aIsPrivate);
in boolean aIsPrivate,
in long aDownloadClassification);
/**
* Same as init, but allows for passing the browsingContext
@ -79,6 +86,7 @@ interface nsITransfer : nsIWebProgressListener2 {
in nsIFile aTempFile,
in nsICancelable aCancelable,
in boolean aIsPrivate,
in long aDownloadClassification,
in BrowsingContext aBrowsingContext,
in boolean aHandleInternally);

View File

@ -1581,10 +1581,22 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
if (mMimeInfo) {
mMimeInfo->GetMIMEType(MIMEType);
}
// Now get the URI
if (aChannel) {
aChannel->GetURI(getter_AddRefs(mSourceUrl));
}
if (!nsContentSecurityUtils::IsDownloadAllowed(aChannel, MIMEType)) {
mDownloadClassification =
nsContentSecurityUtils::ClassifyDownload(aChannel, MIMEType);
if (mDownloadClassification != nsITransfer::DOWNLOAD_ACCEPTABLE) {
// If the download is rated as forbidden,
// we need to silently cancel the request to make sure
// it wont show up in the download ui.
mCanceled = true;
request->Cancel(NS_ERROR_ABORT);
if (mDownloadClassification != nsITransfer::DOWNLOAD_FORBIDDEN) {
CreateFailedTransfer();
}
return NS_OK;
}
@ -1618,11 +1630,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
}
}
// Now get the URI
if (aChannel) {
aChannel->GetURI(getter_AddRefs(mSourceUrl));
}
// retarget all load notifications to our docloader instead of the original
// window's docloader...
RetargetLoadNotifications(request);
@ -2183,11 +2190,12 @@ nsresult nsExternalAppHandler::CreateTransfer() {
rv = transfer->InitWithBrowsingContext(
mSourceUrl, target, EmptyString(), mMimeInfo, mTimeDownloadStarted,
mTempFile, this, channel && NS_UsePrivateBrowsing(channel),
mBrowsingContext, mHandleInternally);
mDownloadClassification, mBrowsingContext, mHandleInternally);
} else {
rv = transfer->Init(mSourceUrl, target, EmptyString(), mMimeInfo,
mTimeDownloadStarted, mTempFile, this,
channel && NS_UsePrivateBrowsing(channel));
channel && NS_UsePrivateBrowsing(channel),
mDownloadClassification);
}
NS_ENSURE_SUCCESS(rv, rv);
@ -2251,12 +2259,13 @@ nsresult nsExternalAppHandler::CreateFailedTransfer() {
rv = transfer->InitWithBrowsingContext(
mSourceUrl, pseudoTarget, EmptyString(), mMimeInfo,
mTimeDownloadStarted, nullptr, this,
channel && NS_UsePrivateBrowsing(channel), mBrowsingContext,
mHandleInternally);
channel && NS_UsePrivateBrowsing(channel), mDownloadClassification,
mBrowsingContext, mHandleInternally);
} else {
rv = transfer->Init(mSourceUrl, pseudoTarget, EmptyString(), mMimeInfo,
mTimeDownloadStarted, nullptr, this,
channel && NS_UsePrivateBrowsing(channel));
channel && NS_UsePrivateBrowsing(channel),
mDownloadClassification);
}
NS_ENSURE_SUCCESS(rv, rv);

View File

@ -341,6 +341,13 @@ class nsExternalAppHandler final : public nsIStreamListener,
*/
uint32_t mReason;
/**
* Indicates if the nsContentSecurityUtils rate this download as
* acceptable, potentialy unwanted or illigal request.
*
*/
int32_t mDownloadClassification;
/**
* Track the executable-ness of the temporary file.
*/