From f90e368cdd11654b7e68fbde98c561177b333671 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Fri, 11 Mar 2016 11:21:13 +0100 Subject: Fix set_cipher_list on modern OpenSSL Also port forward a few changes from #422. --- .travis.yml | 8 ------ doc/api/ssl.rst | 14 ++------- setup.py | 2 +- src/OpenSSL/SSL.py | 27 ++++++++++-------- src/OpenSSL/_util.py | 21 ++++++++++++-- tests/test_ssl.py | 80 ++++++++++++++++++++++++++-------------------------- tox.ini | 2 +- 7 files changed, 79 insertions(+), 75 deletions(-) diff --git a/.travis.yml b/.travis.yml index d65665e..3b2e04e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -85,15 +85,7 @@ matrix: # - Let the cryptography master builds fail because they might be triggered by # cryptography changes beyond our control. - # - Also allow OS X and 0.9.8 to fail at the moment while we fix these new - # build configurations. - # - Also allow lint to fail while we fix existing lint. - # - We alloy pypy to fail until Travis fixes their infrastructure to a pypy - # with a recent enought CFFI library to run cryptography 1.0+. allow_failures: - - language: generic - os: osx - env: TOXENV=py27 OSX_OPENSSL=homebrew - env: TOXENV=py26-cryptographyMaster - env: TOXENV=py27-cryptographyMaster - env: TOXENV=py33-cryptographyMaster diff --git a/doc/api/ssl.rst b/doc/api/ssl.rst index ad4fd05..e3fe8c9 100644 --- a/doc/api/ssl.rst +++ b/doc/api/ssl.rst @@ -333,11 +333,7 @@ Context objects have the following methods: later using the :py:meth:`get_app_data` method. -.. py:method:: Context.set_cipher_list(ciphers) - - Set the list of ciphers to be used in this context. See the OpenSSL manual for - more information (e.g. :manpage:`ciphers(1)`) - +.. automethod:: Context.set_cipher_list .. py:method:: Context.set_info_callback(callback) @@ -592,11 +588,7 @@ Connection objects have the following methods: Retrieve application data as set by :py:meth:`set_app_data`. -.. py:method:: Connection.get_cipher_list() - - Retrieve the list of ciphers used by the Connection object. WARNING: This API - has changed. It used to take an optional parameter and just return a string, - but now it returns the entire list in one go. +.. automethod:: Connection.get_cipher_list .. py:method:: Connection.get_protocol_version() @@ -610,7 +602,7 @@ Connection objects have the following methods: Retrieve the version of the SSL or TLS protocol used by the Connection as a unicode string. For example, it will return ``TLSv1`` for connections - made over TLS version 1, or ``Unknown`` for connections that were not + made over TLS version 1, or ``Unknown`` for connections that were not successfully established. diff --git a/setup.py b/setup.py index 2edb712..510359d 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ # """ -Installation script for the OpenSSL module. +Installation script for the OpenSSL package. """ import codecs diff --git a/src/OpenSSL/SSL.py b/src/OpenSSL/SSL.py index 0dbabc1..5dbe52b 100644 --- a/src/OpenSSL/SSL.py +++ b/src/OpenSSL/SSL.py @@ -5,7 +5,6 @@ from itertools import count, chain from weakref import WeakValueDictionary from errno import errorcode -from six import text_type as _text_type from six import binary_type as _binary_type from six import integer_types as integer_types from six import int2byte, indexbytes @@ -15,6 +14,7 @@ from OpenSSL._util import ( lib as _lib, exception_from_error_queue as _exception_from_error_queue, native as _native, + make_assert as _make_assert, text_to_bytes_and_warn as _text_to_bytes_and_warn, path_string as _path_string, UNSPECIFIED as _UNSPECIFIED, @@ -148,6 +148,7 @@ class Error(Exception): _raise_current_error = partial(_exception_from_error_queue, Error) +_openssl_assert = _make_assert(Error) class WantReadError(Error): @@ -441,7 +442,7 @@ class Session(object): class Context(object): """ - :py:obj:`OpenSSL.SSL.Context` instances define the parameters for setting + :class:`OpenSSL.SSL.Context` instances define the parameters for setting up new SSL connections. """ _methods = { @@ -808,20 +809,22 @@ class Context(object): def set_cipher_list(self, cipher_list): """ - Change the cipher list + Set the list of ciphers to be used in this context. - :param cipher_list: A cipher list, see ciphers(1) + See the OpenSSL manual for more information (e.g. + :manpage:`ciphers(1)`). + + :param bytes cipher_list: An OpenSSL cipher string. :return: None """ - if isinstance(cipher_list, _text_type): - cipher_list = cipher_list.encode("ascii") + cipher_list = _text_to_bytes_and_warn("cipher_list", cipher_list) if not isinstance(cipher_list, bytes): - raise TypeError("cipher_list must be bytes or unicode") + raise TypeError("cipher_list must be a bytes string.") - result = _lib.SSL_CTX_set_cipher_list(self._context, cipher_list) - if not result: - _raise_current_error() + _openssl_assert( + _lib.SSL_CTX_set_cipher_list(self._context, cipher_list) + ) def set_client_ca_list(self, certificate_authorities): """ @@ -1498,9 +1501,9 @@ class Connection(object): def get_cipher_list(self): """ - Get the session cipher list + Retrieve the list of ciphers used by the Connection object. - :return: A list of cipher strings + :return: A list of native cipher strings. """ ciphers = [] for i in count(): diff --git a/src/OpenSSL/_util.py b/src/OpenSSL/_util.py index 074ef3d..cba72ad 100644 --- a/src/OpenSSL/_util.py +++ b/src/OpenSSL/_util.py @@ -1,9 +1,11 @@ -from warnings import warn import sys +import warnings from six import PY3, binary_type, text_type from cryptography.hazmat.bindings.openssl.binding import Binding + + binding = Binding() binding.init_static_locks() ffi = binding.ffi @@ -47,6 +49,21 @@ def exception_from_error_queue(exception_type): raise exception_type(errors) +def make_assert(error): + """ + Create an assert function that uses :func:`exception_from_error_queue` to + raise an exception wrapped by *error*. + """ + def openssl_assert(ok): + """ + If ok is not true-ish, retrieve the error from OpenSSL and raise it. + """ + if not ok: + exception_from_error_queue(error) + + return openssl_assert + + def native(s): """ Convert :py:class:`bytes` or :py:class:`unicode` to the native @@ -116,7 +133,7 @@ def text_to_bytes_and_warn(label, obj): returned. """ if isinstance(obj, text_type): - warn( + warnings.warn( _TEXT_WARNING.format(label), category=DeprecationWarning, stacklevel=3 diff --git a/tests/test_ssl.py b/tests/test_ssl.py index ecdb40c..98b9688 100644 --- a/tests/test_ssl.py +++ b/tests/test_ssl.py @@ -2,7 +2,7 @@ # See LICENSE for details. """ -Unit tests for :py:obj:`OpenSSL.SSL`. +Unit tests for :mod:`OpenSSL.SSL`. """ from gc import collect, get_referrers @@ -17,7 +17,7 @@ from warnings import catch_warnings, simplefilter import pytest -from six import PY3, text_type, u +from six import PY3, text_type from OpenSSL.crypto import TYPE_RSA, FILETYPE_PEM from OpenSSL.crypto import PKey, X509, X509Extension, X509Store @@ -347,6 +347,44 @@ class VersionTests(TestCase): self.assertEqual(len(versions), 5) +@pytest.fixture +def context(): + """ + A simple TLS 1.0 context. + """ + return Context(TLSv1_METHOD) + + +class TestContext(object): + @pytest.mark.parametrize("cipher_string", [ + b"hello world:AES128-SHA", + u"hello world:AES128-SHA", + ]) + def test_set_cipher_list(self, context, cipher_string): + """ + :meth:`Context.set_cipher_list` accepts both :py:obj:`bytes` naming the + ciphers which connections created with the context object will be able + to choose from. + """ + context.set_cipher_list(cipher_string) + conn = Connection(context, None) + + assert "AES128-SHA" in conn.get_cipher_list() + + @pytest.mark.parametrize("cipher_list,error", [ + (object(), TypeError), + ("imaginary-cipher", Error), + ]) + def test_set_cipher_list_wrong_args(self, context, cipher_list, error): + """ + :meth:`Context.set_cipher_list` raises :exc:`TypeError` when passed a + non-string argument and raises :exc:`OpenSSL.SSL.Error` when passed an + incorrect cipher list string. + """ + with pytest.raises(error): + context.set_cipher_list(cipher_list) + + class ContextTests(TestCase, _LoopbackMixin): """ Unit tests for :py:obj:`OpenSSL.SSL.Context`. @@ -1394,44 +1432,6 @@ class ContextTests(TestCase, _LoopbackMixin): # exception. context.set_tmp_ecdh(curve) - def test_set_cipher_list_bytes(self): - """ - :py:obj:`Context.set_cipher_list` accepts a :py:obj:`bytes` naming the - ciphers which connections created with the context object will be able - to choose from. - """ - context = Context(TLSv1_METHOD) - context.set_cipher_list(b"hello world:EXP-RC4-MD5") - conn = Connection(context, None) - self.assertEquals(conn.get_cipher_list(), ["EXP-RC4-MD5"]) - - def test_set_cipher_list_text(self): - """ - :py:obj:`Context.set_cipher_list` accepts a :py:obj:`unicode` naming - the ciphers which connections created with the context object will be - able to choose from. - """ - context = Context(TLSv1_METHOD) - context.set_cipher_list(u("hello world:EXP-RC4-MD5")) - conn = Connection(context, None) - self.assertEquals(conn.get_cipher_list(), ["EXP-RC4-MD5"]) - - def test_set_cipher_list_wrong_args(self): - """ - :py:obj:`Context.set_cipher_list` raises :py:obj:`TypeError` when - passed zero arguments or more than one argument or when passed a - non-string single argument and raises :py:obj:`OpenSSL.SSL.Error` when - passed an incorrect cipher list string. - """ - context = Context(TLSv1_METHOD) - self.assertRaises(TypeError, context.set_cipher_list) - self.assertRaises(TypeError, context.set_cipher_list, object()) - self.assertRaises( - TypeError, context.set_cipher_list, b"EXP-RC4-MD5", object() - ) - - self.assertRaises(Error, context.set_cipher_list, "imaginary-cipher") - def test_set_session_cache_mode_wrong_args(self): """ :py:obj:`Context.set_session_cache_mode` raises :py:obj:`TypeError` if diff --git a/tox.ini b/tox.ini index cf4912b..2d41636 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,7 @@ commands = openssl version python -c "import OpenSSL.SSL; print(OpenSSL.SSL.SSLeay_version(OpenSSL.SSL.SSLEAY_VERSION))" python -c "import cryptography; print(cryptography.__version__)" - coverage run --parallel -m pytest tests + coverage run --parallel -m pytest {posargs} [testenv:py27-twistedMaster] deps = -- cgit v1.2.1