-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SSLContext.hostname_checks_common_name appears to have no effect #87688
Comments
urllib3 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 I would expect that setting 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 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 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 |
Thank you for the quick fix! 🙏 Both the reproducer and the urllib3 test suite run fine with this change. However, we can't trust |
Workaround has been added to upcoming 3.8 to 3.10 releases. Older versions will get fixed by next OpenSSL update. |
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 |
…thonGH-100517) (cherry picked from commit debb138) Co-authored-by: Rami <72725910+ramikg@users.noreply.github.com>
…thonGH-100517) (cherry picked from commit debb138) Co-authored-by: Rami <72725910+ramikg@users.noreply.github.com>
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
The text was updated successfully, but these errors were encountered: