From 614d6737d84294b038eead384100e2a7a65f717b Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Wed, 17 Feb 2021 20:06:26 +0100 Subject: 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: --- CHANGELOG.rst | 3 +++ src/OpenSSL/SSL.py | 21 +++++++++++++++++++-- tests/test_ssl.py | 9 +++++++++ 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 `_ + 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 -- cgit v1.2.1