Committed revision 57568. Anything else I can check in for you? On 8/27/07, Bill Janssen <janssen@parc.com> wrote:

...

Some of the code sets the error string in this directly before returning NULL, and other pieces of the code call PySSL_SetError, which creates the error string. I think some of the places which set the string directly probably shouldn't; instead, they should call PySSL_SetError to cons up the error name directly from the err code. However, PySSL_SetError only works after the construction of an ssl object, which means it can't be used there... I'll take a longer look at it and see if there's a reasonable fix.

Here's a patch which addresses this. It also fixes the indentation in PySSL_SetError, bringing it into line with PEP 7, fixes a compile warning about one of the OpenSSL macros, and makes the namespace a bit more consistent. I've tested it on FC 7 and OS X 10.4.

% ./python ./Lib/test/regrtest.py -R :1: -u all test_ssl test_ssl beginning 6 repetitions 123456 ...... 1 test OK. [29244 refs] %

Bill

Index: Modules/_ssl.c =================================================================== --- Modules/_ssl.c (revision 57564) +++ Modules/_ssl.c (working copy) @@ -122,71 +122,74 @@ char buf[2048]; char *errstr; int err; - enum py_ssl_error p; + enum py_ssl_error p = PY_SSL_ERROR_NONE;

assert(ret <= 0);

- err = SSL_get_error(obj->ssl, ret); + if ((obj != NULL) && (obj->ssl != NULL)) { + err = SSL_get_error(obj->ssl, ret);

- switch (err) { - case SSL_ERROR_ZERO_RETURN: - errstr = "TLS/SSL connection has been closed"; - p = PY_SSL_ERROR_ZERO_RETURN; - break; - case SSL_ERROR_WANT_READ: - errstr = "The operation did not complete (read)"; - p = PY_SSL_ERROR_WANT_READ; - break; - case SSL_ERROR_WANT_WRITE: - p = PY_SSL_ERROR_WANT_WRITE; - errstr = "The operation did not complete (write)"; - break; - case SSL_ERROR_WANT_X509_LOOKUP: - p = PY_SSL_ERROR_WANT_X509_LOOKUP; - errstr = "The operation did not complete (X509 lookup)"; - break; - case SSL_ERROR_WANT_CONNECT: - p = PY_SSL_ERROR_WANT_CONNECT; - errstr = "The operation did not complete (connect)"; - break; - case SSL_ERROR_SYSCALL: - { - unsigned long e = ERR_get_error(); - if (e == 0) { - if (ret == 0 || !obj->Socket) { - p = PY_SSL_ERROR_EOF; - errstr = "EOF occurred in violation of protocol"; - } else if (ret == -1) { - /* the underlying BIO reported an I/O error */ - return obj->Socket->errorhandler(); - } else { /* possible? */ + switch (err) { + case SSL_ERROR_ZERO_RETURN: + errstr = "TLS/SSL connection has been closed"; + p = PY_SSL_ERROR_ZERO_RETURN; + break; + case SSL_ERROR_WANT_READ: + errstr = "The operation did not complete (read)"; + p = PY_SSL_ERROR_WANT_READ; + break; + case SSL_ERROR_WANT_WRITE: + p = PY_SSL_ERROR_WANT_WRITE; + errstr = "The operation did not complete (write)"; + break; + case SSL_ERROR_WANT_X509_LOOKUP: + p = PY_SSL_ERROR_WANT_X509_LOOKUP; + errstr = "The operation did not complete (X509 lookup)"; + break; + case SSL_ERROR_WANT_CONNECT: + p = PY_SSL_ERROR_WANT_CONNECT; + errstr = "The operation did not complete (connect)"; + break; + case SSL_ERROR_SYSCALL: + { + unsigned long e = ERR_get_error(); + if (e == 0) { + if (ret == 0 || !obj->Socket) { + p = PY_SSL_ERROR_EOF; + errstr = "EOF occurred in violation of protocol"; + } else if (ret == -1) { + /* the underlying BIO reported an I/O error */ + return obj->Socket->errorhandler(); + } else { /* possible? */ + p = PY_SSL_ERROR_SYSCALL; + errstr = "Some I/O error occurred"; + } + } else { p = PY_SSL_ERROR_SYSCALL; - errstr = "Some I/O error occurred"; + /* XXX Protected by global interpreter lock */ + errstr = ERR_error_string(e, NULL); } - } else { - p = PY_SSL_ERROR_SYSCALL; - /* XXX Protected by global interpreter lock */ - errstr = ERR_error_string(e, NULL); + break; } - break; - } - case SSL_ERROR_SSL: - { - unsigned long e = ERR_get_error(); - p = PY_SSL_ERROR_SSL; - if (e != 0) - /* XXX Protected by global interpreter lock */ - errstr = ERR_error_string(e, NULL); - else { /* possible? */ - errstr = "A failure in the SSL library occurred"; + case SSL_ERROR_SSL: + { + unsigned long e = ERR_get_error(); + p = PY_SSL_ERROR_SSL; + if (e != 0) + /* XXX Protected by global interpreter lock */ + errstr = ERR_error_string(e, NULL); + else { /* possible? */ + errstr = "A failure in the SSL library occurred"; + } + break; } - break; + default: + p = PY_SSL_ERROR_INVALID_ERROR_CODE; + errstr = "Invalid error code"; + } + } else { + errstr = ERR_error_string(ERR_peek_last_error(), NULL); } - default: - p = PY_SSL_ERROR_INVALID_ERROR_CODE; - errstr = "Invalid error code"; - } - PyOS_snprintf(buf, sizeof(buf), "_ssl.c:%d: %s", lineno, errstr); v = Py_BuildValue("(is)", p, buf); if (v != NULL) { @@ -256,8 +259,8 @@ ret = SSL_CTX_load_verify_locations(self->ctx, cacerts_file, NULL); Py_END_ALLOW_THREADS - if (ret < 1) { - errstr = ERRSTR("SSL_CTX_load_verify_locations"); + if (ret != 1) { + PySSL_SetError(NULL, 0, __FILE__, __LINE__); goto fail; } } @@ -267,8 +270,8 @@ ret = SSL_CTX_use_PrivateKey_file(self->ctx, key_file, SSL_FILETYPE_PEM); Py_END_ALLOW_THREADS - if (ret < 1) { - errstr = ERRSTR("SSL_CTX_use_PrivateKey_file error"); + if (ret != 1) { + PySSL_SetError(NULL, 0, __FILE__, __LINE__); goto fail; }

@@ -276,8 +279,8 @@ ret = SSL_CTX_use_certificate_chain_file(self->ctx, cert_file); Py_END_ALLOW_THREADS - if (ret < 1) { - errstr = ERRSTR("SSL_CTX_use_certificate_chain_file error") ; + if (ret != 1) { + PySSL_SetError(NULL, 0, __FILE__, __LINE__); goto fail; } SSL_CTX_set_options(self->ctx, SSL_OP_ALL); /* ssl compatibility */ @@ -375,7 +378,7 @@ }

static PyObject * -PySocket_ssl(PyObject *self, PyObject *args) +PySSL_sslwrap(PyObject *self, PyObject *args) { PySocketSockObject *Sock; int server_side = 0; @@ -431,6 +434,9 @@ PyObject *pd = PyDict_New(); int index_counter;

+ if (pd == NULL) + return NULL; + for (index_counter = 0; index_counter < X509_NAME_entry_count(xname); index_counter++) @@ -548,7 +554,7 @@ } Py_DECREF(pnotBefore);

- BIO_reset(biobuf); + (void) BIO_reset(biobuf); notAfter = X509_get_notAfter(self->peer_cert); ASN1_TIME_print(biobuf, notAfter); len = BIO_gets(biobuf, buf, sizeof(buf)-1); @@ -857,7 +863,7 @@

static PyTypeObject PySSL_Type = { PyVarObject_HEAD_INIT(NULL, 0) - "socket.SSL", /*tp_name*/ + "ssl.SSLContext", /*tp_name*/ sizeof(PySSLObject), /*tp_basicsize*/ 0, /*tp_itemsize*/ /* methods */ @@ -932,7 +938,7 @@ "RAND_egd(path) -> bytes\n\ \n\ Queries the entropy gather daemon (EGD) on socket path. Returns number\n\ -of bytes read. Raises socket.sslerror if connection to EGD fails or\n\ +of bytes read. Raises ssl.sslerror if connection to EGD fails or\n\ if it does provide enough data to seed PRNG.");

#endif @@ -940,7 +946,7 @@ /* List of functions exported by this module. */

static PyMethodDef PySSL_methods[] = { - {"sslwrap", PySocket_ssl, + {"sslwrap", PySSL_sslwrap, METH_VARARGS, ssl_doc}, #ifdef HAVE_OPENSSL_RAND {"RAND_add", PySSL_RAND_add, METH_VARARGS, @@ -979,7 +985,7 @@ SSLeay_add_ssl_algorithms();

/* Add symbols to module dict */ - PySSLErrorObject = PyErr_NewException("socket.sslerror", + PySSLErrorObject = PyErr_NewException("ssl.sslerror", PySocketModule.error, NULL); if (PySSLErrorObject == NULL)

_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/guido%40python.org

-- --Guido van Rossum (home page: http://www.python.org/~guido/)