set SSL_MODE_RELEASE_BUFFERS

[Lukasa]

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]

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]

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

[Lukasa]

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

[benjaminp]

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]

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

[malemburg]

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]

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]

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]

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]

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

[python-dev]

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