Re: [Python-Dev] Other SSL issues in the tracker have been marked
Bill, Could you also look into this problem: Traceback (most recent call last): File "/home/pybot/buildarea/trunk.klose-debian-ia64/build/Lib/threading.py", line 486, in __bootstrap_inner self.run() File "/home/pybot/buildarea/trunk.klose-debian-ia64/build/Lib/test/test_ssl.py", line 144, in run cert_reqs=self.server.certreqs) File "/home/pybot/buildarea/trunk.klose-debian-ia64/build/Lib/ssl.py", line 98, in __init__ cert_reqs, ssl_version, ca_certs) sslerror: _ssl.c:271: SSL_CTX_use_PrivateKey_file error This occurs on at least 3 of the buildbots (ubuntu and debian on ia64, ppc, and hppa). Here's one example: http://python.org/dev/buildbot/all/ia64%20Ubuntu%20trunk%20trunk/builds/832/... This also looks like it's not working on windows, but there is no info from here: http://python.org/dev/buildbot/all/x86%20XP-3%20trunk/builds/164/step-test/0 Other than: test_ssl The system cannot find the path specified. Which happens after it hangs for 1200 seconds. Thanks, n -- On 8/25/07, Bill Janssen <janssen@parc.com> wrote:
This occurs on at least 3 of the buildbots (ubuntu and debian on ia64, ppc, and hppa). Here's one example:
Unfortunately, I don't have Ubuntu or Debian machines. But I'd bet it's a variation in the specific version of OpenSSL being used. I just tested on Fedora Core 7, though, and test_ssl runs fine there. What version of OpenSSL is being used on the buildbots? Can I log into one of them and look at the configuration?
This must be the call to os.system in test_ssl.py:create_cert_files(). It's very UNIX-y. Can any bi-platform folks suggest a good alternative to os.system( "openssl req -batch -new -x509 -days 10 -nodes -config %s " "-keyout \"%s\" -out \"%s\" > /dev/null < /dev/null 2>&1" % (conffile, crtfile, crtfile)) that would be more Windows-friendly? Bill
That's what we've done, and it seems to have turned the Ubuntu and Debian buildbots green again. The Windows question is harder, though -- I'm still not seeing a clean buildbot test on Windows. By the way, this solution introduces a Y2K problem into the test code. The included cert runs out in February of 2013, and will need to be regenerated then. Bill
If I'm reading this right, it's passing tests on "amd64 gentoo trunk", "x86 gentoo trunk", "g4 osx.4 trunk" (no surprise there). And looking at the community buildbots, it works on "x86 Redhat 9", "x86 Debian unstable", "amd64 Ubuntu gutsy", "G5 OS X", and so on. I've tested it myself on FC 7 and it works. And looking at the "ppc Debian unstable" case, test_socket is also failing there, so the test_ssl failure is not a big surprise. I'm not familiar with what's in Debian "trunk" or Ubuntu "trunk"; any idea what version of OpenSSL they have in them? But I think this exposes a more generic bug in test_ssl.py, which is that the server thread doesn't die when one of these failures occurs. It probably should. I'll make a patch -- but I don't have a system that this fails on, how will I test it? Bill
Here's a patch which makes test_ssl a better player in the buildbots environment. I deep-ended on "try-except-else" clauses. Should I post this as an issue to the Tracker? Bill Index: Lib/ssl.py =================================================================== --- Lib/ssl.py (revision 57506) +++ Lib/ssl.py (working copy) @@ -100,12 +100,13 @@ # see if it's connected try: socket.getpeername(self) - # yes + except: + # no, no connection yet + self._sslobj = None + else: + # yes, create the SSL object self._sslobj = _ssl.sslwrap(self._sock, 0, keyfile, certfile, cert_reqs, ssl_version, ca_certs) - except: - # no - self._sslobj = None self.keyfile = keyfile self.certfile = certfile self.cert_reqs = cert_reqs Index: Lib/test/test_ssl.py =================================================================== --- Lib/test/test_ssl.py (revision 57506) +++ Lib/test/test_ssl.py (working copy) @@ -91,38 +91,66 @@ def testTLSecho (self): s1 = socket.socket() - s1.connect(('127.0.0.1', 10024)) - c1 = ssl.sslsocket(s1, ssl_version=ssl.PROTOCOL_TLSv1) - indata = "FOO\n" - c1.write(indata) - outdata = c1.read() - if outdata != indata.lower(): - sys.stderr.write("bad data <<%s>> received\n" % data) - c1.close() + try: + s1.connect(('127.0.0.1', 10024)) + except: + sys.stdout.write("connection failure:\n" + string.join( + traceback.format_exception(*sys.exc_info()))) + raise test_support.TestFailed("Can't connect to test server") + else: + try: + c1 = ssl.sslsocket(s1, ssl_version=ssl.PROTOCOL_TLSv1) + except: + sys.stdout.write("SSL handshake failure:\n" + string.join( + traceback.format_exception(*sys.exc_info()))) + raise test_support.TestFailed("Can't SSL-handshake with test server") + else: + if not c1: + raise test_support.TestFailed("Can't SSL-handshake with test server") + indata = "FOO\n" + c1.write(indata) + outdata = c1.read() + if outdata != indata.lower(): + raise test_support.TestFailed("bad data <<%s>> received; expected <<%s>>\n" % (data, indata.lower())) + c1.close() def testReadCert(self): s2 = socket.socket() - s2.connect(('127.0.0.1', 10024)) - c2 = ssl.sslsocket(s2, ssl_version=ssl.PROTOCOL_TLSv1, - cert_reqs=ssl.CERT_REQUIRED, ca_certs=CERTFILE) - cert = c2.getpeercert() - if not cert: - raise test_support.TestFailed("Can't get peer certificate.") - if not cert.has_key('subject'): - raise test_support.TestFailed( - "No subject field in certificate: %s." % - pprint.pformat(cert)) - if not (cert['subject'].has_key('organizationName')): - raise test_support.TestFailed( - "No 'organizationName' field in certificate subject: %s." % - pprint.pformat(cert)) - if (cert['subject']['organizationName'] != - "Python Software Foundation"): - raise test_support.TestFailed( - "Invalid 'organizationName' field in certificate subject; " - "should be 'Python Software Foundation'."); - c2.close() + try: + s2.connect(('127.0.0.1', 10024)) + except: + sys.stdout.write("connection failure:\n" + string.join( + traceback.format_exception(*sys.exc_info()))) + raise test_support.TestFailed("Can't connect to test server") + else: + try: + c2 = ssl.sslsocket(s2, ssl_version=ssl.PROTOCOL_TLSv1, + cert_reqs=ssl.CERT_REQUIRED, ca_certs=CERTFILE) + except: + sys.stdout.write("SSL handshake failure:\n" + string.join( + traceback.format_exception(*sys.exc_info()))) + raise test_support.TestFailed("Can't SSL-handshake with test server") + else: + if not c2: + raise test_support.TestFailed("Can't SSL-handshake with test server") + cert = c2.getpeercert() + if not cert: + raise test_support.TestFailed("Can't get peer certificate.") + if not cert.has_key('subject'): + raise test_support.TestFailed( + "No subject field in certificate: %s." % + pprint.pformat(cert)) + if not (cert['subject'].has_key('organizationName')): + raise test_support.TestFailed( + "No 'organizationName' field in certificate subject: %s." % + pprint.pformat(cert)) + if (cert['subject']['organizationName'] != + "Python Software Foundation"): + raise test_support.TestFailed( + "Invalid 'organizationName' field in certificate subject; " + "should be 'Python Software Foundation'."); + c2.close() class threadedEchoServer(threading.Thread): @@ -138,10 +166,22 @@ def run (self): self.running = True - sslconn = ssl.sslsocket(self.sock, server_side=True, - certfile=self.server.certificate, - ssl_version=self.server.protocol, - cert_reqs=self.server.certreqs) + try: + sslconn = ssl.sslsocket(self.sock, server_side=True, + certfile=self.server.certificate, + ssl_version=self.server.protocol, + cert_reqs=self.server.certreqs) + except: + # here, we want to stop the server, because this shouldn't + # happen in the context of our test case + sys.stdout.write("Test server failure:\n" + string.join( + traceback.format_exception(*sys.exc_info()))) + self.running = False + # normally, we'd just stop here, but for the test + # harness, we want to stop the server + self.server.stop() + return + while self.running: try: msg = sslconn.read() @@ -154,15 +194,18 @@ self.server.stop() self.running = False else: - # print "server:", msg.strip().lower() + sys.stdout.write("\nserver: %s\n" % msg.strip().lower()) sslconn.write(msg.lower()) except ssl.sslerror: - sys.stderr.write(string.join( + sys.stdout.write("Test server failure:\n" + string.join( traceback.format_exception(*sys.exc_info()))) sslconn.close() self.running = False + # normally, we'd just stop here, but for the test + # harness, we want to stop the server + self.server.stop() except: - sys.stderr.write(string.join( + sys.stdout.write(string.join( traceback.format_exception(*sys.exc_info()))) def __init__(self, port, certificate, ssl_version=None, @@ -192,21 +235,21 @@ while self.active: try: newconn, connaddr = self.sock.accept() - # sys.stderr.write('new connection from ' + str(connaddr)) + sys.stdout.write('\nserver: new connection from ' + str(connaddr) + '\n') handler = self.connectionHandler(self, newconn) handler.start() except socket.timeout: pass except KeyboardInterrupt: - self.active = False + self.stop() except: - sys.stderr.write(string.join( + sys.stdout.write("Test server failure:\n" + string.join( traceback.format_exception(*sys.exc_info()))) def stop (self): self.active = False + self.sock.close() - CERTFILE_CONFIG_TEMPLATE = """ # create RSA certs - Server
Neal, I'm looking at why we're getting such opaque error messages, and I see that _ssl.c uses a static PyErrorObject that gets created when the module is initialized: PySSLErrorObject = PyErr_NewException("socket.sslerror", PySocketModule.error, NULL); Is this good style? It should be thread-safe, as the GIL is held while the error is being returned to the calling code, but still... 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. Bill
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)
participants (3)
-
Bill Janssen
-
Damien Miller
-
Neal Norwitz