sailfishos / sailfish-secrets

BSD 3-Clause "New" or "Revised" License
24 stars 15 forks source link

Notify on key ring change #61

Open dcaliste opened 6 years ago

dcaliste commented 6 years ago

A use case to keep in mind for future evolution: Keys may be stored in another database that can change through actions outside secrets. There should be a way to notify secret that the keys stored by a plugin somewhere have changed.

Maybe this possibility already exists?

Venemo commented 6 years ago

Currently no. What kind of use case is there for this?

dcaliste commented 6 years ago

Sailfish secrets is not necessary the owner of a key database, especially with plugins. My example is the GnuPG keyring that can be accessed not only from secrets but via other tools : you can import a key frim gpg comnandline tool for instance.

I'm pretty sure that other use cases may be found for some other plugins.

dcaliste commented 6 years ago

I will try to propose an API here, because it's really mandatory. Use case:

There are two things to be done here:

@chriadam and @Venemo what do you think ?

monich commented 6 years ago

Duplication of data seems to be a bad idea to me. If something can get out of sync, sooner or later it will go out of sync.

Plugin specific data are stored on the device in a plugin specific way, right? From the first glance I don't see any good reason for gpg plugin to not use native gpg storage.

There's a problem with synchronizing access to the database between the daemon and the command line tool, but certain measures can be taken to reduce the probability of that to a minimum. And even if that does happen - hei, command line is for advanced users, they should understand what's going on. Or perhaps the plugin can keep the database locked somehow to make command line modifications impossible until the daemon is stopped. I don't know enough about gpg to tell which approach would work better but my feeling is that in any case it's going to be easier and more reliable than synchronizing multiple copies of the same stuff.

dcaliste commented 6 years ago

From the first glance I don't see any good reason for gpg plugin to not use native gpg storage.

That's the case. All keys are stored by GPG only in its own storage place and format. What I have written is a bridge exposing the content of this GPG database to secret API and also to be able to modify this GPG database via the secret API.

But each action (addition, deletion…) done via the secret API, as far as I know, cache the result in a metadata database. So when the GPG storage is modified by something different than the secret API, the metadata databse becomes out of sync.

dcaliste commented 6 years ago

Maybe, the clean way, would be for plugin to declare that they have their own database and that the secret metadata database should not be used. I looks cleaner than syncing I agree. But I guess it's a lot of work…

Venemo commented 6 years ago

This may be a dumb question, but do we really store all the keys in the metadata db? I was under the impression that they are queried from the plugins.

dcaliste commented 6 years ago

It's not dumb. I think before any operation like deletion or addition, a check is done that collection exists (or not) and key exists (or not) from the metadata db. @chriadam may confirm (or not ;) ). There is a PreCheckFromMetadata() or something function in daemon/SecretImpl that does it if I correctly remember.

chriadam commented 6 years ago

We have a hard requirement from OMP that we must support (at least) the following two access control modes: OwnerOnly and NoAccessControl. Specifically, they stated: "1) Any application must be able to generate key and to use it after. 2) There also must be an option to use keys stored in a shared place where any app can use keys from there."

Thus, we need to store metadata on a per-key and per-collection basis, which stores this sort of metadata (i.e. who the "owning" application is, and what the access control semantics are for that datum).

While I agree that this is suboptimal, I don't see any way to avoid this while meeting their requirement, without pushing that down the plugin (i.e. require the plugin itself to store that metadata, which seems suboptimal as some plugins (e.g. USB tokens) may have strict limitations on what they can store).