Closed Bug 1226616 Opened 4 years ago Closed 2 years ago

general.config.filename should be reported on telemetry

Categories

(Core :: AutoConfig (Mission Control Desktop), defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla59
Tracking Status
firefox57 --- verified
firefox58 blocking verified
firefox59 + verified

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [fxsearch])

Attachments

(2 files, 2 obsolete files)

We have reasons to believe the AutoConfig feature (enabled by setting the general.config.filename preference) is being abused. We should add a Telemetry probe to get data about how widespread the abuse is.

We should also make this visible in about:support (there's already bug 1195389 covering this).
Blocks: 1292444
How can we distinguish abuse from a legitimate use?
(In reply to Masatoshi Kimura [:emk] from comment #1)
> How can we distinguish abuse from a legitimate use?

We can't say for sure for any single user, but in aggregate we can guess based on other information:
- if the trend changes, and suddenly we have a huge increase in the proportion of users with an autoconfig file.
- if we see usage of a specific autoconfig file is strongly correlated to some specific search settings that we believe can't have been willingly set by users.
We would want to do a hash of the file, not the name, since most people call the file by the same name.
I've added support to telemetry for querying default prefs, so I think at this point it would be useful to get numbers of simply how many folks have this set. I'll put together a patch.
Recording a scalar from Preferences.cpp instead of recording the value of the preference in the environment (like attachment 8930537 [details] did) because our data stewards are nervous about us collecting the preference value which is a string that could contain anything.
Attachment #8930917 - Flags: review?(n.nethercote)
Assignee: mozilla → florian
Status: NEW → ASSIGNED
Attachment #8930537 - Attachment is obsolete: true
Attachment #8930537 - Flags: review?(florian)
Depends on: 1419557
Priority: -- → P1
Whiteboard: [fxsearch]
Attachment #8930917 - Flags: review?(n.nethercote) → review+
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1603e2488c32
The presence of the general.config.filename preference should be reported on telemetry. r=njn, data-review=francois
https://hg.mozilla.org/mozilla-central/rev/1603e2488c32
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8930917 [details] [diff] [review]
Patch (didn't work)

Approval Request Comment
[Feature/Bug causing the regression]: Not a regression
[User impact if declined]: we'll not be able to know if autoconfig is abused in the wild.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Check in about:telemetry that we are reporting the right thing when an autoconfig file is used.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: very low risk
[Why is the change risky/not risky?]: trivial patch adding one scalar probe
[String changes made/needed]: none
Attachment #8930917 - Flags: approval-mozilla-release?
Attachment #8930917 - Flags: approval-mozilla-beta?
Attachment #8930917 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8930917 - Flags: approval-mozilla-release? → approval-mozilla-release+
s/m-b/m-r/
The patch here doesn't work at all :-(. This is because we attempt to set a scalar before profile-after-change, so Telemetry isn't initialized yet and assumes data collection has been disabled by the user.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8931785 [details] [diff] [review]
Patch recording in the telemetry environment

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

Looks good to me. This may be sufficiently changed from the original implementation to warrant a new Data Collection Review. Check with a data peer just in case, but the data we're recording is the same, it's just in a different place and in every ping when it would have otherwise been in a single ping... so it's probably fine.
Attachment #8931785 - Flags: review?(chutten) → review+
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d72084b1e1b
Backed out changeset 1603e2488c32 as this patch for bug 1226616 didn't work
https://hg.mozilla.org/integration/mozilla-inbound/rev/76c8206c5437
The presence of the general.config.filename preference should be reported on telemetry. r=chutten, data-review=francois
https://hg.mozilla.org/mozilla-central/rev/76c8206c5437
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Attachment #8931727 - Attachment is obsolete: true
Comment on attachment 8931785 [details] [diff] [review]
Patch recording in the telemetry environment

Approval Request Comment
[Feature/Bug causing the regression]: Not a regression
[User impact if declined]: we'll not be able to know if autoconfig is abused in the wild.
[Is this code covered by automated tests?]: the new logic is covered by a test, the recording of the new probe is not
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Check in about:telemetry that we are reporting the right thing when an autoconfig file is used.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: acceptable risk
[Why is the change risky/not risky?]: This patch is less trivial than the  previous one (that didn't work), but is still pretty straightforward.
[String changes made/needed]: none
Attachment #8931785 - Flags: approval-mozilla-release?
Attachment #8931785 - Flags: approval-mozilla-beta?
Attachment #8930917 - Attachment description: Patch → Patch (didn't work)
Attachment #8931785 - Flags: approval-mozilla-release?
Attachment #8931785 - Flags: approval-mozilla-release+
Attachment #8931785 - Flags: approval-mozilla-beta?
Attachment #8931785 - Flags: approval-mozilla-beta+
Verification STR:

1. Open FF application install location.
2. Open folder .../defaults/pref
3. Add test.js
4. Edit test.js and add the following line: pref("general.config.filename", "test"); 
5. Try to open Firefox. (FF fails to open throwing: "Failed to read configuration file...." )
6. Go back to the install folder and create a file test 
7. Open Firefox.
8. Navigate to about:telemetry#environment-data-tab_settings
9. Close Firefox, delete all the changes above (steps 3, 6) and reopen Firefox.
10. Open about:telemetry#environment-data-tab_settings.

Expected Result:
5. Firefox doesn't open showing error message: "Failed to read configuration file...."
7. Firefox is opened.
8. userPrefs.general.config.filename value is <set>.
10. userPrefs.general.config.filename is not found.


Verified on Windows 10 x64, 59.0a1 20171126220311.
Verified fixed on Windows 7 x64, Windows 10 x64, macOS 10.13 and Ubuntu 14.04 x64 using Firefox 57.0.1 (buildID: 20171128222554).
Verified as fixed using Firefox 58 beta 9 under Windows 10 x64, Ubuntu 14.04 x64 and Mac OS X 10.13.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.