portal: Support migration from legacy keyring format

bilelmoussaoui / oo7

James Bond went on a new mission as a Secret Service provider

https://bilelmoussaoui.github.io/oo7/oo7/

MIT License

57 stars 11 forks source link

portal: Support migration from legacy keyring format #76

Closed ueno closed 3 months ago

ueno commented 3 months ago

This adds support for transparent data migration from legacy keyring format (version 0) used by gnome-keyring. If the legacy version is found, it tries to decrypt items in memory, encrypt and store them in a new keyring in the version 1 format.

ueno commented 3 months ago

Thank you for the prompt review!

A6GibKm commented 3 months ago

For the sake of documenting the behavior, gnome-keyring won't be able to read the keyring back right?

ueno commented 3 months ago

For the sake of documenting the behavior, gnome-keyring won't be able to read the keyring back right?

That is true. To be on the safe side, we may want to use a different path somehow for legacy/modern keyrings.

ueno commented 3 months ago

Thinking a bit more about this, I've come up with the following proposal (from the oo7-daemon perspective):

Let's start with a new directory, say ~/.local/share/oo7/keyrings/ instead of ~/.local/share/keyrings/
Make portal::Keyring::load (possibly need a new function) take a keyring name, instead of a path
The function first looks at ~/.local/share/oo7/keyrings/<name>.keyring. If it's found, use it anyway
Otherwise look at ~/.local/share/keyrings/<name>.keyring (which can be either version 0 or 1). If it's found, migrate it to the new location (~/.local/share/oo7/keyrings/<name>.keyring) when writing
Otherwise, just create a new keyring at ~/.local/share/oo7/keyrings/<name>.keyring

How does it sound @A6GibKm @bilelmoussaoui?

bilelmoussaoui commented 3 months ago

Let's start with a new directory, say ~/.local/share/oo7/keyrings/ instead of ~/.local/share/keyrings/

Make portal::Keyring::load (possibly need a new function) take a keyring name, instead of a path

The function first looks at ~/.local/share/oo7/keyrings/<name>.keyring. If it's found, use it anyway

Otherwise look at ~/.local/share/keyrings/<name>.keyring (which can be either version 0 or 1). If it's found, migrate it to the new location (~/.local/share/oo7/keyrings/<name>.keyring) when writing

Otherwise, just create a new keyring at ~/.local/share/oo7/keyrings/<name>.keyring

That sounds sane to me, not sure about using oo7 in the path but we can figure out that later.

bilelmoussaoui commented 3 months ago

Thinking a bit more about this, I've come up with the following proposal (from the oo7-daemon perspective):

* Let's start with a new directory, say `~/.local/share/oo7/keyrings/` instead of `~/.local/share/keyrings/`

* Make `portal::Keyring::load` (possibly need a new function) take a keyring name, instead of a path

* The function first looks at `~/.local/share/oo7/keyrings/<name>.keyring`. If it's found, use it anyway

* Otherwise look at `~/.local/share/keyrings/<name>.keyring` (which can be either version 0 or 1). If it's found, migrate it to the new location (`~/.local/share/oo7/keyrings/<name>.keyring`) when writing

* Otherwise, just create a new keyring at `~/.local/share/oo7/keyrings/<name>.keyring`

How does it sound @A6GibKm @bilelmoussaoui?

Is this something you think we should do as part of this PR? I think it is something that can be done at a later stage in a different PR :)

ueno commented 3 months ago

Is this something you think we should do as part of this PR? I think it is something that can be done at a later stage in a different PR :)

I think it's feature complete now. I can follow up in a separate PR if any issue arises. Thank you for the review!