Skip to content

SSLContext.hostname_checks_common_name appears to have no effect #87688

Closed
Closed
SSLContext.hostname_checks_common_name appears to have no effect#87688
Assignees
tiran
Labels
3.10 (EOL)end of life3.8 (EOL)end of life3.9 (EOL)end of lifetopic-SSLtype-bugAn unexpected behavior, bug, or error

Description

@pquentin
pquentin
mannequin
opened on Mar 16, 2021
BPO 43522
Nosy @tiran, @pquentin
PRs
  • bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) #24899
  • [3.9] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (GH-25451) #25451
  • [3.8] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (GH-25452) #25452
    		•
    Files
  • no_san_ignored.py: Reproducer
  • app.py: Sample Flask app
  • client.pem: client.pem
  • server.pem: server.pem
  • server.key: server.key
    		•

    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:

    assignee = 'https://github.com/tiran'
closed_at = <Date 2021-04-17.10:47:03.885>
created_at = <Date 2021-03-16.20:07:50.463>
labels = ['expert-SSL', 'type-bug', '3.8', '3.9', '3.10']
title = 'SSLContext.hostname_checks_common_name appears to have no effect'
updated_at = <Date 2021-05-04.05:50:13.339>
user = 'https://github.com/pquentin'

    bugs.python.org fields:

    activity = <Date 2021-05-04.05:50:13.339>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2021-04-17.10:47:03.885>
closer = 'christian.heimes'
components = ['SSL']
creation = <Date 2021-03-16.20:07:50.463>
creator = 'Quentin.Pradet'
dependencies = []
files = ['49879', '49880', '49881', '49882', '49883']
hgrepos = []
issue_num = 43522
keywords = ['patch']
message_count = 9.0
messages = ['388875', '388887', '388888', '388907', '391275', '391277', '391278', '391283', '392853']
nosy_count = 2.0
nosy_names = ['christian.heimes', 'Quentin.Pradet']
pr_nums = ['24899', '25451', '25452']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43522'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    Linked PRs

    Activity

    1. pquentin commented on Mar 16, 2021

      @pquentin
      pquentinmannequin
      on Mar 16, 2021
      MannequinAuthor

      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?

    2. pquentin
      added
      3.10 (EOL)end of life
      on Mar 16, 2021
    3. pquentin
      assigned
      tiran
      on Mar 16, 2021
    4. pquentin
      added
      topic-SSL
      3.10 (EOL)end of life
      on Mar 16, 2021
    5. pquentin
      assigned
      tiran
      on Mar 16, 2021
    6. pquentin
      added
      topic-SSL
      on Mar 16, 2021

    7. tiran commented on Mar 16, 2021

      @tiran
      tiran
      on Mar 16, 2021
      Member

      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

    8. tiran commented on Mar 16, 2021

      @tiran
      tiran
      on Mar 16, 2021
      Member

      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

    9. pquentin commented on Mar 17, 2021

      @pquentin
      pquentinmannequin
      on Mar 17, 2021
      MannequinAuthor

      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+.

    10. tiran commented on Apr 17, 2021

      @tiran
      tiran
      on Apr 17, 2021
      Member

      New changeset b467d9a by Christian Heimes in branch 'master':
      bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899)
      b467d9a

    11. tiran commented on Apr 17, 2021

      @tiran
      tiran
      on Apr 17, 2021
      Member

      New changeset cdf0287 by Christian Heimes in branch '3.9':
      [3.9] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (GH-25451)
      cdf0287

    12. tiran commented on Apr 17, 2021

      @tiran
      tiran
      on Apr 17, 2021
      Member

      New changeset f77ca86 by Christian Heimes in branch '3.8':
      [3.8] bpo-43522: Fix SSLContext.hostname_checks_common_name (GH-24899) (GH-25452)
      f77ca86

    13. tiran commented on Apr 17, 2021

      @tiran
      tiran
      on Apr 17, 2021
      Member

      Workaround has been added to upcoming 3.8 to 3.10 releases. Older versions will get fixed by next OpenSSL update.

    14. tiran
      closed this as completedon Apr 17, 2021

    15. 2 remaining items

    16. tiran
      added
      type-bugAn unexpected behavior, bug, or error
      on Apr 17, 2021

    17. tiran commented on May 4, 2021

      @tiran
      tiran
      on May 4, 2021
      Member

      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

    18. tiran
      added
      3.8 (EOL)end of life
      3.9 (EOL)end of life
      on May 4, 2021
    19. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    20. serhiy-storchaka
      added a commit that references this issue on Feb 17, 2024

      gh-87688: Amend SSLContext.hostname_checks_common_name docs (GH-100517)

      debb138
    21. miss-islington
      added 2 commits that reference this issue on Feb 17, 2024

      pythongh-87688: Amend SSLContext.hostname_checks_common_name docs (py…

      29e2d99

      pythongh-87688: Amend SSLContext.hostname_checks_common_name docs (py…

      113b7a9
    22. serhiy-storchaka
      added 2 commits that reference this issue on Feb 17, 2024

      [3.11] gh-87688: Amend SSLContext.hostname_checks_common_name docs (G…

      bc11905

      [3.12] gh-87688: Amend SSLContext.hostname_checks_common_name docs (G…

      068e219
    23. woodruffw
      added a commit that references this issue on Mar 4, 2024

      pythongh-87688: Amend SSLContext.hostname_checks_common_name docs (py…

      0ef07aa
    24. diegorusso
      added a commit that references this issue on Apr 17, 2024

      pythongh-87688: Amend SSLContext.hostname_checks_common_name docs (py…

      a2f8730
    25. LukasWoodtli
      added a commit that references this issue on Jan 22, 2025

      pythongh-87688: Amend SSLContext.hostname_checks_common_name docs (py…

      f810c61
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.10 (EOL)end of life3.8 (EOL)end of life3.9 (EOL)end of lifetopic-SSLtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions