Argon2 + encryption changes

[mymindstorm]

Fixes #370, #366, and #387

Changes:

• key contains random key to encrypt entries with. key is encrypted with password.
• key is hashed with argon2 and is checked on password entry.
• entry.hash is now a random UUID. Entries hashes are changes when migrating to key format and when changing password.
• No cookie again. Cookies can be saved to disk even if we only use a session cookie (MDN, decrypt password not requested after browser shutdown #387). We can't control whether the cookie will be saved to disk or not. The data we put in the cookie should never be saved to disk, and if we can't control that with cookies we shouldn't use them.
• Instead of re-checking the hash of every entry every time an entry is deleted or added, it will only check hash on decrypt.
• When updating / deleting entries it will make changes in application state instead of making the changes in storage and reloading everything.

[mymindstorm]

@Sneezry this has been sitting for a while, do you plan to review this?

[Sneezry]

It's a little long story, and I think we may go back one step.

Why we use secret hash?

When I first design data structure of secret, I use secret hash as key of the dictionary. Because Chrome supports to update part of data by specific key if the data is a dictionary map. So I think if we should always update the entire data (#412), secrest hash is not required. And if so, Argon2 may be not required either?

Event page vs background page

Browser extension should be as lite as possible, and take seriously with background page. I still concern if it is worth to use a background page on user side. I understand we should always pay attention to security, however, there should be a balance.

What I suggest

Use an array to store secrets instead of dictionary maybe a good direction. To security, if we have to use background page, maybe we can dispose all events when user close the popup page.

[mymindstorm]

Secret hashing

argon2 is only used to check the hash of key to make sure the correct password was entered. The purpose of using argon2 is to improve the security of our hashes as argon2 hashes are significantly harder to brute force.

Entries aren't hashed anymore and just get random identifiers (uuidv4) for their keys. It is unnecessary (and very slow with argon2) to hash individual entries as long as the key hash is validated properly.

Cost of using persistent background page

I profiled the background page's memory cost in Firefox. It uses somewhere between 1-2 MB. In a blank profile extensions normally use ~25 MB. In my normal profile extensions (not even counting normal web content) use between 120-250 MB. The memory cost low compared to other extensions already and is negligible compared to the security benefit this provides.

Suggestions

Use an array to store secrets instead of dictionary maybe a good direction.

Sounds like a good idea, this would let us move user config out of localstorage. Other than that, I don't really see that many reasons to move away from the current system. The secret hash problem should be solved by using uuids already.

To security, if we have to use background page, maybe we can dispose all events when user close the popup page.

If 1 MB of background page is too much, the alternative is to forget the user's password every time the popup page is closed. 1-2 MB really isn't that much for a web browser (mine is using over 0.5 GB right now!), and as long as we don't have the background page do too much it should be fine.

[Sneezry]

used to check the hash of key to make sure the correct password was entered.

This is the P1 feature we should have.

If 1 MB of background page is too much...

I checked that in Chrome Task manager, and it costs ~25MB, maybe Firefox can handle background better. If we can dispose the listener when popup is closed, it could be good.

OK, then I think this PR is worth to merge, let's release it.

[mymindstorm]

dispose the listener when popup is closed

I don't fully understand what you mean here. Am I understanding it correctly to mean set persistent: false or do you mean use removeEventListener?

It looks like chrome dedicates an entire process for each extension. It brings in ~20 MB of runtime stuff.

[Sneezry]

removeEventListener is what I mean😄

However, I have no idea how much benefit could we get from that if most memory is brought by runtime.