775253 - Gather statistics on add-ons installed after changes to extensions.autoDisableScopes

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect)

Description

[Kris Maglione [:kmag]]

We've had a lot of reports of external add-on installers overriding extensions.autoDisableScopes and then installing their add-ons. We need data on what add-ons are installed this way so that we can reach out to their authors, or potentially blocklist them.

Ideally, I'd like to see statistics on any externally-installed add-on on the first or second run after the preference is changed, but I think we could manage with knowing what extensions are installed without about:newaddon

I'm filing this bug in Telemetry, but I think that Test Pilot would also be an acceptable place to do this.

Comment 1

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

Would this strategy work:

1) detect all users who have `extensions.autoDisableScopes` set to 0, then
2) sending in a list of all addons they have
3) doing a grep / analysis of those addons

be enough?  Or do we have to catch addons 'in the act'?  

If that is *not enough*, what else do you want to know about addons "while we are in there"?

(cc: Ilana, who is the other tp programmer).

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Gregg Lind (User Research - Test Pilot) from comment #1)
> 1) detect all users who have `extensions.autoDisableScopes` set to 0, then
> 2) sending in a list of all addons they have
> 3) doing a grep / analysis of those addons

We already do #2 in Telemetry (assuming you have it enabled); #1 would be very easy.  #3 is just a data analysis problem to do on the server side of things.

It doesn't quite get at the longitudinal aspect (which addons were installed post-change of extensions.autoDisableScopes), but hopefully you'd see some correlations when doing analysis.

Comment 3

[Kris Maglione [:kmag]]

I think catching them in the act would be best. Sending in an entire list of add-ons would generate too much noise that we'd have to sort out. If we go the telemetry route, it would be quite easy for the add-on manager to collect the add-on ID at install time after checking autoDisableScopes.

Most of the installers that we've come across recently revert autoDisableScopes immediately after their add-on is installed, too, so we have a very short window to catch this in.

Comment 4

[Kris Maglione [:kmag]]

Incidentally, if we go the Telemetry route, I can do the client side portion. I'm not familiar enough with Test Pilot of the server side portions of Telemetry to do either of those. It's mostly a matter of deciding which route is more appropriate at this point.

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

Over to add-on manager to implement a probe or work with the test pilot team on a study.

Comment 6

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

from Test Pilot pov, we can catch it



(Algorithm)
* on study start, get the list of extensions (A)
* instrument extensions.autoDisableScopes prefs 
* on change there, get the list of addons (B)
* villain = B - A

Add-on installations are fairly rare events, of course.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: PoC patch v1

Hmm, so, this is by far the easiest (and most accurate?) to detect within the Add-ons Manager, but with one caveat: remembering it between restarts.

3rd party installers sometimes do the following:
* Quit Firefox
* Install add-on
* Modify prefs.js
* Start Firefox
* Quit Firefox
* Revert prefs.js change
* Start Firefox

Making Firefox exit right after startup is what makes it a little tricky. In the attached patch, it detects such add-ons and immediately gives it to Telemetry. On quitting the application normally, Telemetry will save that data to a file if it hasn't yet had a chance to submit it. However, if the application is killed, then it won't be saved - it'll be lost forever. I don't know how 3rd party installers are exiting Firefox, but its conceivable they can/will just kill the process.

We could save it to a pref, but that makes it incredibly simple for such a 3rd party installer to modify that pref, since they're doing that already.

We could save it to the DB - adding an extra field isn't too difficult (still work, though), and the DB is already open as part of the install process for the new add-on. The trouble there is, on the next restart, we try our best to *not* open the DB - so we can't immediately get that data out again. What we could do it just wait until the next background update check, then add that data to Telemetry. That lag means if the add-on is uninstalled before the next update ping, we won't get data about it. (Note to self: Want to persist the value, but only submit suspicious add-ons that haven't been submitted yet - so use a tri-state field.)


I should note that the attached patch doesn't explicitly detect a change in the autoDisableScopes pref right before a foreign add-on install. I don't think there's a reliable way to do that that isn't easily bypassed. Instead, what it does is detect if that pref has a non-default value that allowed the add-on to be enabled, but the default value wouldn't have. That will increase the rate of false-positives slightly.

Also, the patch added some additional metadata that we don't normally send - such as addon name, author, scope, and the state of the autoDisableScopes pref. I did that to help track down the source of these add-ons, but some more thought should go into that.

Comment 8

[Dave Townsend [:mossop]]

(In reply to Blair McBride (:Unfocused) from comment #7)
> We could save it to the DB - adding an extra field isn't too difficult
> (still work, though), and the DB is already open as part of the install
> process for the new add-on. The trouble there is, on the next restart, we
> try our best to *not* open the DB - so we can't immediately get that data
> out again. What we could do it just wait until the next background update
> check, then add that data to Telemetry. That lag means if the add-on is
> uninstalled before the next update ping, we won't get data about it. (Note
> to self: Want to persist the value, but only submit suspicious add-ons that
> haven't been submitted yet - so use a tri-state field.)

I think this might be a better way to go despite the extra work. We probably don't lose much by waiting for the next telemetry ping, and persisting data on whether the add-on was installed suspiciously or not gives us options to do things in the future, like disabling all suspicious items.

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 660709 [details] [diff] [review]
PoC patch v1

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

Per comment I think this might not be the best option.

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +2703,2 @@
>            newAddon.userDisabled = true;
> +        } else {

Rest of the code style has else on a new line after the brace

Comment 10

[Kris Maglione [:kmag]]

I don't think it's too important that we make this difficult for third parties to circumvent. If it came to it, they could just turn off Telemetry. The process shouldn't be visible to them, and we won't advertise it, so that should be enough for the vast majority of them to ignore it. Still, I agree that we should avoid using preferences to store the data.

Comment 11

[Kris Maglione [:kmag]]

Blair, has there been any progress on this? This is still important.

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Kris Maglione [:kmag] from comment #11)
> Blair, has there been any progress on this? This is still important.

Yes - IIRC, I have an almost finished patch that implements a different/better/safer approach, but it needs some extra work on the tests. I haven't been able to work on it for awhile - might be able to again soon.

Comment 13

[Kris Maglione [:kmag]]

Let's do this via FHR rather than telemtry. It seems the simpler and more appropriate channel.

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Old WiP

This is an old work in progress (including a .rej files I never got around to resolving). Different approach to the old proof-of-concept - this gives us a general framework for reporting behaviour of specific add-ons. IIRC, it needs updating to the newer DB stuff we have.

Going to see if MattN would like to pick this up, as an intro to Add-ons Manager code.

Comment 15

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.