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.

Sign up for GitHub

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

set SSL_MODE_RELEASE_BUFFERS #69858

Closed
Lukasa mannequin opened this issue Nov 19, 2015 · 11 comments
Closed

set SSL_MODE_RELEASE_BUFFERS #69858

Lukasa mannequin opened this issue Nov 19, 2015 · 11 comments
Assignees
@brettcannon
Labels
stdlib Python modules in the Lib dir

Comments

@Lukasa
Copy link
Mannequin

Lukasa mannequin commented Nov 19, 2015

BPO 25672
Nosy @malemburg, @brettcannon, @pitrou, @giampaolo, @tiran, @benjaminp, @alex, @dstufft, @Lukasa
Files
  • ssl.patch
  • ssl2.patch: Second patch
  • ssl3.patch: Third 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 = 'https://github.com/brettcannon'
closed_at = <Date 2016-01-08.05:39:16.082>
created_at = <Date 2015-11-19.18:17:07.013>
labels = ['library']
title = 'set SSL_MODE_RELEASE_BUFFERS'
updated_at = <Date 2016-01-08.05:39:16.080>
user = 'https://github.com/Lukasa'

    bugs.python.org fields:

    activity = <Date 2016-01-08.05:39:16.080>
actor = 'python-dev'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2016-01-08.05:39:16.082>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2015-11-19.18:17:07.013>
creator = 'Lukasa'
dependencies = []
files = ['41083', '41091', '41094']
hgrepos = []
issue_num = 25672
keywords = ['patch']
message_count = 11.0
messages = ['254918', '254919', '254927', '254953', '254960', '254961', '254966', '254971', '257619', '257626', '257737']
nosy_count = 11.0
nosy_names = ['lemburg', 'brett.cannon', 'janssen', 'pitrou', 'giampaolo.rodola', 'christian.heimes', 'benjamin.peterson', 'alex', 'python-dev', 'dstufft', 'Lukasa']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25672'
versions = ['Python 3.6']
    @Lukasa
    Copy link
    Mannequin Author

    Lukasa mannequin commented Nov 19, 2015

    Originally raised by Ben Bangert on the python-dev mailing list.

    It turns out that OpenSSL has a mode setting, SSL_MODE_RELEASE_BUFFERS, that can be set by a call to SSK_CTX_set_mode. This mode can potentially reduce connection overhead by nearly 18kB *per connection*, a reduction of something like 60%. Further, this does not change the behaviour of OpenSSL in any meaningful way.

    For this reason, we should unconditionally set this mode on all SSL Context objects we create.

    I'm happy to submit a patch to the standard library that will do this.

    @Lukasa Lukasa mannequin added the stdlib Python modules in the Lib dir label Nov 19, 2015
    @Lukasa
    Copy link
    Mannequin Author

    Lukasa mannequin commented Nov 19, 2015

    Oh, one further requirement: we should *not* set this mode for OpenSSL releases 1.x through 1.0.1g, which have a NULL pointer dereference vulnerability (CVE 2014-0198). Thanks to Marc-Andre Lemburg for spotting this.

    See also: https://www.rapid7.com/db/vulnerabilities/http-openssl-cve-2014-0198

    @ethanfurman ethanfurman changed the title Unconditionally set SSL_MODE_RELEASE_BUFFERS set SSL_MODE_RELEASE_BUFFERS Nov 19, 2015
    @Lukasa
    Copy link
    Mannequin Author

    Lukasa mannequin commented Nov 19, 2015

    Ok, I've just uploaded an initial draft of the patch for review.

    @benjaminp
    Copy link
    Contributor

    It might be better to do a runtime OpenSSL version check in case someone upgrades or downgrades to an vulnerable version without recompiling Python.

    @Lukasa
    Copy link
    Mannequin Author

    Lukasa mannequin commented Nov 20, 2015

    Good idea Benjamin. I've uploaded a second patch that adjusts the check to be a runtime one, rather than a compiled one.

    @malemburg
    Copy link
    Member

    The release buffer mode bugs were fixed in 1.0.0m and 1.0.1h:

    https://openssl.org/news/vulnerabilities.html#y2014

    CVE-2014-0198 (OpenSSL advisory) 21st April 2014:
    A flaw in the do_ssl3_write function can allow remote attackers to cause a denial of service via a NULL pointer dereference. This flaw only affects OpenSSL 1.0.0 and 1.0.1 where SSL_MODE_RELEASE_BUFFERS is enabled, which is not the default and not common.

        Fixed in OpenSSL 1.0.1h (Affected 1.0.1g, 1.0.1f, 1.0.1e, 1.0.1d, 1.0.1c, 1.0.1b, 1.0.1a, 1.0.1)
    Fixed in OpenSSL 1.0.0m (Affected 1.0.0l, 1.0.0k, 1.0.0j, 1.0.0i, 1.0.0g, 1.0.0f, 1.0.0e, 1.0.0d, 1.0.0c, 1.0.0b, 1.0.0a, 1.0.0)

    CVE-2010-5298 (OpenSSL advisory) 8th April 2014:
    A race condition in the ssl3_read_bytes function can allow remote attackers to inject data across sessions or cause a denial of service. This flaw only affects multithreaded applications using OpenSSL 1.0.0 and 1.0.1, where SSL_MODE_RELEASE_BUFFERS is enabled, which is not the default and not common.

        Fixed in OpenSSL 1.0.1h (Affected 1.0.1g, 1.0.1f, 1.0.1e, 1.0.1d, 1.0.1c, 1.0.1b, 1.0.1a, 1.0.1)
    Fixed in OpenSSL 1.0.0m (Affected 1.0.0l, 1.0.0k, 1.0.0j, 1.0.0i, 1.0.0g, 1.0.0f, 1.0.0e, 1.0.0d, 1.0.0c, 1.0.0b, 1.0.0a, 1.0.0)

    PS: OpenSSL normally doesn't issue betas. All their releases are final. The numbering scheme is a bit weird - perhaps they'll change to a more common one with 1.1 (this will have a beta cycle): https://openssl.org/policies/releasestrat.html

    @Lukasa
    Copy link
    Mannequin Author

    Lukasa mannequin commented Nov 20, 2015

    Thanks for the updated info Marc-Andre.

    Yeah, while generally speaking OpenSSL doesn't ship betas, it does provide them as tarballs. I have a beta of 1.0.2 floating around somewhere on my machine that I was using for ALPN testing back in 2014, and so I can speak from personal experience and say that people do actually work with betas sometimes. On this issue (defending ourselves from a CVE) my instinct is to be conservative. However, we should allow later patch releases of OpenSSL 1.0.0 to have this optimisation if they're safe.

    Therefore, I've uploaded a new patch that does allow for 1.0.0m and later to use this optimisation too. It makes the conditional a little more complex, but c'est la vie.

    @malemburg
    Copy link
    Member

    On 20.11.2015 12:10, Cory Benfield wrote:

    Yeah, while generally speaking OpenSSL doesn't ship betas, it does provide them as tarballs. I have a beta of 1.0.2 floating around somewhere on my machine that I was using for ALPN testing back in 2014, and so I can speak from personal experience and say that people do actually work with betas sometimes. On this issue (defending ourselves from a CVE) my instinct is to be conservative. However, we should allow later patch releases of OpenSSL 1.0.0 to have this optimisation if they're safe.

    Ah, right. For new major release versions such as 1.0.1 or 1.0.2
    they do ship betas, but historically they have often introduced
    new features in their abcde... level releases without doing
    betas for those first - that's what I was thinking of :-)

    Therefore, I've uploaded a new patch that does allow for 1.0.0m and later to use this optimisation too. It makes the conditional a little more complex, but c'est la vie.

    LGTM

    Thanks,

    Marc-Andre Lemburg
    eGenix.com

    @brettcannon
    Copy link
    Member

    I assume this can be checked in, MAL? If you need someone to do it for you, feel free to assign it to me and I can do it when I have a chance.

    @malemburg
    Copy link
    Member

    Thanks, Brett. I'm too busy with other things at the moment.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 8, 2016

    New changeset b5b0394ed20b by Benjamin Peterson in branch 'default':
    merge 3.5 (closes bpo-25672)
    https://hg.python.org/cpython/rev/b5b0394ed20b

    @python-dev python-dev mannequin closed this as completed Jan 8, 2016
    @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

    @brettcannon brettcannon

    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @brettcannon @benjaminp @malemburg