Repository: pyca/pyopenssl
Pull request #876 (2019-11-18 02:17:52)

ALPN: complete handshake without accepting a client's protocols.
The callback passed to `SSL_CTX_set_alpn_select_cb` can return
`SSL_TLSEXT_ERR_NOACK` to allow the handshake to continue without
accepting any of the client's offered protocols.

This commit introduces `NO_OVERLAPPING_PROTOCOLS`, which the Python
callback passed to `Context.set_alpn_select_callback` can return to
achieve the same thing.

It does not change the previous meaning of an empty string, which
still terminates the handshake.


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e0c034d..3b39465 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -30,6 +30,8 @@ Changes:
 
 - Support ``bytearray`` in ``SSL.Connection.send()`` by using cffi's from_buffer.
   `#852 <https://github.com/pyca/pyopenssl/pull/852>`_
+- The ``OpenSSL.SSL.Context.set_alpn_select_callback`` can return a new ``NO_OVERLAPPING_PROTOCOLS`` sentinel value
+  to allow a TLS handshake to complete without an application protocol.
 
 
 ----
diff --git a/doc/api/ssl.rst b/doc/api/ssl.rst
index 1c09237..ead1452 100644
--- a/doc/api/ssl.rst
+++ b/doc/api/ssl.rst
@@ -119,6 +119,15 @@ Context, Connection.
     for details.
 
 
+.. py:data:: NO_OVERLAPPING_PROTOCOLS
+
+    A sentinel value that can be returned by the callback passed to
+    :py:meth:`Context.set_alpn_select_callback` to indicate that
+    the handshake can continue without a specific application protocol.
+
+    .. versionadded:: 19.1
+
+
 .. autofunction:: SSLeay_version
 
 
diff --git a/src/OpenSSL/SSL.py b/src/OpenSSL/SSL.py
index adcfd8f..a228b73 100644
--- a/src/OpenSSL/SSL.py
+++ b/src/OpenSSL/SSL.py
@@ -428,6 +428,9 @@ class _NpnSelectHelper(_CallbackExceptionHelper):
         )
 
 
+NO_OVERLAPPING_PROTOCOLS = object()
+
+
 class _ALPNSelectHelper(_CallbackExceptionHelper):
     """
     Wrap a callback such that it can be used as an ALPN selection callback.
@@ -453,24 +456,32 @@ class _ALPNSelectHelper(_CallbackExceptionHelper):
                     instr = instr[encoded_len + 1:]
 
                 # Call the callback
-                outstr = callback(conn, protolist)
-
-                if not isinstance(outstr, _binary_type):
-                    raise TypeError("ALPN callback must return a bytestring.")
+                outbytes = callback(conn, protolist)
+                any_accepted = True
+                if outbytes is NO_OVERLAPPING_PROTOCOLS:
+                    outbytes = b''
+                    any_accepted = False
+                elif not isinstance(outbytes, _binary_type):
+                    raise TypeError(
+                        "ALPN callback must return a bytestring or the "
+                        "special NO_OVERLAPPING_PROTOCOLS sentinel value."
+                    )
 
                 # Save our callback arguments on the connection object to make
                 # sure that they don't get freed before OpenSSL can use them.
                 # Then, return them in the appropriate output parameters.
                 conn._alpn_select_callback_args = [
-                    _ffi.new("unsigned char *", len(outstr)),
-                    _ffi.new("unsigned char[]", outstr),
+                    _ffi.new("unsigned char *", len(outbytes)),
+                    _ffi.new("unsigned char[]", outbytes),
                 ]
                 outlen[0] = conn._alpn_select_callback_args[0][0]
                 out[0] = conn._alpn_select_callback_args[1]
-                return 0
+                if not any_accepted:
+                    return _lib.SSL_TLSEXT_ERR_NOACK
+                return _lib.SSL_TLSEXT_ERR_OK
             except Exception as e:
                 self._problems.append(e)
-                return 2  # SSL_TLSEXT_ERR_ALERT_FATAL
+                return _lib.SSL_TLSEXT_ERR_ALERT_FATAL
 
         self.callback = _ffi.callback(
             ("int (*)(SSL *, unsigned char **, unsigned char *, "
@@ -1476,8 +1487,12 @@ class Context(object):
 
         :param callback: The callback function.  It will be invoked with two
             arguments: the Connection, and a list of offered protocols as
-            bytestrings, e.g ``[b'http/1.1', b'spdy/2']``.  It should return
-            one of those bytestrings, the chosen protocol.
+            bytestrings, e.g ``[b'http/1.1', b'spdy/2']``.  It can return
+            one of those bytestrings to indicate the chosen protocol, the
+            empty bytestring to terminate the TLS connection, or the
+            :py:obj:`NO_OVERLAPPING_PROTOCOLS` to indicate that no offered
+            protocol was selected, but that the connection should not be
+            aborted.
         """
         self._alpn_select_helper = _ALPNSelectHelper(callback)
         self._alpn_select_callback = self._alpn_select_helper.callback


Test patch:
diff --git a/tests/test_ssl.py b/tests/test_ssl.py
index 16767e9..e2681e3 100644
--- a/tests/test_ssl.py
+++ b/tests/test_ssl.py
@@ -67,7 +67,7 @@ from OpenSSL._util import ffi as _ffi, lib as _lib
 
 from OpenSSL.SSL import (
     OP_NO_QUERY_MTU, OP_COOKIE_EXCHANGE, OP_NO_TICKET, OP_NO_COMPRESSION,
-    MODE_RELEASE_BUFFERS)
+    MODE_RELEASE_BUFFERS, NO_OVERLAPPING_PROTOCOLS)
 
 from OpenSSL.SSL import (
     SSL_ST_CONNECT, SSL_ST_ACCEPT, SSL_ST_MASK,
@@ -1960,6 +1960,83 @@ class TestApplicationLayerProtoNegotiation(object):
 
             assert select_args == [(server, [b'http/1.1', b'spdy/2'])]
 
+        def test_alpn_no_server_overlap(self):
+            """
+            A server can allow a TLS handshake to complete without
+            agreeing to an application protocol by returning
+            ``NO_OVERLAPPING_PROTOCOLS``.
+            """
+            refusal_args = []
+
+            def refusal(conn, options):
+                refusal_args.append((conn, options))
+                return NO_OVERLAPPING_PROTOCOLS
+
+            client_context = Context(SSLv23_METHOD)
+            client_context.set_alpn_protos([b'http/1.1', b'spdy/2'])
+
+            server_context = Context(SSLv23_METHOD)
+            server_context.set_alpn_select_callback(refusal)
+
+            # Necessary to actually accept the connection
+            server_context.use_privatekey(
+                load_privatekey(FILETYPE_PEM, server_key_pem))
+            server_context.use_certificate(
+                load_certificate(FILETYPE_PEM, server_cert_pem))
+
+            # Do a little connection to trigger the logic
+            server = Connection(server_context, None)
+            server.set_accept_state()
+
+            client = Connection(client_context, None)
+            client.set_connect_state()
+
+            # Do the dance.
+            interact_in_memory(server, client)
+
+            assert refusal_args == [(server, [b'http/1.1', b'spdy/2'])]
+
+            assert client.get_alpn_proto_negotiated() == b''
+
+        def test_alpn_select_cb_returns_invalid_value(self):
+            """
+            If the ALPN selection callback returns anything other than
+            a bytestring or ``NO_OVERLAPPING_PROTOCOLS``, a
+            :py:exc:`TypeError` is raised.
+            """
+            invalid_cb_args = []
+
+            def invalid_cb(conn, options):
+                invalid_cb_args.append((conn, options))
+                return u"can't return unicode"
+
+            client_context = Context(SSLv23_METHOD)
+            client_context.set_alpn_protos([b'http/1.1', b'spdy/2'])
+
+            server_context = Context(SSLv23_METHOD)
+            server_context.set_alpn_select_callback(invalid_cb)
+
+            # Necessary to actually accept the connection
+            server_context.use_privatekey(
+                load_privatekey(FILETYPE_PEM, server_key_pem))
+            server_context.use_certificate(
+                load_certificate(FILETYPE_PEM, server_cert_pem))
+
+            # Do a little connection to trigger the logic
+            server = Connection(server_context, None)
+            server.set_accept_state()
+
+            client = Connection(client_context, None)
+            client.set_connect_state()
+
+            # Do the dance.
+            with pytest.raises(TypeError):
+                interact_in_memory(server, client)
+
+            assert invalid_cb_args == [(server, [b'http/1.1', b'spdy/2'])]
+
+            assert client.get_alpn_proto_negotiated() == b''
+
         def test_alpn_no_server(self):
             """
             When clients and servers cannot agree on what protocol to use next