bpo-29136: Add TLS 1.3 cipher suites and OP_NO_TLSv1_3 #1363

tiran · 2017-04-30T10:38:18Z

TLS 1.3 introduces a new, distinct set of cipher suites. The TLS 1.3
cipher suites don't overlap with cipher suites from TLS 1.2 and earlier.
Since Python sets its own set of permitted ciphers, TLS 1.3 handshake
will fail as soon as OpenSSL 1.1.1 is released. Let's enable the common
AES-GCM and ChaCha20 suites.

Additionally the flag OP_NO_TLSv1_3 is added. It defaults to 0 (no op) with
OpenSSL prior to 1.1.1. This allows applications to opt-out from TLS 1.3
now.

Signed-off-by: Christian Heimes [email protected]

https://bugs.python.org/issue29136

mention-bot · 2017-04-30T10:38:22Z

@tiran, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Yhg1s, @benjaminp and @serhiy-storchaka to be potential reviewers.

tiran · 2017-04-30T10:40:34Z

ciphers: https://github.com/openssl/openssl/blob/master/include/openssl/tls1.h#L908

OP_NO flag: https://github.com/openssl/openssl/blob/master/include/openssl/ssl.h#L395

alex · 2017-04-30T10:40:51Z

Lib/ssl.py

@@ -189,13 +190,16 @@
 #   * Disable NULL authentication, NULL encryption, 3DES and MD5 MACs
 #     for security reasons
 _DEFAULT_CIPHERS = (
+    'TLS13-AES-256-GCM-SHA384:TLS13-CHACHA20-POLY1305-SHA256:'
+    'TLS13-AES-128-GCM-SHA256:'


Can this just be expressed as TLS13? Should it?

It can't :( I tried it with latest master:

Error in cipher list 0:error:2506906C:DSO support routines:DSO_pathbyaddr:functionality not supported:crypto/dso/dso_lib.c:315: 0:error:1410D0B9:SSL routines:SSL_CTX_set_cipher_list:no cipher match:ssl/ssl_lib.c:2303:

alex · 2017-04-30T10:41:53Z

Doc/library/ssl.rst

+   Prevents a TLSv1.3 connection. This option is only applicable in conjunction
+   with :const:`PROTOCOL_TLS`. It prevents the peers from choosing TLSv1.3 as
+   the protocol version. TLS 1.3 is available with OpenSSL 1.1.1 or later.
+   With older versions, the flag defaults to *0*.


We don't currently expose OP_NO_TLSv1_2 if your OpenSSL is old enough to not have TLS1.2. Why is this flag special? Should we just change all of them for consistency?

Applications may use hasattr() to detect if TLS 1.1 and 1.2 are available. I'd wish we could change the values to default to 0. It's awkward to require a getattr() call.

PS: OP_NO_TLSv1_1 and OP_NO_TLSv1_2 are going to be available as soon as we remove support for OpenSSL versions that are no longer supported by upstream.

I think these are specialized options (most people should stick to whatever create_default_context() returns), so the need to call getattr() doesn't sound much of a problem IMHO.

@pitrou The PROTOCOL constants with fixed version number are going away. OpenSSL has deprecated them in favor of auto-negotiation. The OP_NO_* constants are the only API that is both available with OpenSSL 1.0.x and 1.1.

Well, other modules don't follow this pattern either, i.e. optional flags are not set to zero when they are not provided by the OS, they are simply not exposed at all. If Twisted wants to be CentOS-compatible, the best thing they can do is to have actual CI on a CentOS system. That's of course complicating things a bit, but it's the only reliable way to reach that goal.

@pitrou: note that in this specific case, the flag being zero means it exactly implements the standard semantics (it successfully disables the feature, because the feature is always disabled). That's not the case for most other flags.

I don't really care that much; obviously the other way has worked fine too. But @tiran's approach has some mild but nice benefits and I'm not sure why this is a point of contention at all – AFAICT all your arguments have been that it doesn't matter either way; I haven't seen a single argument for why having these flags always available is harmful?

I wouldn't mind if this is the first such flag we exposed. But other flags already exist and they don't work the same way.

With OpenSSL 1.1, OP_NO_SSLv2 is 0.

I see. If OpenSSL is playing such games, then I guess it's acceptable for us to do it too...

pitrou · 2017-09-05T20:24:35Z

Doc/library/ssl.rst

@@ -206,6 +206,8 @@ instead.
   .. rubric:: Footnotes
   .. [1] :class:`SSLContext` disables SSLv2 with :data:`OP_NO_SSLv2` by default.
   .. [2] :class:`SSLContext` disables SSLv3 with :data:`OP_NO_SSLv3` by default.
+   .. [3] TLS 1.3 protocol is will be available with :data:`PROTOCOL_TLS` in


Typo: "is will be".

pitrou · 2017-09-05T20:25:06Z

Doc/library/ssl.rst

@@ -206,6 +206,8 @@ instead.
   .. rubric:: Footnotes
   .. [1] :class:`SSLContext` disables SSLv2 with :data:`OP_NO_SSLv2` by default.
   .. [2] :class:`SSLContext` disables SSLv3 with :data:`OP_NO_SSLv3` by default.
+   .. [3] TLS 1.3 protocol is will be available with :data:`PROTOCOL_TLS` in
+      OpenSSL >= 1.1.1. There is no dedicated PROTOCOL for just TLSv1.3.


For clarity, I suggest "There is no dedicated PROTOCOL constant for just TLS 1.3".

TLS 1.3 introduces a new, distinct set of cipher suites. The TLS 1.3 cipher suites don't overlap with cipher suites from TLS 1.2 and earlier. Since Python sets its own set of permitted ciphers, TLS 1.3 handshake will fail as soon as OpenSSL 1.1.1 is released. Let's enable the common AES-GCM and ChaCha20 suites. Additionally the flag OP_NO_TLSv1_3 is added. It defaults to 0 (no op) with OpenSSL prior to 1.1.1. This allows applications to opt-out from TLS 1.3 now. Signed-off-by: Christian Heimes <[email protected]>

gpshead

looks good to me. one comment on some documentation wording for clarity.

gpshead · 2017-09-08T00:20:14Z

Doc/library/ssl.rst

+   Prevents a TLSv1.3 connection. This option is only applicable in conjunction
+   with :const:`PROTOCOL_TLS`. It prevents the peers from choosing TLSv1.3 as
+   the protocol version. TLS 1.3 is available with OpenSSL 1.1.1 or later.
+   With older versions, the flag defaults to *0*.


suggesting wording clarification:

"With older versions, the flag defaults to 0." -> "When Python has been compiled against an older version of OpenSSL, the flag defaults to 0."

miss-islington · 2017-09-08T01:07:02Z

🐍🍒⛏🤖 Thanks @tiran for the PR, and @tiran for merging it 🌮🎉.I'm working now to backport this PR to: 2.7, 3.6.

…H-1363) * bpo-29136: Add TLS 1.3 support TLS 1.3 introduces a new, distinct set of cipher suites. The TLS 1.3 cipher suites don't overlap with cipher suites from TLS 1.2 and earlier. Since Python sets its own set of permitted ciphers, TLS 1.3 handshake will fail as soon as OpenSSL 1.1.1 is released. Let's enable the common AES-GCM and ChaCha20 suites. Additionally the flag OP_NO_TLSv1_3 is added. It defaults to 0 (no op) with OpenSSL prior to 1.1.1. This allows applications to opt-out from TLS 1.3 now. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit cb5b68a)

bedevere-bot · 2017-09-08T02:13:59Z

GH-3444 is a backport of this pull request to the 3.6 branch.

…H-1363) * bpo-29136: Add TLS 1.3 support TLS 1.3 introduces a new, distinct set of cipher suites. The TLS 1.3 cipher suites don't overlap with cipher suites from TLS 1.2 and earlier. Since Python sets its own set of permitted ciphers, TLS 1.3 handshake will fail as soon as OpenSSL 1.1.1 is released. Let's enable the common AES-GCM and ChaCha20 suites. Additionally the flag OP_NO_TLSv1_3 is added. It defaults to 0 (no op) with OpenSSL prior to 1.1.1. This allows applications to opt-out from TLS 1.3 now. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit cb5b68a)

bedevere-bot · 2017-09-08T02:23:58Z

GH-3446 is a backport of this pull request to the 2.7 branch.

#3444) * bpo-29136: Add TLS 1.3 support TLS 1.3 introduces a new, distinct set of cipher suites. The TLS 1.3 cipher suites don't overlap with cipher suites from TLS 1.2 and earlier. Since Python sets its own set of permitted ciphers, TLS 1.3 handshake will fail as soon as OpenSSL 1.1.1 is released. Let's enable the common AES-GCM and ChaCha20 suites. Additionally the flag OP_NO_TLSv1_3 is added. It defaults to 0 (no op) with OpenSSL prior to 1.1.1. This allows applications to opt-out from TLS 1.3 now. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit cb5b68a)

#3446) * bpo-29136: Add TLS 1.3 support TLS 1.3 introduces a new, distinct set of cipher suites. The TLS 1.3 cipher suites don't overlap with cipher suites from TLS 1.2 and earlier. Since Python sets its own set of permitted ciphers, TLS 1.3 handshake will fail as soon as OpenSSL 1.1.1 is released. Let's enable the common AES-GCM and ChaCha20 suites. Additionally the flag OP_NO_TLSv1_3 is added. It defaults to 0 (no op) with OpenSSL prior to 1.1.1. This allows applications to opt-out from TLS 1.3 now. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit cb5b68a)

tiran added needs backport to 2.7 labels Apr 30, 2017

the-knights-who-say-ni added the CLA signed label Apr 30, 2017

alex reviewed Apr 30, 2017

View reviewed changes

brettcannon added the type-feature A feature request or enhancement label May 1, 2017

brettcannon removed the needs backport to 3.5 label Aug 11, 2017

tiran force-pushed the bpo-29136-tls13 branch 2 times, most recently from 2512c9f to 0c22c7f Compare September 5, 2017 20:13

pitrou reviewed Sep 5, 2017

View reviewed changes

tiran added 2 commits September 7, 2017 11:09

Fix typos, thx Antoine

141abfc

tiran force-pushed the bpo-29136-tls13 branch from 6c93dce to 141abfc Compare September 7, 2017 18:09

gpshead reviewed Sep 8, 2017

View reviewed changes

Change wording of OP_NO_TLSv1_3 doc, thx Gregs

bca26e8

tiran merged commit cb5b68a into python:master Sep 8, 2017

tiran deleted the bpo-29136-tls13 branch September 8, 2017 01:07

bedevere-bot removed the needs backport to 3.6 label Sep 8, 2017

bedevere-bot removed the needs backport to 2.7 label Sep 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-29136: Add TLS 1.3 cipher suites and OP_NO_TLSv1_3 #1363

bpo-29136: Add TLS 1.3 cipher suites and OP_NO_TLSv1_3 #1363

tiran commented Apr 30, 2017 •

edited by bedevere-bot

Loading

mention-bot commented Apr 30, 2017

tiran commented Apr 30, 2017

alex Apr 30, 2017

tiran Apr 30, 2017

alex Apr 30, 2017

tiran Apr 30, 2017

tiran Apr 30, 2017

pitrou May 1, 2017

tiran May 1, 2017

pitrou May 2, 2017

njsmith May 2, 2017

pitrou May 2, 2017

tiran May 2, 2017

pitrou May 2, 2017

pitrou Sep 5, 2017

pitrou Sep 5, 2017

gpshead left a comment

gpshead Sep 8, 2017

miss-islington commented Sep 8, 2017

bedevere-bot commented Sep 8, 2017

bedevere-bot commented Sep 8, 2017

bpo-29136: Add TLS 1.3 cipher suites and OP_NO_TLSv1_3 #1363

bpo-29136: Add TLS 1.3 cipher suites and OP_NO_TLSv1_3 #1363

Conversation

tiran commented Apr 30, 2017 • edited by bedevere-bot Loading

mention-bot commented Apr 30, 2017

tiran commented Apr 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gpshead left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miss-islington commented Sep 8, 2017

bedevere-bot commented Sep 8, 2017

bedevere-bot commented Sep 8, 2017

tiran commented Apr 30, 2017 •

edited by bedevere-bot

Loading