python / cpython

The Python programming language
https://www.python.org
Other
62.31k stars 29.93k forks source link

Support TLS SNI extension in ssl module #49889

Closed cde690a3-7412-4f89-bea2-524b3a6dbb2c closed 13 years ago

cde690a3-7412-4f89-bea2-524b3a6dbb2c commented 15 years ago
BPO 5639
Nosy @jcea, @ncoghlan, @pitrou, @vstinner, @giampaolo, @philpennock, @dimaqq, @dstufft
Files
  • python-2.6.1-tlssni.patch: Add support for TLS SNI to Python ssl module
  • python-HEAD-74602-ssl_client_sni.path: python-2.7-svn-ssl_client_sni.patch
  • python-3K-74602-ssl_client_sni.path: python-3k-svn-ssl_client_sni.patch
  • pytest.py: test script for p3k
  • sni.patch
  • unnamed
  • python-2.7.5-tlssni.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: ```python assignee = None closed_at = created_at = labels = ['type-feature', 'library'] title = 'Support TLS SNI extension in ssl module' updated_at = user = 'https://github.com/philpennock' ``` bugs.python.org fields: ```python activity = actor = 'Dima.Tisnek' assignee = 'none' closed = True closed_date = closer = 'pitrou' components = ['Library (Lib)'] creation = creator = 'pdp' dependencies = [] files = ['13534', '14808', '14809', '14810', '19327', '22886', '30779'] hgrepos = [] issue_num = 5639 keywords = ['patch'] message_count = 42.0 messages = ['84984', '85088', '92097', '92098', '92099', '92100', '92118', '92233', '92272', '103748', '106323', '106324', '106327', '106331', '106335', '106336', '106370', '119340', '119397', '141912', '141913', '141946', '141950', '192233', '192234', '192235', '204323', '204324', '207659', '212406', '214170', '214171', '214180', '214199', '214201', '214203', '214211', '214215', '214218', '214222', '214224', '215919'] nosy_count = 16.0 nosy_names = ['jcea', 'exarkun', 'ncoghlan', 'janssen', 'pitrou', 'mnot', 'vstinner', 'giampaolo.rodola', 'scott.tsai', 'pdp', 'grooverdan', 'Dolf.Andringa', 'Dima.Tisnek', 'dstufft', 'markk', 'sag47'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue5639' versions = ['Python 3.1', 'Python 2.7', 'Python 3.2'] ```

    cde690a3-7412-4f89-bea2-524b3a6dbb2c commented 15 years ago

    With TLS it is possible to have the client use an extension (defined in RFC 4366, and RFC 3546 before that) to indicate to the server which hostname it believes it is talking to. The server can then choose TLS certificates accordingly. This makes virtual-hosting possible. Most modern GUI web-browsers support making use of this extension, Server Name Indication (SNI).

    OpenSSL 0.9.8f onwards have optional support for this; OpenSSL needs to have been built with "enable-tlsext" in EXTRACONFIGURE. If that is not present, then there's a guard macro defined to say it's absent.

    This patch, against Python 2.6.1, adds to the standard ssl module the ability to set the extension, using server_hostname as a arg in relevant places. This is only set for client connections and will silently be ignored if the OpenSSL library does not support it.

    I have tested this on FreeBSD 7.0/amd64 with OpenSSL 0.9.8k when talking to Apache 2.2.x with the SNI patches from https://sni.velox.ch/. Below is my simple test program, to dump raw HTTP results back. With this, I can connect to various local https vhosts and get the correct content back.

    I am not a Python core dev and not too enthusiastic at the thought of grabbing latest svn to port this across; I hope that it's still of use.

    \=============

    import socket
    import ssl
    import sys
    
    def dump_https_page(hostname, uri='/'):
    
      sock = socket.socket(socket.AF_INET)
      s = ssl.SSLSocket(sock=sock,
                        ca_certs='/etc/ssl/certs',
                        server_hostname=hostname)
      print 'have socket'
      s.connect((hostname, 443))
      print 'connected'

    print >>s, 'GET %s HTTP/1.0\r\nHost: %s\r\nConnection: close\r\n\r\n' % ( uri, hostname),

      t = s.read()
      while t:
        print t,
        t = s.read()
    
    if __name__ == '__main__':
      for x in sys.argv[1:]:
        dump_https_page(hostname=x)
    cde690a3-7412-4f89-bea2-524b3a6dbb2c commented 15 years ago

    Note: this previous work is client-side only, as noted in the body of the report. I'll look into what's needed for clean server-side support too.

    14786669-223b-4a34-98e8-6cef54412420 commented 15 years ago

    patch against TRUNK (2.7) with self tests and doco. Essentially the same code as pdp with a SSLv2 check before using the SNI extension.

    Contains some spacing cleanups that where highlighted by vim.

    14786669-223b-4a34-98e8-6cef54412420 commented 15 years ago

    py3k version

    14786669-223b-4a34-98e8-6cef54412420 commented 15 years ago

    current self tests cannot fully test the existence of the SNI extension as there is no server side support.

    This client script run with argument sni.velox.ch will show the "Great! Your client ....its ClientHello: sni.velox.ch" on the output.

    14786669-223b-4a34-98e8-6cef54412420 commented 15 years ago

    The small deficiency with these patches is that the specified server_hostname is almost always the hostname that is used in the socket pair of connect. Is it appropriate to grab the hostname value and use it in the SNI extension header?

    cde690a3-7412-4f89-bea2-524b3a6dbb2c commented 15 years ago

    (Sorry for dropping this, lost available time)

    I see your point. OTOH, use of SNI needs to be something that can be disabled and people need to be able to connect to host A while supplying host B, not necessarily using IP addresses for the specificity. Use- case example: someone has a service "www" hosted on ["www-1", "www-2", "www-3"]. They have an SSL certificate for "www" and they want to have a health-checker which probes for "working service, all certs valid and not about to expire".

    Unless s.connect() gains a keep_original_hostname=False option (?), this is hard to do.

    Then there's the principle of least surprise -- while it would be nice to get SNI working automatically for everyone, it's still plausible that amongst the various TLS servers out there are some which break horribly for SNI and upgrading Python shouldn't break the tools in use.

    So I tend towards favouring "make use of the newer, less well tested, protocol feature something that has to be explicitly enabled", even if it adds a line to boilerplate -- if SNI ever takes over the world (as Host: headers in HTTP have) then it can be defaulted on in future perhaps?

    In which case, if the default is to change, the API should be sorted now, so perhaps connect() should take an override_server_hostname=False flag, which will make it pass the connect() hostname parameter instead of self.server_hostname in the call to _ssl.sslwrap()? With checking for an IP address?

    14786669-223b-4a34-98e8-6cef54412420 commented 15 years ago

    Hey Phil,

    (Sorry for dropping this, lost available time) know the feeling :-(

    use of SNI needs to be something that can be disabled maybe. See small rational below:

    and people need to be able to connect to host A while supplying host B This seems to be a fringe case for usage and I seem to thing adding this would overly complicate the API. When testing hosts you would have all the names in DNS I'd assume.

    Unless s.connect() gains a keep_original_hostname=False option (?), this is hard to do. Is this starting to look too complicated?

    Options for client side SNI:

    1. wrapssl() - defaults to SNI enabled on SSL2/TLS1 connections (compile time/module level/object variable disable if really needed?)
    2. wrapssl(server_hostname=True/False) - enable SNI using the connect hostname (if domainname and not IP/socket)
    3. wrapssl(server_hostname=True/False/String) - True - enable SNI as above, False/None- don't use SNI or use hostname if a String.
    4. wrapssl(server-hostname=String)
    5. connect() should an override_server_hostname=False more?

    Then there's the principle of least surprise -- while it would be nice to get SNI working automatically for everyone, it's still plausible that amongst the various TLS servers out there are some which break horribly for SNI and upgrading Python shouldn't break the tools in use.

    Small rational to enable SNI by default:

    1. SNI probably won't break too much stuff. It was only in July 2009 that Apache-2.2.12 got officially released with proper SNI support. Patches existed before then however deployment was limited. mod_gnutls did implemented this earlier however I never thought this was widely used. Vista IE7 got SNI support in \~2005 (http://blogs.msdn.com/ie/archive/2005/10/22/483795.aspx) FF got SNI support in the 2.0 release (October 24, 2006, wikipedia) (https://bugzilla.mozilla.org/show_bug.cgi?id=116169#c26) The OpenSSL server side API for SNI required the registration of a callback to receive the SNI name. It is possible that there are some faulty implementations in this part of the server code. The default case that a TLS server doesn't account for SNI is very safe as it won't ever get a callback for it. The existence of wide spread client side support with limited server support hasn't broken the web.

    So I tend towards favouring "make use of the newer, less well tested, protocol feature something that has to be explicitly enabled", even if it adds a line to boilerplate -- if SNI ever takes over the world (as Host: headers in HTTP have) then it can be defaulted on in future perhaps?

    1. I think with p3k there is an opportunity to put something in that may break stuff. The release noted didn't guarantee stuff would work compatibly.

    2. supporting SNI clients by default may actually break less stuff that not supporting SNI client. e.g. Webhosting companies may embrace the SNI features of Apache for maximum IP utilization breaking the non-SNI aware clients (which won't be the majority of web browsers).

    With checking for an IP address? To be RFC compliant IP addresses shouldn't be used in SNI. Apart from socket family checking (AF_INET/AF_INET6) and doing a regex on the name I couldn't see an easy way to do this even looking at the low level socketmodule.c file. Maybe I need to look deeper. I could cheat and look at the Firefox crypto (NSS) code though.

    just my current thoughts

    cde690a3-7412-4f89-bea2-524b3a6dbb2c commented 15 years ago

    wrapssl(server_hostname=True/False/String) looks good to me.

    Your arguments for enabling by default are compelling, for P3k.

    pitrou commented 14 years ago

    Too late for 2.7 now, but looks like a good idea.

    pitrou commented 14 years ago

    The patch probably needs refreshing now that first SSL contexts are in.

    I wonder whether a combined boolean/string flag is really the best solution.

    I think we could instead enable SNI by default and add an optional "server_hostname" to set the hostname to SSLContext.wrap_socket(), so that people can explicitly set the hostname; and otherwise take it, if possible, from the argument given to connect().

    We can also add an "enable_sni" attribute to SSLContext (True by default) to allow selective disabling. This attribute would raise an exception if SNI support isn't available, which would be a way to test for it.

    8726d1eb-a365-45b6-b81d-c75988975e5a commented 14 years ago

    Here's another possible approach:

    ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
    ctx.set_tlsext_host_name("foo.bar")
    skt = ctx.wrap_socket(socket.socket())
    skt.connect("bar.baz")

    This makes it obvious what the SNI hostname is and what the TCP address to connect to is, and they can easily be different.

    14786669-223b-4a34-98e8-6cef54412420 commented 14 years ago

    msg106323 - Author: Antoine Pitrou (pitrou) Date: 2010-05-22 20:17

    I quite like your proposed alternative here. Not sure when/if I'll get to implement this.

    msg106324 - Author: Jean-Paul Calderone (exarkun) Date: 2010-05-22 22:17 Sorry I don't like this as much. I believe following the RFC for TLS SNI should be implicit and not something the programmer need to put effort into achieving. I acknowledge this approach does go against some explicit behaviour programming quality metrics.

    pitrou commented 14 years ago

    ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.set_tlsext_host_name("foo.bar")

    Well, the hostname should be specific to a connection, so I'm not sure it makes sense to set it on the context. (besides, the OpenSSL APIs only allow it to be set on the SSL structure)

    8726d1eb-a365-45b6-b81d-c75988975e5a commented 14 years ago

    Sorry I don't like this as much. I believe following the RFC for TLS SNI should be implicit and not something the programmer need to put effort into achieving. I acknowledge this approach does go against some explicit behaviour programming quality metrics.

    It's almost always wrong for Python to enforce a particular *policy, particularly in a very low level API (which is what the ssl module should be). Python's main job is to make it *possible to do things. It's the application developer's job to decide what things should be done.

    It would be entirely appropriate, though, for a higher-level interface (for example, the httplib module) to take care of this itself and not require users to explicitly specify things separately.

    Well, the hostname should be specific to a connection, so I'm not sure it makes sense to set it on the context.

    That doesn't make sense to me. For example, consider the case where you're talking to a web service. The hostname lookup might result in 10 A records, which you then drop into a connection pool. Your application doesn't care which server you talk to (and maybe it talks to serveral, or all, of them). But it does want to specify the same hostname for each.

    (besides, the OpenSSL APIs only allow it to be set on the SSL structure)

    Nope, I checked before making the suggestion. There's an SSLCTX version of this API (in addition to the SSL_ version).

    8726d1eb-a365-45b6-b81d-c75988975e5a commented 14 years ago

    Nope, I checked before making the suggestion. There's an SSLCTX version of this API (in addition to the SSL_ version).

    Sorry, I just checked again, and it seems you're right. Perhaps I saw SSL_CTX_set_tlsext_servername_callback and got the two confused.

    pitrou commented 14 years ago

    Python's main job is to make it *possible* to do things. It's the application developer's job to decide what things should be done.

    It would be entirely appropriate, though, for a higher-level interface (for example, the httplib module) to take care of this itself and not require users to explicitly specify things separately.

    Ok, I find this argument rather convincing. Also, enabling implicit SNI with the connect() argument could make user code stop working if he decides to pass the IP instead, without him being able to diagnose precisly what happens.

    As you said, httplib/urllib should probably enable client-side SNI by default.

    pitrou commented 13 years ago

    Here is a patch for py3k, including http.client and urllib support.

    pitrou commented 13 years ago

    Committed with docs in r85793.

    1dec85a9-f5af-41c7-ad0a-45b93f4b7710 commented 13 years ago

    I see the patch has been applied python3 in r85793, but is there any chance there will also be patches for python 2.6 or 2.7? And if so, what release of python (any version) might this patch be included in?

    pitrou commented 13 years ago

    I see the patch has been applied python3 in r85793, but is there any chance there will also be patches for python 2.6 or 2.7

    No, Python 2 only receives bug fixes.

    1dec85a9-f5af-41c7-ad0a-45b93f4b7710 commented 13 years ago

    And python3? Any idea which version the patch will be included there? This might be a good reason to finally take action on migrating my code from python 2.7 to python 3.

    On 11 August 2011 18:49, Antoine Pitrou \report@bugs.python.org\ wrote:

    Antoine Pitrou \pitrou@free.fr\ added the comment:

    > I see the patch has been applied python3 in r85793, but is there any > chance there will also be patches for python 2.6 or 2.7

    No, Python 2 only receives bug fixes.

    ----------


    Python tracker \report@bugs.python.org\ \http://bugs.python.org/issue5639\


    pitrou commented 13 years ago

    And python3? Any idea which version the patch will be included there?

    It was included in Python 3.2.

    778797f2-90c5-4c97-ba8b-e336243e7c90 commented 11 years ago

    Python 2.7 is still used in production.

    Given the scarcity of IPv4-addresses — and with CDNs (think: Amazon, Akamai, EdgeCast…) starting to offer HTTP+SSL — the need for SNI arises in order to avoid pitfalls such as shared certificates.

    The lack of ubiquitous support for TLS SNI could cause delays in HTTPS-everywhere–deployments. Therefore, in the light of the latest revelations about mass surveillance, I'd like even to argue that this is a matter of security and privacy.

    pitrou commented 11 years ago

    Mark, thanks for the patch. However, unless exceptional situations, we don't backport features to bugfix branches. The next Python 2.7 version will probably not be released before 2014, so even if your patch were integrated, widespread deployment would still be delayed significantly. By contrast, Python 3.2 has SNI support and it was released in February 2011.

    778797f2-90c5-4c97-ba8b-e336243e7c90 commented 11 years ago

    Antoine, thank you for the heads-up. As long as I've reminded distribution maintainers of this issue and this or a similar patch (always send a server_hostname with TLS, if one is missing) will be integrated (please do!) I've accomplished my goal.

    BTW, today I've encountered a similar certificate. Semper aliquid haeret:

    subjectAltName=DNS:cdn.cloudtop.org,DNS:barely-legal-spam.com,DNS:*.banging-ham.com,DNS:jimmyforcongress2014.com ;-)
    1e3445ee-1357-417d-a204-0863cf864f91 commented 10 years ago

    Is this really not going into Python2 series?

    It's not a Python feature or a language feature, it's a matter of exporting OpenSSL feature.

    Furthermore it's a matter of security, same as support for session tickets is a matter of performance.

    SNI was first introduced in 2004, RFC in 2006, libcurl supported it from 2008, you had a patch since 2009 and still it's not in?

    Are you guys intentionally trying to cripple Python2?

    What do you think is likelier outcome, faster Python3 adoption or Python labelled insecure?

    pitrou commented 10 years ago

    It's not a Python feature or a language feature, it's a matter of exporting OpenSSL feature.

    It's a feature regardless (from our POV), and Python 2.x has been in bug fix mode for a long time now. Please understand that this is how our release process works.

    4593697b-ec62-482c-a3e8-80fd64349535 commented 10 years ago

    This is *not* a feature request, it's a bug fix in the underlying protocols.

    Client sides that do not send SNI are actively hurting the Web and the Internet by constraining the deployment of TLS.

    The closest analogy would be if Python's HTTP client side didn't emit a Host header, and the excuse were "But we only advertise ourselves as HTTP/1.0." The biggest difference being that this has additional security impact.

    The pain of lack of support for SNI is completely borne by the server-side, not the client (here, Python). As such, this is not a feature for Python client-side developers, but an interop / scaling / security issue for the Web and Internet overall.

    02fd638a-ffde-44d3-b9e8-0dc931d5d308 commented 10 years ago

    Are you kidding me? I can't believe SNI isn't being backported to python 2.x. This is ridiculous in my opinion. The bug fix needs to be back ported.

    ncoghlan commented 10 years ago

    I'd be happy to add a disclaimer to the Python 2.7 docs directing users to use the requests module instead (https://pypi.python.org/pypi/requests).

    People *really *really *really* should be using requests on the client side when doing HTTP/HTTPS in Python 2.x - the standard library support is now too old to have kept up with the evolution of web security and standard.

    dstufft commented 10 years ago

    To be clear, to get SNI with requests on 2.x you need requests, pyopenssl, ndg-httpsclient, and pyasn1 (which also pulls in cryptography, six, cffi, and pycparser). So that's 8 dependencies to get SNI on Python 2.x.

    At least it's doable but it's kind of really unfriendly :/ Also the error message you get when you need SNI and it's not available is basically the most obtuse thing ever. You get told that the SSL verification failed for \<some other domain> that isn't what you asked for but when you go to it in your Browser it'll all work kosher with no indication it was using SNI.

    It's generally a good idea to install those extra things anyways because the SSL lib on Python 2.7 has other actual security issues which those address (IIRC it still has TLS compression on, I think it's default cipher list is rather poor, doesn't support TLS 1.2, etc).

    1e3445ee-1357-417d-a204-0863cf864f91 commented 10 years ago

    +dstufft is absolutely right.

    SNI needs to be enabled on lower level than "user" python code. if it is, requests and most other http client libs get it for free without dependencies.

    pitrou commented 10 years ago

    Nick: rather than direct users to use requests, we should direct them to use Python 3, which has had SNI support for 3+ years now.

    If client programs choose to remain on Python 2, it's *their* fault, not Python's.

    1e3445ee-1357-417d-a204-0863cf864f91 commented 10 years ago

    Antoine, was Python 2.x a mistake?

    I don't think so.

    SNI is not a language feature, it's not even a python extension feature. It's a feature of and existing protocol and the underlying library.

    pitrou commented 10 years ago

    Antoine, was Python 2.x a mistake?

    Really, can you stop arguing about this? If you want to know what Python considers features and bug fixes, then get acquainted with the development process instead of bickering.

    ncoghlan commented 10 years ago

    I'm currently discussing some options with Donald and Christian. While it's annoying that a developer from a certain large corporate user of Python (a director of the PSF, no less) is whining at volunteers on the internet instead of actually helping by encouraging their employer or the board to help fund the creation and publication of an up to date TLS module for Python 2, griping about the endemic problem of corporate users taking community developed software for granted won't make the underlying problem go away: an unfortunate amount of Python code is currently improperly secured because it is using outdated SSL support, and there isn't currently a good alternative available for users that aren't in a position to immediately migrate to Python 3.

    pitrou commented 10 years ago

    I'm missing some context to appreciate your message, Nick, but please note that SNI is not in itself a security feature. It just enables interoperability with TLS virtual hosts (aka. hosting several TLS-enabled domains behind a single IP and port).

    dstufft commented 10 years ago

    It's somewhat of a grey area of security feature. It's not directly a security feature but if you don't have SNI and you hit a site that requires it then your error message is going to be something like what people run into with PyPI[1] which is "Cannot verify pypi.python.org, does not match hostname *.a.ssl.fastly.net". At this point most people go "What?" and assume the site is at fault and disable verification. Even more frustrating is this is going to work fine in their browser. The answer of how to actually verify this is without SNI is (once you even figure out the problem is SNI, which is non obvious) verify against what's actually in the CN of the cert, and send a Host header for what site you actually want. So while it is not strictly a security feature, it is fairly important for reasonably securely connecting to a site that requires SNI for the lay person.

    [1] PyPI's problem is no SNI but that some clients don't support SAN certificates, but the error message is exactly the same.

    pitrou commented 10 years ago

    Understood, but that's no different from trying to connect with an old Windows or MSIE version (which I'm sure will also fail on some websites).

    Client-side SNI support has been added in Python 3.2, and 3.4 is now out. People who migrated their code to Python 3 have been enjoying SNI support for years now, and they're gradually getting more TLS features at every new feature release.

    vstinner commented 10 years ago

    Please discuss the Python 2 documentation in a new issue, this one is now closed and so hidden from the list of bugs.

    1e3445ee-1357-417d-a204-0863cf864f91 commented 10 years ago

    Hopefully pep-466 resolves this for 2.x series.