Giter Club home page Giter Club logo

Comments (14)

dstufft avatar dstufft commented on July 20, 2024

So I think this is a bad idea.

Essentially it boils down to false positives. Now you've correctly identified that this will have a false positive and mentioned that Debian and OpenSSL will simply patch their pyOpenSSL to make it build correctly. However what you've failed to account for is that a lot of people install from PyPI using either easy_install or pip. In fact pyOpenSSL has been downloaded 157,518 times in the past month from PyPI, which we can assume a significant portion of that will be by installers. The proposed change will basically break pip and easy_install completely for all of the supported Linux OSs in the Debian family, Also the Fedora and RHEL/CentOS 6.5 family. It's essentially going to break things for a ton of people.

I also think that this bug was particularly bad, but I think it's also getting an unreasonable amount of attention. There are other bugs in the stack that affect pyOpenSSL (Python bugs, Kernel bugs, OpenSSL bugs), should pyOpenSSL attempt to block all access to these systems as well? Should it attempt to maintain a list of every security related bug and reject them? This quickly becomes an unreasonable problem (as you can guess!). However it's not really clear where you would draw the line if you would draw it. Certainly some kernel bugs are as bad as or worse than this bug. Python has had things that would allow an RCE too.

By pushing this off to the system it's a clear signal that you have to do your due diligence for your entire system, whereas pyOpenSSL implementing these checks creates a false sense of security that pyOpenSSL is going to audit your system for you when in reality it's only going to Audit for one specific bug that happened to get a whole lot of press.

from pyopenssl.

zookoatleastauthoritycom avatar zookoatleastauthoritycom commented on July 20, 2024

Oh, I see. I had neglected the fact that the proposed release would result in a lot of people who are not packagers-of-software building the resulting version on a platform (e.g. Debian/Ubuntu/etc.) which has patched the vuln without explicitly so indicating.

Dammit. I withdraw my suggestion.

Some of your other objections are off-the-mark, but that's probably not important for this ticket. Heartbleed is a critical vuln for Tahoe-LAFS users, and it is I think the only critical vuln that I can think of in the 7 years of Tahoe-LAFS's life, in the entire stack of Python and userspace C code that we rely on. Maybe there has been an RCE in Python during that time, too, but I don't remember. Anyway, I believe that for Linux kernel issues, Python issues, etc., we can rely on other social and technical infrastructure to patch it, but that for heartbleed we can't, and our users will continue to be vulnerable to it for years to come unless we figure out some kind of workaround. ☹

from pyopenssl.

dstufft avatar dstufft commented on July 20, 2024

Well I didn't mean for Tahoe-LAFS, but for users of pyOpenSSL which have a much more general audience where failing in OpenSSL (or other parts of code) are not as well fenced off.

from pyopenssl.

daira avatar daira commented on July 20, 2024

pyOpenSSL 13.1 and 14.1 could completely avoid false positives, by checking the behaviour of tls1_process_heartbeat. Also, the policy described at https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2215#comment:4 does not only cover heartbleed, it takes account of other critical OpenSSL bugs as well.

I agree that the effect on installs via pip and easy_install needs to be carefully considered. The issue here is not really false positives, though. The issue is true positives: packages that didn't explicitly depend on 13.1 or 14.1, but end up using 14.1 just because it's a more recent version, and therefore have the OpenSSL version check applied when the depending package metadata didn't ask for it. As it happens, there's a very good chance that those installs will break anyway unless libffi is installed.

Remember that:

  • heartbleed is being actively exploited
  • there's been little or no attempt to convince end-users who are not running servers to upgrade; there's been a little bit of media attention on so-called "reverse heartbleed" but in practice, clients aren't going to be upgraded for years unless we do something to force it.

from pyopenssl.

daira avatar daira commented on July 20, 2024

Zooko wrote: "Heartbleed is a critical vuln for Tahoe-LAFS users, and it is I think the only critical vuln that I can think of in the 7 years of Tahoe-LAFS's life, in the entire stack of Python and userspace C code that we rely on. Maybe there has been an RCE in Python during that time, too, but I don't remember."

There has, and we responded to it in the same way, by refusing to run on vulnerable Python versions.

from pyopenssl.

dstufft avatar dstufft commented on July 20, 2024

So if the detection can be changed from "Check if it's a vulnerable version string" to "Actually detect if it's vulnerable", remembering that a lot of Linux distros backported the fix and didn't bump version numbers or didn't disable heartbeats, then my major concern with doing this is gone and the question simply becomes, "Should pyOpenSSL attempt to protect it's users (and the users of things that depend on it) from a Heartbleed vulnerable system". I don't have an answer to that and that's probably something @exarkun would need to answer.

from pyopenssl.

exarkun avatar exarkun commented on July 20, 2024

First, let me thank everyone for contributing their thoughts and some great information on this issue.

The purpose of pyOpenSSL is to assist with the development of Python software which can be operated in a secure manner. So far as it is possible for pyOpenSSL to protect anyone using software based on it from the heartbleed vulnerability, I am in favor of it doing so.

Therefore, in principal, I am in favor of making changes to pyOpenSSL to protect end-users against this vulnerability (to which they are exposed due to the software that pyOpenSSL is directly built upon).

In practice I'm still not clear on how exactly to do that in this case. @daira mentioned "tls1_process_heartbeat". I see that this is an API related to OpenSSL's implementation of the heartbeat functionality but it's not obvious to me what check pyOpenSSL can perform to determine if the library version is vulnerable or not. I do see a OPENSSL_NO_HEARTBEATS symbol which supposedly exists but offhand (ie, with no research whatsoever) I am suspicious of the efficacy of any solution based only on a check against this symbol (due to the shennanigans I have frequently seen committed against OPENSSL_ feature-marker macros).

The second practical concern is that the pyOpenSSL project's ability to build Windows binaries has essentially been lost at this point (indeed, this was a large motivating factor for the switch to cffi). I am unsure what the consequences would be if there were to be a pyOpenSSL 0.14.1 release which was effectively only made available to non-Windows users (particularly given that Windows users are most likely to be affected due to the lack of package management software as well as the fact that there is no organization that has claimed responsibility for notifying Windows OpenSSL users of necessary security updates). This is not to say I think the proposed 0.14.1 is useless without Windows binaries: it is to say just what I said - I am unsure what the consequences would be. Given a large chunk of time I could probably arrange to build new binary packages for Windows but I can say that, unfortunately, it's very unlikely that I will actually be able to arrange to have such a chunk of time.

If it is possible to reliably, correctly detect whether the OpenSSL library in use is vulnerable or not (and someone points me to a reference explaining this and perhaps offers assistance in devising a testing strategy) and if someone I can trust (I feel it would be irresponsible for me to allow a well-meaning but otherwise unknown to me volunteer from the community to put pyOpenSSL binaries onto PyPI) wants to volunteer to build Windows binaries then I'd be happy to see this effort go forward.

from pyopenssl.

zooko avatar zooko commented on July 20, 2024

LeastAuthority.com is willing to contribute time and even money (if it would help) to make this happen. The reason we can contribute money is that we received a generous grant from OpenITP (https://openitp.org/) to improve Tahoe-LAFS packaging on Windows and Macintosh, and fixing this problem is a necessary step to that.

In our proposal to OpenITP we specified that we would do all of our work in a way that is as reproducible and transparent as possible, and specifically that we would set up a buildbot to publicly and reproducibly build Windows packages of pyOpenSSL instead of building packages manually.

Note: at this moment we may not actually have time to contribute, although it is possible we can squeeze it into our weekends and evenings. ☺ Also we can perhaps juggle our other time-obligations to get time. Also if we could find someone else who could do the necessary work in return for money, and then we could give them some of the money from the OpenITP grant.

from pyopenssl.

daira avatar daira commented on July 20, 2024

Hmm, so OpenSSL._util.lib.tls1_process_heartbeat can't be called directly because tls1_process_heartbeat isn't exported (at least on Linux). It is called by ssl3_read_bytes ( http://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=ssl/s3_pkt.c;h=b9e45c74bc5c8c04cc7c2e023eb484162491e362;hb=HEAD#l1368 ) which is exported, but it's necessary to first simulate sending a ClientHello so that the SSL argument is in the right state. This is a bit trickier than I thought, although I believe it's still possible.

from pyopenssl.

daira avatar daira commented on July 20, 2024

This seems to work as a proof-of-concept, but I'd like to do more testing and find a more robust way to check the server response, rather than just looking at its length:

#!/usr/bin/python

# As simple as possible, but no simpler.
CLIENT_HELLO = (
  '\x16'                 # Handshake
  '\x03\x02'             # TLS version 1.1
  '\x00\x34'             # length of ClientHello
  '\x01'                 #   Handshake type (ClientHello)
  '\x00\x00\x30'         #   length
  '\x03\x02'             #     TLS version 1.1
  '\x53\x43\x5b\x90'     #     timestamp
  '\x9d\x9b\x72\x0b\xbc\x0c\xbc\x2b\x92\xa8\x48\x97\xcf\xbd'
  '\x39\x04\xcc\x16\x0a\x85\x03\x90\x9f\x77\x04\x33\xd4\xde' # client random
  '\x00'                 #     length of session ID (not resuming session)
  '\x00\x02'             #     length of ciphersuites
  '\x00\x0a'             #       TLS_RSA_WITH_3DES_EDE_CBC_SHA
  '\x01'                 #     length of compression methods
  '\x00'                 #       null compression
  '\x00\x05'             #     length of extensions
  '\x00\x0f\x00\x01\x01' #       heartbeat extension
)

HEARTBEAT = (
  '\x18'                 # Heartbeat
  '\x03\x02'             # TLS version 1.1
  '\x00\x03'             # length
  '\x01'                 #   heartbeat request
  '\x40\x00'             #   payload length (16384 bytes)
)

def verify_callback(connection, x509, errnum, errdepth, ok):
    return ok

def is_vulnerable(SSL):
    ctx = SSL.Context(SSL.TLSv1_1_METHOD)
    ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3)
    ctx.use_certificate_file('test.crt')
    ctx.use_privatekey_file('test.key')
    ctx.set_cipher_list('DES-CBC3-SHA') # TLS_RSA_WITH_3DES_EDE_CBC_SHA
    ctx.set_verify(SSL.VERIFY_NONE, verify_callback)

    server = SSL.Connection(ctx, None)
    server.set_accept_state()
    server.bio_write(CLIENT_HELLO + HEARTBEAT)

    server_response = bytearray()
    try:
        server.do_handshake()
    except SSL.WantReadError:
        pass # this is expected
    while True:
        try:
            server_response += server.bio_read(32768)
        except SSL.WantReadError:
            break
    #print len(server_response)
    # TODO: make this more reliable
    return len(server_response) > 16384


from OpenSSL import SSL
print is_vulnerable(SSL)

from pyopenssl.

daira avatar daira commented on July 20, 2024

Oh, this relies on a dummy key+cert generated by

openssl req -new -x509 -nodes -out test.crt -keyout test.key

(which is ugly, I know).

from pyopenssl.

daira avatar daira commented on July 20, 2024

The advantage of the above code using only the public API (rather than directly calling the OpenSSL bindings) is that it should work with pyOpenSSL either pre- or post-0.14.

from pyopenssl.

zooko avatar zooko commented on July 20, 2024

Work in progress: https://github.com/tahoe-lafs/tahoe-lafs/commits/2215-refuse-vulnerable-openssl-2

from pyopenssl.

alex avatar alex commented on July 20, 2024

People upgraded their OpenSSLs in the end.

from pyopenssl.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.