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

bpo-43640: Update TLS/SSL security consids. due to TLS 1.0 and TLS 1.1 deprecation #25040

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

illia-v
Copy link
Contributor

@illia-v illia-v commented Mar 27, 2021

TLS 1.0 and TLS 1.1 have recently been deprecated.
https://datatracker.ietf.org/doc/rfc8996/

https://bugs.python.org/issue43640

@illia-v illia-v requested a review from tiran as a code owner Mar 27, 2021
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Mar 27, 2021
Copy link
Member

@tiran tiran left a comment

I don't like to litter documentation with warnings. Warnings should be use sparsely. Otherwise users start to just ignore warnings. Could you please update the Security considerations section instead?

@bedevere-bot
Copy link

bedevere-bot commented Mar 27, 2021

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@tiran
Copy link
Member

tiran commented Mar 27, 2021

Thanks, instead of adding warnings to PROTOCOL_TLSv1, PROTOCOL_TLSv1_1, OP_NO_TLSv1, OP_NO_TLSv1_1, TLSv1, and TLSv1_1, let's rather document problematic version in the security consideration block.

I'll soon post a new PEP based on the RFC, too.

@illia-v
Copy link
Contributor Author

illia-v commented Mar 27, 2021

I don't like to litter documentation with warnings. Warnings should be use sparsely. Otherwise users start to just ignore warnings. Could you please update the Security considerations section instead?

Fair enough.

Do you think it will suffice to change "SSL versions 2 and 3 are considered insecure" to "SSL versions 2 and 3, TLS 1.0, and TLS 1.1 are considered insecure" in the beginning of this section?

Also, should we leave the old warnings of PROTOCOL_SSLv2 and PROTOCOL_SSLv3?

@illia-v
Copy link
Contributor Author

illia-v commented Mar 27, 2021

I'll soon post a new PEP based on the RFC, too.

Are you going to disable TLS 1.0 and 1.1 by default?

@tiran
Copy link
Member

tiran commented Mar 27, 2021

Also, should we leave the old warnings of PROTOCOL_SSLv2 and PROTOCOL_SSLv3?

Good point! Please consolidate the old warnings, too.

@illia-v illia-v changed the title bpo-43640: Add warnings to ssl.PROTOCOL_TLSv1 and ssl.PROTOCOL_TLSv1_1 bpo-43640: Update TLS/SSL security consids. due to TLS 1.0 and TLS 1.1 deprecation Mar 27, 2021
@illia-v illia-v requested a review from tiran Mar 27, 2021
@github-actions
Copy link

github-actions bot commented Apr 27, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 27, 2021
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 6, 2022
@@ -2669,7 +2661,8 @@ to specify :const:`CERT_REQUIRED` and similarly check the client certificate.
Protocol versions
'''''''''''''''''

SSL versions 2 and 3 are considered insecure and are therefore dangerous to
SSL versions 2 and 3, TLS 1.0, and TLS 1.1 are considered insecure and are
therefore dangerous to
use. If you want maximum compatibility between clients and servers, it is
Copy link
Contributor

@slateny slateny Dec 10, 2022

Choose a reason for hiding this comment

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

If https://datatracker.ietf.org/doc/rfc8996/ is authoritative, maybe adding it as a source would be useful, something like ... to use (see https://datatracker.ietf.org/doc/rfc8996/)?

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

Successfully merging this pull request may close these issues.

None yet

7 participants