1650825 - Ignore bloom filter block result for add-ons unknown to AMO

Status: RESOLVED FIXED

(Toolkit :: Blocklist Implementation, task, P1)

Description

[Rob Wu [:robwu]]

Blocklist v3 consists of two parts: stashes and bloom filters.

The bloom filter part is a probabilistic data structure, whose results are only meaningful/accurate if the input add-on is known to the bloom filter generator. In https://bugzilla.mozilla.org/show_bug.cgi?id=1631018#c9 , we found that sometimes privileged add-ons got blocked. In the following comments it became apparent that these add-ons are signed independently of AMO, so AMO is unable to construct a bloom filter without false positives for these add-ons.

Let's skip the bloomfilter check for these kinds of add-ons. If needed, the client still supports blocking them through stashes.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1650825 - Only use bloom filters for AMO-signed add-ons

Comment 2

[Rob Wu [:robwu]]

Are we going to ensure that AMO knows about privileged / system add-ons ?
If so then we won't need this patch in its current form.

Comment 3

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

Are we going to ensure that AMO knows about privileged / system add-ons ?

Yes, but I don't think it's going to be enforced automatically, so there's still a chance that we'll be back to having this problem sometime in the future. I'm in favor of keeping this patch to better cover our bases.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dbea60830c7
Only use bloom filters for AMO-signed add-ons r=Gijs

Comment 5

[Stuart Colville [:muffinresearch]]

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

Are we going to ensure that AMO knows about privileged / system add-ons ?
If so then we won't need this patch in its current form.

Adding all of the system / privileged add-ons requires knowing not only the guids but the versions too. Given this might be difficult to get in a concrete way, with no central records (Afaik) this patch seems like a better way to fix the problem in a more robust way so it makes sense to keep it. However, it does also raise an additional concern wrt being able to block a system add-on with this patch in place. We'll need to understand what options are available for that independently.

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/2dbea60830c7

Comment 7

[Rob Wu [:robwu]]

Comment on attachment 9161644 [details]
Bug 1650825 - Only use bloom filters for AMO-signed add-ons

Beta/Release Uplift Approval Request

• User impact if declined: Privileged add-ons unknown to AMO have a 0.36% chance of inadvertently being blocked by the add-ons blocklist.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Covered by unit tests.

The blocklist is still capable of enforcing blocks of privileged add-ons (via stashes).

• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: We want to enable blocklist v3 (bug 1620621) on ESR78.
• User impact if declined: We can either not enable blocklist v3 (and users have less coverage from blocklist v2 due to scalability issues),
  or we enable blocklist v3 and privileged add-ons unknown to AMO have a 0.36% chance of inadvertently being blocked by the add-ons blocklist.
• Fix Landed on Version: 80
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Covered by unit tests. Currently pref-off-by-default (but already tested with a staged rollout on Beta 78/79).
• String or UUID changes made by this patch:

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9161644 [details]
Bug 1650825 - Only use bloom filters for AMO-signed add-ons

Needed for migration to the v3 blocklist. Approved for 79.0b6 and 78.1esr.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3d7fc5d1d499

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/6efe9012737d