Skip to content

Conversation

@tiran
Copy link
Member

@tiran tiran commented Jun 27, 2019

SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue37428

@ned-deily
Copy link
Member

FWIW, I tested a manual cherrypick of this to 3.7 on macOS with OpenSSL 1.1.1c and it didn't cause any new failures in test_ssl and it wasn't failing before in this environment. However, FTR, the following non-fatal traceback consistently shows up both before and after this change and both with and without test -j:

[...]
test_pha_required_nocert (test.test_ssl.TestPostHandshakeAuth) ... Exception in thread Thread-242:
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/test/test_ssl.py", line 2293, in run
    msg = self.read()
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/test/test_ssl.py", line 2270, in read
    return self.sslconn.read()
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ssl.py", line 920, in read
    return self._sslobj.read(len)
ssl.SSLError: [SSL: PEER_DID_NOT_RETURN_A_CERTIFICATE] peer did not return a certificate (_ssl.c:2508)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 926, in _bootstrap_inner
    self.run()
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/test/test_ssl.py", line 2379, in run
    raise ssl.SSLError('tlsv13 alert certificate required')
ssl.SSLError: ('tlsv13 alert certificate required',)

ok
test_pha_setter (test.test_ssl.TestPostHandshakeAuth) ... ok
test_get_server_certificate_ipv6 (test.test_ssl.NetworkedTests) ... skipped "Resource 'ipv6.google.com' is not available"
test_timeout_connect_ex (test.test_ssl.NetworkedTests) ... ok

----------------------------------------------------------------------

Ran 153 tests in 76.861s

OK (skipped=10)
test_ssl passed in 1 min 16 sec

@ned-deily
Copy link
Member

Anything we can do to expedite this? It's currently blocking 3.7.4 final and could shortly block 3.8.0 b2. @alex, would you be able to review this? Thanks!

Copy link
Member

@alex alex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks right to me -- but I confess this code has changed a lot since I last looked at it, so I'd love if another person had eyes on it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is bpo37337 correct here? The issue addressed seems to be: https://bugs.python.org/issue37428

This might be more of a question.
I thouhght, the test should have two parts.

  1. Server verifying SSL_VERIFY_POST_HANDSHAKE flag set.
  2. Client which asserts SSL_VERIFY_POST_HANDSHAKE not set.

I could not figure out if the test covers both these scenarios.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the BPO number in the name of the test.

The new test verifies that client_context.post_handshake_auth does not implicitly enable server cert validation on the client side. There are already several other tests that verify several combinations of PHA.

  • the test case test_pha_required verifies that SSL_VERIFY_POST_HANDSHAKE on a server context works as expected. The server does request a client during handshake.
  • SSL_VERIFY_POST_HANDSHAKE must not be set on a client context. This was the original bug. The _set_verify_mode helper prevents this already.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for answering these, @tiran. LGTM.

SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran force-pushed the bpo-37428-pha-server branch from 06c3021 to 83d7c57 Compare June 30, 2019 20:16
Copy link
Member

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@miss-islington miss-islington merged commit f0f5930 into python:master Jul 1, 2019
@miss-islington
Copy link
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.7, 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 1, 2019
SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>

https://bugs.python.org/issue37428
(cherry picked from commit f0f5930)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot
Copy link

GH-14493 is a backport of this pull request to the 3.7 branch.

@miss-islington
Copy link
Contributor

Sorry, @tiran, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker f0f5930ac88482ef896283db5be9b8d508d077db 2.7

@miss-islington miss-islington self-assigned this Jul 1, 2019
@miss-islington
Copy link
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @tiran, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker f0f5930ac88482ef896283db5be9b8d508d077db 3.8

tiran added a commit to tiran/cpython that referenced this pull request Jul 1, 2019
…4421)

SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>

https://bugs.python.org/issue37428
(cherry picked from commit f0f5930)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot
Copy link

GH-14494 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Jul 1, 2019
…H-14493)

SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>



https://bugs.python.org/issue37428
(cherry picked from commit f0f5930)


Co-authored-by: Christian Heimes <christian@python.org>


https://bugs.python.org/issue37428
@tiran tiran deleted the bpo-37428-pha-server branch July 1, 2019 07:06
ned-deily pushed a commit to ned-deily/cpython that referenced this pull request Jul 2, 2019
…4421) (pythonGH-14493)

SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>



https://bugs.python.org/issue37428
(cherry picked from commit f0f5930)


Co-authored-by: Christian Heimes <christian@python.org>


https://bugs.python.org/issue37428
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>



https://bugs.python.org/issue37428
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
SSLContext.post_handshake_auth = True no longer sets
SSL_VERIFY_POST_HANDSHAKE verify flag for client connections. Although the
option is documented as ignored for clients, OpenSSL implicitly enables cert
chain validation when the flag is set.

Signed-off-by: Christian Heimes <christian@python.org>



https://bugs.python.org/issue37428
rickprice added a commit to ActiveState/cpython that referenced this pull request Jan 26, 2024
rickprice added a commit to ActiveState/cpython that referenced this pull request Jan 26, 2024
rickprice added a commit to ActiveState/cpython that referenced this pull request Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants