From e70d5d69aa0d7a210d35d033d2a7193fb7051108 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sat, 10 Feb 2024 09:04:03 -0800 Subject: [PATCH] Explain why #2356 "works". (#2358) The explanation was hard won and a bit embarrasing in outcome. The original behavior was correct afaict and PBS use on a RedHat system with custom RedHat OpenSSL config keys should fail, obviously with a much better error message, and prompt OpenSSL configuration evaluation on the machine. That said, the behavior is released now and Pex stands behind it. It may make sense to add a `--strict-ssl` option or something similar to restore the old behavior and let the (confusing) error bubble, perhaps with a pointer to what may be wrong. This should also serve to close https://github.com/indygreg/python-build-standalone/issues/207 or at least give Gregory enough information to decide what to do over in PBS. Closes the loose ends in #2355. --- pex/fetcher.py | 35 ++++++++++++++++++++++++++++ tests/integration/test_issue_2355.py | 3 ++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/pex/fetcher.py b/pex/fetcher.py index 402dd06a5..bbf842519 100644 --- a/pex/fetcher.py +++ b/pex/fetcher.py @@ -67,6 +67,41 @@ def create(cls, network_configuration=None): def create_ssl_context(self): # type: () -> SSLContext + + # These shenanigans deserve some explanation, since, in OpenSSL 3.0 anyhow, it is perfectly + # fine to create an SSL Context (`SSL_CTX_new`) in any thread: + # + # It turns out that, in typical use, the CPython ssl module hides OpenSSL configuration + # issues through no real fault of its own. This is due to the fact that an import of the + # `ssl` module, which generally happens in the main thread, triggers, through instantiation + # of the `ssl.Purpose` enum type [^1] a call to OpenSSL's `OBJ_obj2nid` [^2][^3] which + # loads OpenSSL config but throws away the return value; thus hiding errors in config. Since + # the OpenSSL config scheme is to load it at most once per thread, this means subsequent + # OpenSSL call paths in the same thread that imported `ssl` (like the one generated via + # `ssl.create_default_context`) that _do_ check the return value of config loading [^4][^5], + # will not have to load config (since it's been done once already in the thread) and thus + # will not get the chance to check the config load function return value and will thus not + # error. This default behavior is almost certainly bad, since it allows invalid OpenSSL + # configs to go partially read at best. That said, this is the default Python + # single-threaded behavior and, right or wrong, we preserve that here by forcing our SSL + # initialization to happen in the main thread, keeping any OpenSSL misconfiguration silently + # ignored. + # + # The only solace here is that the use cases where OpenSSL config can be bad on a machine + # and the machine still function are narrow. The case we know of, that triggered creation of + # this machinery, involves the combination of a modern PBS Python [^6] (which has a vanilla + # OpenSSL statically linked into the Python binary) running on a RedHat OS that expresses + # custom RedHat configuration keys [^7] in its OpenSSL config. These custom keys are only + # supported by RedHat patches to OpenSSL and cause vanilla versions of OpenSSL to error when + # loading config due to unknown configuration options. + # + # [^1]: https://github.com/python/cpython/blob/5a173efa693a053bf4a059c82c1c06c82a9fa8fb/Lib/ssl.py#L394-L419 + # [^2]: https://github.com/python/cpython/blob/5a173efa693a053bf4a059c82c1c06c82a9fa8fb/Modules/_ssl.c#L5534-L5552 + # [^3]: https://github.com/openssl/openssl/blob/c3cc0f1386b0544383a61244a4beeb762b67498f/crypto/objects/obj_dat.c#L326-L340 + # [^4]: https://github.com/openssl/openssl/blob/c3cc0f1386b0544383a61244a4beeb762b67498f/ssl/ssl_lib.c#L3194-L3212 + # [^5]: https://github.com/openssl/openssl/blob/c3cc0f1386b0544383a61244a4beeb762b67498f/ssl/ssl_init.c#L86-L116 + # [^6]: https://github.com/indygreg/python-build-standalone/releases/tag/20240107 + # [^7]: https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/merge_requests/110/diffs#269a48e71ac25ad1d07ff00db2390834c8ba7596_11_16 asserts.production_assert( in_main_thread(), msg=( diff --git a/tests/integration/test_issue_2355.py b/tests/integration/test_issue_2355.py index 1271dda0f..c8dd39586 100644 --- a/tests/integration/test_issue_2355.py +++ b/tests/integration/test_issue_2355.py @@ -71,7 +71,8 @@ def test_ssl_context( "docker", "run", "--rm", - "-v" "{pex_project_dir}:/code".format(pex_project_dir=pex_project_dir), + "-v", + "{pex_project_dir}:/code".format(pex_project_dir=pex_project_dir), "-v", "{work_dir}:/work".format(work_dir=work_dir), "-w",