Skip to content

Loading…

Disable SSL compression for security reasons. #1857

Closed
t-8ch opened this Issue · 15 comments

6 participants

@t-8ch

After the recent discussion about SSL compression both in shazow/urllib3#109
and kennethreitz/requests#1583 (this issue was about performance reason, it got me to look at other projects, performance is not the reason for this issue) I looked at the behaviour of other popular open source projects. It turned out that the following projects disable the compression by default for [security reasons](https://en.wikipedia.org/wiki/CRIME_(security_exploit\)):

This are the only projects I have looked at. I am sure we can find more if necessary.
As the stdlib ssl module does not allow us to change this parameter before 3.3
I propose to raise the issue on the CPython bug tracker, so that SSL
compression will be disabled by default (with the possibility to manually
enable it on 3.3 and later).

The current handlich of CPython 3.4 and up only disables compression on openssl
1.0 and up, as the relevant constant has not introduced before. However the
nginx changelog claims to also disable compression on earlier versions. I will
look into this.

This issue is meant to gather feedback and momentum before raising the issue with CPython (and maybe also the other implementations)

/cc @lukasa @sigmavirus24 @shazow @alex @jmhodges

@shazow
Collaborator

Fwiw, urllib3 just merged a PR which disables this by default in Py32+ and PyOpenSSL, big thanks to @dbrgn for bringing it up and writing the patch. shazow/urllib3#309

@t-8ch

Ah, I linked the wrong urllib3 issue. This should of course have been issue 309.

@Lukasa
Collaborator

I'm 100% in favour of this. 2.7 is clearly accepting some SSL related changes in the next version (see 20207 from @alex), so it doesn't seem unreasonable to make a push for this as well.

Unfortunately, it doesn't save requests/urllib3 entirely, because both projects support 2.6 and 2.6 is deader-than-dead.

@t-8ch

If the enterprise distributions still supporting 2.6 see that the (security) changes are applied to the still maintained versions of Python they know they should backport those patches.

@shazow
Collaborator

I've resolved to letting 2.6 get whatever subset of functionality we're able to reasonably provide, until it becomes too much of a hassle and we have to drop it.

@Lukasa
Collaborator

Fine then. So we just need to work out how to do it. It's clear that @t-8ch has some plans to investigate nginx, so I'm open to doing that for now.

@alex
@t-8ch

Here is what nginx is doing:
(if SSL_OP_NO_COMPRESSION is not available, openssl < 1.0.0)

int                  n;
STACK_OF(SSL_COMP)  *ssl_comp_methods;

ssl_comp_methods = SSL_COMP_get_compression_methods();
n = sk_SSL_COMP_num(ssl_comp_methods);

while (n--) {
    (void) sk_SSL_COMP_pop(ssl_comp_methods);
}

(Nginx is 2-clause BSD licensed, so we should be fine)

@Lukasa
Collaborator

Yeah, that looks reasonable to me. Any idea when the relevant functions were introduced to OpenSSL?

@t-8ch

Nginx gates this code behind

#if OPENSSL_VERSION_NUMBER >= 0x0090800fL

It seems compression itself has been implemented some versions earlier but openssl >= 0.9.8 should cover most installations.

@jmhodges

Am I right to understand that a CPython 2.7 ticket needs to be made to disable TLS compression?

@alex
@t-8ch

@jmhodges This is the plan.
@alex I guess your are running it with openssl > 1.0.0. If yes, the last point of my initial list applies.

@Lukasa
Collaborator

What's the state of this?

@Lukasa
Collaborator

Closed for massive inactivity. =D

@Lukasa Lukasa closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.