Skip to content

Rephrase section on distutil's security#100

Merged
sigmavirus24 merged 1 commit intopypa:masterfrom
untitaker:distutils-is-secure-now
Apr 7, 2015
Merged

Rephrase section on distutil's security#100
sigmavirus24 merged 1 commit intopypa:masterfrom
untitaker:distutils-is-secure-now

Conversation

@untitaker
Copy link
Copy Markdown
Contributor

See #93

Comment thread README.rst Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Only recently" is deceptive and either needs to be expanded or there needs to be a footnote explaining what that even means. Expecting people to read a bpo issue to try to determine when it stopped does not seem reasonable to me. Further, not using HTTP simply isn't enough to make distutils secure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"Only recently" is deceptive and either needs to be expanded

I thought the Versions field of the linked bugreport would be informative enough while not distracting the user from twine's rationale. I have updated the whole paragraph though.

Further, not using HTTP simply isn't enough to make distutils secure.

I didn't write that anywhere. Even the commit message doesn't, although I tried hard to keep that line within 50 chars.

@untitaker untitaker force-pushed the distutils-is-secure-now branch 2 times, most recently from 44fb240 to fb9bf2d Compare April 6, 2015 13:56
Comment thread README.rst Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably also say "easily sniffed" because there's no verification of the connection and someone using a tool like mitmproxy could easily sniff the credentials over TLS that is not verified.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there's no verification of the connection

Are you saying that twine doesn't check SSL certificates?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you saying that twine doesn't check SSL certificates?

No. distutils (even thought it uses HTTPS) does not check SSL certificates. Twine does.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, brainfart. Will fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just did s/sniffed/easily sniffed/g.

@untitaker untitaker force-pushed the distutils-is-secure-now branch from fb9bf2d to 0dc6dda Compare April 6, 2015 16:13
sigmavirus24 added a commit that referenced this pull request Apr 7, 2015
Rephrase section on distutil's security
@sigmavirus24 sigmavirus24 merged commit 28a69e0 into pypa:master Apr 7, 2015
@sigmavirus24
Copy link
Copy Markdown
Member

Thanks @untitaker

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.

2 participants