Use Better Default Ciphers for the SSL Module

[dstufft]

BPO	20995
Nosy	@malemburg, @jcea, @ncoghlan, @pitrou, @vstinner, @tiran, @benjaminp, @ezio-melotti, @alex, @bitdancer, @dstufft
Files	better-ciphers.diff better-ciphers-better-priority.diff better-ciphers-dss.diff better-ciphers-with-docs.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 = None
closed_at = <Date 2014-03-22.17:27:59.923>
created_at = <Date 2014-03-20.14:11:16.718>
labels = []
title = 'Use Better Default Ciphers for the SSL Module'
updated_at = <Date 2014-04-23.14:28:37.006>
user = 'https://github.com/dstufft'

bugs.python.org fields:

activity = <Date 2014-04-23.14:28:37.006>
actor = 'alex'
assignee = 'none'
closed = True
closed_date = <Date 2014-03-22.17:27:59.923>
closer = 'dstufft'
components = []
creation = <Date 2014-03-20.14:11:16.718>
creator = 'dstufft'
dependencies = []
files = ['34539', '34546', '34548', '34558']
hgrepos = []
issue_num = 20995
keywords = ['patch']
message_count = 69.0
messages = ['214239', '214240', '214243', '214244', '214249', '214251', '214253', '214271', '214277', '214278', '214286', '214290', '214291', '214292', '214293', '214294', '214295', '214296', '214297', '214298', '214300', '214301', '214303', '214304', '214305', '214306', '214307', '214309', '214310', '214315', '214347', '214348', '214351', '214353', '214356', '214357', '214358', '214359', '214360', '214361', '214362', '214363', '214387', '214393', '214394', '214396', '214401', '214402', '214403', '214404', '214414', '214416', '214417', '214418', '214420', '214422', '214423', '214425', '214428', '214429', '214488', '214495', '214496', '217066', '217067', '217068', '217069', '217070', '217071']
nosy_count = 14.0
nosy_names = ['lemburg', 'jcea', 'ncoghlan', 'pitrou', 'vstinner', 'christian.heimes', 'benjamin.peterson', 'ezio.melotti', 'Arfrever', 'alex', 'r.david.murray', 'python-dev', 'dstufft', 'markk']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue20995'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4', 'Python 3.5']

[dstufft]

As of right now the default cipher list for the ssl module is DEFAULT:!aNULL:!eNULL:!LOW:!EXPORT:!SSLv2, additionally on Python 3.4 when you use create_default_context() then you also additionally get HIGH:!aNULL:!RC4:!DSS.

I think we should change this to the cipher string:

ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:DH+3DES:RSA+AESGCM:RSA+AES:RSA+3DES:!aNULL:!MD5:!DSS

This will:

• Prefer cipher suites that offer perfect forward secrecy (DHE/ECDHE)
• prefer ECDHE over DHE for better performance
• prefer any AES-GCM over any AES-CBC for better performance and security
• use 3DES as fallback which is secure but slow
• disable NULL authentication, MD5 MACs and DSS for security reasons

This cipher string is taken from urllib3 where it was compiled through the resources of:

• https://www.ssllabs.com/projects/best-practices/index.html
• https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/

The compatibility of this is pretty good. The only time this should cause a connection to *fail* is if a server is using an insecure cipher and in that case you can re-enable it by simply passing the original cipher list through the ssl.wrap_socket ciphers function.

[pitrou]

I really don't think hardcoding specific ciphers is a good idea.

[malemburg]

On 20.03.2014 15:11, Donald Stufft wrote:

The compatibility of this is pretty good. The only time this should cause a connection to *fail* is if a server is using an insecure cipher and in that case you can re-enable it by simply passing the original cipher list through the ssl.wrap_socket ciphers function.

Depends on who "you" is :-) Most of the time this will be the user of
some script or application with no clue as to how to change this or
what a cipher string is.

I think we should leave this decision to the OpenSSL lib vendors
and developers.

[bitdancer]

create_default_context is about best practices, though, so it seems to me it wouldn't be crazy to do it there.

[pitrou]

create_default_context is about best practices, though, so it seems to
me it wouldn't be crazy to do it there.

Agreed, but the real problem here is maintenance. Hardcoding a list of
specific ciphers means someone must follow closely the introduction of
new ciphers in OpenSSL, and choose whether or not to include them in the
list.

I'd prefer an open-ended cipher string. Here is a proposal:
'ECDH:EDH:AESGCM:HIGH:!eNULL:!aNULL:!DSS'

It prioritizes Diffie-Hellman key exchange (for perfect forward
secrecy), and AESGCM for the symmetric cipher; it also lets OpenSSL
append other possible ciphers.

BTW, apparently removing RC4 prevents ECDHE in SSv23 mode:

$ ./python -c 'import ssl, socket; ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23); ctx.set_ciphers("EECDH:EDH:AESGCM:HIGH:!eNULL:!aNULL");  s = ctx.wrap_socket(socket.socket()); s.connect(("linuxfr.org", 443)); print(s.cipher()); s.close()'
('ECDHE-RSA-RC4-SHA', 'TLSv1/SSLv3', 128)

$  ./python -c 'import ssl, socket; ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23); ctx.set_ciphers("EECDH:EDH:AESGCM:HIGH:!eNULL:!aNULL:!RC4");  s = ctx.wrap_socket(socket.socket()); s.connect(("linuxfr.org", 443)); print(s.cipher()); s.close()'
('DHE-RSA-AES256-SHA', 'TLSv1/SSLv3', 256)

[alex]

That's because of the set of ciphersuites offered by the server (see https://www.ssllabs.com/ssltest/analyze.html?d=linuxfr.org), it's not an inevitable property of TLS. For example jenkins.cryptography.io (see https://www.ssllabs.com/ssltest/analyze.html?d=jenkins.cryptography.io) offers ECDHE suites without any RC4 at all.

[dstufft]

Yea I noticed that, so I was doing some more testing, here's what I think we should be using (It Adds back in RC4):

ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:DH+3DES:RSA+AESGCM:RSA+AES:RSA+3DES:ECDH+RC4:DH+RC4:RSA+RC4!aNULL:!MD5:!DSS

This gives us everything that DEFAULT:!aNULL:!eNULL:!LOW:!EXPORT:!SSLv2 does except for the ciphers list here https://gist.github.com/dstufft/251dbeb8962e2182e668 on my OpenSSL 1.0.1f install.

Antoine, your cipher string priortizes ECDHE RC4 over DHE AES or even just plain AES. The string I'm proposing has been carefully crafted in order to get the ciphers in a very particular order. That order is basically - 1) Security of the cipher itself 2) PFS 3) Performance while also maintaining compatibility both forwards and backwards.

RC4 is in a precarious condition and it's use should be heavily discouraged. It is still required in some cases which is why my revised default cipher suggestion includes it, but at the end as a last fall back. At that point if RC4 gets selected it's the servers fault and the client did everything it could except refuse.

I still do believe that this should be the default ciphers while my original string should be the "restricted" ciphers that create_default_context() uses.

[ncoghlan]

In terms of following closely, I'd be willing to encourage Red Hat's SRT to
keep an eye on this.

[dstufft]

Another bit of maintenance here:

If a new cipher suite is added to OpenSSL it won' be generally available for a long while so very few if any services are going to be willing to depend on *only* it. For the very rare and unlikely case that somebody does setup a service that requires some brand new cipher they can override this list easily.

Using the default or the "wide" open strings are inherently more dangerous because of the wide range of OpenSSL's that are in production use. It's hard without auditing every version of OpenSSL to figure out what ciphers will be available in what circumstances. It also means that if OpenSSL adds a new cipher that ends up being insecure that it will be picked up automatically. Therefore the strings I've posted take the opinion that a whitelist is more secure than a blacklist and whitelist the cipher suites to a very specific set that happen to be best practices at this current time.

The only *required* maintenance would be if one of the selected ciphers are found to be insecure. However that was already a required maintenance because (again) of the wide range of OpenSSL versions available and the fact that these strings don't *add* any new ciphers, only remove some and create an explicit priority.

[alex]

It's also worth noting that users appear to be FAR more likely to have an up to date Python than they are an up to date OpenSSL, meaning that if a change needs to be made, we're much better situated to get that disseminated to actual users than OpenSSL is

[pitrou]

The string I'm proposing has been carefully crafted in order to get
the ciphers in a very particular order. That order is basically - 1)
Security of the cipher itself 2) PFS 3) Performance while also
maintaining compatibility both forwards and backwards.

I still think the ciphers list should be open-ended, i.e. have "HIGH" somewhere at the end.

[dstufft]

Why? At best users will get yet another secure algorithm and at worst they'll get an insecure algorithm.

[pitrou]

See http://bugs.python.org/issue20995#msg214249

[pitrou]

By the way:

Using the default or the "wide" open strings are inherently more
dangerous because of the wide range of OpenSSL's that are in
production use. It's hard without auditing every version of OpenSSL to
figure out what ciphers will be available in what circumstances

This doesn't parse. If the system OpenSSL isn't maintained properly, it's not Python's job to workaround that. And we certainly don't have the required knowledge and dedication anyway.

[dstufft]

This doesn't parse. If the system OpenSSL isn't maintained properly, it's not Python's job to workaround that. And we certainly don't have the required knowledge and dedication anyway.

Please let's not have a repeat of https://bugs.ruby-lang.org/issues/9424, Python is in a better place to workaround that than anyone else.