[Add-ons] disable misbehaving or malicious add-ons on devices

RESOLVED FIXED in Firefox 45

Status

P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mars, Assigned: fabrice)

Tracking

unspecified
FxOS-S10 (30Oct)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.5+, firefox45 fixed)

Details

User Story

As a FxOS add-on reviewer, I want to disable misbehaving or malicious addons that have already been downloaded to user's devices.  (requested by pauljt)

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8668740 [details] [diff] [review]
addon-blocklist.patch

That let us leverage the blocklist support we already use for desktop addons, plugins and some other things.

I still need to add tests and discuss with people that actually create and deploy the lists.
Assignee: nobody → fabrice
I was told yesterday that we weren't using desktop addons kind of blocklisting for 2.5... has that now changed?
(Assignee)

Comment 3

3 years ago
Who told you that?
It was discussed in the FxOS 2.5 Addons : Weekly Escalations / Open Issues meeting. Pretty sure Jonas said that.
(In reply to David Durst [:ddurst] from comment #4)
> It was discussed in the FxOS 2.5 Addons : Weekly Escalations / Open Issues
> meeting. Pretty sure Jonas said that.

I wasn't at that meeting I think, but we definitely need blocklisting from a security perspective. The only question I was aware of was whether we use the existing apps mechanism or use the add-ons way. Both require work, and that add-on mechanism is much preferable due to the limitations of the existing process (and the fact that we have never actually used it also worries me, where as for add-on we regularly ship add-on blocks).
(Reporter)

Updated

3 years ago
Target Milestone: --- → FxOS-S9 (16Oct)

Updated

3 years ago
QA Whiteboard: [COM=Add-on]
Fabrice, 

Can you please help out here? Should this block 2.5 release? 

Thanks
Flags: needinfo?(fabrice)
(Assignee)

Comment 7

3 years ago
Yes, we need to block. I'm working on the patch.
Flags: needinfo?(fabrice)
(Assignee)

Comment 8

3 years ago
Created attachment 8679581 [details] [diff] [review]
addon-blocklist.patch

Part 1: hooking up the blocklist service to b2g web extensions.

Dave, can you take a look at the changes I made to nsBlocklistService.js:
- switch to use AppConstants.jsm instead of preprocessing.
- add b2g specific paths in _loadBlocklistFromString since we don't use the addon manager because web extensions are shipped as apps on b2g.

Fernando, the changes in this patch are:
- adding blocklistId and blockedStatus in the registry.
- forcibly disable apps when told so by the blocklist service.
I think we also will want to check blocked status at install and update time for webextensions (this patch just set it to unblocked), but that can be done in a followup since for now only the marketplace can ship webextensions.

Tests are coming in part 2.
Attachment #8679581 - Flags: review?(ferjmoreno)
Attachment #8679581 - Flags: review?(dtownsend)
Is the plan to use the same blocklist server that Firefox uses or something else?
Flags: needinfo?(fabrice)
(Assignee)

Comment 10

3 years ago
Yes, we are reusing the same blocklist server, using b2g/b2gdroid app IDs. Last I heard the AMO team was doing the last verifications on this side.
Flags: needinfo?(fabrice)
(Reporter)

Updated

3 years ago
Depends on: 1218582
Comment on attachment 8679581 [details] [diff] [review]
addon-blocklist.patch

Review of attachment 8679581 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for the toolkit/mozapps/extensions pieces
Attachment #8679581 - Flags: review?(dtownsend) → review+
Comment on attachment 8679581 [details] [diff] [review]
addon-blocklist.patch

Review of attachment 8679581 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/apps/ImportExport.jsm
@@ +398,5 @@
>        meta.installerIsBrowser = false;
>        meta.role = manifest.role;
>  
> +      // If there is an id in the mini-manifest, use it for blocklisting purposes.
> +      if (isPackage && updateManifest ("id" in updateManifest)) {

isPackage && updateManifest && ("id" in updateManifest) ?

::: dom/apps/Webapps.jsm
@@ +1237,5 @@
> +    let app;
> +    let runtime = Services.appinfo.QueryInterface(Ci.nsIXULRuntime);
> +
> +    aExtensions.filter(extension => {
> +      // Filter out id-less items and those who don't have a matching insalled

typo: installed
Attachment #8679581 - Flags: review?(ferjmoreno) → review+
(Assignee)

Comment 13

3 years ago
Created attachment 8680255 [details] [diff] [review]
blocklist-tests.patch
Attachment #8668740 - Attachment is obsolete: true
Attachment #8680255 - Flags: review?(ferjmoreno)
Attachment #8680255 - Flags: review?(ferjmoreno) → review+
Fabrice, you need to update the commit message of both patches.
(Assignee)

Comment 18

3 years ago
ha damn... on emulators we run mochitests in a child process and we can't load nsBlocklistService.js there...
Flags: needinfo?(fabrice)
(Assignee)

Comment 20

3 years ago
I moved the test to run on mulet this it has MOZ_B2G defined and runs mochitests in the parent process.

Comment 21

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/93976c034d03
https://hg.mozilla.org/mozilla-central/rev/7dfbcf02d4cc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: FxOS-S9 (16Oct) → FxOS-S10 (30Oct)
(Reporter)

Updated

3 years ago
Depends on: 1220197
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
status-b2g-v2.5: fixed → ---
(Assignee)

Comment 24

3 years ago
Carsten, I'm confused by your previous comment. Where has this been reverted? This needs to be on the mozilla-b2g44_v2_5 branch.
You need to log in before you can comment on or make changes to this bug.