classification
Title: SSLContext.post_handshake_auth implicitly enables cert validation
Type: behavior Stage: patch review
Components: SSL Versions: Python 3.9, Python 3.8, Python 3.7, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: The Compiler, alex, benjamin.peterson, christian.heimes, lukasz.langa, miss-islington, ned.deily
Priority: high Keywords: patch

Created on 2019-06-27 10:33 by christian.heimes, last changed 2019-08-29 02:01 by benjamin.peterson.

Pull Requests
URL Status Linked Edit
PR 14421 merged christian.heimes, 2019-06-27 11:31
PR 14493 merged miss-islington, 2019-07-01 06:29
PR 14494 merged christian.heimes, 2019-07-01 06:31
Messages (13)
msg346725 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-06-27 10:33
Enabling TLS 1.3 post handshake auth also enables cert chain validation. OpenSSL documents SSL_VERIFY_POST_HANDSHAKE as ignored for client side. However tls_process_server_certificate in the client state machine code does not ignore the flag and checks for a correct cert chain.

see https://github.com/openssl/openssl/issues/9259 and https://github.com/openssl/openssl/blob/743694a6c29e5a6387819523fad5e3b7e613f1ee/ssl/statem/statem_clnt.c#L1899-L1918
msg346787 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-06-27 23:33
Christian, just confirming that, since you have not set this as a "release blocker", 3.7.4 will go out without it.
msg346818 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-06-28 13:59
This issue breaks some stuff at work. I would appreciate if we can get the fix into 3.7.4. I wasn't aware that we are so close to cut-off to 3.7.4 release.


What does the fix do?
I moved all PHA related flags / options from SSL_CTX* to SSL*. The flags and options now depend on the socket type and existing flags.

For a server-side socket, the SSL_VERIFY_POST_HANDSHAKE verify flag is now only set when the server socket is configured to verify client certs. Server sockets without SSL_VERIFY_PEER flag don't set the option. The presence of SSL_VERIFY_POST_HANDSHAKE without SSL_VERIFY_PEER sometimes triggers handshake errors like "extension not received". The official documentation says "This flag must be used together with SSL_VERIFY_PEER.". The ssl.CERT_OPTIONAL and ssl.CERT_REQURED both set SSL_VERIFY_PEER. SSL_set_post_handshake_auth() is not enabled for server-side sockets.

For client side sockets, PHA is only enabled with SSL_set_post_handshake_auth(ssl, 1). The SSL_VERIFY_POST_HANDSHAKE flag is no longer set.

https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_post_handshake_auth.html
msg346819 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-06-28 14:16
"Assuming no critical problems are found prior to 2019-06-28, no code changes are planned between these release candidates and the final releases."

We were planning to start producing the final release artifacts in a couple of hours so we need to make a decision very quickly.  If you think this is necessary, we can delay the release a bit.  In the worst case, we could do a second release candidate but I *really* do not want to do that unless it is absolutely necessary.  Also note that with 3.7.4, we have updated the Windows and macOS installers to use OpenSSL 1.1.1 (c) instead of 1.1.0.
msg346824 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-06-28 14:58
There are currently two issues with TLS 1.3 in Python. The issue https://bugs.python.org/issue37440 can be worked around easily with a custom SSLContext. This issue is a bigger problem because there is no possible workaround.

The bug is going to break applications that verify clients with a certificate but accept untrusted server certificates. It's not a common scenario, but I just happen to run into this issue for a project at work.

I'm sorry for the mess. :( I noticed the bug a couple of days ago. It took me a while to understand the root cause.
msg346880 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-06-29 12:59
Christian, do you have an estimate for when these issues will be resolved?  We are holding 3.7.4 right now.
msg346960 - (view) Author: miss-islington (miss-islington) Date: 2019-07-01 06:29
New changeset f0f5930ac88482ef896283db5be9b8d508d077db by Miss Islington (bot) (Christian Heimes) in branch 'master':
bpo-37428: Don't set PHA verify flag on client side (GH-14421)
https://github.com/python/cpython/commit/f0f5930ac88482ef896283db5be9b8d508d077db
msg346965 - (view) Author: miss-islington (miss-islington) Date: 2019-07-01 06:51
New changeset cf7617460a920dd75ced017792045d3ae77648ad by Miss Islington (bot) in branch '3.7':
[3.7] bpo-37428: Don't set PHA verify flag on client side (GH-14421) (GH-14493)
https://github.com/python/cpython/commit/cf7617460a920dd75ced017792045d3ae77648ad
msg346971 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-07-01 07:25
New changeset f22c4cf11d10f52faa86e0b308dd28f11819efd8 by Christian Heimes in branch '3.8':
[3.8] bpo-37428: Don't set PHA verify flag on client side (GH-14494)
https://github.com/python/cpython/commit/f22c4cf11d10f52faa86e0b308dd28f11819efd8
msg347158 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-07-02 22:34
New changeset 5b45fb0a449543fab6e7b606e51b739cb316d3c4 by Ned Deily (Miss Islington (bot)) in branch '3.7':
[3.7] bpo-37428: Don't set PHA verify flag on client side (GH-14421) (GH-14493)
https://github.com/python/cpython/commit/5b45fb0a449543fab6e7b606e51b739cb316d3c4
msg350286 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-08-23 14:02
Should this be closed?
msg350656 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-08-28 05:21
3.7 to 3.9 are fixed.

Benjamin, do you want the fix in 2.7?
msg350701 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2019-08-29 02:01
Yes, please.
History
Date User Action Args
2019-08-29 02:01:31benjamin.petersonsetmessages: + msg350701
2019-08-28 05:21:17christian.heimessetpriority: release blocker -> high

messages: + msg350656
2019-08-23 14:02:52lukasz.langasetmessages: + msg350286
2019-07-02 22:34:02ned.deilysetmessages: + msg347158
2019-07-01 07:25:53christian.heimessetmessages: + msg346971
2019-07-01 06:51:43miss-islingtonsetmessages: + msg346965
2019-07-01 06:31:15christian.heimessetpull_requests: + pull_request14310
2019-07-01 06:29:31miss-islingtonsetpull_requests: + pull_request14309
2019-07-01 06:29:21miss-islingtonsetnosy: + miss-islington
messages: + msg346960
2019-06-30 09:53:47The Compilersetnosy: + The Compiler
2019-06-29 12:59:02ned.deilysetpriority: deferred blocker -> release blocker
nosy: + benjamin.peterson, lukasz.langa
messages: + msg346880

2019-06-28 14:58:40christian.heimessetmessages: + msg346824
2019-06-28 14:44:25christian.heimeslinkissue37440 dependencies
2019-06-28 14:16:50ned.deilysetmessages: + msg346819
2019-06-28 13:59:05christian.heimessetpriority: high -> deferred blocker
nosy: + alex
messages: + msg346818

2019-06-27 23:33:28ned.deilysetnosy: + ned.deily
messages: + msg346787
2019-06-27 11:31:45christian.heimessetkeywords: + patch
stage: patch review
pull_requests: + pull_request14237
2019-06-27 10:33:29christian.heimescreate