classification
Title: set SSL_MODE_RELEASE_BUFFERS
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Lukasa, alex, benjamin.peterson, brett.cannon, christian.heimes, dstufft, giampaolo.rodola, janssen, lemburg, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2015-11-19 18:17 by Lukasa, last changed 2016-01-08 05:39 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
ssl.patch Lukasa, 2015-11-19 19:34 review
ssl2.patch Lukasa, 2015-11-20 08:59 Second patch review
ssl3.patch Lukasa, 2015-11-20 11:09 Third patch review
Messages (11)
msg254918 - (view) 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.
msg254919 - (view) 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
msg254927 - (view) Author: Cory Benfield (Lukasa) * Date: 2015-11-19 19:34
Ok, I've just uploaded an initial draft of the patch for review.
msg254953 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) 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.
msg254960 - (view) 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.
msg254961 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) 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
msg254966 - (view) 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.
msg254971 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) 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
msg257619 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) 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.
msg257626 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2016-01-06 18:14
Thanks, Brett. I'm too busy with other things at the moment.
msg257737 - (view) Author: Roundup Robot (python-dev) (Python triager) 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
2016-01-08 05:39:16python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg257737

resolution: fixed
stage: commit review -> resolved
2016-01-06 18:14:07lemburgsetassignee: brett.cannon
messages: + msg257626
2016-01-06 17:43:04brett.cannonsetnosy: + brett.cannon

messages: + msg257619
stage: commit review
2015-11-20 11:51:36lemburgsetmessages: + msg254971
2015-11-20 11:10:00Lukasasetfiles: + ssl3.patch

messages: + msg254966
2015-11-20 09:12:51lemburgsetnosy: + lemburg
messages: + msg254961
2015-11-20 08:59:33Lukasasetfiles: + ssl2.patch

messages: + msg254960
2015-11-20 07:37:40benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg254953
2015-11-19 19:34:08Lukasasetfiles: + ssl.patch
keywords: + patch
messages: + msg254927
2015-11-19 18:22:20ethan.furmansetnosy: + 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:06Lukasasetmessages: + msg254919
2015-11-19 18:17:07Lukasacreate