classification
Title: SSLContext.hostname_checks_common_name appears to have no effect
Type: Stage: patch review
Components: SSL Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: Quentin.Pradet, christian.heimes
Priority: normal Keywords: patch

Created on 2021-03-16 20:07 by Quentin.Pradet, last changed 2021-03-17 06:01 by Quentin.Pradet.

Files
File name Uploaded Description Edit
no_san_ignored.py Quentin.Pradet, 2021-03-16 20:07 Reproducer
app.py Quentin.Pradet, 2021-03-16 20:08 Sample Flask app
client.pem Quentin.Pradet, 2021-03-16 20:08 client.pem
server.pem Quentin.Pradet, 2021-03-16 20:08 server.pem
server.key Quentin.Pradet, 2021-03-16 20:08 server.key
Pull Requests
URL Status Linked Edit
PR 24899 open christian.heimes, 2021-03-16 21:41
Messages (4)
msg388875 - (view) Author: Quentin Pradet (Quentin.Pradet) * Date: 2021-03-16 20:07
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 `hostname_checks_common_name` to False on Python 3.7+ and OpenSSL 1.1.0+. (In other cases, we use a modified version of `ssl.match_hostname` that does not consider common names.)

I would expect that setting `hostname_checks_common_name` to 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 2020` and 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?
msg388887 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-03-16 21:29
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.

https://github.com/openssl/openssl/blob/081a7061f3da07318c4b0f5de67b82285630bf6b/crypto/x509/x509_vpm.c#L202-L213
msg388888 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-03-16 21:31
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
msg388907 - (view) Author: Quentin Pradet (Quentin.Pradet) * Date: 2021-03-17 06:01
Thank 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_NAME` anymore, because it will be True in Python versions where `hostname_checks_common_name` does not work. Is it possible to have, uh, `REALLY_HAS_NEVER_CHECK_COMMON_NAME_I_PROMISE` or something like that? :D It could even be private. Otherwise, we will only be able to use `hostname_checks_common_name` in Python 3.10.0a7+.
History
Date User Action Args
2021-03-17 06:01:40Quentin.Pradetsetmessages: + msg388907
2021-03-16 21:41:38christian.heimessetkeywords: + patch
stage: patch review
pull_requests: + pull_request23663
2021-03-16 21:31:14christian.heimessetmessages: + msg388888
2021-03-16 21:29:37christian.heimessetmessages: + msg388887
2021-03-16 20:09:00Quentin.Pradetsetfiles: + server.key
2021-03-16 20:08:43Quentin.Pradetsetfiles: + server.pem
2021-03-16 20:08:32Quentin.Pradetsetfiles: + client.pem
2021-03-16 20:08:13Quentin.Pradetsetfiles: + app.py
2021-03-16 20:07:50Quentin.Pradetcreate