Skip to content

bpo-36037: Fix test_ssl for strict OpenSSL policy#11940

Merged
vstinner merged 2 commits into
python:masterfrom
vstinner:test_ssl_rhel8
Feb 19, 2019
Merged

bpo-36037: Fix test_ssl for strict OpenSSL policy#11940
vstinner merged 2 commits into
python:masterfrom
vstinner:test_ssl_rhel8

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Feb 19, 2019 •
edited by bedevere-bot
Loading

Fix test_ssl for strict OpenSSL configuration like RHEL8 strict
crypto policy.

https://bugs.python.org/issue36037

@vstinner vstinner requested review from gpshead, pitrou and tiran February 19, 2019 14:23
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting merge labels Feb 19, 2019
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Feb 19, 2019

cc @stratakis

@stratakis
Copy link
Copy Markdown
Contributor

stratakis commented Feb 19, 2019

Tested it on a RHEL8 system and I confirm this PR actually fixes the tests.

@vstinner
bpo-36037: Fix min ver in test_ssl for strict policy
936612f 
Fix test_ssl for strict OpenSSL configuration like RHEL8 strict crypto policy.
Use older TLS version for minimum TLS version of the server SSL context if
needed, to test TLS version older than default minimum TLS version.
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Feb 19, 2019

I used git push --force to elaborate the commit message and NEWS entry: mention that the fix changes the minimum version.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Feb 19, 2019

Hmm, I'll let @tiran comment on this. I really haven't kept up with the ssl testing infrastructure, nor with the ssl module itself.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Feb 19, 2019

Oh macOS on Azure faild with:

AttributeError: 'SSLContext' object has no attribute 'minimum_version'

pythoninfo:

ssl.HAS_SNI: True
ssl.OPENSSL_VERSION: OpenSSL 1.0.2q  20 Nov 2018
ssl.OPENSSL_VERSION_INFO: (1, 0, 2, 17, 15)
ssl.OP_ALL: 0x800003ff
ssl.OP_NO_TLSv1_1: 0x10000000

Ah, that's pre-OpenSSL 1.1.1.

I fixed my PR.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Feb 19, 2019

I tested manually the PR on Debian Buster:

  • Without this change: "FAILED (failures=1, errors=2, skipped=9)"
  • With this change: "Tests result: SUCCESS"

@vstinner vstinner merged commit 3ef6344 into python:master Feb 19, 2019
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Feb 19, 2019

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@vstinner vstinner deleted the test_ssl_rhel8 branch February 19, 2019 17:06
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Feb 19, 2019

GH-11942 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2019
@vstinner @miss-islington
bpo-36037: Fix test_ssl for strict OpenSSL policy (pythonGH-11940)
53baeb2 
Fix test_ssl for strict OpenSSL configuration like RHEL8 strict crypto policy.
Use older TLS version for minimum TLS version of the server SSL context if
needed, to test TLS version older than default minimum TLS version.
(cherry picked from commit 3ef6344)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
miss-islington added a commit that referenced this pull request Feb 19, 2019
@miss-islington @vstinner
bpo-36037: Fix test_ssl for strict OpenSSL policy (GH-11940)
e8bf04d 
Fix test_ssl for strict OpenSSL configuration like RHEL8 strict crypto policy.
Use older TLS version for minimum TLS version of the server SSL context if
needed, to test TLS version older than default minimum TLS version.
(cherry picked from commit 3ef6344)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
gpshead
gpshead reviewed Feb 19, 2019
View reviewed changes
Comment thread Lib/test/test_ssl.py
if (min_version is not None
# SSLContext.minimum_version is only available on recent OpenSSL
# (setter added in OpenSSL 1.1.0, getter added in OpenSSL 1.1.1)
and hasattr(server_context, 'minimum_version')
Copy link
Copy Markdown
Member

@gpshead gpshead Feb 19, 2019

Choose a reason for hiding this comment

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

style nit (not worth another PR, just leave it), I'd have indented these lines to match the column of min_version after your opening ( above.

Copy link
Copy Markdown
Member Author

@vstinner vstinner Feb 19, 2019

Choose a reason for hiding this comment

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

He he, I expected that someone would complain about that 😁 Feel free to change it if you want.

@terryjreedy terryjreedy mentioned this pull request May 23, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead left review comments

@tiran tiran Awaiting requested review from tiran

@pitrou pitrou Awaiting requested review from pitrou

Assignees

No one assigned

Labels

tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@vstinner @stratakis @pitrou @miss-islington @bedevere-bot @gpshead @the-knights-who-say-ni