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

Status: RESOLVED FIXED

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

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: 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.

Comment 2

[David Durst [:ddurst]]

I was told yesterday that we weren't using desktop addons kind of blocklisting for 2.5... has that now changed?

Comment 3

[[:fabrice] Fabrice Desré]

Who told you that?

Comment 4

[David Durst [:ddurst]]

It was discussed in the FxOS 2.5 Addons : Weekly Escalations / Open Issues meeting. Pretty sure Jonas said that.

Comment 5

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(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).

Comment 6

[Mahendra Potharaju [:mahe]]

Fabrice, 

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

Thanks

Comment 7

[[:fabrice] Fabrice Desré]

Yes, we need to block. I'm working on the patch.

Comment 8

[[:fabrice] Fabrice Desré]

Attachment: 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.

Comment 9

[Dave Townsend [:mossop]]

Is the plan to use the same blocklist server that Firefox uses or something else?

Comment 10

[[:fabrice] Fabrice Desré]

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.

Comment 11

[Dave Townsend [:mossop]]

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

Comment 12

[Fernando Jiménez Moreno [:ferjm]]

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

Comment 13

[[:fabrice] Fabrice Desré]

Attachment: blocklist-tests.patch

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

Fabrice, you need to update the commit message of both patches.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/6dff307959f2
https://hg.mozilla.org/integration/b2g-inbound/rev/c38225ee4378

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/d2f87bf3aced

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

All backed out in https://hg.mozilla.org/integration/b2g-inbound/rev/62167d24ed1c for emulator mochitest bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=3199695&repo=b2g-inbound

Comment 18

[[:fabrice] Fabrice Desré]

ha damn... on emulators we run mochitests in a child process and we can't load nsBlocklistService.js there...

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/93976c034d03
https://hg.mozilla.org/integration/b2g-inbound/rev/7dfbcf02d4cc

Comment 20

[[:fabrice] Fabrice Desré]

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

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/93976c034d03
https://hg.mozilla.org/mozilla-central/rev/7dfbcf02d4cc

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/93976c034d03
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/7dfbcf02d4cc

Comment 23

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion

Comment 24

[[:fabrice] Fabrice Desré]

Carsten, I'm confused by your previous comment. Where has this been reverted? This needs to be on the mozilla-b2g44_v2_5 branch.