Repository navigation
SSLContext.hostname_checks_common_name appears to have no effect #87688
Description
Activity
pquentin commented
on Mar 16, 2021 pquentinmannequinMannequinAuthorurllib3 is preparing a v2 with various SSL improvements, such as leaning on the ssl module to match hostnames when possible and reject certificates without a SAN. See https://urllib3.readthedocs.io/en/latest/v2-roadmap.html#modern-security-by-default for more details.
For this reason, we want to set
hostname_checks_common_nameto False on Python 3.7+ and OpenSSL 1.1.0+. (In other cases, we use a modified version ofssl.match_hostnamethat does not consider common names.)I would expect that setting
hostname_checks_common_nameto False would rejects certificates without SANs, but that does not appear to be the case. I used the following Python code:import socket import ssl print(ssl.OPENSSL_VERSION) hostname = 'localhost' context = ssl.create_default_context() context.load_verify_locations("client.pem") context.hostname_checks_common_name = False with socket.create_connection((hostname, 8000)) as sock: with context.wrap_socket(sock, server_hostname=hostname) as ssock: assert "subjectAltName" not in ssock.getpeercert()
which prints
OpenSSL 1.1.1i 8 Dec 2020and does not fail as expected. I'm testing this on macOS 11.2.2 but this currently breaks our test suite on Ubuntu, Windows and macOS, including on Python 3.10, see https://github.com/urllib3/urllib3/runs/2122811894?check_suite_focus=true.To reproduce this, I used trustme (https://trustme.readthedocs.io/en/latest/). I modified the code to not include a SAN at all and ran
gunicorn --keyfile server.key --certfile server.pem app:app, with app being the Flask quickstart application. I'll try to attach all those files if I manage to do it.What am I missing?
Oh heck, this is a genuine bug. I'm not yet sure if it's an undocumented API quirk in OpenSSL, a design bug in OpenSSL, or a bug in my code.
Python sets the host flags on the X509_VERIFY_PARAM of the *SSL_CTX. All flags get copied to *SSL struct and later to *X509_STORE_CTX struct. At least I thought that all flags get copied. Apparently hostflags aren't copied from *SSL_CTX to *SSL because the *SSL_CTX doesn't have any verify hosts configured. They are only ever configured on *SSL struct.
PS: I don't see any remark or warning about the behavior on the man pages https://www.openssl.org/docs/man1.1.1/man3/X509_VERIFY_PARAM_set_flags.html and https://www.openssl.org/docs/man1.1.1/man3/X509_check_host.html
pquentin commented
on Mar 17, 2021 pquentinmannequinMannequinAuthorThank you for the quick fix! 🙏 Both the reproducer and the urllib3 test suite run fine with this change.
However, we can't trust
HAS_NEVER_CHECK_COMMON_NAMEanymore, because it will be True in Python versions wherehostname_checks_common_namedoes not work. Is it possible to have, uh,REALLY_HAS_NEVER_CHECK_COMMON_NAME_I_PROMISEor something like that? :D It could even be private. Otherwise, we will only be able to usehostname_checks_common_namein Python 3.10.0a7+.Workaround has been added to upcoming 3.8 to 3.10 releases. Older versions will get fixed by next OpenSSL update.
2 remaining items
Seth's urllib3 newsletter reminded me that I forgot to link to OpenSSL issues here.
The problem was caused by a bug in OpenSSL. The issue is fixed in OpenSSL default branch and is scheduled to land in next 1.1.1 release. My changes to Python's ssl module are backports of my OpenSSL fixes for older 1.1.1 releases.
openssl/openssl#14579
openssl/openssl#14856
openssl/openssl@dfccfde
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs