Skip to content

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

Merged
merged 1 commit into from
May 23, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 11 additions & 19 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,7 @@ typedef struct {
PyObject *Socket; /* weakref to socket on which we're layered */
SSL *ssl;
PySSLContext *ctx; /* weakref to SSL context */
X509 *peer_cert;
char shutdown_seen_zero;
char handshake_done;
enum py_ssl_server_or_client socket_type;
PyObject *owner; /* Python level "owner" passed to servername callback */
PyObject *server_hostname;
Expand Down Expand Up @@ -595,12 +593,10 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
if (self == NULL)
return NULL;

self->peer_cert = NULL;
self->ssl = NULL;
self->Socket = NULL;
self->ctx = sslctx;
self->shutdown_seen_zero = 0;
self->handshake_done = 0;
self->owner = NULL;
self->server_hostname = NULL;
if (server_hostname != NULL) {
Expand Down Expand Up @@ -748,13 +744,6 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
if (ret < 1)
return PySSL_SetError(self, ret, __FILE__, __LINE__);

if (self->peer_cert)
X509_free (self->peer_cert);
PySSL_BEGIN_ALLOW_THREADS
self->peer_cert = SSL_get_peer_certificate(self->ssl);
PySSL_END_ALLOW_THREADS
self->handshake_done = 1;

Py_RETURN_NONE;

error:
Expand Down Expand Up @@ -1506,25 +1495,30 @@ _ssl__SSLSocket_peer_certificate_impl(PySSLSocket *self, int binary_mode)
/*[clinic end generated code: output=f0dc3e4d1d818a1d input=8281bd1d193db843]*/
{
int verification;
X509 *peer_cert;
PyObject *result;

if (!self->handshake_done) {
if (!SSL_is_init_finished(self->ssl)) {
PyErr_SetString(PyExc_ValueError,
"handshake not done yet");
return NULL;
}
if (!self->peer_cert)
peer_cert = SSL_get_peer_certificate(self->ssl);
Copy link
Member

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.

if (peer_cert == NULL)
Py_RETURN_NONE;

if (binary_mode) {
/* return cert in DER-encoded format */
return _certificate_to_der(self->peer_cert);
result = _certificate_to_der(peer_cert);
} else {
verification = SSL_CTX_get_verify_mode(SSL_get_SSL_CTX(self->ssl));
if ((verification & SSL_VERIFY_PEER) == 0)
return PyDict_New();
result = PyDict_New();
else
return _decode_certificate(self->peer_cert);
result = _decode_certificate(peer_cert);
}
X509_free(peer_cert);
return result;
}

static PyObject *
Expand Down Expand Up @@ -1845,8 +1839,6 @@ Passed as \"self\" in servername callback.");

static void PySSL_dealloc(PySSLSocket *self)
{
if (self->peer_cert) /* Possible not to have one? */
X509_free (self->peer_cert);
if (self->ssl)
SSL_free(self->ssl);
Py_XDECREF(self->Socket);
Expand Down Expand Up @@ -2442,7 +2434,7 @@ static int PySSL_set_session(PySSLSocket *self, PyObject *value,
"Cannot set session for server-side SSLSocket.");
return -1;
}
if (self->handshake_done) {
if (SSL_is_init_finished(self->ssl)) {
PyErr_SetString(PyExc_ValueError,
"Cannot set session after handshake.");
return -1;
Expand Down