-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
[2.7] bpo-33570: TLS 1.3 ciphers for OpenSSL 1.1.1 (GH-6976) (GH-8760) #10607
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
Conversation
…ythonGH-8760) Change TLS 1.3 cipher suite settings for compatibility with OpenSSL 1.1.1-pre6 and newer. OpenSSL 1.1.1 will have TLS 1.3 cipers enabled by default. Also update multissltests to test with latest OpenSSL. Signed-off-by: Christian Heimes <christian@python.org>. (cherry picked from commit 3e630c5) Co-authored-by: Christian Heimes <christian@python.org>
This PR is based on #8771 however the additional changes to multissltests.py were added. |
Some notes: Removed the testing 0.9.8zc and 0.9.8zh versions of openssl. Updated as well the libressl recent versions to 2.6.5 and 2.7.4 respectively. |
] | ||
env = os.environ.copy() | ||
# set rpath | ||
env["LD_RUN_PATH"] = self.lib_dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here
cpython/Tools/ssl/multissltests.py
Line 283 in 9fb051f
if self.system: |
The '--system', argument is missing as well which was added on e5f41d2. Should it be added? |
Also all tests pass when utilizing multissltests.py |
Yeah, I noticed that. IMHO it can be added later. To be honest, I don't know what is the purpose of this new command line option. |
"1.1.0f", | ||
"1.0.2p", | ||
"1.1.0i", | ||
# "1.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it skipped on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, with this PR, python2 is not yet completely compatible with openssl 1.1.1, it will be with the follow-up PR's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I preferred to ask.
I'm very confused by the change title and description. It says "Change TLS 1.3 cipher suite settings" but I don't see any change in ssl.py nor _ssl.c, only in tests. The main change of this PR is to remove test_default_ciphers() and add a new test_no_shared_ciphers() in test_ssl. |
Can you mention e8eb6cb in the commit message? Your PR seems to be a backport of this commit from master to 2.7. |
The title is taken from the PR title, this commit was cherry-pick from |
Alright, I noticed that later :-) Maybe mention the cherry-picked commit to avoid confusion ;-) |
It is actually a backport of 3e630c5 which is essentially the same but for the 3.6 branch, where this PR is mainly based of, and it's mentioned in the commit message. However the main commit from the master branch is e8eb6cb. I can change the message to point to it, if that's the better case. |
@stratakis: Thanks, I merged your first PR for OpenSSL 1.1.1 in 2.7! Sorry, I was confused between the commit message and the PR description. PR description didn't contain the cherry-picked commit, whereas commit message did. Since you reused the commit message unmodified, it makes sense to leave it unchanged even if it's surprising (it doesn't change the default list of ciphers, it's done in your second PR). |
Change TLS 1.3 cipher suite settings for compatibility with OpenSSL
1.1.1-pre6 and newer. OpenSSL 1.1.1 will have TLS 1.3 cipers enabled by
default.
Also update multissltests to test with latest OpenSSL.
https://bugs.python.org/issue33570