gh-117784: Only reference PHA functions ifndef SSL_VERIFY_POST_HANDSHAKE#117785
gh-117784: Only reference PHA functions ifndef SSL_VERIFY_POST_HANDSHAKE#117785encukou merged 6 commits intopython:mainfrom
Conversation
4d15119 to
4c8cee2
Compare
|
My apologies on the force-push in this draft. I inadvertently rebased latest from upstream main. No significant commit history was lost; I'll refrain from rebasing main again on this PR. |
|
I'm not a SSL expert, but I'm concerned about relying on an Does AWS-LC still define |
1ded2dc to
6743dc0
Compare
PR #1526 introduced the `OPENSSL_NO_TLS_PHA` directive mostly for the purposes of AWS-LC's compatibility with CPython, but in [cpython PR #117785](python/cpython#117785) @encukou points out that detecting the absence of OpenSSL's own `SSL_VERIFY_POST_HANDSHAKE` directive is sufficient. This change removes AWS-LC's `OPENSSL_NO_TLS_PHA` directive in favor of detecting absence of `SSL_VERIFY_POST_HANDSHAKE`.
Lib/test/test_ssl.py
Outdated
| psk = bytes.fromhex('deadbeef') | ||
|
|
||
| client_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) | ||
| client_context, server_context, _ = testing_context() |
There was a problem hiding this comment.
Is this change needed?
Not being an OpenSSL expert, I'd rather avoid having to review it :)
There was a problem hiding this comment.
It's required for this test to pass when built against AWS-LC, but I'll remove it to conform to your guidance here:
IMO we could get to the point where you can get CPython to compile with AWS-LC without special-casing it (i.e. no Py_OPENSSL_IS_AWSLC), but relevant test cases still fail.
You’ll still need a patch, but a more focused one.
Modules/_ssl.c
Outdated
|
|
||
|
|
||
| #if !defined(SSL_VERIFY_POST_HANDSHAKE) || !defined(TLS1_3_VERSION) || defined(OPENSSL_NO_TLS1_3) | ||
| #define PY_SSL_NO_POST_HS_AUTH |
There was a problem hiding this comment.
I'd prefer a positive name: it is always used with ! later and double negatives are harder to read.
Perhaps PySSL_HAVE_POST_HS_AUTH
| } | ||
|
|
||
| #ifdef TLS1_3_VERSION | ||
| #if defined(TLS1_3_VERSION) && !defined(OPENSSL_NO_TLS1_3) |
There was a problem hiding this comment.
Are tickets used only for post-handshake authentication? From the docs it looks like a separate TLSv1.3 feature.
There was a problem hiding this comment.
No, tickets are used for session resumption and are unrelated to post-handshake authentication. I believe get_num_tickets pertains to a post-handshake message added in TLSv1.3. SSL_CTX_get_num_tickets's docs indicate as much.
This change just updates that guard to honor OpenSSL's OPENSSL_NO_TLS1_3 as other parts of _ssl.c already do.
|
Thank you @encukou! Would it be possible to backport this to 3.13? I see that 3.12 is currently in "bugfix", so I assume this patch isn't applicable to <= 3.12. |
As of [CPython PR #117785][1], CPython can now build against AWS-LC without any source code modifications. The only patches we still require are to configure the build and work around ([expected][2]) test failures. [1]: python/cpython#117785 [2]: https://discuss.python.org/t/support-building-ssl-and-hashlib-modules-against-aws-lc/44505/4
[CPython PR #117785][1], CPython can now build against AWS-LC without any source code modifications. The only patches we still require are to configure the build and work around ([expected][2]) test failures. [1]: python/cpython#117785 [2]: https://discuss.python.org/t/support-building-ssl-and-hashlib-modules-against-aws-lc/44505/4
…HANDSHAKE (pythonGH-117785) With this change, builds with OpenSSL forks that don't have this functionalty (like AWS-LC or BoringSSL) will require less patching.
…HANDSHAKE (pythonGH-117785) With this change, builds with OpenSSL forks that don't have this functionalty (like AWS-LC or BoringSSL) will require less patching.
…HANDSHAKE (pythonGH-117785) With this change, builds with OpenSSL forks that don't have this functionalty (like AWS-LC or BoringSSL) will require less patching.
Notes
AWS-LC recently defined a new configuration macro
OPENSSL_NO_TLS_PHAto signal lack of support for TLSv1.3 post-handshake authentication. Other cryptography libraries may find this useful. We then adjustModules/_ssl.cto detect this macro, guarding usages of PHA-related functions. We also update two PSK tests to generate client/server contexts withtest_ssl.py's conventionaltesting_context()utility.This pull request implements Issue #117784.