Keyring support should require an --enable-keyring flag

[pfmoore]

What's the problem this feature will solve?
Keyring support is enabled silently when the keyring module is installed in the user's system. There's no way for the user to say whether or not they want keyring support, or to turn it off when it causes an issue. The pip tests don't test the behaviour of the keyring module (just the integration) and there have been a number of issues reported which are down to keyring behaviour: #8634, #8485, #8443, #8090

Describe the solution you'd like
Add an --enable-keyring flag to pip that acts as an explicit "opt in" to keyring support. Users who want keyring enabled by default can set this via an environment variable or config file.

This would ensure that users have a clear understanding that they are opting into keyring usage, and would ensure that pip's behaviour can't be changed by unrelated changes to the user's system. It would also allow users having issues with keyring to easily disable it (by omitting the flag).

Also, if the user requests keyring support and the module is not available for some reason, it is easy to give an explicit message explaining the issue, whereas at the moment, pip just silently disables the feature with no explanation.

Alternative Solutions
Having an option to disable keyring support would allow users to opt out if they hit issues. But it would do nothing to help users to have a clear understanding of when they have keyring support enabled. It also doesn't address the "silent behaviour change" issue.

Additional context
When implemented, keyring support was actively debated, because the approach taken (enabling the feature if a particular package was available) is non-standard. Normally, pip vendors dependencies, and won't implement features that need modules that don't conform to its vendoring requirements. The keyring support was merged, but to be honest the concerns about the exception to our vendoring policy (see the thread starting here for details) were never really addressed. In my view, the current issues stem from that, and this issue is an attempt to contain the problem by making it explicitly "opt in".

[pradyunsg]

I'm strongly in favor of making the keyring support opt-in instead of opt-out.

Nearly all users I've interacted with have mentioned how the keyring support in pip is actually surprising rather than useful to them.

[chrahunt]

We generally see these kinds of requests alongside workflow-impacting changes:

1. this should be available in requirements.txt so that its usage can be standardized across a team or require fewer setup steps
2. a flag also needs to be present to disable it, so if it is enabled at a system config or user config level then it can be disabled on a per-query basis

I think it would be good to understand our position now enough to be able to help answer those questions later.

---

For the implementation of this, we would probably want to defer importing keyring at all until we confirm the opt-in. From discussions in at lease one keyring issue it's possible it could be made to do the backend check lazily, but there's no telling when that makes it into distributions. It would also be nice if we can say "just upgrade pip" to resolve issues like #8485.

[pfmoore]

Good points. Personally, I'd go with:

1. No, it's global only, and we explicitly don't support it in requirements.txt. That's to limit the complexity in general, and because there's no such support at the moment, so we shouldn't be adding yet more features as part of this issue, which is all about keeping the impact of keyring support limited.
2. Again, no. There's no way to disable it now short of uninstalling keyring or using whatever options keyring has to switch it off, why should we add one as part of this change?

For the implementation of this, we would probably want to defer importing keyring at all until we confirm the opt-in.

Probably, but only if it's easy to do. If it's complicated, then I'd say defer it to a separate feature request, and leave this as simply switching off the functionality.

(I should probably be clear here, I never actually liked the way the keyring feature was implemented, and if I had my way I'd just rip it out completely. But we're stuck with it now and my main aim is to limit the damage, and make it easier to not have it affect people who don't opt into it. I'm particularly offended by #8485 in this regard.)

Also, I should probably have added under Alternative Solutions - Implement a more robust plugin system for pip that allows reliable addition of "optional" features that rely on non-vendored libraries. But that's easy to say, and much harder to actually do, so I basically just consider keyring support as a demonstration of why such a thing is harder than it looks 🙂

[Julian]

Not sure if it's included in the issues alluded to here (or discussed in that original ticket that I haven't read yet), but similar issues to #6668 arise with the way keyring support works today as well.

Specifically -- because it's not vendored, if you're going to use e.g. tox to run some tests, and need keyring in that venv, it is difficult or impossible today to ensure keyring is installed at the right moment (so fetch keyring from PyPI, then proceed with tox installing everything else from potentially some other place that now needs keyring present).

[pradyunsg]

PRs to implement this are very welcome.

[Jmennius]

Is there a chance the keyring support will be improved by vendoring the package?
My concern is mostly because of virtual environments - they break the experience as the system keyring package is not accessible inside a venv.

[pfmoore]

The support cannot be vendored because it includes a C extension somewhere in the dependency chain, and pip only ships as a "pure Python" universal wheel.

[Jmennius]

The support cannot be vendored because it includes a C extension somewhere in the dependency chain, and pip only ships as a "pure Python" universal wheel.

Understood, otherwise, you would consider vendoring it?

[pfmoore]

Personally, I'd be against doing so. The known issues with keyring, with long delays on import, people getting advised to disable it, etc, suggest that vendoring it would add an unacceptable amount to our support burden. Also, I don't know how keyring backends work - we wouldn't want to vendor those so how much would vendoring just keyring help?

[Jmennius]

I agree that making it opt-in is likely the best solution.
But when you actually want to use it there is that barrier that keyring is actually a python package and is subject to related restictions (mostly because of venvs).

Usually, keyring backends work as daemons and are preinstalled on desktop operating systems. On Linux, communication with the daemon (Freedesktop SecretService API) is via DBus.
DBus may be the reason there are long import delays (there is an option to start the service on the first request to it), and 25 seconds resembles some timeout. I will check upstream if I can help.
Maybe, this is the reason people are having issues on Ubuntu WSL, maybe the daemon is just not installed (kind of headless installation, there is even a manual on how to install and use it in headless mode).

For example, with git, you have an ability to configure credential helpers, one of them is libsecret.
All you have to do to opt-in is install a package (git-credential-libsecret on Fedora) or compile a single file (Ubuntu/Debian somehow don't package it but the source is included...) and then configure it in your gitconfig.

As i understand from your previous comment the best alternative to bundling is to have a plugin system?

[pradyunsg]

@Jmennius That's how keyrings generally work -- not the keyring package in Python.

We can't vendor keyring since adapters/plugins for it assume that the package is available under the keyring namespace, and if we vendor it, it won't be -- it'd be pip._vendor.keyring.

[pfmoore]

@Jmennius jaraco/keyring#403 is probably where you would find the best place to try to improve things.

Honestly, pip isn't set up for this type of thing - yes, a plugin system would be the best approach, but we don't have one because we don't have a stable programming API for plugins to use, etc, etc. It's an ongoing debate that we aren't likely to resolve in the short term

The way keyring is handled is essentially a "better than nothing" solution, but like any such compromise it's not ideal for anyone 🙁

[Jmennius]

Thank you for your inputs, this clears up the situation for me at least.

[dquitmann-op]

Is there any progress/time plan for this?

I literally spend half a day to debug the "can not uninstall cffi on windows" problem until I found #8443 which in the end leads to here.

I don't want to blame anyone and appreciate the work you are putting into this great tool (without which python really would not be the same), but for this particular thing my feeling is: this should be rolled back as soon as possible (for your own sake). While keyring support might be nice, the issues (especially #8443 breaking pip) speak for themselves, that this was not a thoughtful shot.

Sorry for the rage, this had to go somewhere...

[uranusjr]

I believe everyone basically agrees this needs to be done, but nobody is having enough interest to actually write the code. A PR would definitely be welcomed, and likely promptly reviewed, if you would bother to implement it 🙂

[dumblob]

@nbraud it seems more or less finished in your branch - would you dare to make a PR to get it reviewed?

[yan12125]

would you dare to make a PR to get it reviewed?

Apparently nbraud already submitted a PR at #9434.

By the way, before that PR is merged, this issue can work-arounded by specifying the "null" keyring backend if you have python-keyring 19.3.0 or newer (see discussions below). That prevents pip from using the system keyring (KWallet, GNOME Keyring, KeePassXC, ...)

PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring pip install ...

[piksel]

Setting that environment variable didn't help in my case:

$ PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring pip install -v fabulous

^CTraceback (most recent call last):
  File "/usr/bin/pip", line 11, in <module>
    load_entry_point('pip==20.0.2', 'console_scripts', 'pip')()
  File "/usr/lib/python3/dist-packages/pip/_internal/cli/main.py", line 73, in main
    command = create_command(cmd_name, isolated=("--isolated" in cmd_args))
  File "/usr/lib/python3/dist-packages/pip/_internal/commands/__init__.py", line 96, in create_command
    module = importlib.import_module(module_path)
  File "/usr/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 848, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/lib/python3/dist-packages/pip/_internal/commands/install.py", line 24, in <module>
    from pip._internal.cli.req_command import RequirementCommand
  File "/usr/lib/python3/dist-packages/pip/_internal/cli/req_command.py", line 19, in <module>
    from pip._internal.network.session import PipSession
  File "/usr/lib/python3/dist-packages/pip/_internal/network/session.py", line 26, in <module>
    from pip._internal.network.auth import MultiDomainBasicAuth
  File "/usr/lib/python3/dist-packages/pip/_internal/network/auth.py", line 36, in <module>
    import keyring  # noqa
  File "/usr/lib/python3/dist-packages/keyring/__init__.py", line 3, in <module>
    from .core import (
  File "/usr/lib/python3/dist-packages/keyring/core.py", line 189, in <module>
    init_backend()
  File "/usr/lib/python3/dist-packages/keyring/core.py", line 93, in init_backend
    keyrings = filter(limit, backend.get_all_keyring())
  File "/usr/lib/python3/dist-packages/keyring/util/__init__.py", line 21, in wrapper
    func.always_returns = func(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/keyring/backend.py", line 210, in get_all_keyring
    return list(rings)
  File "/usr/lib/python3/dist-packages/keyring/util/__init__.py", line 31, in suppress_exceptions
    for callable in callables:
  File "/usr/lib/python3/dist-packages/keyring/util/properties.py", line 26, in __get__
    return self.fget.__get__(None, owner)()
  File "/usr/lib/python3/dist-packages/keyring/backend.py", line 67, in viable
    cls.priority
  File "/usr/lib/python3/dist-packages/keyring/util/properties.py", line 26, in __get__
    return self.fget.__get__(None, owner)()
  File "/usr/lib/python3/dist-packages/keyring/backends/kwallet.py", line 140, in priority
    return super(DBusKeyringKWallet4, cls).priority - 1
  File "/usr/lib/python3/dist-packages/keyring/util/properties.py", line 26, in __get__
    return self.fget.__get__(None, owner)()
  File "/usr/lib/python3/dist-packages/keyring/backends/kwallet.py", line 37, in priority
    bus = dbus.SessionBus(mainloop=DBusGMainLoop())
  File "/usr/lib/python3/dist-packages/dbus/_dbus.py", line 212, in __new__
    return Bus.__new__(cls, Bus.TYPE_SESSION, private=private,
  File "/usr/lib/python3/dist-packages/dbus/_dbus.py", line 102, in __new__
    bus = BusConnection.__new__(subclass, bus_type, mainloop=mainloop)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 124, in __new__
    bus = cls._new_for_bus(address_or_type, mainloop=mainloop)
  File "/usr/lib/python3/dist-packages/dbus/exceptions.py", line 47, in __init__
    def __init__(self, *args, **kwargs):
KeyboardInterrupt

It still hangs indefinitely in keychain/dbus, but at least it lead me to the only way to enable pip again:

apt remove python3-keychain

[uranusjr]

Your installed version of keyring is likely too old to support the flag. If that’s not the case, please report the issue to keyring.

[yan12125]

Looks like @piksel had python-keyring older than 19.3, where all keyrings are still attempted even when $PYTHON_KEYRING_BACKEND is set [1]. I've updated my previous comment to include version constraints.

[1] jaraco/keyring#404

[piksel]

Indeed, Ubuntu 20.04 (LTS) uses:

python3-keyring/focal,focal 18.0.1-2ubuntu1 all

which was installed as a dependency of ubuntu-server

[suhailxeaser]

Setting that environment variable didn't help in my case:

$ PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring pip install -v fabulous
It still hangs indefinitely in keychain/dbus, but at least it lead me to the only way to enable pip again:

apt remove python3-keychain

What command I use for windows instead of apt ?

[Darsstar]

FYI https://github.com/Darsstar/pip/tree/vendor-keyring-subprocess is a proof of concept that keyring could be vendored if we are OK interfacing with keyring via its CLI interface instead of its Python API. (GitHub Action "CI" succeeded.)

The C deps are not vendored because they are not strictly necessary when explicitly setting a different keyring backend. The top commit is the most interesting, HEAD^1 just vendors everything. (Well, vendor.txt might be interesting.)

It would also make keyring support opt-in without adding a new flag because the executable (currently keyring-subprocess) needs to be present on the PATH. (It does not prevent adding the flag, so it can be added in addition to vendoring.)

The longs delays on importing keyring mentioned earlier in this issue should be solved. They should have been causef because no backend is set/configured and it falling back to discovering backends registered as entry points. That is the last of the three fallbacks. The first fallback is looking at environment variables. The second is looking at a config file, if it exists. But the very first thing keyring checks is if keyring.set_keyring() has been called to explcitly set a keyring backend. That is what my POC does.

Security wise it doesn't seem any more or less secure since adding a keyring.py in the right location would also let someone execute their code. So it is probably just a matter of taste to keep using keyring's Python API instead of it's CLI API.

---

Some possible motivation:

• I have only glaced at the pip as a zipapp issues/discussions, but keyring was mentioned in those.
  • This is what pushed me from thinking "calling an executable on the PATH is probably a big no-no" to "well, I should probably throw the idea out there and have others make that decission instead of implicitly making it for them by not mentioning it".
• I created keyring-subprocess to make bootstrapping easier for me and my colleagues by including a Virtualenv seeder, but Pipx and pyprojectx don't use virtualenv to create venvs. Having keyring-subprocess or something similar vendored into Pip (and have Add a mechanism to query keyring for credentials in combination with --no-input #11020 be resolved) would be a massive win for us. (And I would get to remove the seeder implementation.)

---

I do appologive for the structure of this comment/post. Hopefully it is sufficient even though I'm not happy with it.

So I guess the only thing left is: is this something I should make a polished PR for or is #11215 the only PR that should be merged to close this issue?

[Darsstar]

I turned the PoC into an actual PR: #11399. The test failure seems is unrelated to the changes in the PR.

[SnoopJ]

PRs to implement this are very welcome.

A PR would definitely be welcomed, and likely promptly reviewed, if you would bother to implement it 🙂

@pradyunsg @uranusjr any chance for some review of #11399 and/or #11215? 😁

[Darsstar]

(There is a question for maintainsers at the end.)

So I closed #11399 but @uranusjr made a comment relevant to this issue. Keyring can now be queried by importing and calling its API or the cli interface in a subprocess. Should we instead have more influence than just on/off? And instead be able to pick from disabled, import, subprocess and maybe try-import-then-subprocess (which I would name auto)

Ik have a branch for the first three options and can add the fouth. It is currently based on #11029 PR which is at the time of writing part of the 23.0 milestone.

Should I rebase that branch on main before making a PR? Should I include it in #11029 instead? Should I polish it and make a PR without rebasing? Or is a on/off switch still prefered even though new keyring related functionality got added?

PS. Sorry, I wasn't able to actually make it one question for guidance without it being to vague about the sort of answer I expect.

[uranusjr]

I like the option idea.

[Darsstar]

I like the option idea.

#11698 is ready for review