-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-29334: Fix ssl.getpeercert for auto-handshake #1769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Drop handshake_done and peer_cert members from PySSLSocket struct. The peer certificate can be acquired from *SSL directly. SSL_get_peer_certificate() does not trigger any network activity. Instead of manually tracking the handshake state, simply use SSL_is_init_finished(). In combination these changes fix auto-handshake for non-blocking MemoryBIO connections. Signed-off-by: Christian Heimes <[email protected]>
@tiran, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @Yhg1s, @janssen and @tiran to be potential reviewers. |
PyErr_SetString(PyExc_ValueError, | ||
"handshake not done yet"); | ||
return NULL; | ||
} | ||
if (!self->peer_cert) | ||
peer_cert = SSL_get_peer_certificate(self->ssl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we released the GIL around this -- this seems a bit silly to me, but I just wanted to note that this is a behavior change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides that comment, assuming tests pass, this LGTM.
The appveyor failure appear unrelated. |
Drop handshake_done and peer_cert members from PySSLSocket struct. The peer certificate can be acquired from *SSL directly. SSL_get_peer_certificate() does not trigger any network activity. Instead of manually tracking the handshake state, simply use SSL_is_init_finished(). In combination these changes fix auto-handshake for non-blocking MemoryBIO connections. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit 66dc33b)
Memo to me: add Misc/NEWS entry |
Drop handshake_done and peer_cert members from PySSLSocket struct. The peer certificate can be acquired from *SSL directly. SSL_get_peer_certificate() does not trigger any network activity. Instead of manually tracking the handshake state, simply use SSL_is_init_finished(). In combination these changes fix auto-handshake for non-blocking MemoryBIO connections. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit 66dc33b)
) Drop handshake_done and peer_cert members from PySSLSocket struct. The peer certificate can be acquired from *SSL directly. SSL_get_peer_certificate() does not trigger any network activity. Instead of manually tracking the handshake state, simply use SSL_is_init_finished(). In combination these changes fix auto-handshake for non-blocking MemoryBIO connections. Signed-off-by: Christian Heimes <[email protected]>. (cherry picked from commit 66dc33b)
Drop handshake_done and peer_cert members from PySSLSocket struct. The
peer certificate can be acquired from *SSL directly.
SSL_get_peer_certificate() does not trigger any network activity.
Instead of manually tracking the handshake state, simply use
SSL_is_init_finished().
In combination these changes fix auto-handshake for non-blocking
MemoryBIO connections.
Signed-off-by: Christian Heimes [email protected]