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 libsecret backend #521

Merged
merged 6 commits into from Sep 11, 2021
Merged

Add libsecret backend #521

merged 6 commits into from Sep 11, 2021

Conversation

a-andre
Copy link

@a-andre a-andre commented Aug 7, 2021

With the libsecret backend python-keyring can be used in flatpak without opening a hole for org.freedesktop.secrets which allows cross-application password access.

The backend works also outside of flatpak.

(Secret-portal support isn't in the scope of SecretStorage)

keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
@a-andre
Copy link
Author

a-andre commented Aug 8, 2021

Thanks for the feedback.

Copy link
Collaborator

@mitya57 mitya57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better now, but I still have some comments.

Also please fix the style errors from Black which make the CI fail: https://github.com/jaraco/keyring/pull/521/checks?check_run_id=3280102581.

keyring/backends/libsecret.py Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
keyring/backends/libsecret.py Outdated Show resolved Hide resolved
Copy link
Owner

@jaraco jaraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach looks sound.

All that's needed is some background on what this backend is meant to accomplish. Normally, the built-in backends are meant to be backends that are generally useful, and keyring recommends for less common backends to be implemented as an optionally-installed plugin.

What is the role of libsecret? What is the relationship of flatpack to libsecret? How does libsecret relate to SecretService and kwallet backends? What users will benefit from this change? What users will be possibly negatively affected by this change?

I see the priority is set to 5, meaning it's exactly as preferable as SecretService. Do you expect both of these to be viable in the same environment?

@mitya57
Copy link
Collaborator

mitya57 commented Aug 16, 2021

@jaraco libsecret is a library from GNOME project whose initial purpose is wrapping the Secret Service D-Bus API, so in this aspect it is very similar to SecretStorage module in Python.

However, starting with version 0.20.0, libsecret also provides a second backend, that is using a local file instead of Secret Service. It is used mainly for flatpak applications, where for security reasons access to Secret Service API is not granted. Flatpak is a technology for app isolation. See https://gitlab.gnome.org/GNOME/libsecret/-/merge_requests/6 for some details.

So having a libsecret backend in Keyring will help flatpak applications to store their passwords.

This new backend is compatible with the existing SecretStorage-based backend when flatpak is not used:

>>> k1 = keyring.backends.libsecret.Keyring()
>>> k2 = keyring.backends.SecretService.Keyring()
>>> k1.set_password('foo', 'bar', 'baz')
>>> k2.get_password('foo', 'bar')
'baz'

It may sound like we don't need the SecretStorage backend anymore. But it's not true — SecretStorage and jeepney are pure Python libraries and they can be easily installed from PyPI. It is not true for libsecret: libsecret requires PyGObject which needs to be installed from system packages or built from source.

Regarding the priorities, I don't have a strong opinion but maybe I would lower the priority of new backend from 5 to something like 4.8, just to make sure the behavior is deterministic and not random (4.9 is taken by KWallet).

@mitya57 mitya57 requested a review from jaraco August 19, 2021 17:28
@jaraco
Copy link
Owner

jaraco commented Aug 20, 2021

I notice the tests don't get run. I thought I remembered that at least one Linux backend had its tests run in CI when running under Linux. Perhaps that functionality was lost when migrating from Travis to Github. Is it possible for the tests for libsecret to be run?

@a-andre
Copy link
Author

a-andre commented Aug 30, 2021

With pygobject; sys_platform=="linux" added to tox testenv dependencies, the tests are run. But they fail with
gi.repository.GLib.GError: g-io-error-quark: Cannot autolaunch D-Bus without X11 $DISPLAY (0).

@mitya57
Copy link
Collaborator

mitya57 commented Sep 10, 2021

You should use dbus-run-session to run tests in a headless environment. Look what I do in SecretStorage project:

https://github.com/mitya57/secretstorage/blob/master/.github/workflows/main.yml#L19-L25

I use a mock server there, but having a real gnome-keyring installed should also work.

@takluyver
Copy link
Contributor

It may sound like we don't need the SecretStorage backend anymore. But it's not true — SecretStorage and jeepney are pure Python libraries and they can be easily installed from PyPI. It is not true for libsecret: libsecret requires PyGObject which needs to be installed from system packages or built from source.

Just to note, if people wanted, it should be possible (and maybe an interesting exercise :-) to build a Python library which wraps the portals secret API and stores/retrieves passwords in an encrypted file - either copying whatever libsecret does, or based on one of the existing encrypted file backends (keyrings.cryptfile or EncryptedKeyring in keyrings.alt). This example script shows how to call a portal API (albeit FileChooser rather than Secret).

It might be trickier to make this easily installable with only pure Python packages, because I think most of the crypto packages need extension modules for speed. But there might be contexts where being independent of the GObject universe is an advantage.

If you do this for real world use, obviously getting the encrypted file right is important - it would probably be easy to do something that works but isn't very secure.

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.

None yet

4 participants