bpo-35031: Fix test_start_tls_server_1 on FreeBSD buildbots by pablogsal · Pull Request #10011 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Tested on the buildbot itself:
CURRENT-amd64% uname -a FreeBSD CURRENT-amd64 12.0-ALPHA10 FreeBSD 12.0-ALPHA10 r339405 GENERIC-NODEBUG amd64 CURRENT-amd64% ./python -m test test_asyncio -m test_start_tls_server_1 -v == CPython 3.8.0a0 (tags/v3.7.0a4-747-g35230d08e0:35230d08e0, Oct 21 2018, 03:49:42) [Clang 6.0.1 (tags/RELEASE_601/final 335540)] == FreeBSD-12.0-ALPHA10-amd64-64bit-ELF little-endian == cwd: /usr/home/pablo/cpython/build/test_python_32337 == CPU count: 4 == encodings: locale=UTF-8, FS=utf-8 Run tests sequentially 0:00:00 load avg: 3.63 [1/1] test_asyncio test_start_tls_server_1 (test.test_asyncio.test_sslproto.ProactorStartTLSTests) ... skipped 'Windows only' test_start_tls_server_1 (test.test_asyncio.test_sslproto.SelectorStartTLSTests) ... ok
Ran 2 tests in 0.424s
OK (skipped=1) 1 test OK.
Total duration: 1 sec Tests result: SUCCESS CURRENT-amd64%
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Honestly, I'm not a TLS expert, but this bug is annoying and makes the buildbot fail randomly. So I suggest to apply it as soon as possible :-)
If it's wrong, it can be fixed later, but at least the buildbot will become useful again and stop to spam buildbot-status and random PRs.
| @@ -429,6 +429,7 @@ def test_start_tls_server_1(self): |
|---|
| server_context = test_utils.simple_server_sslcontext() |
| client_context = test_utils.simple_client_sslcontext() |
| client_context.options |= ssl.OP_NO_TLSv1_3 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment explaining this line?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 93e66f6
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please restrict this to FreeBSD?
@1st1 In the long run, we should have parametrized tests to test TLS 1.2 and TLS 1.3 behavior. The protocols behave differently on mulitple accounts. For client cert auth, we even need to have TLS 1.3 with and without PHA.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please limit ssl.OP_NO_TLSv1_3 to FreeBSD for now.
| @@ -429,6 +429,7 @@ def test_start_tls_server_1(self): |
|---|
| server_context = test_utils.simple_server_sslcontext() |
| client_context = test_utils.simple_client_sslcontext() |
| client_context.options |= ssl.OP_NO_TLSv1_3 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please restrict this to FreeBSD?
@1st1 In the long run, we should have parametrized tests to test TLS 1.2 and TLS 1.3 behavior. The protocols behave differently on mulitple accounts. For client cert auth, we even need to have TLS 1.3 with and without PHA.
@tiran I restricted the fix for FreeBSD in commit f777fa5.
Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖
Sorry, @pablogsal, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.cherry_picker f6a47f3e316cede2a07a1f74a509f6d80ab8fef0 3.6
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
Some FreeBSD buildbots fail to run this test as the eof was not being received by the server if the size is not big enough. This behaviour only appears if the client is using TLS1.3. (cherry picked from commit f6a47f3)
Co-authored-by: Pablo Galindo Pablogsal@gmail.com
Backporting after checking that it works on the buildbots.
miss-islington added a commit that referenced this pull request
Some FreeBSD buildbots fail to run this test as the eof was not being received by the server if the size is not big enough. This behaviour only appears if the client is using TLS1.3. (cherry picked from commit f6a47f3)
Co-authored-by: Pablo Galindo Pablogsal@gmail.com