classification
Title: SSLContext.hostname_checks_common_name appears to have no effect
Type: behavior Stage: resolved
Components: SSL Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
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-05-04 05:50 by christian.heimes. This issue is now closed.

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 merged christian.heimes, 2021-03-16 21:41
PR 25451 merged christian.heimes, 2021-04-17 09:06
PR 25452 merged christian.heimes, 2021-04-17 09:07
Messages (9)
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+.
msg391275 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-17 08:07
New changeset b467d9a24011992242c95d9157d3455f8a84466b by Christian Heimes in branch 'master':
bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899)
https://github.com/python/cpython/commit/b467d9a24011992242c95d9157d3455f8a84466b
msg391277 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-17 09:35
New changeset cdf02879790b8e52456df6e9d58fb8c0842fc359 by Christian Heimes in branch '3.9':
[3.9] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (GH-25451)
https://github.com/python/cpython/commit/cdf02879790b8e52456df6e9d58fb8c0842fc359
msg391278 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-17 09:36
New changeset f77ca86f75d5ad9b52e5f3cd19c0024b204b168c by Christian Heimes in branch '3.8':
[3.8] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (GH-25452)
https://github.com/python/cpython/commit/f77ca86f75d5ad9b52e5f3cd19c0024b204b168c
msg391283 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-04-17 10:47
Workaround has been added to upcoming 3.8 to 3.10 releases. Older versions will get fixed by next OpenSSL update.
msg392853 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-05-04 05:50
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.

https://github.com/openssl/openssl/issues/14579
https://github.com/openssl/openssl/pull/14856
https://github.com/openssl/openssl/commit/dfccfde06562ac87fe5e5f9401ba86cad050d9a2
History
Date User Action Args
2021-05-04 05:50:13christian.heimessetmessages: + msg392853
versions: + Python 3.8, Python 3.9
2021-04-17 10:47:03christian.heimessetstatus: open -> closed
type: behavior
messages: + msg391283

resolution: fixed
stage: patch review -> resolved
2021-04-17 09:36:04christian.heimessetmessages: + msg391278
2021-04-17 09:35:48christian.heimessetmessages: + msg391277
2021-04-17 09:07:15christian.heimessetpull_requests: + pull_request24181
2021-04-17 09:06:24christian.heimessetpull_requests: + pull_request24180
2021-04-17 08:07:26christian.heimessetmessages: + msg391275
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