1593467 - Automatically restore from logins-backup.json when logins.json is missing or corrupt

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[vinkof]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

Today after an update to 70.0.1 (64-bit) all my passwords in Firefox Lockwise disappeared - which was shocking. I googled and I found this: https://www.reddit.com/r/firefox/comments/clhoj1/saved_logins_box_empty_firefox_690b10/
I went to Profiles folder. Then I first copied logins.json and logins.json.corrupt to a backup folder. Then I moved logins.json away. Then I reanamed logins.json.corrupt to logins.json. And it worked. It seems all passwords are now back. I hope I didn't lose any passwords and that all the passwords were in the logins.json.corrupt file which I renamed.

Actual results:

Today after an update to 70.0.1 (64-bit) all my passwords in Firefox Lockwise disappeared - which was shocking. All passwords gone !!! The list was empty !

Expected results:

All passwords should be visible and there should be more information on a way to fix and avoid this problem.

Comment 1

[:Gijs (he/him)]

Not a security exploit that needs to stay hidden.

Matt, how can we gather debug info here?

Comment 2

[Matthew N. [:MattN]]

I doubt there is much more we can do here… we would need Browser Console logs from the first startup where they disappeared. Do you know what the timestamp of logins.json.corrupt was before you renamed it? Was it recent?

Do you use AVG or any other non-Defender anti-virus?

Comment 3

[vinkof]

logins.json.corrupt, Date Modified, Saturday, ‎2 ‎November ‎2019, ‏‎13:01:56

I am using AVG AntiVirus FREE, ver. 19.8.3108 (build 19.8.4793.544)

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:MattN, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[Matthew N. [:MattN]]

Hmm… bug 1558765 is a case of AVG causing this problem in the past. I wonder if they are still intermittently causing this problem due to some kind of race condition? There haven't been any changes in Firefox related to login storage. It'll be hard to know for sure since it's intermittent. I believe you can disable the protection of logins.json from AVG but we'd have to wait a while to see if that solved the problem.

Leif, how hard would it be to extend your pwmgr data loss analysis to break it down by A/V software (as a proportion of the users who use that software)? I believe environment.system.sec.antivirus is the data you need but I think it's only available on recent Windows versions. If we see that the problem is significantly more prevalent with AVG users then we can do outreach.

Comment 6

[Leif Oines [:loines]]

Potentially could do this yea, but I'd have to investigate whether I can just tweak that notebook given the change to gcp or if large parts of it would have to be re-written. I'll open a bug in data science.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:MattN, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Matthew N. [:MattN]]

I'll make this a tracking issue to figure out the root cause using telemetry data in related bugs.

Comment 9

[Matthew N. [:MattN]]

Hey Prathiksha, while testing out the patch for bug 1597358, I noticed that it spams the Browser Console whenever consumers other than autofill and logins use JSONFile.jsm so I guess we need to duplicate the telemetry allow list in the code to avoid that spam. I thought that was a case where telemetry silently ignored the mismatch but apparently not. Can you address this in this bug too?

Thanks

Comment 10

[:prathiksha]

Attachment: Bug 1593467 - Automatically restore from logins-backup.json when logins.json is missing or corrupt. r?MattN

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8754a74ef0f8
Automatically restore from logins-backup.json when logins.json is missing or corrupt. r=MattN

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8754a74ef0f8

Comment 13

[Matthew N. [:MattN]]

Prathiksha and Timea, can you work together to ensure the feature is working properly? Perhaps the data or analysis is wrong but it seems like this may not be working: https://sql.telemetry.mozilla.org/embed/query/73323/visualization/183571?api_key=CZ6KjzyWFYCheq7RZpQuOZlhNx3hbboUL3MiENAq&

Comment 14

[Timea Cernea [:tbabos][inactive]]

The data is correct, I can reproduce that issue by updating from Firefox 79 Release (signon.backup.enabled is preffed off by default) to Firefox 80 Release (signon.backup.enabled preffed on by default). The trick is to not save any new credentials after updating Firefox to 80. In this case, the "logins-backup.json" file is not created and credentials can't be restored if the logins.json created in Firefox 79 is deleted or corrupted.

I also attached a video of 2 parts on how it can be reproduced with the update scenario but it works ok on the builds where the logins restored from backup functionality is enabled by default (such as how the verification was done in Bug 1597358)

Steps to repro:

1. Have Firefox 79 installed and opened with several logins saved
2. Make sure the update is downloaded for FX80 and a restart is pending for it
3. Close Firefox
   4 Start Firefox and make sure it is updated to version 80
4. Visit about:logins and see that all the saved logins are still there.
5. Check the profile directory and see that there is no logins-backup file created for the existing "old" logins.json
6. Close again Firefox and delete the "logins.json" file
7. Start Firefox once again and check about:logins (credentials are missing) since logins.json couldn't be restored from a backup file. Browser console error is also displayed.

We might need to create the backup file of the existing logins.json after Firefox is updated to the version where restore is preffed on for cases where the old Firefox build had it preffed off. Doing this scenario (as well as the corrupted one) on Firefox builds such as Nightly 82, Beta81 and Release 80 will work correctly. The high number of "File Not found" makes sense since users updated from Firefox 79 to Firefox 80 and their original logins.json file does not have a backup unless the users saved new credentials on the new Firefox builds on the same profile.

Tested on: Windows 10:
82.0a1 (2020-09-07) (64-bit)
81.0b7 (64-bit)
80.0.1 (64-bit)
79.0. (64-bit)

Prathiksha, if you need any further help please let me know.

Comment 15

[Timea Cernea [:tbabos][inactive]]

Attachment: Repro part 1

Comment 16

[Timea Cernea [:tbabos][inactive]]

Attachment: Repro part 2

Comment 17

[Matthew N. [:MattN]]

Yeah, I'm not too worried about the case where the user hasn't made any changes since upgrading… even "using" a login (i.e. submitting a form with it filled should cause the backup to be created) and that case is accounted for separately in the graph. Is the feature working fine for when a backup does exist? Maybe the telemetry being recorded is different than my analysis expects in the success case?

Comment 18

[Timea Cernea [:tbabos][inactive]]

It worked just fine on the latest versions I tested.
After the backup file was created, I closed firefox, deleted de logins.json and re-launched. The logins.json was created again from the backup file.
Also corrupted the logins.json by editing gibberish text into it. A corrupted file was created in the process and the new logins.json file was created from the backup on browser launch.
Is there any other scenario I am missing or maybe I am not doing this right?

Comment 19

[Sam Foster [:sfoster] (he/him)]

I tried editing my logins.json to make it invalid JSON, then starting back up and loading about:logins.
about:telemetry shows:

1474 	jsonfile 	load 	logins 	
1486 	jsonfile 	load 	logins 	invalid_json
1488 	jsonfile 	load 	logins 	used_backup

..which agrees with what I expect the code to do. But I'm going to need time/help to unpack the query and figure out if that is looking for the correct values. It is looking for udf.get_key(payload.processes.parent.keyed_scalars.telemetry_event_counts, "jsonfile#load#logins") = 1 but I confess I don't know where that '1' would come from, given the data I'm seeing above.

Comment 20

[:prathiksha]

Sorry for the late response. I was on PTO. I just verified too that backup is correctly generated and used with the Firefox update.