1631018 - Flip blocklist v3 preferences and uplift through ESR 78

Status: RESOLVED FIXED

(Toolkit :: Blocklist Implementation, task, P1)

Description

[Jorge Villalobos [:jorgev] (he/him)]

Once we're done with the rollout of blocklist v3 on Release, we'll flip the pref on ESR so the next dot release turns it on for all ESR users.

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

Note there are two boolean prefs, both of which should be set to true:

extensions.blocklist.useMLBF
extensions.blocklist.useMLBF.stashes

The second one enables incremental updates. It's not necessary for v3 to work, but it enables much more efficient updating.

Comment 2

[Shane Caraveo (:mixedpuppy)]

[Tracking Requested - why for this release]: We'll want this uplifted into 79 and esr-78 (point release). My understanding is that this will be rolled out gradually for 78-release.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1631018 set blocklist V3 as the default blocklist

Comment 4

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4707acf1f36acbfff6fb2279e9181a8857bfde5f

Comment 5

[Rob Wu [:robwu]]

Attachment: Bug 1631018 - Enable blocklist v3 by default on desktop

The blocklist v2 implementation will be removed in bug 1649906.

Comment 6

[Rob Wu [:robwu]]

Attachment: Bug 1631018 - Run test_blocklistchange.js with v2 and v3 blocklist

To preserve the test coverage for addon updates and appversion-specific
blocks, test_blocklistchange.js now runs with blocklist v2 and v3.

This provides NEW test coverage for features that already worked but
lacked automated test coverage. Bug 1649896 is a follow-up and concerned
with creating a new test file to achieve the same coverage in a cleaner
way.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11f135d60922
Enable blocklist v3 by default on desktop r=Gijs
https://hg.mozilla.org/integration/autoland/rev/7f27080ffee3
Run test_blocklistchange.js with v2 and v3 blocklist r=Gijs

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on browser_aboutdebugging_addons_manifest_url.js

backout: https://hg.mozilla.org/integration/autoland/rev/ec01c146f756b74d18e4892b4fd3aecba00da93e

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7f27080ffee35521c42fe9d4025caabef7b6258c&searchStr=windows%2C7%2Cdebug%2Cmochitests%2Ctest-windows7-32%2Fdebug-mochitest-devtools-chrome-e10s-5%2Cm%28dt5%29

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308578905&repo=autoland&lineNumber=1905

[task 2020-07-04T18:10:31.727Z] 18:10:31 INFO - Console message: [JavaScript Error: "NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIProcess.init]" {file: "resource://devtools/client/shared/remote-debugging/adb/adb-process.js" line: 109}]
[task 2020-07-04T18:10:31.727Z] 18:10:31 INFO - Buffered messages finished
[task 2020-07-04T18:10:31.727Z] 18:10:31 INFO - TEST-UNEXPECTED-FAIL | devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_manifest_url.js | Test timed out -
[task 2020-07-04T18:10:31.727Z] 18:10:31 INFO - Removing tab.
[task 2020-07-04T18:10:31.727Z] 18:10:31 INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2020-07-04T18:10:31.728Z] 18:10:31 INFO - Got event: 'TabClose' on [object XULElement].
[task 2020-07-04T18:10:31.728Z] 18:10:31 INFO - GECKO(5656) | [Parent 5944, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorProtocol.cpp, line 172
[task 2020-07-04T18:10:31.728Z] 18:10:31 INFO - GECKO(5656) | [Parent 5944, Main Thread] WARNING: '!inner', file /builds/worker/checkouts/gecko/dom/ipc/JSWindowActorProtocol.cpp, line 172
[task 2020-07-04T18:10:31.728Z] 18:10:31 INFO - Tab removed and finished closing
[task 2020-07-04T18:10:31.767Z] 18:10:31 INFO - TEST-PASS | devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_manifest_url.js | The main process DevToolsServer has no pending connection when the test ends -
[task 2020-07-04T18:10:31.810Z] 18:10:31 INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - TEST-UNEXPECTED-FAIL | devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_manifest_url.js | A promise chain failed to handle a rejection: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIProcess.init] - stack: start@resource://devtools/client/shared/remote-debugging/adb/adb-process.js:109:13
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - async*_startTracking@resource://devtools/client/shared/remote-debugging/adb/adb.js:106:22
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - _updateAdbProcess@resource://devtools/client/shared/remote-debugging/adb/adb.js:139:12
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - _emit@resource://devtools/shared/event-emitter.js:226:34
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - emit@resource://devtools/shared/event-emitter.js:172:18
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - emit@resource://devtools/shared/event-emitter.js:324:18
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - set status@resource://devtools/client/shared/remote-debugging/adb/adb-addon.js:55:12
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - updateInstallStatus@resource://devtools/client/shared/remote-debugging/adb/adb-addon.js:71:7
[task 2020-07-04T18:10:31.813Z] 18:10:31 INFO - async*ADBAddon/addonsListener.onUninstalled@resource://devtools/client/shared/remote-debugging/adb/adb-addon.js:46:12

Comment 9

[Rob Wu [:robwu]]

According to the log, the test add-on fails to install because the add-on is blocked by the blocklist. The blocklist relies on the assumption that all signed add-ons are known at the time of the blocklist generation. The failure here suggests that this is not the case (details below).

Andrew, does AMO account for add-ons signed with the privileged signature? (OU="Mozilla Extensions")?

Gijs, should we exclude privileged and system add-ons from the MLBF check? If really needed, we can still block add-ons through stashes even if the MLBF is skipped.

---

When I manually check [e-mail]:0.0.2 against the packaged MLBF dump referenced at https://searchfox.org/mozilla-central/rev/e11d919cdc8a909354eb2c3e19904d9229c55d88/services/settings/dumps/blocklists/addons-bloomfilters/addons-mlbf.bin.meta.json , then I can confirm that the entry is part of the MLBF. However, when I check the stashes and the other bloom filters referenced by the addons-bloomfilters collection, then [e-mail]:0.0.2 is not part of it. This inconsistent result implies that the add-on is not known by the blocklist generator (run by the addons-server), and consequently the block decision is unreliable.

To fix this the AMO generator should either include them, or the client should skip signed add-ons that are known to be unknown to AMO (i.e. having a privileged signature?).

Comment 10

[Andrew Williamson [:eviljeff]]

(In reply to Rob Wu [:robwu] from comment #9)

According to the log, the test add-on fails to install because the add-on is blocked by the blocklist. The blocklist relies on the assumption that all signed add-ons are known at the time of the blocklist generation. The failure here suggests that this is not the case (details below).

Andrew, does AMO account for add-ons signed with the privileged signature? (OU="Mozilla Extensions")?

If the file has been submitted to AMO, yes. (If not, AMO won't know about the guid and/or version so can't).

To fix this the AMO generator should either include them, or the client should skip signed add-ons that are known to be unknown to AMO (i.e. having a privileged signature?).

If we don't start requiring all (versions of) privileged addons to be submitted to AMO, then the blocklist check is going to have to be skipped for those add-ons. Given we tightly control those addons, I guess that's fine?

If really needed, we can still block add-ons through stashes even if the MLBF is skipped.

I'm not sure what solution you're proposing here, but manually adding extra content to stash JSON isn't a feature for the v3 blocklist on AMO.

Comment 11

[:Gijs (he/him)]

Skipping the blocklist for the Mozilla Extensions signature seems OK-ish, assuming that the fallback solution of using stashes / explicitly disallowing it works (which I'm not clear about given comment #10), in case we end up needing that.

The other thing I'm confused about is how this affects local / development add-ons. Are they going to be blocked because they're not known to the blocklist? That seems... bad?

Comment 12

[Rob Wu [:robwu]]

Thanks for your feedback. I'll add a patch today to skip the MLBF for the edge cases where the signature is not known to AMO.

(In reply to :Gijs (he/him) from comment #11)

Skipping the blocklist for the Mozilla Extensions signature seems OK-ish, assuming that the fallback solution of using stashes / explicitly disallowing it works (which I'm not clear about given comment #10), in case we end up needing that.

(In reply to Andrew Williamson [:eviljeff] from comment #10)

(In reply to Rob Wu [:robwu] from comment #9)

If really needed, we can still block add-ons through stashes even if the MLBF is skipped.

I'm not sure what solution you're proposing here, but manually adding extra content to stash JSON isn't a feature for the v3 blocklist on AMO.

AMO currently lack support for "manually" adding items to the stash, but the fact that the client checks stashes before the MLBF means that we aren't completely losing the ability to block some classes of add-ons. How the collection is populated isn't a concern right now, because we aren't expecting abuse from privileged signatures.

In practice, the full collection is rarely reset, and the default effect of adding an item to the blocklist is that it is added to a stash. This is good enough for me. If we unexpectedly need the ability to ensure the addition of an item to the stash after resetting the collection, then I can imagine an explicit unblock followed by a block, or even asking ops to manually making an API call to RemoteSettings to add a new stash. Again, I'm not expecting this to be needed, but if we do, we will have ways to do it (until the server is ready to officially support the feature).

The other thing I'm confused about is how this affects local / development add-ons. Are they going to be blocked because they're not known to the blocklist? That seems... bad?

Local add-ons aren't signed, so these are not affected. The blocklist is only enforced for signed add-ons.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa24360c5b70
Enable blocklist v3 by default on desktop r=Gijs
https://hg.mozilla.org/integration/autoland/rev/3e296ea3a883
Run test_blocklistchange.js with v2 and v3 blocklist r=Gijs

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/aa24360c5b70
https://hg.mozilla.org/mozilla-central/rev/3e296ea3a883

Comment 15

[Ryan VanderMeulen [:RyanVM]]

IIUC, we're targeting Fx80/78.2esr for this change now?

Comment 16

[Rob Wu [:robwu]]

Quoting jorgev:

just had a meeting and decided on this plan: Keep ESR 78.0 and Release 78 on v2. Patch bug 1650825 on 79 and uplift to ESR 78.1 (if RelMan is okay with it). Rollout v3 on Release 79. Turn on the pref permanently on Release 80 and ESR 78.2 if the rollout goes well.

So everything in this bug will ride the train and then be uplifted to 78.2ESR.

bug 1650825 should be uplifted to 79 (I'll file an uplift request tomorrow after confirming in a 1:1).
(and then bug 1631008 will be concerned with flipping the pref on release 79)

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Is this on track for 78.2esr uplift still?

Comment 18

[Rob Wu [:robwu]]

Yes it's still on track. Should I file an uplift request, and by when preferably?

Comment 19

[Ryan VanderMeulen [:RyanVM]]

By August 14th please.

Comment 20

[Rob Wu [:robwu]]

Comment on attachment 9160794 [details]
Bug 1631018 - Enable blocklist v3 by default on desktop

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This enables a more scalable blocklist implementation on ESR.
• User impact if declined: ESR users would use blocklist v2, which doesn't offer the same coverage as blocklist v3.
• Fix Landed on Version: 80
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The feature is covered by unit tests and has already rolled out on release (bug 1631008).
  Telemetry shows that blocklist updates are propagated as expected: https://sql.telemetry.mozilla.org/queries/73919

In theory toggling the blocklist implementation could result break the blocklist for users who customized their blocklist endpoint. I don't expect enterprise users to be affected, because there are no such issues when we migrated from blocklist v1 (XML) to blocklist v2 (RemoteSettings) according to mkaply.

• String or UUID changes made by this patch:

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9160794 [details]
Bug 1631018 - Enable blocklist v3 by default on desktop

Approved for 78.2esr.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/f4e931882d26
https://hg.mozilla.org/releases/mozilla-esr78/rev/198b5174ca31