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

SSLContext.wrap_socket sends SNI Extension when server_hostname is IP #76366

Closed
nitzmahone mannequin opened this issue Nov 30, 2017 · 9 comments
Closed

SSLContext.wrap_socket sends SNI Extension when server_hostname is IP #76366

nitzmahone mannequin opened this issue Nov 30, 2017 · 9 comments
Assignees
@tiran
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-SSL type-bug An unexpected behavior, bug, or error

Comments

@nitzmahone
Copy link
Mannequin

nitzmahone mannequin commented Nov 30, 2017

BPO 32185
Nosy @pfmoore, @pitrou, @tiran, @tjguk, @alex, @zware, @zooba, @dstufft, @nitzmahone
PRs
  • bpo-32185: Don't send IP in SNI TLS extension #4938
  • bpo-32185: Don't send IP in SNI TLS extension #5865
  • [2.7] bpo-32185: Don't send IP in SNI TLS extension (GH-5865) #5871
  • bpo-33184: Update Windows build to use OpenSSL 1.0.2o (GH-6464) #6474
    		•

    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 2018-02-25.09:19:20.791>
created_at = <Date 2017-11-30.18:05:09.552>
labels = ['expert-SSL', '3.8', 'type-bug', '3.7']
title = 'SSLContext.wrap_socket sends SNI Extension when server_hostname is IP'
updated_at = <Date 2018-04-14.22:19:00.613>
user = 'https://github.com/nitzmahone'

    bugs.python.org fields:

    activity = <Date 2018-04-14.22:19:00.613>
actor = 'steve.dower'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2018-02-25.09:19:20.791>
closer = 'christian.heimes'
components = ['SSL']
creation = <Date 2017-11-30.18:05:09.552>
creator = 'nitzmahone'
dependencies = []
files = []
hgrepos = []
issue_num = 32185
keywords = ['patch', '3.5regression']
message_count = 9.0
messages = ['307333', '307334', '308712', '308713', '308715', '308716', '312782', '312786', '312787']
nosy_count = 10.0
nosy_names = ['paul.moore', 'janssen', 'pitrou', 'christian.heimes', 'tim.golden', 'alex', 'zach.ware', 'steve.dower', 'dstufft', 'nitzmahone']
pr_nums = ['4938', '5865', '5871', '6474']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32185'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']
    @nitzmahone
    Copy link
    Mannequin Author

    nitzmahone mannequin commented Nov 30, 2017

    The current implementation of SSLContext.wrap_socket blindly sends whatever is passed in server_hostname in the SNI extension, assuming it's a DNS hostname. RFC6066 describes the SNI TLS extension, and specifically states that 'Literal IPv4 and IPv6 addresses are not permitted in "HostName".' The RFC makes no recommendation on how a server implementation that violates this requirement should behave; Microsoft's kernel HTTP listener (http.sys) chooses to abort the connection if SNI has been enabled. In the http.sys case, SNI is a global setting, currently off by default, but if any registered listener has SNI enabled, the connection abort behavior applies to all listeners.

    SSLContext.wrap_socket() should determine whether server_hostname is an IP address before including the SNI extension.

    I've submitted a PR to work around this issue in urllib3 (urllib3/urllib3#1287) in the meantime, but would be good to get this fixed, especially if Microsoft decides to enable SNI by default at some point.

    @nitzmahone nitzmahone mannequin added the 3.7 (EOL) end of life label Nov 30, 2017
    @nitzmahone nitzmahone mannequin assigned tiran Nov 30, 2017
    @nitzmahone nitzmahone mannequin added the topic-SSL label Nov 30, 2017
    @tiran
    Copy link
    Member

    tiran commented Nov 30, 2017

    Thanks!

    3.4 and 3.5 are out of scope. They only receive security fixes.

    For 3.7 master...tiran:openssl_check_hostname will take care of the issue

    2.7 and 3.6 are a bit tricky. There is no platform-compatible way to detect if a string is an IP address. inet_pton() is not available on Windows. I cannot use the OpenSSL parser because it is only available in 1.0.2+. 2.7 and 3.6 still support 0.9.8.

    @tiran tiran added the type-bug An unexpected behavior, bug, or error label Dec 20, 2017
    @pitrou
    Copy link
    Member

    pitrou commented Dec 20, 2017

    There is no platform-compatible way to detect if a string is an IP address.

    Actually, you could use the ipaddress module.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 20, 2017

    By the way, Windows nowadays has inet_pton():
    https://msdn.microsoft.com/en-us/library/windows/desktop/cc805844(v=vs.85).aspx

    Are there other platforms without it? inet_pton() is part of POSIX.

    @tiran
    Copy link
    Member

    tiran commented Dec 20, 2017

    The code works on all platforms with OpenSSL >= 1.0.2. OpenSSL 1.0.1, 0.9.8 and earlier are no longer supported by upstream. Anybody with even marginal interest in secure TLS/SSL should update. Python.org's Windows and macOS binaries are good.

    The inet_pton() code in my patch is for those poor souls that are stuck with RHEL 6, CentOS 6, or Ubuntu 14.04. RHEL 7, CentOS 7, and Ubuntu 16.04 have OpenSSL 1.0.2.

    The IP address module is slow and hard to use from C code.

    @tiran
    Copy link
    Member

    tiran commented Dec 20, 2017

    PS: With OpenSSL >= 1.0.2, inet_pton() is not required.

    @tiran
    Copy link
    Member

    tiran commented Feb 25, 2018

    New changeset e9370a4 by Christian Heimes in branch '3.6':
    bpo-32185: Don't send IP in SNI TLS extension (bpo-5865)
    e9370a4

    @tiran
    Copy link
    Member

    tiran commented Feb 25, 2018

    New changeset a5c9112 by Christian Heimes (Miss Islington (bot)) in branch '2.7':
    [2.7] bpo-32185: Don't send IP in SNI TLS extension (GH-5865) (bpo-5871)
    a5c9112

    @tiran
    Copy link
    Member

    tiran commented Feb 25, 2018

    The issue has been fixed in 2.7, 3.6-3.8 for OpenSSL >= 1.0.2 or platforms with inet_pton. I didn't bother to fix platforms without inet_pton since OpenSSL 1.0.1 and earlier are no longer support any way.

    @tiran tiran added the 3.8 only security fixes label Feb 25, 2018
    @tiran tiran closed this as completed Feb 25, 2018
    @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 topic-SSL type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @tiran @pitrou