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

Update distributing guide to prefer twine's keyring support, and discourage use of pypirc #297

Open
theacodes opened this Issue Apr 19, 2017 · 22 comments

Comments

Projects
None yet
10 participants
@theacodes
Member

theacodes commented Apr 19, 2017

This can't be done until this upstream issue is resolved: pypa/twine#216
Original context: #271 (comment)

@glyph

This comment has been minimized.

Show comment
Hide comment
@glyph

glyph Oct 23, 2017

I think this might actually be doable now. twine can't put a password in the keyring, but it can read one.

glyph commented Oct 23, 2017

I think this might actually be doable now. twine can't put a password in the keyring, but it can read one.

@theacodes

This comment has been minimized.

Show comment
Hide comment
@theacodes

theacodes Oct 23, 2017

Member

@glyph what would it take for twine to be able to put a password in the keyring?

Member

theacodes commented Oct 23, 2017

@glyph what would it take for twine to be able to put a password in the keyring?

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan Oct 24, 2017

Member

@takluyver may know, as I believe flit already does this (at least on Linux).

Member

ncoghlan commented Oct 24, 2017

@takluyver may know, as I believe flit already does this (at least on Linux).

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Oct 24, 2017

Member

Yup, flit uses keyring on all platforms, so long as it's available (it's a soft dependency).

My strategy is to first try various means to get a stored or provided password. If none is found, we prompt the user, and then if keyring is importable, we store the password. You can see the code to do it here:

https://github.com/takluyver/flit/blob/8e1077c4d425239fea860b143714a2622c4d16db/flit/upload.py#L138-L168

The main UI weakness I'm aware of is that flit provides no mechanism to change the stored password; if you type it wrong, or change it later, you have to go through whatever mechanism the platform provides to change or remove it.

I hope that twine is storing the passwords in a similar way so that both tools can use one stored password.

Member

takluyver commented Oct 24, 2017

Yup, flit uses keyring on all platforms, so long as it's available (it's a soft dependency).

My strategy is to first try various means to get a stored or provided password. If none is found, we prompt the user, and then if keyring is importable, we store the password. You can see the code to do it here:

https://github.com/takluyver/flit/blob/8e1077c4d425239fea860b143714a2622c4d16db/flit/upload.py#L138-L168

The main UI weakness I'm aware of is that flit provides no mechanism to change the stored password; if you type it wrong, or change it later, you have to go through whatever mechanism the platform provides to change or remove it.

I hope that twine is storing the passwords in a similar way so that both tools can use one stored password.

@theacodes theacodes self-assigned this Dec 13, 2017

@theacodes theacodes changed the title from Update distributing guide to mention twine configuration for storing passwords in the keyring to Update distributing guide to prefer twine's keyring support, and discourage use of pypirc Dec 13, 2017

@theacodes

This comment has been minimized.

Show comment
Hide comment
@theacodes

theacodes Dec 13, 2017

Member

@ncoghlan after discussing with Sumana on IRC about the security implications of .pypirc, I strongly think that the tutorials should discourage the use of .pypirc altogether and we should work towards our tools having keyring support. Let me know if you have any thoughts/reservations.

Member

theacodes commented Dec 13, 2017

@ncoghlan after discussing with Sumana on IRC about the security implications of .pypirc, I strongly think that the tutorials should discourage the use of .pypirc altogether and we should work towards our tools having keyring support. Let me know if you have any thoughts/reservations.

@dstufft

This comment has been minimized.

Show comment
Hide comment
@dstufft

dstufft Dec 13, 2017

Member

I think something like twine configure would be awesome.

Member

dstufft commented Dec 13, 2017

I think something like twine configure would be awesome.

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan Dec 14, 2017

Member
Member

ncoghlan commented Dec 14, 2017

@mbacchi

This comment has been minimized.

Show comment
Hide comment
@mbacchi

mbacchi Jan 3, 2018

Here's a blog post I did on how to prevent .pypirc from being leaked for reference: http://mbacchi.github.io/2017/12/22/3-ways-prevent-secret-leaks-github.html

FYI @brainwane

mbacchi commented Jan 3, 2018

Here's a blog post I did on how to prevent .pypirc from being leaked for reference: http://mbacchi.github.io/2017/12/22/3-ways-prevent-secret-leaks-github.html

FYI @brainwane

@theacodes

This comment has been minimized.

Show comment
Hide comment
@theacodes

theacodes Jan 3, 2018

Member

Thank you, @mbacchi! very helpful.

Member

theacodes commented Jan 3, 2018

Thank you, @mbacchi! very helpful.

@brainwane

This comment has been minimized.

Show comment
Hide comment
@brainwane

brainwane Feb 1, 2018

Member

I'm on Debian 9.3 (stretch, e.g., stable) and today I successfully made a username/password/website credential available for python-keyring to retrieve (so twine can use it when uploading a package).

I followed, basically, the instructions for Ubuntu 16.04 plus @jaraco's explanation in pypa/twine#208 (comment) , and everything worked out. Starting in a Python 3 virtualenv, and eliding a lot of output and a test python session where I tested that I could set and get a credential, here's what I did:

$ sudo apt install python3-venv libdbus-glib-1-dev
$ pip install secretstorage dbus-python
$ pip install keyring
$ keyring set https://test.pypi.org/legacy/ brainwane
Password for 'brainwane' in 'https://test.pypi.org/legacy/': # [TYPE THIS IN. NOT ECHOED.]
$ python setup.py sdist
$ python setup.py bdist_wheel
$ twine upload --repository-url https://test.pypi.org/legacy/ dist/*
Uploading distributions to https://test.pypi.org/legacy/
Enter your username: brainwane
Uploading Forms990_analysis-1.0.0-py3-none-any.whl
Uploading Forms990_analysis-1.0.1-py3-none-any.whl
Uploading Forms990-analysis-1.0.0.tar.gz
Uploading Forms990-analysis-1.0.1.tar.gz

My opinion: we should put a variant of this into the uploading tutorial and the guide to using Test PyPI, with adjustments for the user's OS.

Member

brainwane commented Feb 1, 2018

I'm on Debian 9.3 (stretch, e.g., stable) and today I successfully made a username/password/website credential available for python-keyring to retrieve (so twine can use it when uploading a package).

I followed, basically, the instructions for Ubuntu 16.04 plus @jaraco's explanation in pypa/twine#208 (comment) , and everything worked out. Starting in a Python 3 virtualenv, and eliding a lot of output and a test python session where I tested that I could set and get a credential, here's what I did:

$ sudo apt install python3-venv libdbus-glib-1-dev
$ pip install secretstorage dbus-python
$ pip install keyring
$ keyring set https://test.pypi.org/legacy/ brainwane
Password for 'brainwane' in 'https://test.pypi.org/legacy/': # [TYPE THIS IN. NOT ECHOED.]
$ python setup.py sdist
$ python setup.py bdist_wheel
$ twine upload --repository-url https://test.pypi.org/legacy/ dist/*
Uploading distributions to https://test.pypi.org/legacy/
Enter your username: brainwane
Uploading Forms990_analysis-1.0.0-py3-none-any.whl
Uploading Forms990_analysis-1.0.1-py3-none-any.whl
Uploading Forms990-analysis-1.0.0.tar.gz
Uploading Forms990-analysis-1.0.1.tar.gz

My opinion: we should put a variant of this into the uploading tutorial and the guide to using Test PyPI, with adjustments for the user's OS.

@di

This comment has been minimized.

Show comment
Hide comment
@di

di Feb 1, 2018

Member

I want to reiterate a comment I made here:

But arguably this isn't worth the effort, since the default access control granted by keyring is that any other Python application may access it (via keyring.get_password) and there doesn't seem to be a way to scope this just to twine (but perhaps I'm missing something).

To be more explicit, the following lines (run anywhere, not just from within twine) would give @brainwane back the password in plaintext:

>>> import keyring
>>> keyring.get_password('https://test.pypi.org/legacy/', 'brainwane')

I don't really see the value in putting this in the guide unless it's at least marginally more secure that keeping a password in ~/.pypirc, which it seems like it's not (but again, totally possible that I'm missing something here).

EDIT: Perhaps this is all understood, and it could be argued that using the keyring over ~/.pypirc helps prevents the leaking of passwords, so maybe it is worth doing after all. Just wanted to raise that point. 🙂

Member

di commented Feb 1, 2018

I want to reiterate a comment I made here:

But arguably this isn't worth the effort, since the default access control granted by keyring is that any other Python application may access it (via keyring.get_password) and there doesn't seem to be a way to scope this just to twine (but perhaps I'm missing something).

To be more explicit, the following lines (run anywhere, not just from within twine) would give @brainwane back the password in plaintext:

>>> import keyring
>>> keyring.get_password('https://test.pypi.org/legacy/', 'brainwane')

I don't really see the value in putting this in the guide unless it's at least marginally more secure that keeping a password in ~/.pypirc, which it seems like it's not (but again, totally possible that I'm missing something here).

EDIT: Perhaps this is all understood, and it could be argued that using the keyring over ~/.pypirc helps prevents the leaking of passwords, so maybe it is worth doing after all. Just wanted to raise that point. 🙂

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Feb 1, 2018

Member

It doesn't protect against untrusted code running in your user account, but storing the password in the keyring is an improvement in other ways:

  • If your computer is stolen, the thief can't access the keyring data unless they know your login password - the keyring is encrypted even if you haven't set up an encrypted filesystem (AIUI).
  • Your .pypirc file might be readable by other users who can log on to the same computer, if the permissions are not set correctly. The guide currently describes creating this file with no mention of setting its permissions, and at least on Linux the default file permissions allow anyone to read it.

There's arguably also a psychological reason: putting PyPI passwords in a plaintext config file signals that they're not that important. Using a specific mechanism for storing secrets would suggest that PyPA thinks how you store your password matters.

We definitely shouldn't see using keyring as the end of the road on this. Glyph has done some work on getting the credentials from a password manager, for instance. And hopefully Warehouse will add features like tokens to upload a specific project. But if we can recommend keyring rather than storing passwords in .pypirc, I think that's a step forwards.

Member

takluyver commented Feb 1, 2018

It doesn't protect against untrusted code running in your user account, but storing the password in the keyring is an improvement in other ways:

  • If your computer is stolen, the thief can't access the keyring data unless they know your login password - the keyring is encrypted even if you haven't set up an encrypted filesystem (AIUI).
  • Your .pypirc file might be readable by other users who can log on to the same computer, if the permissions are not set correctly. The guide currently describes creating this file with no mention of setting its permissions, and at least on Linux the default file permissions allow anyone to read it.

There's arguably also a psychological reason: putting PyPI passwords in a plaintext config file signals that they're not that important. Using a specific mechanism for storing secrets would suggest that PyPA thinks how you store your password matters.

We definitely shouldn't see using keyring as the end of the road on this. Glyph has done some work on getting the credentials from a password manager, for instance. And hopefully Warehouse will add features like tokens to upload a specific project. But if we can recommend keyring rather than storing passwords in .pypirc, I think that's a step forwards.

@di

This comment has been minimized.

Show comment
Hide comment
@di

di Feb 1, 2018

Member

All great points @takluyver. Consider me convinced, and thanks for laying out an explanation.

Member

di commented Feb 1, 2018

All great points @takluyver. Consider me convinced, and thanks for laying out an explanation.

@brainwane

This comment has been minimized.

Show comment
Hide comment
@brainwane

brainwane Mar 8, 2018

Member

We'll be publicizing pypi.org widely probably within the next two weeks, and I would very much welcome help on this, so the packaging guide (which we link to consistently within https://pypi.org/help/ and elsewhere on the site) mentions keying as a preferred credential store.

I've just put out a new release of twine and would be happy to put out a point release that includes command-line help with keyring plus guidance in the README (per pypa/twine#277). Then we could update the packaging and distribution guide, remove or move the .pypirc guidance, and point to the twine/keyring documentation.

Member

brainwane commented Mar 8, 2018

We'll be publicizing pypi.org widely probably within the next two weeks, and I would very much welcome help on this, so the packaging guide (which we link to consistently within https://pypi.org/help/ and elsewhere on the site) mentions keying as a preferred credential store.

I've just put out a new release of twine and would be happy to put out a point release that includes command-line help with keyring plus guidance in the README (per pypa/twine#277). Then we could update the packaging and distribution guide, remove or move the .pypirc guidance, and point to the twine/keyring documentation.

@brainwane

This comment has been minimized.

Show comment
Hide comment
@brainwane

brainwane Mar 15, 2018

Member

I'm working towards getting Twine 1.10.1 out in the next few days and would strongly welcome doc improvements related to keyring to get into that release!

Member

brainwane commented Mar 15, 2018

I'm working towards getting Twine 1.10.1 out in the next few days and would strongly welcome doc improvements related to keyring to get into that release!

@brainwane

This comment has been minimized.

Show comment
Hide comment
@brainwane

brainwane Mar 20, 2018

Member

Twine 1.11.0 is now out and includes guidance in the README for using keyring.

@jonparrott and I have, in our experience, run into hiccups in getting keyring support to work well on Linux. We'll file bugs appropriately. Once the user experience is smoother, then the packaging/distro guide should replace .pypirc recommendations with keyring recommendations.

Member

brainwane commented Mar 20, 2018

Twine 1.11.0 is now out and includes guidance in the README for using keyring.

@jonparrott and I have, in our experience, run into hiccups in getting keyring support to work well on Linux. We'll file bugs appropriately. Once the user experience is smoother, then the packaging/distro guide should replace .pypirc recommendations with keyring recommendations.

@jakethedev

This comment has been minimized.

Show comment
Hide comment
@jakethedev

jakethedev Jun 29, 2018

Hey all, I'm a bit new to python packaging and have a general question about this particular bit of the docs, no idea about the best way to ask but this issue seems to be the most relevant place

The docs mention that one should install twine and run twine upload --repository-url https://test.pypi.org/legacy/ dist/* - but setup has this functionality built in, doesn't it? What's the difference between the twine command and python setup.py bdist_wheel (or any other preferred versions) upload -r https://test.pypi.org/legacy/? Is it mostly that twine has better authentication tooling?

I was thinking I'd pop over here and make a PR to add the setup-upload info to the docs and found this thread, so I hope it doesn't hurt to ask

jakethedev commented Jun 29, 2018

Hey all, I'm a bit new to python packaging and have a general question about this particular bit of the docs, no idea about the best way to ask but this issue seems to be the most relevant place

The docs mention that one should install twine and run twine upload --repository-url https://test.pypi.org/legacy/ dist/* - but setup has this functionality built in, doesn't it? What's the difference between the twine command and python setup.py bdist_wheel (or any other preferred versions) upload -r https://test.pypi.org/legacy/? Is it mostly that twine has better authentication tooling?

I was thinking I'd pop over here and make a PR to add the setup-upload info to the docs and found this thread, so I hope it doesn't hurt to ask

@ncoghlan

This comment has been minimized.

Show comment
Hide comment
@ncoghlan

ncoghlan Jun 30, 2018

Member

@jaktonik Depending on exactly which Python environment you're using, the native distutils upload may happen over HTTP, exposing your PyPI password as cleartext. There's also some newer metadata fields which distutils on its own won't extract and publish correctly, but twine makes sure are handled.

We should add that info as a footnote on https://packaging.python.org/guides/tool-recommendations/, perhaps by linking out to https://twine.readthedocs.io/en/latest/#why-should-i-use-this

Member

ncoghlan commented Jun 30, 2018

@jaktonik Depending on exactly which Python environment you're using, the native distutils upload may happen over HTTP, exposing your PyPI password as cleartext. There's also some newer metadata fields which distutils on its own won't extract and publish correctly, but twine makes sure are handled.

We should add that info as a footnote on https://packaging.python.org/guides/tool-recommendations/, perhaps by linking out to https://twine.readthedocs.io/en/latest/#why-should-i-use-this

@illume

This comment has been minimized.

Show comment
Hide comment
@illume

illume Aug 17, 2018

Contributor

Hello wonderful people! 👋

I feel like the section in the python packaging guide about storing your password in plaintext in pypirc should be removed immediately. It's a separate issue of if twine keyring support is ready for everyone, or if alternative instructions are ready.

https://packaging.python.org/guides/using-testpypi/?highlight=username#setting-up-testpypi-in-pypirc

The note about setting permissions 'properly' is also not good advice. Instead it should just say Do not store passwords in the pypirc file. Storing passwords in plain text is never a good idea., and not have instructions on how to store plain text passwords in files.

The important part is removing the recommendation on storing passwords like this as soon as possible.

Contributor

illume commented Aug 17, 2018

Hello wonderful people! 👋

I feel like the section in the python packaging guide about storing your password in plaintext in pypirc should be removed immediately. It's a separate issue of if twine keyring support is ready for everyone, or if alternative instructions are ready.

https://packaging.python.org/guides/using-testpypi/?highlight=username#setting-up-testpypi-in-pypirc

The note about setting permissions 'properly' is also not good advice. Instead it should just say Do not store passwords in the pypirc file. Storing passwords in plain text is never a good idea., and not have instructions on how to store plain text passwords in files.

The important part is removing the recommendation on storing passwords like this as soon as possible.

@illume

This comment has been minimized.

Show comment
Hide comment
@illume

illume Aug 17, 2018

Contributor

Here's where the python docs talk about storing the password in pypirc: https://docs.python.org/3/distutils/packageindex.html#the-pypirc-file

Contributor

illume commented Aug 17, 2018

Here's where the python docs talk about storing the password in pypirc: https://docs.python.org/3/distutils/packageindex.html#the-pypirc-file

@theacodes

This comment has been minimized.

Show comment
Hide comment
@theacodes

theacodes Aug 17, 2018

Member

Hi @illume, feel free to see a pull request!

Member

theacodes commented Aug 17, 2018

Hi @illume, feel free to see a pull request!

@illume

This comment has been minimized.

Show comment
Hide comment
@illume

illume Aug 17, 2018

Contributor

Thanks @theacodes :)

Contributor

illume commented Aug 17, 2018

Thanks @theacodes :)

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