Default keyring is insecure

[jaraco]

Just using the default from the keyring module defaults to a plaintext keyring file, which is very insecure. Either default to the system keyring, an otherwise encrypted keyring, or just fail with an error if you can't actually provide a secure keyring.

Making plaintext the default will just lead users to think they are secure when their passwords are all dumped to the disk in plain text, which is insane.

---

• Bitbucket: https://bitbucket.org/kang/python-keyring-lib/issue/117
• Originally reported by: Stavros Korokithakis
• Originally created at: 2013-09-27T14:11:44.531

[jaraco]

It's not the case that the default keyring is insecure. The actual implementation prefers the most secure key ring available based on the environment.

On Mac OS and Windows systems, the system security provider is used, and they provide high-security.

On Linux, the situation is more complicated. There are secure providers for gnome and KDE, but those are only available in the GUI.

The fallback of course is the encrypted key ring, but that requires PyCrypto to be installed.

I agree there's a use case here to be considered, and that use case is, as you describe, for a new, novice user. Maybe you're right, and The only time the plaintext key ring should be used is when it's explicitly selected in configuration.

Such a change would affect users on the next systems where access to the file system is not a concern, such as with encrypted file systems, isolated backend systems, or non-shared systems.

---

Original comment by: Jason R. Coombs

[jaraco]

Hmm, I'm using Ubuntu with Unity, wouldn't the Gnome keyring be preferred in this case? Would you happen to know why it fell back to the plaintext keyring?

---

Original comment by: Stavros Korokithakis

[jaraco]

Looking at the Gnome backend, it imports gi.repository.GnomeKeyring, and if that's not present, it's not considered a viable keyring. So it seems that PyGI must be installed for Gnome support. I also just noted that PyGI appears to be deprecated according to their wiki. I guess that just means that keyring should use PyGObject directly, but that's a separate issue.

In general, you can test for the viability and priority of any keyring by querying the .priority of its class (requires keyring >= 2.0):

>>> import keyring
>>> keyring.backends.Gnome
<module 'keyring.backends.Gnome' from 'c:\\python\\lib\\site-packages\\keyring-3.1b1-py3.3.egg\\keyring\\backends\\Gnome.py'>
>>> keyring.backends.file.PlaintextKeyring.priority
0.5
>>> keyring.backends.Gnome.Keyring.priority
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\python\lib\site-packages\keyring-3.1b1-py3.3.egg\keyring\util\properties.py", line 22, in __get__
    return self.fget.__get__(None, owner)()
  File "c:\python\lib\site-packages\keyring-3.1b1-py3.3.egg\keyring\backends\Gnome.py", line 32, in priority
    raise RuntimeError("GnomeKeyring module required")
RuntimeError: GnomeKeyring module required

As you can see on my Windows box, the GnomeKeyring module has no priority, so would never be selected.

Can you try installing the PyGI in your environment and see if it then uses Gnome Keyring?

I don't disagree that the behavior you described is insecure, though it's been the behavior of keyring since its inception, so I'm somewhat reluctant to change it for that reason, and especially as other use cases almost certainly depend on the existing behavior.

I invite you or others to comment on how to address this issue. One possibility is that a 'minimum priority' could be enforced, which could exclude viable but not-recommended keyring backends like PlaintextKeyring.

---

Original comment by: Jason R. Coombs

[jaraco]

I tried doing that but I didn't manage to find a way to install PyGI and gave up after a few minutes. It's good that the priorities are sane, but I do think there should be some sort of warning before using an insecure backend, at least. Both I and a friend went with what I later discovered to be the plaintext backend because there was no indication that that was being used.

---

Original comment by: Stavros Korokithakis

[jaraco]

PyGI is not deprecated, it's that wiki page that is deprecated (PyGI is now part of PyGObject package).

In Ubuntu 13.10, Python-Keyring packages pull in SecretStorage module, so that backend should be always used. 13.04 has an old version of Python-Keyring, which just uses Secret Service directly — that should also work fine in most cases.

P.S. If you use 13.10 and want GnomeKeyring backend to be available, install gir1.2-gnomekeyring-1.0 package.

---

Original comment by: Dmitry Shachnev

[jaraco]

Ah, thank you for the info. I'm not running 13.10 yet, but I will try this when I upgrade. For now, a warning (or at least a mention in the documentation) should suffice for people who are using the plaintext backend, I think.

---

Original comment by: Stavros Korokithakis

[jaraco]

I just spent several hours working around this issue on Ubuntu 13.04. I want my app to use the keyring to store database passwords securely, so I used keyring with the default implementation. Imagine my surprise when I decided to see what backend it was using and I see PlainText!

FWIW, I would strongly recommend the default option to be to throw an error when only insecure backends are available and, if the developer wants to allow insecure backends, make that be an extra hoop to jump through. Explicit is better than implicit.

#!python

keyring.allow_insecure_backend = True
...

I'm currently restricting usage of any backends with priority < 1, but don't think that's a great solution since priority is arbitrary. I'd prefer to see a property like:

#!python

if not backend.is_secure:
    raise ...

Also, it turns out that I had to do some extra work to get keyring in a virtualenv. Here are the details:

https://gist.github.com/rsyring/6895791

---

Original comment by: Randy Syring

[jaraco]

If you want a specific keyring, why don't you just use it explicitly?

>>> import keyring.backends.SecretService
>>> keyring = keyring.backends.SecretService.Keyring()

Re 13.04 package, it is really too old, and I don't think we are going to patch it any more (13.04 support ends in 3 months).

---

Original comment by: Dmitry Shachnev

[jaraco]

@rsyring, thanks for that venv info, I hit the same problem too.

@mitya57, it's not about wanting a specific keyring, it's about expecting a secure implementation and getting an insecure one.

---

Original comment by: Stavros Korokithakis

[jaraco]

@Stavros Well, it should work with the latest Python-Keyring. Bugs in distro releases are out of scope for this tracker.

Also, Ubuntu 13.04 support really ends in January, see e.g. the announcement.

---

Original comment by: Dmitry Shachnev

[jaraco]

@rsyring I'm reluctant to implement is_secure and allow_secure, as the term 'secure' is somewhat subjective. In some environments, a plaintext storage is adequate. It doesn't provide security of any kind, and it's the responsibility of the admin to secure the system or environment. The plain text keyring is suitably secure when stored on an encrypted directory or volume as well.

So I try a different take on your issue - does the keyring backend "provide" the security. However, in this case, only the Crypto keyring qualifies (and the other system-backed keyrings don't).

So I struggle to make a judgment on what is secure and what is not.

Although the use of the priority < 1 seems arbitrary, the measure has a documented purpose. A priority of < 1, > 0 is suitable but a priority >= 1 is recommended. I believe that's the best indicator keyring can give about backends without more detail about the user's environment and usage.

I can imagine some solutions here:

One would be to move the Plaintext keyring out of the project altogether and require it to be included explicitly as a separate package if the user desires for it to be available.

A less invasive approach could be to disable the Plaintext Keyring by default, and only enable it by API or environment variable (opt-in), or enable it by default, but disable it explicitly (opt-out).

Another approach could involve raising a UserWarning when passwords are saved in the plaintext keyring (and implementers could suppress that warning in the API or with an environment variable).

The latter two options might prove inconvenient on systems where security of the file is not a concern.

I can imagine other solutions, too. None of them strike me as particularly optimal.

I'm eager to hear your responses on these proposals. I still agree that it's important to address this issue and find a less surprising outcome in this case.

---

Original comment by: Jason R. Coombs

[jaraco]

Jason,

Thanks for the response. I'm going to share two assumptions I have which is driving my perspective on this issue:

1. Anyone going through the extra work of using the keyring library wants a more secure implementation than plaintext, otherwise they would likely have just used a config file.
2. Many/most developers don't read documentation and will use default settings unless the program flat-out doesn't work, at which point they will read more closely.

I realize that these assumptions may not always be valid, but IMO we should approach the solution to this issue assuming the above is true and then allowing devs to make exceptions as needed.

Although the use of the priority < 1 seems arbitrary, the measure has a documented purpose. A priority of < 1, > 0 is suitable but a priority >= 1 is recommended. I believe that's the best indicator keyring can give about backends without more detail about the user's environment and usage.

I understand that there is some ambiguity here based on a user's use-case. I guess, I would argue, that the plain-text option is really "unsuitable" in most instances and sometimes suitable. Maybe the terms should be updated to "available" and "recommended."

One would be to move the Plaintext keyring out of the project altogether and require it to be included explicitly as a separate package if the user desires for it to be available.

Seems like too much work to me. :)

A less invasive approach could be to disable the Plaintext Keyring by default, and only enable it by API or environment variable (opt-in)

I like this. Although, I'd like to adjust the proposal to make anything less than recommended opt-in. That way, the default implementation is most secure and there is extra effort required on the devs part to make less secure. Requiring extra effort will also likely lead to the dev reading the docs more closely and understanding better the tradeoffs for what they are choosing to do.

I propose a api variable something like:

#!python

keyring.allow_unrecommended_backends = True

or, if you don't like the connotation of "unrecommended", then maybe:

#!python

keyring.allow_less_secure_backends = True

Either one will, IMO, encourage devs to read the docs more carefully and understand the tradeoffs of which backend they are using.

If no recommended backends are available, and the API setting isn't enabled, then I would prefer to see an exception thrown indicating such.

I realize this would lead to some backwards incompatibility issues. IMO, that is acceptable. In order to ease the transition, I would encourage you to make a an initial release that issues a deprecation warning if an unrecommended backend is used by default without the explicit opt-in. Then, make a new major release 3-6 months later which implements the API change.

Another approach could involve raising a UserWarning when passwords are saved in the plaintext keyring (and implementers could suppress that warning in the API or with an environment variable).

This is better than nothing, but I like the above option better. If you are going to require the dev to make changes anyway, then lets just make the implementation opt-in. Most secure by default, less secure if they choose.

The latter two options might prove inconvenient on systems where security of the file is not a concern.

Based on my first assumption, this is going to be the less-common case. Therefore, I'm proposing that we accept the inconvenience in favor of a default implementation that is most secure.

I'm eager to hear your responses on these proposals. I still agree that it's important to address this issue and find a less surprising outcome in this case.

Thanks for inviting feedback. I hope my thoughts are helpful.

---

Original comment by: Randy Syring

[jaraco]

Maybe this should be it's own Issue... but the default password storage of keyring in Windows is easily readable from any python program. I think that win32cred may not be used properly. If you "import win32cred" and "print win32cred.CredEnumerate()" you can see all the passwords stored by keyring without any need to provide any username or password to access the information. Is this expected behavior? I definitely agree with the error when only unsecure options are available, but it seems like the default windows option is unsecure (even though it's using win32cred).

---

Original comment by: David Johnson

[jaraco]

That should definitely be its own issue, but to reassure you some, the credential vault is secured by the Windows Login of that account. The credentials should not be retrievable without entering that user's password... And I believe though haven't verified that the passwords would be wiped or rendered inaccessible if the password were forcibly changed (ie by another admin).

Please follow up in a second ticket or on the mailing list with your concerns.

---

Original comment by: Jason R. Coombs

[jaraco]

@rsyring As you can see, I've added a commit that starts to think about this issue, but I'm unsure what to do in the case where no recommended keyring is available. As it's not suitable to fail on import, and because the default keyring is configured on import, a whole new behavior must be created. I'm thinking that leaving the default backend as None is reasonable, but then calls to get_password and set_password will have an attribute error. I'm considering adding a ReadOnlyPlainTextKeyring backend that would error on set/delete operations, but otherwise function to retrieve passwords.

How would you envision this working in its final form?

[rsyring]

@jaraco I think it depends on how big of BC break you are willing to make in the next release. Some of the pain of such an event could be mitigated by issuing a point release with deprecation warnings.

A few initial thoughts:

• Remove the idea of a default keyring and make the dev set it up explicitly, even if there is a recommended keyring available. I like this approach the most because it requires the dev to read the docs further to understand what a backend is and why they would care. There would be more pain associated with this approach.
  • I also wonder if it wouldn't make sense then to completely get rid of the module-level keyring methods and simply have the methods available on the module deal with setting up a backend. Once the backend is available, then all interaction happens with the backend instance directly, and not through the keyring module methods. The same API could be required for every backend, so you still have API compatibility. But this may be more BC churn than it is worth.
• Agreed that the module can't fail on import otherwise you never get access to the API to be able to set the default keyring. Also don't like the options that leave you with an AttributeError, that wouldn't be friendly to the developer at all.
• A new backend, NoBackendAvailable or something, could be created that simply throws an exception whenever any methods are called. The exception would indicate that a backend needs to be chosen and give a URL to the official docs describing the situation.

[mitya57]

Remove the idea of a default keyring and make the dev set it up explicitly, even if there is a recommended keyring available. I like this approach the most because it requires the dev to read the docs further to understand what a backend is and why they would care.

Sorry, this makes no sense. The idea of Keyring module is making it easier to write cross-platform apps, without caring about implementation details on every particular platform.

If a developer needs only one particular backend, then they may use the underlying platform API directly, why use Keyring at all?

I.e. if one wants to support only Secret Service, they can use secretstorage library which provides much more powerful API than Keyring.

[rsyring]

Sorry, this makes no sense. The idea of Keyring module is making it easier to write cross-platform apps, without caring about implementation details on every particular platform.

This proposal would not require you to understand the implementation details of every platform. It would require a developer to understand that there are different backend implementations and those implementations have different security trade offs. In particular, the default backend on some systems is a plain text format which, IMO, and as reasoned in previous comments, should not be happening.

Having a unified API for accessing the backends is still a huge value. But completely hiding the backend realities only brings a sense of false security. Making the dev at least consider the security implications of each backend and have an explicit API step that requires the dev to opt-in to the backends they are comfortable with is reasonable.

If a developer needs only one particular backend, then they may use the underlying platform API directly, why use Keyring at all?

No one said they would be restricted to only one backend. Just that there would be some kind of opt-in mechanism for declaring which backends the developer "trusts" and desires to use. IMO, it's very difficult for a library to decide something like this and provide a default. It's in the realm of the app developer to weigh the security considerations of their particular app and then choose the backend that meets their needs.

[jaraco]

I'm disinclined to remove the default keyrings as well, especially the recommended ones, essentially leaving it to every developer to keep up with the library and opt-in to every backend. So let's assume that's off the table for now.

It's currently possible for any developer to readily take control over which backends are loaded and available through the public interface.

I'd like to focus on an implementation that retains the zero config simple use case.

A new backend, NoBackendAvailable or something, could be created that simply throws an exception whenever any methods are called. The exception would indicate that a backend needs to be chosen and give a URL to the official docs describing the situation.

This is similar to what I was considering, a Null backend.

It could raise an Exception on any method, or it could raise exceptions only on set/delete methods and return None on the get_password method (similar to what the other backends do when no password has been set). I note that this backend would be a special case to the 'recommended' paradigm. It would have a priority less than 1 (maybe 0 or -1), but would be included as the fallback when a recommended keyring is indicated but no recommended keyrings are available.

Given the convergence of thought on this idea, I'm going to push forward with an implementation in this spirit.

[rsyring]

Sounds good.

It could raise an Exception on any method, or it could raise exceptions only on set/delete methods and return None on the get_password method.

I would vote for an exception in all cases. Getting a None returned is probably never going to be the actual desired behavior but it's not immediately obvious what the problem is. An exception with a good error message avoids all ambiguity and would result in better troubleshooting IMO.

[jaraco]

I've got a draft implementation in place, but it's failing due to circular imports on Python < 3.4. Otherwise, I think it's ready for a release that will enable forward compatibility to the version that only loads recommended backends by default.

[jaraco]

I'm reconsidering how to impose the backward compatibility. My latest thinking is - instead of requiring an API call to enable all available backends, which would require developer intervention to enable backends or require the user to specify a backend, to simply remove all but the recommended backends from this package, and instead make those backends available in another package. In that way, not only does the developer have the ability to include the less secure backends, but so also does the system or the user through exactly the same technique, but it's still an explicit action.

[jaraco]

I've released 7.3 with API support for limiting backends (arbitrarily, but also with a convenience filter for only 'recommended' backends).

[jaraco]

In https://github.com/jaraco/keyrings.alt, I've started work on a project to host the non-preferred keyrings that are currently part of the keyring library, basically everything but SecretService, kwallet.DbusWallet, OS_X, and Windows.WinVaultKeyring.

[jaraco]

@skorokithakis and @rsyring, can you confirm that the implementation in the keyring-exodus branch, referenced in the commit above, achieves your needs and solves the issue?

[mitya57]

@jaraco: FYI, that approach will break some apps that currently use try to use the old backends explicitly. For example:

• https://bitbucket.org/pypa/wheel/src/tip/wheel/tool/__init__.py#__init__.py-50
• https://github.com/engla/kupfer/blob/master/kupfer/plugin_support.py#L171

Though the second is not working anyway, and for the first you have the rights to fix it :)

[jaraco]

mitya57: Thanks for finding this. I've submitted wheel PR 61 to address the first.

[ibotty]

Why is the gnome keyring not a preferred backend and moved to keyrings.alt?

[mitya57]

@ibotty That is because libgnome-keyring is deprecated, and using the Secret Service D-Bus API is the recommended alternative. (libsecret is the C wrapper around that API, and SecretStorage is the Python wrapper.)

[ibotty]

I could replicate (my reasonably simple) use of keyring using secretservice as follows:

import keyring
keyring.get_password('category', 'secret_name')

became

import secretstorage
bus = secretstorage.dbus_init()
collection = secretstorage.get_default_collection(bus)
collection.unlock()
next(secrets.search_items({'name': 'category/secret_name'})).get_secret()

Can you provide a wrapper around secretstorage to get it to work with the simpler keyring API?

[mitya57]

@ibotty Keyring does use SecretStorage. It is the default backend on Linux nowadays.

[jaraco]

See the README for hints on running with SecretStorage.

[ibotty]

Oh, I see, it did not work here because I did not have secretstorage installed when I initially tested it. Now that I do, it works beautifully. Thanks.