Issue 25672: set SSL_MODE_RELEASE_BUFFERS - Python tracker (original) (raw)
Created on 2015-11-19 18:17 by Lukasa, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Messages (11)
Author: Cory Benfield (Lukasa) *
Date: 2015-11-19 18:17
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.
Author: Cory Benfield (Lukasa) *
Date: 2015-11-19 18:20
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
Author: Cory Benfield (Lukasa) *
Date: 2015-11-19 19:34
Ok, I've just uploaded an initial draft of the patch for review.
Author: Benjamin Peterson (benjamin.peterson) * 
Date: 2015-11-20 07:37
It might be better to do a runtime OpenSSL version check in case someone upgrades or downgrades to an vulnerable version without recompiling Python.
Author: Cory Benfield (Lukasa) *
Date: 2015-11-20 08:59
Good idea Benjamin. I've uploaded a second patch that adjusts the check to be a runtime one, rather than a compiled one.
Author: Marc-Andre Lemburg (lemburg) *
Date: 2015-11-20 09:12
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
Author: Cory Benfield (Lukasa) *
Date: 2015-11-20 11:09
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.
Author: Marc-Andre Lemburg (lemburg) *
Date: 2015-11-20 11:51
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
Author: Brett Cannon (brett.cannon) *
Date: 2016-01-06 17:43
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.
Author: Marc-Andre Lemburg (lemburg) *
Date: 2016-01-06 18:14
Thanks, Brett. I'm too busy with other things at the moment.
Author: Roundup Robot (python-dev) 
Date: 2016-01-08 05:39
New changeset b5b0394ed20b by Benjamin Peterson in branch 'default': merge 3.5 (closes #25672) https://hg.python.org/cpython/rev/b5b0394ed20b
History
Date
User
Action
Args
2022-04-11 14:58:24
admin
set
github: 69858
2016-01-08 05:39:16
python-dev
set
status: open -> closed
nosy: + python-dev
messages: +
resolution: fixed
stage: commit review -> resolved
2016-01-06 18:14:07
lemburg
set
assignee: brett.cannon
messages: +
2016-01-06 17:43:04
brett.cannon
set
nosy: + brett.cannon
messages: +
stage: commit review
2015-11-20 11:51:36
lemburg
set
messages: +
2015-11-20 11:10:00
Lukasa
set
files: + ssl3.patch
messages: +
2015-11-20 09:12:51
lemburg
set
nosy: + lemburg
messages: +
2015-11-20 08:59:33
Lukasa
set
files: + ssl2.patch
messages: +
2015-11-20 07:37:40
benjamin.peterson
set
nosy: + benjamin.peterson
messages: +
2015-11-19 19:34:08
Lukasa
set
files: + ssl.patch
keywords: + patch
messages: +
2015-11-19 18:22:20
ethan.furman
set
nosy: + janssen, pitrou, giampaolo.rodola, christian.heimes, alex, dstufft
title: Unconditionally set SSL_MODE_RELEASE_BUFFERS -> set SSL_MODE_RELEASE_BUFFERS
2015-11-19 18:20:06
Lukasa
set
messages: +
2015-11-19 18:17:07
Lukasa
create