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 does not accept IP Address #67428

Closed
dmZsigmond mannequin opened this issue Jan 14, 2015 · 14 comments
Closed

SSL match_hostname does not accept IP Address #67428

dmZsigmond mannequin opened this issue Jan 14, 2015 · 14 comments
Labels
extension-modules C modules in the Modules dir topic-SSL type-feature A feature request or enhancement

Comments

@dmZsigmond
Copy link
Mannequin

dmZsigmond mannequin commented Jan 14, 2015

BPO 23239
Nosy @pitrou, @tiran, @alex, @bitdancer, @msabramo, @dstufft
Files
  • ip_certs.patch
  • ip_certs_comment.patch
    		•

    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 = None
closed_at = <Date 2021-04-17.18:23:06.360>
created_at = <Date 2015-01-14.09:04:52.726>
labels = ['extension-modules', 'expert-SSL', 'type-feature']
title = 'SSL match_hostname does not accept IP Address'
updated_at = <Date 2021-04-17.18:23:06.359>
user = 'https://bugs.python.org/dmZsigmond'

    bugs.python.org fields:

    activity = <Date 2021-04-17.18:23:06.359>
actor = 'christian.heimes'
assignee = 'none'
closed = True
closed_date = <Date 2021-04-17.18:23:06.360>
closer = 'christian.heimes'
components = ['Extension Modules', 'SSL']
creation = <Date 2015-01-14.09:04:52.726>
creator = '\xc3\x81d\xc3\xa1m.Zsigmond'
dependencies = []
files = ['37746', '42186']
hgrepos = []
issue_num = 23239
keywords = ['patch']
message_count = 14.0
messages = ['234017', '234038', '234190', '236051', '244665', '246327', '246671', '261917', '262987', '262988', '303155', '303156', '314190', '391301']
nosy_count = 10.0
nosy_names = ['pitrou', 'christian.heimes', 'alex', 'r.david.murray', 'python-dev', 'Marc.Abramowitz', 'dstufft', '\xc3\x81d\xc3\xa1m.Zsigmond', 'Alex Gaynor', 'j-harbott']
pr_nums = []
priority = 'high'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue23239'
versions = ['Python 2.7']
    @dmZsigmond
    Copy link
    Mannequin Author

    dmZsigmond mannequin commented Jan 14, 2015

    ssl.match_hostname does not accept the ca certificate if the hostname matches the ip address.

    I am trying to connect to a servert with a cacert by IP address but I get an error message like: '42.42.42.42' hostname does not match '<hostname_in_cacert>'

    The IP Address is in the ca certificate, so it should be accepted.

    @dmZsigmond dmZsigmond mannequin added type-security A security issue extension-modules C modules in the Modules dir labels Jan 14, 2015
    @pitrou pitrou added type-feature A feature request or enhancement and removed type-security A security issue labels Jan 14, 2015
    @pitrou
    Copy link
    Member

    pitrou commented Jan 14, 2015

    This is a feature request. Not supporting IP addresses is a documented limitation of the current implementation.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 17, 2015

    Here is a patch.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 15, 2015

    New changeset b15a5f239e8a by Antoine Pitrou in branch 'default':
    Issue bpo-23239: ssl.match_hostname() now supports matching of IP addresses.
    https://hg.python.org/cpython/rev/b15a5f239e8a

    @pitrou pitrou closed this as completed Feb 15, 2015
    @tiran
    Copy link
    Member

    tiran commented Jun 2, 2015

    The patch has a couple of issues

    1. match_hostname()'s doc string needs to be updated. It still contains "but IP addresses are not accepted for *hostname*"

    2. The stdlib uses server_hostname for SNI and matching. An IP address in the SNI TLS extension violates RF 3546 https://tools.ietf.org/html/rfc3546#page-9

      Literal IPv4 and IPv6 addresses are not permitted in "HostName".

    3. The code doesn't match IP addresses in dNSName and DNS names in IP Address fields. Hynek's service identity module and Mozilla's NSS [1] agree with you. As far as I have studied OpenSSL 1.0.2, it has a different opinion. I'm in favor for the current check. I suggest to document the decision in the code and raise a more explicit exception. The current message is a bit confusing:

    ssl.CertificateError: hostname '127.0.0.1' doesn't match '127.0.0.1'

    1. The code doesn't check the CN field for IP address as NSS does. [2]

    In order to fix 2) and make the check more explicit I like to suggest an API change. Don't convert the host name to an IP address implicitly. If the user wants to validate an IP address, then she must pass in an ipaddress object as server_hostname. In that case SSLSocket.server_hostname is set to the ipaddress object. socket._wrap_socket() is called with server_hostname=None for ipaddress. That fixes the RFC violation.

    [1] cert_VerifySubjectAltName() https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/certdb/certdb.c#1427
    [2] CERT_VerifyCertName https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/certdb/certdb.c#1769

    @tiran tiran reopened this Jun 2, 2015
    @tiran
    Copy link
    Member

    tiran commented Jul 5, 2015

    ping

    @pitrou
    Copy link
    Member

    pitrou commented Jul 12, 2015

    ping

    Sorry. I do not have time currently to tackle this issue. Feel free to submit and/or commit improvements if you feel like it.

    @msabramo
    Copy link
    Mannequin

    msabramo mannequin commented Mar 17, 2016

    Patch to update the comment to remove "IP addresses are not accepted for *hostname*", because supported for IP addresses was added earlier by @pitrou in https://hg.python.org/cpython/rev/b15a5f239e8a

    @msabramo
    Copy link
    Mannequin

    msabramo mannequin commented Apr 7, 2016

    ip_certs_comment.patch is a simple patch that just removes the verbiage about not supporting IP addresses in hostnames, as that restriction was removed by an earlier commit from Antoine.

    @tiran
    Copy link
    Member

    tiran commented Apr 7, 2016

    I'm -1 on the patch for a practical reason: The current API is broken and I don't want to have it documented as officially supported.

    In fact it is not only broken but also incompatible with more modern releases of OpenSSL. Recently OpenSSL got proper implementation of hostname and IP checking. Hostname and IP must be set with different API calls:

    https://www.openssl.org/docs/manmaster/crypto/X509_VERIFY_PARAM_add1_host.html
    https://www.openssl.org/docs/manmaster/crypto/X509_check_host.html

    @tiran tiran added the 3.7 (EOL) end of life label Sep 8, 2016
    @tiran tiran self-assigned this Sep 8, 2016
    @j-harbott
    Copy link
    Mannequin

    j-harbott mannequin commented Sep 27, 2017

    The original issue still exists in py27, is there a chance to get the fix backported? See pyca/cryptography#3943 and urllib3/urllib3#1269 for sample issues that arise because we need to work around this one.

    @AlexGaynor
    Copy link
    Mannequin

    AlexGaynor mannequin commented Sep 27, 2017

    I'd be in favor of backporting this to the 2.x - encouraging reliance on the nonsense behaviour of putting IPAddresses in DNS Names or relying on CN over SAN is bad, and we shouldn't encourage it.

    @tiran
    Copy link
    Member

    tiran commented Mar 21, 2018

    bpo-32819 and bpo-32185 have solved the last outstanding bugs with IP address validation and handling. I'm fine with a backport of the feature to 2.7 now.

    @tiran tiran removed the 3.7 (EOL) end of life label Mar 21, 2018
    @tiran tiran removed their assignment Mar 21, 2018
    @tiran
    Copy link
    Member

    tiran commented Apr 17, 2021

    Python 2 is out of support. Python 3 can verify IP addresses in certificates correctly.

    @tiran tiran closed this as completed Apr 17, 2021
    @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
    No one assigned
    Labels
    extension-modules C modules in the Modules dir topic-SSL type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @tiran @pitrou