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

Using python-keyring with KeePassXC #448

Closed
runiq opened this issue Jul 15, 2020 · 14 comments · Fixed by #586
Closed

Using python-keyring with KeePassXC #448

runiq opened this issue Jul 15, 2020 · 14 comments · Fixed by #586

Comments

@runiq
Copy link

runiq commented Jul 15, 2020

Describe the bug

The format of data in a KeePassXC database is different from what the SecretService backend expects, which means there's no way to get data created natively in KeePassXC.

To Reproduce

  1. Download KeePassXC (2.6.0 has an appimage version, just download and run)

  2. Create a new database for testing

  3. Enable Secret Service Integration, expose your database

  4. Create a new entry, title foo, username bar, password baz

  5. Create a new entry from python-keyring in the database, but this time using python-keyring:

    import keyring as k
    k.set_keyring(k.backends.SecretService.Keyring())
    k.set_password('kfoo', 'kbar', 'kbaz')
  6. Observe the different entry layouts. (The attributes set by python-keyring can be found by right-clicking the entry, then "Properties", then "Advanced" on the left.)

  7. Note that there's no way to get at a "natively created" KeePassXC entry using the Secret Service backend.

Expected behavior

  • keyring.get_password() works correctly through KeePassXC's Secret Service integration.
  • A way to get entries created in KeePassXC using python-keyring.

Environment

  • OS: Fedora 32
$ pip list | grep keyring
keyring 21.2.0
$ keyring --list-backends
keyring.backends.chainer.ChainerBackend (priority: 10)
keyring.backends.kwallet.DBusKeyring (priority: 5.1)
keyring.backends.fail.Keyring (priority: 0)
keyring.backends.SecretService.Keyring (priority: 5)

Additional context

What I would really like to do is to use the Ansible keyring lookup plugin to find credentials stored in KeePassXC, but that won't be possible unless python-keyring understands the format provided by KeePassXC, I believe.

This is how secret-tool formats the KeePassXC generated entry:

[/org/freedesktop/secrets/collection/runiq/6318916706d54321a7ffd0b0a744c05a]
label = foo
secret = baz
created = 2020-07-15 17:18:11
modified = 2020-07-15 17:18:19
attribute.Title = foo
attribute.URL =
attribute.UserName = bar
attribute.Path = /foo
attribute.Uuid = 6318916706d54321a7ffd0b0a744c05a
attribute.Notes =

And this is the python-keyring generated entry:

[/org/freedesktop/secrets/collection/runiq/e50ead9611834503acd02bedd3791ce4]
label = Password for 'kbar' on 'kfoo'
secret = kbaz
created = 2020-07-15 18:00:25
modified = 2020-07-15 18:00:25
attribute.URL =
attribute.Title = Password for 'kbar' on 'kfoo'
attribute.UserName = runiq
attribute.Notes =
attribute.application = Python keyring library
attribute.Uuid = e50ead9611834503acd02bedd3791ce4
attribute.username = kbar
attribute.Path = /Password for 'kbar' on 'kfoo'
attribute.service = kfoo
@runiq
Copy link
Author

runiq commented Jul 15, 2020

I've also found this from this issue, which seems relevant, but I'm not really sure…

@jaraco
Copy link
Owner

jaraco commented Aug 22, 2020

Thanks for the report, especially capturing the entries from the two systems.

Have a look at the code where SecretService sets and gets passwords. As you can see, that's where the attributes username and service are set and queried.

I suspect if you were to change those keys to UserName and Title, that those entries would match the convention used by KeePassXC. Or if you were to file a bug with KeePassXC, they could change their convention to match that of Python-keyring. But even that would only work to make those two applications match. What about other consumers of the APIs?

Is there a place where these conventions are more rigorously designed and defined?

Assuming we can identify a preferred convention to follow, this library will need to provide some sort of process to transition from the old convention to the new.

Any chance you'd be willing to spearhead this effort, investigate the problem space, devise a plan, and implement a solution?

@lovetox
Copy link
Contributor

lovetox commented Aug 22, 2020

Hi,

It seems the API for example of libsecret was designed so you need to know the attributes to find an item.

Is it really the right way to try to get all projects to use the same convention? Surely possible but considering the migration work we may consider easier solutions.

Could we not add a KeePassXC backend which is only a hook into the SecretService Backend with different attributes?

@jaraco
Copy link
Owner

jaraco commented Dec 22, 2020

Is it really the right way to try to get all projects to use the same convention?

Not necessarily, but it becomes increasingly complicated to support and maintain support for more conventions.

Could we not add a KeePassXC backend which is only a hook into the SecretService Backend with different attributes?

Yes, that's one option (although you may have trouble superseding SecretService, as Chainer will involve all viable backends). Another option might be to provide detection or awareness of KeePassXC within SecretService or to provide a configuration setting that enables another scheme. The readme describes a recent innovation that allows a backend to solicit parameters from the environment. That may be a possible approach.

@Aetf
Copy link

Aetf commented Jan 5, 2021

The fundamental problem is that the spec doesn't include anything about the semantics of attributes.

For @runiq's particular use case, it is actually achievable. And I've done exactly the same thing before. The trick is to create another entry in KeePassXC, whose password field refers to the main entry. You can configure this new entry with whatever attributes required by keyring to find it.

Or, if the main entry is already in the exposed database group, why not just adding those attributes to it?

@jaraco
Copy link
Owner

jaraco commented Jan 24, 2021

In #485, we learned that there's a concept in SecretService of a "Schema". Does KeyPassXC define such a schema?

@Aetf
Copy link

Aetf commented Jan 24, 2021

No, KeePassXC doesn't define its own schema, and doesn't treat the attribute specially. It simply stores whatever the client wants to save.

@lovetox
Copy link
Contributor

lovetox commented Jul 31, 2022

I thought about this and i think there is some confusion about what python-keyring should or can provide.

Secret Service Backend

The secret-service spec provides a way for application to store a secret together with attributes. The attributes can be almost anything. Which makes sense because a project like secret-service can not define sensible attributes for all applications and usecases. It seems they not even tried. So its up to user facing applications to define specs. For example mail clients could standardize on attributes like

mail-account -> mail@domain.net
mail-protocol -> imap

if python-keyring is used by applications to store credentials it uses

username -> what-ever-username
service -> App or Service

KeepassXC decided on

UserName -> what-ever-username
Title -> App or Service

There is no standard or spec how credentials should be saved by applications, and python-keyring should not try to do some magic. Applications must agree on it or spec it themself. python-keyring needs to make it possible to retrieve and store passwords in any possible way, but must not mandate how they are saved (how attributes are named etc), except for the use case where an application which uses python-keyring chooses actively to not care and let python-keyring chose. (See simple use case below)

I propose python-keyring should fulfill 2 use cases

Simple

As an application i don’t care about other applications to retrieve my stored passwords. I just want to store passwords and retrieve it myself later.

This use case is already fulfilled with the current get_password() and set_password() methods. And works to my knowledge even if KeepassXC is used as secret-service implementation.

Advanced

As an application i need to store my passwords in a specific way, this may be because of standards and agreements with other applications. The passwords stored need to be retrievable by other applications. I want to retrieve passwords other applications have stored.

This is currently not possible because we hard code the attribute names service and username when using the secret-service backend.

Proposal

So i propose backend specific get_password_advanced() and set_password_advanced() methods which allow to specify various attributes which are then stored or retrieved.

These methods would be backend specific, the signature would look potentially different depending on the backend or not even be available. Applications would need to check themself which backend or platform is chosen and act on it.

This would basically be a new feature in python-keyring for advanced usage, not affecting the Simple use case.

This would allow applications to store and retrieve passwords in a way agreed with KeepassXC. How they find out that KeepassXC runs and is used, is out of scope for python-keyring, as we simply use the secret-service backend.

Its confusing that KeepassXC acts here as an implementation of the secret service API, but also at the same time is a user facing application that stores secrets how it sees fit. In this part is not different to e.g. a mail client. As KeepassXC is just another application, we should not add code to make it "compatible". As we would never add code just because some mail client decides to store its passwords with a weird attribute name. But it would be a great feature if python-keyring allows applications to store/retrieves passwords however they want, this gives them the flexibility to have agreements and standards in the future.

@mitya57
Copy link
Collaborator

mitya57 commented Aug 2, 2022

Applications must agree on it or spec it themself. python-keyring needs to make it possible to retrieve and store passwords in any possible way, but must not mandate how they are saved (how attributes are named etc), except for the use case where an application which uses python-keyring chooses actively to not care and let python-keyring chose.

If you end up writing some code which will deal with specifics of Secret Service backend, then you don't need keyring library, you can use SecretStorage library directly, with which you can set any attributes you want:
https://secretstorage.readthedocs.io/en/latest/

@lovetox
Copy link
Contributor

lovetox commented Aug 2, 2022

I agree its a valid argument to say, python-keyring provides only one unified API over all backends.
But this means the use case to retrieve arbitrary secrets (stored by other applications) is not in scope of the project.

This still would leave the option to treat KeepassXC as its own backend, basically a slight variant of the libsecret backend.

What is in my opinion unreasonable, is to change the way python-keyring stores its passwords currently with the libsecret backend and migrate everything to the way KeepassXC stores it. There is no agreement with KeepassXC, no standard or any spec about the attributes they use to store their secrets. And we cannot depend on them to not change it if they feel like it. Further other password managers could also implement the secret-service API and store their passwords again differently.

Writing a slight variation of the libsecret backend is very easy and flexible for the future. It also does not add the migration related risks.

jaraco added a commit that referenced this issue Aug 2, 2022
…eypassXC' to use the username/system convention from KeypassXC, either with KEYRING_PROPERTY_SCHEME or keyring.with_properties(scheme='KeypassXC'). Fixes #448.
@jaraco
Copy link
Owner

jaraco commented Aug 2, 2022

Thanks. I think lovetox makes a compelling argument. I'd like to explore a possible implementation. In a branch, I've drafted a change that might support allowing the user to customize the scheme. Install with pip install git+https://github.com/jaraco/keyring@feature/secretservice-scheme and then either set the environment variable KEYRING_PROPERTY_SCHEME=KeypassXC or get a keyring programmatically and use keyring.with_properties(scheme='KeypassXC'), and then set/get/delete/credential and let me know how it works for you.

@lovetox
Copy link
Contributor

lovetox commented Aug 4, 2022

I tested this and it works for retrieving passwords.

It would also work for storing passwords but its blocked by another bug i discovered mitya57/secretstorage#39

I applied a fix for that bug locally and it worked from that point on.

So i think LTGM

@lovetox
Copy link
Contributor

lovetox commented Aug 4, 2022

I don’t understand why there is a libsecret backend if the SecretService backend does the same thing.

But can this patch also be applied to the libsecret backend?

@jaraco
Copy link
Owner

jaraco commented Aug 4, 2022

I don’t understand why there is a libsecret backend if the SecretService backend does the same thing.

#521 (comment)

But can this patch also be applied to the libsecret backend?

Yes, probably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants