Closed Bug 1208242 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox OS Graveyard :: Gaia::System, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(feature-b2g:2.5+, firefox45 fixed)

RESOLVED FIXED
FxOS-S10 (30Oct)
feature-b2g 2.5+
Tracking Status
firefox45 --- fixed

People

(Reporter: mars, Assigned: fabrice)

References

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 files, 1 obsolete file)

No description provided.
Attached patch addon-blocklist.patch (obsolete) — Splinter Review
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?
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).
Target Milestone: --- → FxOS-S9 (16Oct)
QA Whiteboard: [COM=Add-on]
Fabrice, 

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

Thanks
Flags: needinfo?(fabrice)
Yes, we need to block. I'm working on the patch.
Flags: needinfo?(fabrice)
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)
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)
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+
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.
ha damn... on emulators we run mochitests in a child process and we can't load nsBlocklistService.js there...
Flags: needinfo?(fabrice)
I moved the test to run on mulet this it has MOZ_B2G defined and runs mochitests in the parent process.
https://hg.mozilla.org/mozilla-central/rev/93976c034d03
https://hg.mozilla.org/mozilla-central/rev/7dfbcf02d4cc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: FxOS-S9 (16Oct) → FxOS-S10 (30Oct)
Depends on: 1220197
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
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.