element-hq / element-desktop

A glossy Matrix collaboration client for desktop.
https://element.io
GNU Affero General Public License v3.0
1.15k stars 261 forks source link

element-desktop needs to move away from Keytar now it is deprecated #1947

Open jejb opened 2 hours ago

jejb commented 2 hours ago

Your use case

keytar is used to securely store keys on the desktop. However, atom is sunsetting the project:

https://github.com/atom/node-keytar/issues/482

Several of the keytar users are migrating to electron SafeStorage

https://github.com/filecoin-station/desktop/issues/1656 https://github.com/mountainash/taskana/issues/142

Which seems like it would be the best solution for element-desktop as well

Have you considered any alternatives?

There is no alternative to replacing a deprecated project. It already seems to be causing problems with later electron versions (https://github.com/element-hq/element-desktop/pull/1931#issuecomment-2417375615) so it's going to become a big issue soon. SafeStorage looks like the easiest path forwards, but there may be others I haven't found

Additional context

No response

t3chguy commented 2 hours ago

Migrating to SafeStorage doesn't really solve the issue, if we need to avoid data loss we would need to ship them side-by-side for a long period of time to allow a migration to occur

jejb commented 2 hours ago

Well, there are two known ways to do that. One is to save the room keys in a protected file and restore from that. The other is to get the keys from another instance (say your phone). I'm not saying either is foolproof or painless, but it is possible.

I'm not saying shipping both and trying to migrate isn't a better solution, just that, given the compile issues, it won't be viable for very long. Probably the best way to do it would be to have one version that does the migration and have later versions insist that you must first upgrade to that one version to preserve your keys.

t3chguy commented 2 hours ago

One is to save the room keys in a protected file and restore from that.

This has the same downsides to requiring both at once, as if a user skips a version which dumps to this protected file then they will simply suffer data loss

jejb commented 2 hours ago

This has the same downsides to requiring both at once, as if a user skips a version which dumps to this protected file then they will simply suffer data loss

I proposed a solution in the previous comment: have a version which users must upgrade through to preserve their keys. If that version is N then the N+M version would warn if it is an upgrade from N-P. It shouldn't force this, but it should strongly warn that you'll lose your keys and have to re-import them if you don't first upgrade to N.

I think this will have to be done regardless of which solution is taken since keytar isn't going to compile much longer (or electron will have to be held back)

t3chguy commented 1 hour ago

I proposed a solution in the previous comment: have a version which users must upgrade through to preserve their keys.

Sure but this isn't something we can achieve with the update mechanisms available to us. The update platform runs via an R2 (S3-alike) bucket and thus cannot vary update version based on your current version.

jejb commented 1 hour ago

Sure but this isn't something we can achieve with the update mechanisms available to us. The update platform runs via an R2 (S3- alike) bucket and thus cannot vary update version based on your current version.

I maintain distro packages for element-desktop so it's definitely something they can do. However, even for people installing directly, the package has various databases which go through schema updates which can be intercepted and used to flag update and deduce the old version. You can even introduce specific schema updates for the migrate and post migrate versions that would allow you to detect any N- to N+ migration.

t3chguy commented 1 hour ago

@jejb on Linux, sure, on Windows & macOS with the in-Electron auto-updater though that would not be the case. The Squirrel.Mac & Squirrel.Windows updaters just hit a URL on packages.element.io which receives a static file from an R2 bucket and fetches the new update from there.

jejb commented 1 hour ago

@jejb on Linux, sure, on Windows & macOS with the in-Electron auto- updater though that would not be the case. The Squirrel.Mac & Squirrel.Windows updaters just hit a URL on packages.element.io which receives a static file from an R2 bucket and fetches the new update from there.

I get that ... that's why in the previous comment I recommended using a schema version update trigger for this case. It's somewhat similar to what was done when the name was changed from Riot.im to Element.

t3chguy commented 1 hour ago

Right, but if a user updates from version A -> C (missing B)

where A is only Keytar (old version from today) B is Keytar + Migration code + SafeStorage C is Migration code + SafeStorage

then how does the user on C not end up with data corruption however you trigger it. Without Keytar you can't access the data it was storing on your behalf, so you have to start your SafeStorage in a clean slate i.e. data loss.

jejb commented 1 hour ago

Right, but if a user updates from version A -> C (missing B)

where A is only Keytar (old version from today) B is Keytar + Migration code + SafeStorage C is Migration code + SafeStorage

then how does the user on C not end up with data corruption however you trigger it. Without Keytar you can't access the data it was storing on your behalf, so you have to start your SafeStorage in a clean slate i.e. data loss.

Well because a trigger on db upgrade gets called before anything destructive is done. That way you can ask the user from the trigger to proceed and destroy their keys (if they have a backup they can restore from) or advise them to cancel and install B to preserve the keys.

t3chguy commented 57 minutes ago

Trying to support "manually install version B" where different customer builds have different version schemes and different support policies would be nigh impossible. The better alternative I can think of is making a standalone static build of keytar + safeStorage which we build once and include for a long time as a migration layer which we depend on and run before loading the app if necessary