Skip to content
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

Jump to bottom

ssl.match_hostname() ignores extra string after whitespace in IPv4 address #81644

Closed
tiran opened this issue Jul 1, 2019 · 12 comments
Closed

ssl.match_hostname() ignores extra string after whitespace in IPv4 address #81644

tiran opened this issue Jul 1, 2019 · 12 comments
Assignees
@tiran
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes release-blocker topic-SSL type-security A security issue

Comments

@tiran
Copy link
Member

tiran commented Jul 1, 2019

BPO 37463
Nosy @vstinner, @tiran, @ned-deily, @alex, @ambv, @dstufft, @miss-islington, @ret2libc
PRs
  • bpo-37463: match_hostname requires quad-dotted IPv4 #14499
  • [3.8] bpo-37463: match_hostname requires quad-dotted IPv4 (GH-14499) #14559
  • [3.7] bpo-37463: match_hostname requires quad-dotted IPv4 (GH-14499) #14560
    		•

    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 2019-07-02.21:25:53.737>
created_at = <Date 2019-07-01.06:47:04.103>
labels = ['type-security', 'expert-SSL', '3.8', '3.9', 'release-blocker', '3.7']
title = 'ssl.match_hostname() ignores extra string after whitespace in IPv4 address'
updated_at = <Date 2019-07-05.09:49:51.485>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2019-07-05.09:49:51.485>
actor = 'vstinner'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2019-07-02.21:25:53.737>
closer = 'christian.heimes'
components = ['SSL']
creation = <Date 2019-07-01.06:47:04.103>
creator = 'christian.heimes'
dependencies = []
files = []
hgrepos = []
issue_num = 37463
keywords = ['patch', '3.7regression']
message_count = 12.0
messages = ['346964', '346980', '347121', '347122', '347123', '347151', '347152', '347154', '347155', '347156', '347162', '347323']
nosy_count = 9.0
nosy_names = ['janssen', 'vstinner', 'christian.heimes', 'ned.deily', 'alex', 'lukasz.langa', 'dstufft', 'miss-islington', 'rschiron']
pr_nums = ['14499', '14559', '14560']
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue37463'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']
    @tiran
    Copy link
    Member Author

    tiran commented Jul 1, 2019

    inet_aton accepts trailing characterrs after a valid IP (
    https://bugzilla.redhat.com/show_bug.cgi?id=1347549). This, in combination with its use inside ssl.match_hostname, allows the following code to work when it should fail:

    import ssl
cert = {'subjectAltName': (('IP Address', '1.1.1.1'),)}
ssl.match_hostname(cert, '1.1.1.1 ; this should not work but does')

    The bug was initially found by Dominik Czarnota and reported by Paul Kehrer.

    The issue was introduced in commit aef1283 / bpo-32819. Only 3.7 and newer are affected. It's a potential security bug although low severity. For one Python 3.7 and newer no longer use ssl.match_hostname() to verify hostnames and IP addresses of a certificate. Matching is performed by OpenSSL.

    @tiran tiran added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes release-blocker labels Jul 1, 2019
    @tiran tiran self-assigned this Jul 1, 2019
    @tiran tiran added topic-SSL type-security A security issue labels Jul 1, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Jul 1, 2019

    It's a potential security bug although low severity.

    What is the worst that can happen with this issue?

    Other the client doesn't validate the cert at all, and so this issue has no impact, or the client validates the cert and trusts the CA, but the host isn't fully validated... Ok, but could someone abuse "1.1.1.1 ; this should not work but does"? Does a web browser accept such hostname? Or can it be used to inject SQL or a shell command for example?

    @ned-deily
    Copy link
    Member

    Ping. At the moment, this is the only release blocker preventing the release of 3.7.4rc2.

    @ret2libc
    Copy link
    Mannequin

    ret2libc mannequin commented Jul 2, 2019

    As far as I know you can't request a hostname with spaces in it (which seems to be a precondition to trigger this bug) so I think an attacker cannot even create a malicious CA that would be mistakenly accepted by match_hostname.

    @tiran
    Copy link
    Member Author

    tiran commented Jul 2, 2019

    Riccardo, the issue is about parsing the user supplied hostname/ipaddress, not the IPAddress field of the certificate. X.509 certs store IP addresses as fixed-size binary data, 4 bytes for IPv4 or 16 bytes for IPv6. There can't be any additional payload.

    The bug is in the code that parses the user supplied "hostname" parameter to ssl.match_hostname(cert, hostname). The bug allows an attacker to pass an IPv4 address with additional content and ssl.match_hostname() ignores this additional content. This example should fail, but does not fail with an exception:

    >> import ssl
    >> cert = {'subjectAltName': [('IP Address', '127.0.0.1 additional payload')]}
    >> ssl.match_hostname(cert, '127.0.0.1')

    @ambv
    Copy link
    Contributor

    ambv commented Jul 2, 2019

    FTR 3.8b2 is also waiting for this fix due to the expert's (that's you, Christian!) priority setting.

    @miss-islington
    Copy link
    Contributor

    New changeset 477b1b2 by Miss Islington (bot) (Christian Heimes) in branch 'master':
    bpo-37463: match_hostname requires quad-dotted IPv4 (GH-14499)
    477b1b2

    @miss-islington
    Copy link
    Contributor

    New changeset 3cba3d3 by Miss Islington (bot) in branch '3.8':
    bpo-37463: match_hostname requires quad-dotted IPv4 (GH-14499)
    3cba3d3

    @miss-islington
    Copy link
    Contributor

    New changeset 024ea21 by Miss Islington (bot) in branch '3.7':
    bpo-37463: match_hostname requires quad-dotted IPv4 (GH-14499)
    024ea21

    @tiran
    Copy link
    Member Author

    tiran commented Jul 2, 2019

    Ned, Łukasz, thanks for your patience.

    @tiran tiran closed this as completed Jul 2, 2019
    @ned-deily
    Copy link
    Member

    New changeset 070fae6 by Ned Deily (Christian Heimes) in branch '3.7':
    bpo-37463: match_hostname requires quad-dotted IPv4 (GH-14499)
    070fae6

    @vstinner vstinner changed the title socket.inet_aton IP parsing issue in ssl.match_hostname ssl.match_hostname() ignores extra string after a space in IPv4 address Jul 5, 2019
    @tiran tiran changed the title ssl.match_hostname() ignores extra string after a space in IPv4 address ssl.match_hostname() ignores extra string after whitespace in IPv4 address Jul 5, 2019
    @vstinner
    Copy link
    Member

    vstinner commented Jul 5, 2019

    inet_aton accepts trailing characterrs after a valid IP (
    https://bugzilla.redhat.com/show_bug.cgi?id=1347549).

    There is a little bit of confusion between getaddrinfo() and inet_aton() here (https://bugzilla.redhat.com/show_bug.cgi?id=1347549 is about getaddrinfo()). getaddrinfo() has been fixed:
    https://sourceware.org/bugzilla/show_bug.cgi?id=20018

    But glibc devs don't want to fix inet_aton() to keep the backward compatibility ("for historic reasons"): more info in bpo-37495 "socket.inet_aton parsing issue on some libc versions".

    This issue is about ssl.match_hostname() which uses internally socket.inet_aton(). ssl.match_hostname() has been fixed to implement further checks to workaround inet_aton() behavior (ignore extra string after a whitespace).

    I also removed inet_aton() from the title of this issue to reduce confusion ;-)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @tiran tiran

    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes release-blocker topic-SSL type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @ambv @vstinner @tiran @ned-deily @miss-islington