Check return code of SSL_[CTX_]set_alpn_protos (#993)

* check return code of SSL_CTX_set_alpn_protos, fix #992

* paint it black!

* fix line lengths as well :upside_down_face:

3 files changed, 31 insertions, 2 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 2951256..220ee00 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -16,6 +16,9 @@ Deprecations:
 Changes:
 ^^^^^^^^
 
+- Raise an error when an invalid ALPN value is set.
+  `#993 <https://github.com/pyca/pyopenssl/pull/993>`_
+
 20.0.1 (2020-12-15)
 -------------------
 
diff --git a/src/OpenSSL/SSL.py b/src/OpenSSL/SSL.py
index 1900b8c..cd1e9be 100644
--- a/src/OpenSSL/SSL.py
+++ b/src/OpenSSL/SSL.py
@@ -1375,7 +1375,17 @@ class Context(object):
         # Build a C string from the list. We don't need to save this off
         # because OpenSSL immediately copies the data out.
         input_str = _ffi.new("unsigned char[]", protostr)
-        _lib.SSL_CTX_set_alpn_protos(self._context, input_str, len(protostr))
+
+        # https://www.openssl.org/docs/man1.1.0/man3/SSL_CTX_set_alpn_protos.html:
+        # SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos()
+        # return 0 on success, and non-0 on failure.
+        # WARNING: these functions reverse the return value convention.
+        _openssl_assert(
+            _lib.SSL_CTX_set_alpn_protos(
+                self._context, input_str, len(protostr)
+            )
+            == 0
+        )
 
     @_requires_alpn
     def set_alpn_select_callback(self, callback):
@@ -2393,7 +2403,14 @@ class Connection(object):
         # Build a C string from the list. We don't need to save this off
         # because OpenSSL immediately copies the data out.
         input_str = _ffi.new("unsigned char[]", protostr)
-        _lib.SSL_set_alpn_protos(self._ssl, input_str, len(protostr))
+
+        # https://www.openssl.org/docs/man1.1.0/man3/SSL_CTX_set_alpn_protos.html:
+        # SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos()
+        # return 0 on success, and non-0 on failure.
+        # WARNING: these functions reverse the return value convention.
+        _openssl_assert(
+            _lib.SSL_set_alpn_protos(self._ssl, input_str, len(protostr)) == 0
+        )
 
     @_requires_alpn
     def get_alpn_proto_negotiated(self):
diff --git a/tests/test_ssl.py b/tests/test_ssl.py
index 8fdcae2..27f2d43 100644
--- a/tests/test_ssl.py
+++ b/tests/test_ssl.py
@@ -1888,6 +1888,15 @@ class TestApplicationLayerProtoNegotiation(object):
         assert server.get_alpn_proto_negotiated() == b"spdy/2"
         assert client.get_alpn_proto_negotiated() == b"spdy/2"
 
+    def test_alpn_call_failure(self):
+        """
+        SSL_CTX_set_alpn_protos does not like to be called with an empty
+        protocols list. Ensure that we produce a user-visible error.
+        """
+        context = Context(SSLv23_METHOD)
+        with pytest.raises(Error):
+            context.set_alpn_protos([])
+
     def test_alpn_set_on_connection(self):
         """
         The same as test_alpn_success, but setting the ALPN protocols on