From 17ed58daa9573458157b02f822f5dc471d954298 Mon Sep 17 00:00:00 2001 From: Alex Stapleton Date: Tue, 21 Jan 2014 20:19:17 +0000 Subject: Python implementation of OpenSSL locking callback --- cryptography/hazmat/backends/openssl/backend.py | 2 + cryptography/hazmat/bindings/openssl/binding.py | 45 ++++++++++ cryptography/hazmat/bindings/openssl/crypto.py | 7 +- tests/hazmat/bindings/test_openssl.py | 106 ++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 1 deletion(-) diff --git a/cryptography/hazmat/backends/openssl/backend.py b/cryptography/hazmat/backends/openssl/backend.py index ee82ba71..504ad551 100644 --- a/cryptography/hazmat/backends/openssl/backend.py +++ b/cryptography/hazmat/backends/openssl/backend.py @@ -44,6 +44,8 @@ class Backend(object): self._ffi = self._binding.ffi self._lib = self._binding.lib + self._binding.init_static_locks() + # adds all ciphers/digests for EVP self._lib.OpenSSL_add_all_algorithms() # registers available SSL/TLS ciphers and digests diff --git a/cryptography/hazmat/bindings/openssl/binding.py b/cryptography/hazmat/bindings/openssl/binding.py index 1c17a5b2..cde3bdbd 100644 --- a/cryptography/hazmat/bindings/openssl/binding.py +++ b/cryptography/hazmat/bindings/openssl/binding.py @@ -14,6 +14,7 @@ from __future__ import absolute_import, division, print_function import sys +import threading from cryptography.hazmat.bindings.utils import build_ffi @@ -70,6 +71,10 @@ class Binding(object): "x509v3", ] + _locks = None + _lock_cb_handle = None + _lock_init_lock = threading.Lock() + ffi = None lib = None @@ -95,3 +100,43 @@ class Binding(object): def is_available(cls): # OpenSSL is the only binding so for now it must always be available return True + + @classmethod + def init_static_locks(cls): + with cls._lock_init_lock: + cls._ensure_ffi_initialized() + + if not cls._lock_cb_handle: + cls._lock_cb_handle = cls.ffi.callback( + "void(int, int, const char *, int)", + cls._lock_cb + ) + + # use Python's implementation if available + + __import__("_ssl") + + if cls.lib.CRYPTO_get_locking_callback() != cls.ffi.NULL: + return + + # otherwise setup our version + + num_locks = cls.lib.CRYPTO_num_locks() + cls._locks = [threading.Lock() for n in range(num_locks)] + + cls.lib.CRYPTO_set_locking_callback(cls._lock_cb_handle) + + @classmethod + def _lock_cb(cls, mode, n, file, line): + lock = cls._locks[n] + + if mode & cls.lib.CRYPTO_LOCK: + lock.acquire() + elif mode & cls.lib.CRYPTO_UNLOCK: + lock.release() + else: + raise RuntimeError( + "Unknown lock mode {0}: lock={1}, file={2}, line={3}".format( + mode, n, file, line + ) + ) diff --git a/cryptography/hazmat/bindings/openssl/crypto.py b/cryptography/hazmat/bindings/openssl/crypto.py index 40d91bf2..81d13b73 100644 --- a/cryptography/hazmat/bindings/openssl/crypto.py +++ b/cryptography/hazmat/bindings/openssl/crypto.py @@ -27,6 +27,11 @@ static const int CRYPTO_MEM_CHECK_ON; static const int CRYPTO_MEM_CHECK_OFF; static const int CRYPTO_MEM_CHECK_ENABLE; static const int CRYPTO_MEM_CHECK_DISABLE; +static const int CRYPTO_LOCK; +static const int CRYPTO_UNLOCK; +static const int CRYPTO_READ; +static const int CRYPTO_WRITE; +static const int CRYPTO_LOCK_SSL; """ FUNCTIONS = """ @@ -43,6 +48,7 @@ void CRYPTO_set_locking_callback(void(*)(int, int, const char *, int)); void CRYPTO_set_id_callback(unsigned long (*)(void)); unsigned long (*CRYPTO_get_id_callback(void))(void); void (*CRYPTO_get_locking_callback(void))(int, int, const char *, int); +void CRYPTO_lock(int, int, const char *, int); void OPENSSL_free(void *); """ @@ -51,7 +57,6 @@ MACROS = """ void CRYPTO_add(int *, int, int); void CRYPTO_malloc_init(void); void CRYPTO_malloc_debug_init(void); - """ CUSTOMIZATIONS = """ diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py index d1e85058..b2264fb5 100644 --- a/tests/hazmat/bindings/test_openssl.py +++ b/tests/hazmat/bindings/test_openssl.py @@ -11,6 +11,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import threading +import time + +import pytest + from cryptography.hazmat.bindings.openssl.binding import Binding @@ -23,3 +28,104 @@ class TestOpenSSL(object): def test_is_available(self): assert Binding.is_available() is True + + def test_crypto_lock_init(self): + b = Binding() + b.init_static_locks() + lock_cb = b.lib.CRYPTO_get_locking_callback() + assert lock_cb != b.ffi.NULL + + def test_our_crypto_lock(self, capfd): + b = Binding() + b.init_static_locks() + + # only run this test if we are using our locking cb + original_cb = b.lib.CRYPTO_get_locking_callback() + if original_cb != b._lock_cb_handle: + pytest.skip("Not using Python locking callback implementation") + + # check that the lock state changes appropriately + lock = b._locks[b.lib.CRYPTO_LOCK_SSL] + + assert lock.acquire(False) + + lock.release() + + b.lib.CRYPTO_lock( + b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ, + b.lib.CRYPTO_LOCK_SSL, + b.ffi.NULL, + 0 + ) + + assert not lock.acquire(False) + + b.lib.CRYPTO_lock( + b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ, + b.lib.CRYPTO_LOCK_SSL, + b.ffi.NULL, + 0 + ) + + assert lock.acquire(False) + lock.release() + + # force the error path to run. + + b.lib.CRYPTO_lock( + 0, + b.lib.CRYPTO_LOCK_SSL, + b.ffi.NULL, + 0 + ) + + lock.acquire(False) + lock.release() + + out, err = capfd.readouterr() + assert "RuntimeError: Unknown lock mode" in err + + def test_crypto_lock_mutex(self): + b = Binding() + b.init_static_locks() + + # make sure whatever locking system we end up with actually acts + # like a mutex. + + self._shared_value = 0 + + def critical_loop(): + for i in range(10): + b.lib.CRYPTO_lock( + b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ, + b.lib.CRYPTO_LOCK_SSL, + b.ffi.NULL, + 0 + ) + + assert self._shared_value == 0 + self._shared_value += 1 + time.sleep(0.01) + assert self._shared_value == 1 + self._shared_value = 0 + + b.lib.CRYPTO_lock( + b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ, + b.lib.CRYPTO_LOCK_SSL, + b.ffi.NULL, + 0 + ) + + threads = [] + for x in range(10): + t = threading.Thread(target=critical_loop) + t.daemon = True + t.start() + + threads.append(t) + + while threads: + for t in threads: + t.join(0.1) + if not t.is_alive(): + threads.remove(t) -- cgit v1.2.3 From 06649cf8bcd764c51c9ee819f43a5a0a29290a38 Mon Sep 17 00:00:00 2001 From: Alex Stapleton Date: Wed, 22 Jan 2014 15:07:38 +0000 Subject: Also test the locking cb directly --- tests/hazmat/bindings/test_openssl.py | 39 ++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py index b2264fb5..e5094133 100644 --- a/tests/hazmat/bindings/test_openssl.py +++ b/tests/hazmat/bindings/test_openssl.py @@ -47,43 +47,46 @@ class TestOpenSSL(object): # check that the lock state changes appropriately lock = b._locks[b.lib.CRYPTO_LOCK_SSL] + # starts out unlocked assert lock.acquire(False) - lock.release() b.lib.CRYPTO_lock( b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ, - b.lib.CRYPTO_LOCK_SSL, - b.ffi.NULL, - 0 + b.lib.CRYPTO_LOCK_SSL, b.ffi.NULL, 0 ) + # becomes locked assert not lock.acquire(False) b.lib.CRYPTO_lock( b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ, - b.lib.CRYPTO_LOCK_SSL, - b.ffi.NULL, - 0 + b.lib.CRYPTO_LOCK_SSL, b.ffi.NULL, 0 ) + # then unlocked assert lock.acquire(False) lock.release() - # force the error path to run. + # then test directly - b.lib.CRYPTO_lock( - 0, - b.lib.CRYPTO_LOCK_SSL, - b.ffi.NULL, - 0 - ) + with pytest.raises(RuntimeError): + b._lock_cb(0, b.lib.CRYPTO_LOCK_SSL, "", 1) - lock.acquire(False) + # errors shouldnt cause locking + assert lock.acquire(False) lock.release() - out, err = capfd.readouterr() - assert "RuntimeError: Unknown lock mode" in err + b._lock_cb(b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ, + b.lib.CRYPTO_LOCK_SSL, "", 1) + # locked + assert not lock.acquire(False) + + b._lock_cb(b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ, + b.lib.CRYPTO_LOCK_SSL, "", 1) + # unlocked + assert lock.acquire(False) + lock.release() def test_crypto_lock_mutex(self): b = Binding() @@ -119,9 +122,7 @@ class TestOpenSSL(object): threads = [] for x in range(10): t = threading.Thread(target=critical_loop) - t.daemon = True t.start() - threads.append(t) while threads: -- cgit v1.2.3 From fcae15014dd9510969615a7f5100704955bbeb64 Mon Sep 17 00:00:00 2001 From: Alex Stapleton Date: Thu, 23 Jan 2014 20:43:34 +0000 Subject: Some docs --- docs/hazmat/bindings/openssl.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/hazmat/bindings/openssl.rst b/docs/hazmat/bindings/openssl.rst index 373fe472..77623b22 100644 --- a/docs/hazmat/bindings/openssl.rst +++ b/docs/hazmat/bindings/openssl.rst @@ -22,6 +22,26 @@ These are `CFFI`_ bindings to the `OpenSSL`_ C library. This is a ``cffi`` library. It can be used to call OpenSSL functions, and access constants. + .. classmethod:: init_static_locks + + Enables the best available locking callback for OpenSSL. + See :ref:`openssl-threading`. + +.. _openssl-threading: + +Threading +--------- + +`cryptography` enables OpenSSLs `thread safety facilities`_ in two different +ways depending on the configuration of your system. Normally the locking +callbacks provided by your Python implementation specifically for OpenSSL will +be used. However if you have linked `cryptography` to a different version of +OpenSSL than that used by your Python implementation we enable an alternative +locking callback. This version is implemented in Python and so may result in +lower performance in some situations. In particular parallelism is reduced +because it has to acquire the GIL whenever any lock operations occur within +OpenSSL. .. _`CFFI`: https://cffi.readthedocs.org/ .. _`OpenSSL`: https://www.openssl.org/ +.. _`thread safety facilities`: http://www.openssl.org/docs/crypto/threads.html -- cgit v1.2.3 From 7ce51f26bb10dc56483a5f0e9639092ccb2f9d5c Mon Sep 17 00:00:00 2001 From: Alex Stapleton Date: Thu, 23 Jan 2014 20:43:49 +0000 Subject: Split a test in half --- tests/hazmat/bindings/test_openssl.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py index e5094133..43a07760 100644 --- a/tests/hazmat/bindings/test_openssl.py +++ b/tests/hazmat/bindings/test_openssl.py @@ -35,14 +35,20 @@ class TestOpenSSL(object): lock_cb = b.lib.CRYPTO_get_locking_callback() assert lock_cb != b.ffi.NULL - def test_our_crypto_lock(self, capfd): - b = Binding() - b.init_static_locks() - + def _skip_if_not_fallback_lock(self, b): # only run this test if we are using our locking cb original_cb = b.lib.CRYPTO_get_locking_callback() if original_cb != b._lock_cb_handle: - pytest.skip("Not using Python locking callback implementation") + pytest.skip( + "Not using the fallback Python locking callback " + "implementation. Probably because import _ssl set one" + ) + + def test_fallback_crypto_lock_via_openssl_api(self): + b = Binding() + b.init_static_locks() + + self._skip_if_not_fallback_lock(b) # check that the lock state changes appropriately lock = b._locks[b.lib.CRYPTO_LOCK_SSL] @@ -68,7 +74,13 @@ class TestOpenSSL(object): assert lock.acquire(False) lock.release() - # then test directly + def test_fallback_crypto_lock_via_binding_api(self): + b = Binding() + b.init_static_locks() + + self._skip_if_not_fallback_lock(b) + + lock = b._locks[b.lib.CRYPTO_LOCK_SSL] with pytest.raises(RuntimeError): b._lock_cb(0, b.lib.CRYPTO_LOCK_SSL, "", 1) -- cgit v1.2.3 From 5fb7eb442d56642f201ba016fa5e8f99943f0bfe Mon Sep 17 00:00:00 2001 From: Alex Stapleton Date: Fri, 24 Jan 2014 19:02:33 +0000 Subject: Backticks --- docs/hazmat/bindings/openssl.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/hazmat/bindings/openssl.rst b/docs/hazmat/bindings/openssl.rst index 77623b22..557f8c4d 100644 --- a/docs/hazmat/bindings/openssl.rst +++ b/docs/hazmat/bindings/openssl.rst @@ -32,10 +32,10 @@ These are `CFFI`_ bindings to the `OpenSSL`_ C library. Threading --------- -`cryptography` enables OpenSSLs `thread safety facilities`_ in two different +``cryptography`` enables OpenSSLs `thread safety facilities`_ in two different ways depending on the configuration of your system. Normally the locking callbacks provided by your Python implementation specifically for OpenSSL will -be used. However if you have linked `cryptography` to a different version of +be used. However if you have linked ``cryptography`` to a different version of OpenSSL than that used by your Python implementation we enable an alternative locking callback. This version is implemented in Python and so may result in lower performance in some situations. In particular parallelism is reduced -- cgit v1.2.3 From 29446cd8315985680fd2af0d0137c3d1c4c2a4a1 Mon Sep 17 00:00:00 2001 From: Alex Stapleton Date: Fri, 24 Jan 2014 19:04:01 +0000 Subject: Remove the contenious test of contention --- tests/hazmat/bindings/test_openssl.py | 46 ----------------------------------- 1 file changed, 46 deletions(-) diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py index 43a07760..35eb7e8d 100644 --- a/tests/hazmat/bindings/test_openssl.py +++ b/tests/hazmat/bindings/test_openssl.py @@ -11,9 +11,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import threading -import time - import pytest from cryptography.hazmat.bindings.openssl.binding import Binding @@ -99,46 +96,3 @@ class TestOpenSSL(object): # unlocked assert lock.acquire(False) lock.release() - - def test_crypto_lock_mutex(self): - b = Binding() - b.init_static_locks() - - # make sure whatever locking system we end up with actually acts - # like a mutex. - - self._shared_value = 0 - - def critical_loop(): - for i in range(10): - b.lib.CRYPTO_lock( - b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ, - b.lib.CRYPTO_LOCK_SSL, - b.ffi.NULL, - 0 - ) - - assert self._shared_value == 0 - self._shared_value += 1 - time.sleep(0.01) - assert self._shared_value == 1 - self._shared_value = 0 - - b.lib.CRYPTO_lock( - b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ, - b.lib.CRYPTO_LOCK_SSL, - b.ffi.NULL, - 0 - ) - - threads = [] - for x in range(10): - t = threading.Thread(target=critical_loop) - t.start() - threads.append(t) - - while threads: - for t in threads: - t.join(0.1) - if not t.is_alive(): - threads.remove(t) -- cgit v1.2.3