Merge lp:~myers-1/pyopenssl/npn into lp:~exarkun/pyopenssl/trunk

Proposed by Myers Carpenter on 2012-02-10
Status: Work in progress
Proposed branch: lp:~myers-1/pyopenssl/npn
Merge into: lp:~exarkun/pyopenssl/trunk
Diff against target: 409 lines (+292/-0)
5 files modified
OpenSSL/ssl/connection.c (+33/-0)
OpenSSL/ssl/connection.h (+1/-0)
OpenSSL/ssl/context.c (+212/-0)
OpenSSL/ssl/context.h (+2/-0)
OpenSSL/test/test_ssl.py (+44/-0)
To merge this branch: bzr merge lp:~myers-1/pyopenssl/npn
Reviewer Review Type Date Requested Status
Jean-Paul Calderone 2012-02-10 Needs Fixing on 2012-04-03
Review via email: mp+92416@code.launchpad.net

Description of the change

This adds support for Next Protocol Negotiation found in OpenSSL 1.0.1. It includes a new test that exercises the functionality.

To post a comment you must log in.
lp:~myers-1/pyopenssl/npn updated on 2012-02-10
165. By Myers Carpenter on 2012-02-10

add ifdefs so this will build with older versions of openssl

Myers Carpenter (myers-1) wrote :

OpenSSL 1.0.1 is out with NPN built it.

Jean-Paul Calderone (exarkun) wrote :

Thanks! A few comments:

  1. Where `global_next_protos_advertised_callback` invokes application code, `next_protos_advertised_callback`, it needs to check for an exception being raised and propagate it. It also needs to make sure the return value is a sequence and raise its own exception (likely a `TypeError`) if something else is returned. Notice the "XXX" near the end of `global_next_protos_advertised_callback`: that's the code doing the exception check. It doesn't do any good down there, after `ret` has been used. :)
  1. `global_next_proto_select_callback` needs to check the return value of `PyList_New`. It may return NULL to signal error.
  1. Is the reference counting for `conn->proto_selected` / `ret` correct? Does it need to be incref'd after `ret` is assigned to `conn->proto_selected`? I don't know who owns the reference returned by `PyEval_CallObject`.
  1. The `PyCallback_Check` in `ssl_Context_set_next_protos_advertised_callback` isn't necessary. Just try calling the object. If it fails, the exception handling code at the call site (still needs to be added :) will handle it.
  1. Same comment about `ssl_Context_set_next_proto_select_callback`.
  1. The unit tests should exercise as many of the error cases as possible.
  1. Avoid using assertEqual inside a callback in the unit test. There's no guarantee that the exception it raises will propagate to the test runner and be reported (and indeed, the current implementation will crash instead). Record the arguments and make assertions about them afterwards.

Thanks again! Looking forward to the next iteration.

review: Needs Fixing

Unmerged revisions

165. By Myers Carpenter on 2012-02-10

add ifdefs so this will build with older versions of openssl

164. By Myers Carpenter on 2012-02-10

add support for next protocol neg added in OpenSSL 1.0.1.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'OpenSSL/ssl/connection.c'
--- OpenSSL/ssl/connection.c 2011-07-16 05:14:58 +0000
+++ OpenSSL/ssl/connection.c 2012-02-10 13:47:29 +0000
@@ -327,6 +327,29 @@
327 }327 }
328}328}
329329
330#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
331static char ssl_Connection_get_next_proto_negotiated_doc[] = "\n\
332Get protocol that the client selected\n\
333\n\
334:return: A string with the protocol the client selected\n\
335";
336static PyObject *
337ssl_Connection_get_next_proto_negotiated(ssl_ConnectionObj *self, PyObject *args)
338{
339 const unsigned char *data;
340 unsigned len;
341
342 if (!PyArg_ParseTuple(args, ":get_next_proto_negotiated"))
343 return NULL;
344
345 SSL_get0_next_proto_negotiated(self->ssl, &data, &len);
346 if (len == 0) {
347 Py_INCREF(Py_None);
348 return Py_None;
349 }
350 return PyBytes_FromStringAndSize((const char *)data, (Py_ssize_t)len);
351}
352#endif
330353
331static char ssl_Connection_set_tlsext_host_name_doc[] = "\n\354static char ssl_Connection_set_tlsext_host_name_doc[] = "\n\
332Set the value of the servername extension to send in the client hello.\n\355Set the value of the servername extension to send in the client hello.\n\
@@ -1271,6 +1294,9 @@
1271 ADD_METHOD(get_context),1294 ADD_METHOD(get_context),
1272 ADD_METHOD(set_context),1295 ADD_METHOD(set_context),
1273 ADD_METHOD(get_servername),1296 ADD_METHOD(get_servername),
1297#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
1298 ADD_METHOD(get_next_proto_negotiated),
1299#endif
1274 ADD_METHOD(set_tlsext_host_name),1300 ADD_METHOD(set_tlsext_host_name),
1275 ADD_METHOD(pending),1301 ADD_METHOD(pending),
1276 ADD_METHOD(send),1302 ADD_METHOD(send),
@@ -1339,6 +1365,9 @@
1339 Py_INCREF(sock);1365 Py_INCREF(sock);
1340 self->socket = sock;1366 self->socket = sock;
13411367
1368 Py_INCREF(Py_None);
1369 self->proto_selected = Py_None;
1370
1342 self->ssl = NULL;1371 self->ssl = NULL;
1343 self->from_ssl = NULL;1372 self->from_ssl = NULL;
1344 self->into_ssl = NULL;1373 self->into_ssl = NULL;
@@ -1466,6 +1495,8 @@
1466 ret = visit((PyObject *)self->context, arg);1495 ret = visit((PyObject *)self->context, arg);
1467 if (ret == 0 && self->socket != NULL)1496 if (ret == 0 && self->socket != NULL)
1468 ret = visit(self->socket, arg);1497 ret = visit(self->socket, arg);
1498 if (ret == 0 && self->proto_selected != NULL)
1499 ret = visit((PyObject *)self->proto_selected, arg);
1469 if (ret == 0 && self->app_data != NULL)1500 if (ret == 0 && self->app_data != NULL)
1470 ret = visit(self->app_data, arg);1501 ret = visit(self->app_data, arg);
1471 return ret;1502 return ret;
@@ -1484,6 +1515,8 @@
1484 self->context = NULL;1515 self->context = NULL;
1485 Py_XDECREF(self->socket);1516 Py_XDECREF(self->socket);
1486 self->socket = NULL;1517 self->socket = NULL;
1518 Py_XDECREF(self->proto_selected);
1519 self->proto_selected = NULL;
1487 Py_XDECREF(self->app_data);1520 Py_XDECREF(self->app_data);
1488 self->app_data = NULL;1521 self->app_data = NULL;
1489 self->into_ssl = NULL; /* was cleaned up by SSL_free() */1522 self->into_ssl = NULL; /* was cleaned up by SSL_free() */
14901523
=== modified file 'OpenSSL/ssl/connection.h'
--- OpenSSL/ssl/connection.h 2011-03-03 00:26:20 +0000
+++ OpenSSL/ssl/connection.h 2012-02-10 13:47:29 +0000
@@ -42,6 +42,7 @@
42 SSL *ssl;42 SSL *ssl;
43 ssl_ContextObj *context;43 ssl_ContextObj *context;
44 PyObject *socket;44 PyObject *socket;
45 PyObject *proto_selected;
45 PyThreadState *tstate; /* This field is no longer used. */46 PyThreadState *tstate; /* This field is no longer used. */
46 PyObject *app_data;47 PyObject *app_data;
47 BIO *into_ssl, *from_ssl; /* for connections without file descriptors */48 BIO *into_ssl, *from_ssl; /* for connections without file descriptors */
4849
=== modified file 'OpenSSL/ssl/context.c'
--- OpenSSL/ssl/context.c 2011-09-11 13:35:32 +0000
+++ OpenSSL/ssl/context.c 2012-02-10 13:47:29 +0000
@@ -237,6 +237,140 @@
237 return;237 return;
238}238}
239239
240#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
241/*
242 * Globally defined next proto callback. This is called from OpenSSL internally.
243 * The GIL will not be held when this function is invoked. It must not be held
244 * when the function returns.
245 *
246 * Arguments: ssl - The Connection
247 * **out - handle to where we can put a new string to be returned
248 * *outlen - pointer to where we can put the len of the string we want to return
249 * Returns: SSL_TLSEXT_ERR_OK
250 */
251static int
252global_next_protos_advertised_callback(SSL *ssl, const unsigned char **out, unsigned int *outlen, void *arg)
253{
254 ssl_ConnectionObj *conn = (ssl_ConnectionObj *)SSL_get_app_data(ssl);
255 PyObject *argv, *ret, *item;
256 Py_ssize_t length, i, strlength;
257 unsigned char *outptr;
258
259 *outlen = 0;
260
261 /*
262 * GIL isn't held yet. First things first - acquire it, or any Python API
263 * we invoke might segfault or blow up the sun. The reverse will be done
264 * before returning.
265 */
266 MY_END_ALLOW_THREADS(conn->tstate);
267
268 argv = Py_BuildValue("(O)", (PyObject *)conn);
269 ret = PyEval_CallObject(conn->context->next_protos_advertised_callback, argv);
270 Py_DECREF(argv);
271
272 length = PySequence_Size(ret);
273 for (i = 0; i < length; i++) {
274 strlength = PyBytes_Size(PySequence_GetItem(ret, i));
275 if (strlength >= 254) {
276 PyErr_SetString(PyExc_TypeError, "string returned by next_protos_advertised_callback was greater than 254 characters");
277 goto out;
278 }
279 *outlen += 1 + strlength;
280 }
281 *out = outptr = OPENSSL_malloc(*outlen + 1);
282 for (i = 0; i < length; i++) {
283 item = PySequence_GetItem(ret, i);
284 strlength = PyBytes_Size(item);
285 outptr[0] = Py_SAFE_DOWNCAST(strlength, Py_ssize_t, unsigned char);
286 outptr++;
287 memcpy(outptr, PyBytes_AsString(item), strlength);
288 outptr += strlength;
289 }
290
291 if (ret == NULL) {
292 /*
293 * XXX - This should be reported somehow. -exarkun
294 */
295 PyErr_Clear();
296 } else {
297 Py_DECREF(ret);
298 }
299
300 out:
301 /*
302 * This function is returning into OpenSSL. Release the GIL again.
303 */
304 MY_BEGIN_ALLOW_THREADS(conn->tstate);
305 return SSL_TLSEXT_ERR_OK;
306}
307
308/*
309 * Globally defined next proto select callback. This is called from OpenSSL internally.
310 * The GIL will not be held when this function is invoked. It must not be held
311 * when the function returns.
312 *
313 * Arguments: ssl - The Connection
314 * **out - handle to where we can put the proto we want to pick
315 * *outlen - pointer to where we can put the len of the proto we pick
316 * *in - handle to where the list of protocols the server is advertising
317 * inlen - pointer to where the lenght of protocols we can select
318 * Returns: SSL_TLSEXT_ERR_OK
319 */
320static int
321global_next_proto_select_callback(SSL *ssl, unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg)
322{
323 ssl_ConnectionObj *conn = (ssl_ConnectionObj *)SSL_get_app_data(ssl);
324 PyObject *argv, *proto_list, *str, *ret;
325 int retval = SSL_TLSEXT_ERR_ALERT_FATAL;
326 int i;
327
328 *outlen = 0;
329 /*
330 * GIL isn't held yet. First things first - acquire it, or any Python API
331 * we invoke might segfault or blow up the sun. The reverse will be done
332 * before returning.
333 */
334 MY_END_ALLOW_THREADS(conn->tstate);
335
336 proto_list = PyList_New(0);
337 for (i = 0; i < inlen; i++) {
338 str = PyBytes_FromStringAndSize((const char *)in+i+1, (Py_ssize_t)in[i]);
339 PyList_Append(proto_list, str);
340 Py_DECREF(str);
341 i += in[i];
342 }
343
344 argv = Py_BuildValue("(OO)", (PyObject *)conn, proto_list);
345 ret = PyEval_CallObject(conn->context->next_proto_select_callback, argv);
346 Py_DECREF(argv);
347 Py_DECREF(proto_list);
348
349 if (ret == NULL) {
350 goto out;
351 }
352 if (!PyBytes_Check(ret)) {
353 /*
354 * XXX Returned something that wasn't a string. This is bogus. We'll
355 * return 0 and OpenSSL will treat it as an error, resulting in an
356 * exception from whatever Python API triggered this callback.
357 */
358 Py_DECREF(ret);
359 goto out;
360 }
361 Py_DECREF(conn->proto_selected);
362 conn->proto_selected = ret;
363 PyBytes_AsStringAndSize(ret, (char **)out, (Py_ssize_t *)outlen);
364 retval = SSL_TLSEXT_ERR_OK;
365 out:
366 /*
367 * This function is returning into OpenSSL. Release the GIL again.
368 */
369 MY_BEGIN_ALLOW_THREADS(conn->tstate);
370 return retval;
371}
372#endif
373
240/*374/*
241 * Globally defined TLS extension server name callback. This is called from375 * Globally defined TLS extension server name callback. This is called from
242 * OpenSSL internally. The GIL will not be held when this function is invoked.376 * OpenSSL internally. The GIL will not be held when this function is invoked.
@@ -1030,6 +1164,66 @@
1030 return Py_None;1164 return Py_None;
1031}1165}
10321166
1167#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
1168static char ssl_Context_set_next_protos_advertised_callback_doc[] = "\n\
1169Set the next protos advertised callback\n\
1170\n\
1171:param callback: The Python callback to use\n\
1172:return: None\n\
1173";
1174static PyObject *
1175ssl_Context_set_next_protos_advertised_callback(ssl_ContextObj *self, PyObject *args)
1176{
1177 PyObject *callback;
1178
1179 if (!PyArg_ParseTuple(args, "O:set_next_protos_advertised_callback", &callback))
1180 return NULL;
1181
1182 if (!PyCallable_Check(callback))
1183 {
1184 PyErr_SetString(PyExc_TypeError, "expected PyCallable");
1185 return NULL;
1186 }
1187
1188 Py_DECREF(self->next_protos_advertised_callback);
1189 Py_INCREF(callback);
1190 self->next_protos_advertised_callback = callback;
1191 SSL_CTX_set_next_protos_advertised_cb(self->ctx, global_next_protos_advertised_callback, NULL);
1192
1193 Py_INCREF(Py_None);
1194 return Py_None;
1195}
1196
1197static char ssl_Context_set_next_proto_select_callback_doc[] = "\n\
1198Set the next protos select callback\n\
1199\n\
1200:param callback: The Python callback to use. This will take a list as an arg and return a str\n\
1201:return: None\n\
1202";
1203static PyObject *
1204ssl_Context_set_next_proto_select_callback(ssl_ContextObj *self, PyObject *args)
1205{
1206 PyObject *callback;
1207
1208 if (!PyArg_ParseTuple(args, "O:set_next_proto_select_callback", &callback))
1209 return NULL;
1210
1211 if (!PyCallable_Check(callback))
1212 {
1213 PyErr_SetString(PyExc_TypeError, "expected PyCallable");
1214 return NULL;
1215 }
1216
1217 Py_DECREF(self->next_proto_select_callback);
1218 Py_INCREF(callback);
1219 self->next_proto_select_callback = callback;
1220 SSL_CTX_set_next_proto_select_cb(self->ctx, global_next_proto_select_callback, NULL);
1221
1222 Py_INCREF(Py_None);
1223 return Py_None;
1224}
1225#endif
1226
1033static char ssl_Context_get_app_data_doc[] = "\n\1227static char ssl_Context_get_app_data_doc[] = "\n\
1034Get the application data (supplied via set_app_data())\n\1228Get the application data (supplied via set_app_data())\n\
1035\n\1229\n\
@@ -1187,6 +1381,10 @@
1187 ADD_METHOD(set_timeout),1381 ADD_METHOD(set_timeout),
1188 ADD_METHOD(get_timeout),1382 ADD_METHOD(get_timeout),
1189 ADD_METHOD(set_info_callback),1383 ADD_METHOD(set_info_callback),
1384#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
1385 ADD_METHOD(set_next_protos_advertised_callback),
1386 ADD_METHOD(set_next_proto_select_callback),
1387#endif
1190 ADD_METHOD(get_app_data),1388 ADD_METHOD(get_app_data),
1191 ADD_METHOD(set_app_data),1389 ADD_METHOD(set_app_data),
1192 ADD_METHOD(get_cert_store),1390 ADD_METHOD(get_cert_store),
@@ -1241,6 +1439,12 @@
1241 self->info_callback = Py_None;1439 self->info_callback = Py_None;
12421440
1243 Py_INCREF(Py_None);1441 Py_INCREF(Py_None);
1442 self->next_protos_advertised_callback = Py_None;
1443
1444 Py_INCREF(Py_None);
1445 self->next_proto_select_callback = Py_None;
1446
1447 Py_INCREF(Py_None);
1244 self->tlsext_servername_callback = Py_None;1448 self->tlsext_servername_callback = Py_None;
12451449
1246 Py_INCREF(Py_None);1450 Py_INCREF(Py_None);
@@ -1320,6 +1524,10 @@
1320 ret = visit((PyObject *)self->verify_callback, arg);1524 ret = visit((PyObject *)self->verify_callback, arg);
1321 if (ret == 0 && self->info_callback != NULL)1525 if (ret == 0 && self->info_callback != NULL)
1322 ret = visit((PyObject *)self->info_callback, arg);1526 ret = visit((PyObject *)self->info_callback, arg);
1527 if (ret == 0 && self->next_protos_advertised_callback != NULL)
1528 ret = visit((PyObject *)self->next_protos_advertised_callback, arg);
1529 if (ret == 0 && self->next_proto_select_callback != NULL)
1530 ret = visit((PyObject *)self->next_proto_select_callback, arg);
1323 if (ret == 0 && self->app_data != NULL)1531 if (ret == 0 && self->app_data != NULL)
1324 ret = visit(self->app_data, arg);1532 ret = visit(self->app_data, arg);
1325 return ret;1533 return ret;
@@ -1342,6 +1550,10 @@
1342 self->verify_callback = NULL;1550 self->verify_callback = NULL;
1343 Py_XDECREF(self->info_callback);1551 Py_XDECREF(self->info_callback);
1344 self->info_callback = NULL;1552 self->info_callback = NULL;
1553 Py_XDECREF(self->next_protos_advertised_callback);
1554 self->next_protos_advertised_callback = NULL;
1555 Py_XDECREF(self->next_proto_select_callback);
1556 self->next_proto_select_callback = NULL;
1345 Py_XDECREF(self->app_data);1557 Py_XDECREF(self->app_data);
1346 self->app_data = NULL;1558 self->app_data = NULL;
1347 return 0;1559 return 0;
13481560
=== modified file 'OpenSSL/ssl/context.h'
--- OpenSSL/ssl/context.h 2011-05-26 22:47:00 +0000
+++ OpenSSL/ssl/context.h 2012-02-10 13:47:29 +0000
@@ -29,6 +29,8 @@
29 *passphrase_userdata,29 *passphrase_userdata,
30 *verify_callback,30 *verify_callback,
31 *info_callback,31 *info_callback,
32 *next_protos_advertised_callback,
33 *next_proto_select_callback,
32 *tlsext_servername_callback,34 *tlsext_servername_callback,
33 *app_data;35 *app_data;
34 PyThreadState *tstate;36 PyThreadState *tstate;
3537
=== modified file 'OpenSSL/test/test_ssl.py'
--- OpenSSL/test/test_ssl.py 2011-09-11 14:01:31 +0000
+++ OpenSSL/test/test_ssl.py 2012-02-10 13:47:29 +0000
@@ -587,6 +587,50 @@
587 # Kind of lame. Just make sure it got called somehow.587 # Kind of lame. Just make sure it got called somehow.
588 self.assertTrue(called)588 self.assertTrue(called)
589589
590 def test_next_proto(self):
591 if not hasattr(Context, 'set_next_proto_select_callback'):
592 return
593
594 (server, client) = socket_pair()
595
596 server_protos = ['spdy/2', 'http/1.1',]
597
598 clientContext = Context(TLSv1_METHOD)
599 select_called = []
600 def next_proto_select(conn, protos):
601 self.assertEqual(server_protos, protos)
602 select_called.append(conn)
603 return protos[0]
604 clientContext.set_next_proto_select_callback(next_proto_select)
605 clientSSL = Connection(clientContext, client)
606 clientSSL.set_connect_state()
607
608 serverContext = Context(TLSv1_METHOD)
609 advertised_called = []
610 def next_protos(conn):
611 advertised_called.append(conn)
612 return server_protos
613 serverContext.set_next_protos_advertised_callback(next_protos)
614 serverContext.use_certificate(
615 load_certificate(FILETYPE_PEM, cleartextCertificatePEM))
616 serverContext.use_privatekey(
617 load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM))
618
619 serverSSL = Connection(serverContext, server)
620 serverSSL.set_accept_state()
621
622 while not (advertised_called and select_called):
623 for ssl in clientSSL, serverSSL:
624 try:
625 ssl.do_handshake()
626 except WantReadError:
627 pass
628
629 # Kind of lame. Just make sure it got called somehow.
630 self.assertTrue(select_called)
631 self.assertTrue(advertised_called)
632 self.assertEqual(server_protos[0], serverSSL.get_next_proto_negotiated())
633
590634
591 def _load_verify_locations_test(self, *args):635 def _load_verify_locations_test(self, *args):
592 """636 """

Subscribers

People subscribed via source and target branches

to status/vote changes: