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
1=== modified file 'OpenSSL/ssl/connection.c'
2--- OpenSSL/ssl/connection.c 2011-07-16 05:14:58 +0000
3+++ OpenSSL/ssl/connection.c 2012-02-10 13:47:29 +0000
4@@ -327,6 +327,29 @@
5 }
6 }
7
8+#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
9+static char ssl_Connection_get_next_proto_negotiated_doc[] = "\n\
10+Get protocol that the client selected\n\
11+\n\
12+:return: A string with the protocol the client selected\n\
13+";
14+static PyObject *
15+ssl_Connection_get_next_proto_negotiated(ssl_ConnectionObj *self, PyObject *args)
16+{
17+ const unsigned char *data;
18+ unsigned len;
19+
20+ if (!PyArg_ParseTuple(args, ":get_next_proto_negotiated"))
21+ return NULL;
22+
23+ SSL_get0_next_proto_negotiated(self->ssl, &data, &len);
24+ if (len == 0) {
25+ Py_INCREF(Py_None);
26+ return Py_None;
27+ }
28+ return PyBytes_FromStringAndSize((const char *)data, (Py_ssize_t)len);
29+}
30+#endif
31
32 static char ssl_Connection_set_tlsext_host_name_doc[] = "\n\
33 Set the value of the servername extension to send in the client hello.\n\
34@@ -1271,6 +1294,9 @@
35 ADD_METHOD(get_context),
36 ADD_METHOD(set_context),
37 ADD_METHOD(get_servername),
38+#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
39+ ADD_METHOD(get_next_proto_negotiated),
40+#endif
41 ADD_METHOD(set_tlsext_host_name),
42 ADD_METHOD(pending),
43 ADD_METHOD(send),
44@@ -1339,6 +1365,9 @@
45 Py_INCREF(sock);
46 self->socket = sock;
47
48+ Py_INCREF(Py_None);
49+ self->proto_selected = Py_None;
50+
51 self->ssl = NULL;
52 self->from_ssl = NULL;
53 self->into_ssl = NULL;
54@@ -1466,6 +1495,8 @@
55 ret = visit((PyObject *)self->context, arg);
56 if (ret == 0 && self->socket != NULL)
57 ret = visit(self->socket, arg);
58+ if (ret == 0 && self->proto_selected != NULL)
59+ ret = visit((PyObject *)self->proto_selected, arg);
60 if (ret == 0 && self->app_data != NULL)
61 ret = visit(self->app_data, arg);
62 return ret;
63@@ -1484,6 +1515,8 @@
64 self->context = NULL;
65 Py_XDECREF(self->socket);
66 self->socket = NULL;
67+ Py_XDECREF(self->proto_selected);
68+ self->proto_selected = NULL;
69 Py_XDECREF(self->app_data);
70 self->app_data = NULL;
71 self->into_ssl = NULL; /* was cleaned up by SSL_free() */
72
73=== modified file 'OpenSSL/ssl/connection.h'
74--- OpenSSL/ssl/connection.h 2011-03-03 00:26:20 +0000
75+++ OpenSSL/ssl/connection.h 2012-02-10 13:47:29 +0000
76@@ -42,6 +42,7 @@
77 SSL *ssl;
78 ssl_ContextObj *context;
79 PyObject *socket;
80+ PyObject *proto_selected;
81 PyThreadState *tstate; /* This field is no longer used. */
82 PyObject *app_data;
83 BIO *into_ssl, *from_ssl; /* for connections without file descriptors */
84
85=== modified file 'OpenSSL/ssl/context.c'
86--- OpenSSL/ssl/context.c 2011-09-11 13:35:32 +0000
87+++ OpenSSL/ssl/context.c 2012-02-10 13:47:29 +0000
88@@ -237,6 +237,140 @@
89 return;
90 }
91
92+#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
93+/*
94+ * Globally defined next proto callback. This is called from OpenSSL internally.
95+ * The GIL will not be held when this function is invoked. It must not be held
96+ * when the function returns.
97+ *
98+ * Arguments: ssl - The Connection
99+ * **out - handle to where we can put a new string to be returned
100+ * *outlen - pointer to where we can put the len of the string we want to return
101+ * Returns: SSL_TLSEXT_ERR_OK
102+ */
103+static int
104+global_next_protos_advertised_callback(SSL *ssl, const unsigned char **out, unsigned int *outlen, void *arg)
105+{
106+ ssl_ConnectionObj *conn = (ssl_ConnectionObj *)SSL_get_app_data(ssl);
107+ PyObject *argv, *ret, *item;
108+ Py_ssize_t length, i, strlength;
109+ unsigned char *outptr;
110+
111+ *outlen = 0;
112+
113+ /*
114+ * GIL isn't held yet. First things first - acquire it, or any Python API
115+ * we invoke might segfault or blow up the sun. The reverse will be done
116+ * before returning.
117+ */
118+ MY_END_ALLOW_THREADS(conn->tstate);
119+
120+ argv = Py_BuildValue("(O)", (PyObject *)conn);
121+ ret = PyEval_CallObject(conn->context->next_protos_advertised_callback, argv);
122+ Py_DECREF(argv);
123+
124+ length = PySequence_Size(ret);
125+ for (i = 0; i < length; i++) {
126+ strlength = PyBytes_Size(PySequence_GetItem(ret, i));
127+ if (strlength >= 254) {
128+ PyErr_SetString(PyExc_TypeError, "string returned by next_protos_advertised_callback was greater than 254 characters");
129+ goto out;
130+ }
131+ *outlen += 1 + strlength;
132+ }
133+ *out = outptr = OPENSSL_malloc(*outlen + 1);
134+ for (i = 0; i < length; i++) {
135+ item = PySequence_GetItem(ret, i);
136+ strlength = PyBytes_Size(item);
137+ outptr[0] = Py_SAFE_DOWNCAST(strlength, Py_ssize_t, unsigned char);
138+ outptr++;
139+ memcpy(outptr, PyBytes_AsString(item), strlength);
140+ outptr += strlength;
141+ }
142+
143+ if (ret == NULL) {
144+ /*
145+ * XXX - This should be reported somehow. -exarkun
146+ */
147+ PyErr_Clear();
148+ } else {
149+ Py_DECREF(ret);
150+ }
151+
152+ out:
153+ /*
154+ * This function is returning into OpenSSL. Release the GIL again.
155+ */
156+ MY_BEGIN_ALLOW_THREADS(conn->tstate);
157+ return SSL_TLSEXT_ERR_OK;
158+}
159+
160+/*
161+ * Globally defined next proto select callback. This is called from OpenSSL internally.
162+ * The GIL will not be held when this function is invoked. It must not be held
163+ * when the function returns.
164+ *
165+ * Arguments: ssl - The Connection
166+ * **out - handle to where we can put the proto we want to pick
167+ * *outlen - pointer to where we can put the len of the proto we pick
168+ * *in - handle to where the list of protocols the server is advertising
169+ * inlen - pointer to where the lenght of protocols we can select
170+ * Returns: SSL_TLSEXT_ERR_OK
171+ */
172+static int
173+global_next_proto_select_callback(SSL *ssl, unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg)
174+{
175+ ssl_ConnectionObj *conn = (ssl_ConnectionObj *)SSL_get_app_data(ssl);
176+ PyObject *argv, *proto_list, *str, *ret;
177+ int retval = SSL_TLSEXT_ERR_ALERT_FATAL;
178+ int i;
179+
180+ *outlen = 0;
181+ /*
182+ * GIL isn't held yet. First things first - acquire it, or any Python API
183+ * we invoke might segfault or blow up the sun. The reverse will be done
184+ * before returning.
185+ */
186+ MY_END_ALLOW_THREADS(conn->tstate);
187+
188+ proto_list = PyList_New(0);
189+ for (i = 0; i < inlen; i++) {
190+ str = PyBytes_FromStringAndSize((const char *)in+i+1, (Py_ssize_t)in[i]);
191+ PyList_Append(proto_list, str);
192+ Py_DECREF(str);
193+ i += in[i];
194+ }
195+
196+ argv = Py_BuildValue("(OO)", (PyObject *)conn, proto_list);
197+ ret = PyEval_CallObject(conn->context->next_proto_select_callback, argv);
198+ Py_DECREF(argv);
199+ Py_DECREF(proto_list);
200+
201+ if (ret == NULL) {
202+ goto out;
203+ }
204+ if (!PyBytes_Check(ret)) {
205+ /*
206+ * XXX Returned something that wasn't a string. This is bogus. We'll
207+ * return 0 and OpenSSL will treat it as an error, resulting in an
208+ * exception from whatever Python API triggered this callback.
209+ */
210+ Py_DECREF(ret);
211+ goto out;
212+ }
213+ Py_DECREF(conn->proto_selected);
214+ conn->proto_selected = ret;
215+ PyBytes_AsStringAndSize(ret, (char **)out, (Py_ssize_t *)outlen);
216+ retval = SSL_TLSEXT_ERR_OK;
217+ out:
218+ /*
219+ * This function is returning into OpenSSL. Release the GIL again.
220+ */
221+ MY_BEGIN_ALLOW_THREADS(conn->tstate);
222+ return retval;
223+}
224+#endif
225+
226 /*
227 * Globally defined TLS extension server name callback. This is called from
228 * OpenSSL internally. The GIL will not be held when this function is invoked.
229@@ -1030,6 +1164,66 @@
230 return Py_None;
231 }
232
233+#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
234+static char ssl_Context_set_next_protos_advertised_callback_doc[] = "\n\
235+Set the next protos advertised callback\n\
236+\n\
237+:param callback: The Python callback to use\n\
238+:return: None\n\
239+";
240+static PyObject *
241+ssl_Context_set_next_protos_advertised_callback(ssl_ContextObj *self, PyObject *args)
242+{
243+ PyObject *callback;
244+
245+ if (!PyArg_ParseTuple(args, "O:set_next_protos_advertised_callback", &callback))
246+ return NULL;
247+
248+ if (!PyCallable_Check(callback))
249+ {
250+ PyErr_SetString(PyExc_TypeError, "expected PyCallable");
251+ return NULL;
252+ }
253+
254+ Py_DECREF(self->next_protos_advertised_callback);
255+ Py_INCREF(callback);
256+ self->next_protos_advertised_callback = callback;
257+ SSL_CTX_set_next_protos_advertised_cb(self->ctx, global_next_protos_advertised_callback, NULL);
258+
259+ Py_INCREF(Py_None);
260+ return Py_None;
261+}
262+
263+static char ssl_Context_set_next_proto_select_callback_doc[] = "\n\
264+Set the next protos select callback\n\
265+\n\
266+:param callback: The Python callback to use. This will take a list as an arg and return a str\n\
267+:return: None\n\
268+";
269+static PyObject *
270+ssl_Context_set_next_proto_select_callback(ssl_ContextObj *self, PyObject *args)
271+{
272+ PyObject *callback;
273+
274+ if (!PyArg_ParseTuple(args, "O:set_next_proto_select_callback", &callback))
275+ return NULL;
276+
277+ if (!PyCallable_Check(callback))
278+ {
279+ PyErr_SetString(PyExc_TypeError, "expected PyCallable");
280+ return NULL;
281+ }
282+
283+ Py_DECREF(self->next_proto_select_callback);
284+ Py_INCREF(callback);
285+ self->next_proto_select_callback = callback;
286+ SSL_CTX_set_next_proto_select_cb(self->ctx, global_next_proto_select_callback, NULL);
287+
288+ Py_INCREF(Py_None);
289+ return Py_None;
290+}
291+#endif
292+
293 static char ssl_Context_get_app_data_doc[] = "\n\
294 Get the application data (supplied via set_app_data())\n\
295 \n\
296@@ -1187,6 +1381,10 @@
297 ADD_METHOD(set_timeout),
298 ADD_METHOD(get_timeout),
299 ADD_METHOD(set_info_callback),
300+#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
301+ ADD_METHOD(set_next_protos_advertised_callback),
302+ ADD_METHOD(set_next_proto_select_callback),
303+#endif
304 ADD_METHOD(get_app_data),
305 ADD_METHOD(set_app_data),
306 ADD_METHOD(get_cert_store),
307@@ -1241,6 +1439,12 @@
308 self->info_callback = Py_None;
309
310 Py_INCREF(Py_None);
311+ self->next_protos_advertised_callback = Py_None;
312+
313+ Py_INCREF(Py_None);
314+ self->next_proto_select_callback = Py_None;
315+
316+ Py_INCREF(Py_None);
317 self->tlsext_servername_callback = Py_None;
318
319 Py_INCREF(Py_None);
320@@ -1320,6 +1524,10 @@
321 ret = visit((PyObject *)self->verify_callback, arg);
322 if (ret == 0 && self->info_callback != NULL)
323 ret = visit((PyObject *)self->info_callback, arg);
324+ if (ret == 0 && self->next_protos_advertised_callback != NULL)
325+ ret = visit((PyObject *)self->next_protos_advertised_callback, arg);
326+ if (ret == 0 && self->next_proto_select_callback != NULL)
327+ ret = visit((PyObject *)self->next_proto_select_callback, arg);
328 if (ret == 0 && self->app_data != NULL)
329 ret = visit(self->app_data, arg);
330 return ret;
331@@ -1342,6 +1550,10 @@
332 self->verify_callback = NULL;
333 Py_XDECREF(self->info_callback);
334 self->info_callback = NULL;
335+ Py_XDECREF(self->next_protos_advertised_callback);
336+ self->next_protos_advertised_callback = NULL;
337+ Py_XDECREF(self->next_proto_select_callback);
338+ self->next_proto_select_callback = NULL;
339 Py_XDECREF(self->app_data);
340 self->app_data = NULL;
341 return 0;
342
343=== modified file 'OpenSSL/ssl/context.h'
344--- OpenSSL/ssl/context.h 2011-05-26 22:47:00 +0000
345+++ OpenSSL/ssl/context.h 2012-02-10 13:47:29 +0000
346@@ -29,6 +29,8 @@
347 *passphrase_userdata,
348 *verify_callback,
349 *info_callback,
350+ *next_protos_advertised_callback,
351+ *next_proto_select_callback,
352 *tlsext_servername_callback,
353 *app_data;
354 PyThreadState *tstate;
355
356=== modified file 'OpenSSL/test/test_ssl.py'
357--- OpenSSL/test/test_ssl.py 2011-09-11 14:01:31 +0000
358+++ OpenSSL/test/test_ssl.py 2012-02-10 13:47:29 +0000
359@@ -587,6 +587,50 @@
360 # Kind of lame. Just make sure it got called somehow.
361 self.assertTrue(called)
362
363+ def test_next_proto(self):
364+ if not hasattr(Context, 'set_next_proto_select_callback'):
365+ return
366+
367+ (server, client) = socket_pair()
368+
369+ server_protos = ['spdy/2', 'http/1.1',]
370+
371+ clientContext = Context(TLSv1_METHOD)
372+ select_called = []
373+ def next_proto_select(conn, protos):
374+ self.assertEqual(server_protos, protos)
375+ select_called.append(conn)
376+ return protos[0]
377+ clientContext.set_next_proto_select_callback(next_proto_select)
378+ clientSSL = Connection(clientContext, client)
379+ clientSSL.set_connect_state()
380+
381+ serverContext = Context(TLSv1_METHOD)
382+ advertised_called = []
383+ def next_protos(conn):
384+ advertised_called.append(conn)
385+ return server_protos
386+ serverContext.set_next_protos_advertised_callback(next_protos)
387+ serverContext.use_certificate(
388+ load_certificate(FILETYPE_PEM, cleartextCertificatePEM))
389+ serverContext.use_privatekey(
390+ load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM))
391+
392+ serverSSL = Connection(serverContext, server)
393+ serverSSL.set_accept_state()
394+
395+ while not (advertised_called and select_called):
396+ for ssl in clientSSL, serverSSL:
397+ try:
398+ ssl.do_handshake()
399+ except WantReadError:
400+ pass
401+
402+ # Kind of lame. Just make sure it got called somehow.
403+ self.assertTrue(select_called)
404+ self.assertTrue(advertised_called)
405+ self.assertEqual(server_protos[0], serverSSL.get_next_proto_negotiated())
406+
407
408 def _load_verify_locations_test(self, *args):
409 """

Subscribers

People subscribed via source and target branches

to status/vote changes: