Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a secret portal #359

Merged
merged 1 commit into from
Sep 22, 2019
Merged

Conversation

ueno
Copy link
Contributor

@ueno ueno commented Aug 19, 2019

This adds a portal interface to provide sandboxed applications with master secret to locally encrypt credentials. It works as follows: the application first creates a pipe, sends the write end to the portal, and the portal sends back a secret value through the FD. That way, we can avoid any transport encryption.

If the master secret doesn't exist in keyring, the portal generates a random secret. This step needs to be done with a standard conforming crypto library as the secret is used for encryption. Therefore the backend implementation resides in gnome-keyring.

See also:

Fixes #14

@ueno
Copy link
Contributor Author

ueno commented Aug 19, 2019

Here is a quick demo https://asciinema.org/a/9gAd1DMQuucd3brCGp3SynBtj

@ueno ueno force-pushed the wip/dueno/secret-portal branch from 7065eea to 387cf5b Compare August 19, 2019 16:48
@matthiasclasen
Copy link
Contributor

What is WIP about this ? It looks basically good to merge, to me

@TingPing
Copy link
Member

If the master secret doesn't exist in keyring, the portal generates a random secret. This step needs to be done with a standard conforming crypto library as the secret is used for encryption. Therefore the backend implementation resides in gnome-keyring.

How unresonable would it be to de-couple this from gnome-keyring. I know it isn't a crazy dep but I personally don't want to have to tell all flatpak users it depends on gnome-kerying now.

@matthiasclasen
Copy link
Contributor

If the master secret doesn't exist in keyring, the portal generates a random secret. This step needs to be done with a standard conforming crypto library as the secret is used for encryption. Therefore the backend implementation resides in gnome-keyring.

How unresonable would it be to de-couple this from gnome-keyring. I know it isn't a crazy dep but I personally don't want to have to tell all flatpak users it depends on gnome-kerying now.

It is a somewhat soft dep though - we depend on a implementation for org.freedesktop.impl.portal.Secret to be present, and as things are now, the only one will be in gnome-keyring. It could be moved out to xdg-desktop-portal-gtk, if that is feasible.

But I don't think it is such a big problem. If the backend is not there, the secret portal interface will simply not be provided.

@TingPing
Copy link
Member

But I don't think it is such a big problem. If the backend is not there, the secret portal interface will simply not be provided.

And every application depending on it won't work... Its a hard dep IMO. It has been how many years since the Secret service API was made and gnome-keyring is still the only shipped implementation AFAIK.

I don't think its a blocker to be merged but I think moving it somewhere else would be a win.

@matthiasclasen
Copy link
Contributor

I don't think its a blocker to be merged but I think moving it somewhere else would be a win.

Yes. Daiki, would it be hard to separate the backend implementation from gnome-keyring, and move it to xdg-desktop-portal-gtk? It would of course still be the sole implementation and likely still depend on gnome-keying, so you trade a hard direct dependency on gnome-keyring with a hard indirect dependency....

@ueno
Copy link
Contributor Author

ueno commented Sep 20, 2019

I don't think its a blocker to be merged but I think moving it somewhere else would be a win.

Yes. Daiki, would it be hard to separate the backend implementation from gnome-keyring, and move it to xdg-desktop-portal-gtk? It would of course still be the sole implementation and likely still depend on gnome-keying, so you trade a hard direct dependency on gnome-keyring with a hard indirect dependency....

What I'm most concerned is that that would bring in a direct dependency on a crypto library (OpenSSL, GnuTLS, libgcrypt; each has pros and cons) from xdg-desktop-portal-gtk to generate secure random bytes that can be used as encryption key.

And also, the generated key must be sent to gnome-keyring through the custom wire encryption, which I want to dismiss in the next few years.

@TingPing
Copy link
Member

What I'm most concerned is that that would bring in a direct dependency on a crypto library (OpenSSL, GnuTLS, libgcrypt; each has pros and cons) from xdg-desktop-portal-gtk to generate secure random bytes that can be used as encryption key.

I don't think that is really a concern of xdg-desktop-portal-gtk. The license of gnutls isn't a problem and glib-networking in practice always uses gnutls.

If it always has to talk to gnome-keyring its a moot point anyway.

@ueno
Copy link
Contributor Author

ueno commented Sep 20, 2019

If it always has to talk to gnome-keyring its a moot point anyway.

I'm afraid that's the case, unless xdg-desktop-portal-gtk implements what gnome-keyring is doing to protect secrets on the host side (e.g., encrypting secrets with login password through pam).

@TingPing
Copy link
Member

I guess we'll just have to live with it then. Note to future @matthiasclasen, mention in release notes for packagers of the added runtime dep.

@matthiasclasen
Copy link
Contributor

Daiki, is there anything left here that is WIP ?

Is this ready to merge from your perspective ?

@ueno
Copy link
Contributor Author

ueno commented Sep 20, 2019

I marked this as WIP because the other parts (libsecret and gnome-keyring) are still under review. If it is fine I'll update the doc and remove WIP.

@matthiasclasen
Copy link
Contributor

I marked this as WIP because the other parts (libsecret and gnome-keyring) are still under review. If it is fine I'll update the doc and remove WIP.

Yes, please. I think this probably needs to land first, right ? Since both of the other changes will need the D-Bus interfaces defined here.

This adds a portal interface to provide sandboxed applications with
master secret to locally encrypt credentials. It works as follows: the
application first creates a pipe, sends the write end to the portal,
and the portal sends back a secret value through the FD. That way, we
can avoid any transport encryption.

If the master secret doesn't exist in keyring, the portal generates a
random secret. This step needs to be done with a standard conforming
crypto library as the secret is used for encryption. Therefore the
backend implementation resides in gnome-keyring.
@ueno ueno force-pushed the wip/dueno/secret-portal branch from 387cf5b to aa42ec9 Compare September 22, 2019 07:19
@ueno ueno changed the title WIP: Add a secret portal Add a secret portal Sep 22, 2019
@matthiasclasen matthiasclasen merged commit 16f47ee into flatpak:master Sep 22, 2019
AsavarTzeth added a commit to flathub/org.keepassxc.KeePassXC that referenced this pull request Nov 11, 2019
Cannot be guaranteed to work by default, due to unresolvable conflicts
with gnome-keyring.

Gnome Keyring includes a org.freedesktop.secrets D-Bus service that
allow dependent apps to launch it, if it isn't running. Depending on
the distribution or desktop environment it can be very difficult or
unpractical to disable this. Uninstalling it may also not be an option
due to package dependencies.

- From a trust perspective (not to mention Flathub policy?) it might be
  questionable to allow the app to own org.freedesktop.secrets.

- Future xdg portals, will depend on a sandbox compatible implementation
  of the service and because of this gnome-keyring could become a
  xdg-desktop-portal dependency.
  See: flatpak/xdg-desktop-portal#311
       flatpak/xdg-desktop-portal#359

- Any sandbox compatible implementation replacing gnome-keyring would
  surely need to run on its own, outside the application sandbox.

- Removing gnome-keyring and replacing it with an incomplete
  replacement could break other Flatpak apps in the future.
@runiq
Copy link

runiq commented Nov 11, 2019

It has been how many years since the Secret service API was made and gnome-keyring is still the only shipped implementation AFAIK.

I'd like to point out that, with the recent KeePassXC release, the above is no longer true. KeePassXC 2.5 has an implementation of the Secret Storage API.

AsavarTzeth added a commit to flathub/org.keepassxc.KeePassXC that referenced this pull request Nov 12, 2019
Cannot be guaranteed to work by default, due to unresolvable conflicts
with gnome-keyring.

Gnome Keyring includes a org.freedesktop.secrets D-Bus service that
allow dependent apps to launch it, if it isn't running. Depending on
the distribution or desktop environment it can be very difficult or
unpractical to disable this. Uninstalling it may also not be an option
due to package dependencies.

- From a trust perspective (not to mention Flathub policy?) it might be
  questionable to allow the app to own org.freedesktop.secrets.

- Future xdg portals, will depend on a sandbox compatible implementation
  of the service and because of this gnome-keyring could become a
  xdg-desktop-portal dependency.
  See: flatpak/xdg-desktop-portal#311
       flatpak/xdg-desktop-portal#359

- Any sandbox compatible implementation replacing gnome-keyring would
  surely need to run on its own, outside the application sandbox.

- Removing gnome-keyring and replacing it with an incomplete
  replacement could break other Flatpak apps in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a portal for keyring access
4 participants