Disable TLS Compression

[dstufft]

BPO	20994
Nosy	@birkenfeld, @ncoghlan, @pitrou, @tiran, @alex, @dstufft
Superseder	bpo-28043: Sane defaults for SSLContext options and ciphers
Files	disable-ssl-compression-default.diff disable-ssl-compression-2.7.diff

^{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/tiran'
closed_at = <Date 2017-09-06.00:55:56.849>
created_at = <Date 2014-03-20.13:53:36.291>
labels = ['type-security', 'expert-SSL']
title = 'Disable TLS Compression'
updated_at = <Date 2017-09-06.00:55:56.848>
user = 'https://github.com/dstufft'

bugs.python.org fields:

activity = <Date 2017-09-06.00:55:56.848>
actor = 'christian.heimes'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2017-09-06.00:55:56.849>
closer = 'christian.heimes'
components = ['SSL']
creation = <Date 2014-03-20.13:53:36.291>
creator = 'dstufft'
dependencies = []
files = ['34531', '34532']
hgrepos = []
issue_num = 20994
keywords = ['patch']
message_count = 14.0
messages = ['214234', '214235', '214237', '214238', '214246', '214248', '214254', '214258', '225884', '225885', '227924', '275037', '276525', '301420']
nosy_count = 8.0
nosy_names = ['georg.brandl', 'ncoghlan', 'pitrou', 'christian.heimes', 'Arfrever', 'alex', 'Alex.Stapleton', 'dstufft']
pr_nums = []
priority = 'high'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '28043'
type = 'security'
url = 'https://bugs.python.org/issue20994'
versions = ['Python 2.7', 'Python 3.5']

[dstufft]

Since Python 3.3 the ssl module has supported the ability to opt in to disabling TLS Compression [1]. However TLS Compression has the problem that it typically leaks data through an attack known as CRIME. CRIME is specific to HTTP but the type of attack it employs is not.

I believe that CPython should just flat out disable TLS Compression and it should do so in all currently active branches (2.7, 3.2+). The patch is fairly minor however there is the question of how that should be handled in 3.3+ where there would be a now useless flag and method on SSLContext. The likelhood for breakage is fairly low and all modern browsers have already permanently disabled it.

[1] http://bugs.python.org/issue13634

[pitrou]

It would probably be sufficient to add OP_NO_COMPRESSION to OP_ALL.

[dstufft]

Ah, I hadn't noticed the OP_ALL thing, in 3.3+ adding OP_NO_COMPRESSION to OP_ALL would be reasonable. That would disable TLS Compression by default, still provide people the ability to disable TLS Compression if they don't use OP_ALL, and provide a way to enable it if they want it.

Do you think it'd be OK to just disable TLS Compression in 2.7 and 3.2 without the option to turn it back on? I think that would be fine personally.

[pitrou]

Do you think it'd be OK to just disable TLS Compression in 2.7 and 3.2
without the option to turn it back on? I think that would be fine
personally.

I'm not enough of a TLS expert, but it sounds ok.

[AlexStapleton]

CRIME is not universally applicable to all TLS connections and it requires some cooperation from the application to work. In fact for a Python TLS client it seems quite unlikely for an application to be vulnerable. The attack in the paper leverages an insecure website to inject JavaScript to issue crafted requests to a secure one. i.e. It requires both compression and some degree of remote code execution to work. Perhaps there are ways to extend the attack to apply to more common Python TLS client usage though?

Also some users will absolutely want to manually re-enable compression, please don't disable it entirely.

[dstufft]

To be specific it doesn't require any remote code execution to work, it just requires you to be able to influence the content of the responses that the client is receiving.

[dstufft]

This is a simple patch, it simple disables TLS Compression by default. If a user wants to add it back they can create their own SSLContext and do

ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
ctx.options &= ~ssl.OP_NO_COMPRESSION

This should be able to apply against 3.2+ although it would only be 3.3+ that ssl.OP_NO_COMPRESSION is available to disable it, although a user could still hard code the constant in themselves.

This still leaves 2.7 out in the open here, what I'd like to do is just disable it and if someone really *needs* TLS Compression they can use pyopenssl to get that back. This is a reversal of the current situation where in order to get the safer value you have to use pyopenssl.

[dstufft]

Here's the same patch for Python 2.7, it's basically the same thing just at a different location.

[alex]

Pinging on this, since the SSL backport landed, concerns about an inability to change this behavior on python2 are no longer there. At a minimum I think we should include this flag in the default and stdlib contexts.

[pitrou]

Now that the backport has landed, I think you're welcome to do any further necessary tweaks.

By the way, as mentioned in the comments, I think we could add SSL_OP_NO_COMPRESSION to ssl.OP_ALL in all versions.

[birkenfeld]

I wouldn't consider this important enough for 3.2; since it lacks the means to do the opt-back-in.

[tiran]

the default context sets context.options |= getattr(_ssl, "OP_NO_COMPRESSION", 0), _create_unverified_context() is missing that line.

[tiran]

For 3.6 and 3.7, _ssl__SSLContext_impl() now sets NO_COMPRESSION.