Skip to content

Commit d862933

Browse files
alexreaperhulk
authored andcommitted
C locking callback (pyca#3226)
* Remove Python OpenSSL locking callback and replace it with one in C The Python OpenSSL locking callback is unsafe; if GC is triggered during the callback's invocation, it can result in the callback being invoked reentrantly, which can lead to deadlocks. This patch replaces it with one in C that gets built at compile time via cffi along with the rest of the OpenSSL binding. * fixes for some issues * unused * revert these changes * these two for good measure * missing param * sigh, syntax * delete tests that assumed an ability to mess with locks * style fixes * licensing stuff * utf8 * Unicode. Huh. What it isn't good for, absolutely nothing.
1 parent 562b9a9 commit d862933

File tree

5 files changed

+118
-95
lines changed

5 files changed

+118
-95
lines changed

Diff for: LICENSE

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
This software is made available under the terms of *either* of the licenses
22
found in LICENSE.APACHE or LICENSE.BSD. Contributions to cryptography are made
33
under the terms of *both* these licenses.
4+
5+
The code used in the OpenSSL locking callback is derived from the same in
6+
Python itself, and is licensed under the terms of the PSF License Agreement.

Diff for: LICENSE.PSF

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
1. This LICENSE AGREEMENT is between the Python Software Foundation ("PSF"), and
2+
the Individual or Organization ("Licensee") accessing and otherwise using Python
3+
2.7.12 software in source or binary form and its associated documentation.
4+
5+
2. Subject to the terms and conditions of this License Agreement, PSF hereby
6+
grants Licensee a nonexclusive, royalty-free, world-wide license to reproduce,
7+
analyze, test, perform and/or display publicly, prepare derivative works,
8+
distribute, and otherwise use Python 2.7.12 alone or in any derivative
9+
version, provided, however, that PSF's License Agreement and PSF's notice of
10+
copyright, i.e., "Copyright © 2001-2016 Python Software Foundation; All Rights
11+
Reserved" are retained in Python 2.7.12 alone or in any derivative version
12+
prepared by Licensee.
13+
14+
3. In the event Licensee prepares a derivative work that is based on or
15+
incorporates Python 2.7.12 or any part thereof, and wants to make the
16+
derivative work available to others as provided herein, then Licensee hereby
17+
agrees to include in any such work a brief summary of the changes made to Python
18+
2.7.12.
19+
20+
4. PSF is making Python 2.7.12 available to Licensee on an "AS IS" basis.
21+
PSF MAKES NO REPRESENTATIONS OR WARRANTIES, EXPRESS OR IMPLIED. BY WAY OF
22+
EXAMPLE, BUT NOT LIMITATION, PSF MAKES NO AND DISCLAIMS ANY REPRESENTATION OR
23+
WARRANTY OF MERCHANTABILITY OR FITNESS FOR ANY PARTICULAR PURPOSE OR THAT THE
24+
USE OF PYTHON 2.7.12 WILL NOT INFRINGE ANY THIRD PARTY RIGHTS.
25+
26+
5. PSF SHALL NOT BE LIABLE TO LICENSEE OR ANY OTHER USERS OF PYTHON 2.7.12
27+
FOR ANY INCIDENTAL, SPECIAL, OR CONSEQUENTIAL DAMAGES OR LOSS AS A RESULT OF
28+
MODIFYING, DISTRIBUTING, OR OTHERWISE USING PYTHON 2.7.12, OR ANY DERIVATIVE
29+
THEREOF, EVEN IF ADVISED OF THE POSSIBILITY THEREOF.
30+
31+
6. This License Agreement will automatically terminate upon a material breach of
32+
its terms and conditions.
33+
34+
7. Nothing in this License Agreement shall be deemed to create any relationship
35+
of agency, partnership, or joint venture between PSF and Licensee. This License
36+
Agreement does not grant permission to use PSF trademarks or trade name in a
37+
trademark sense to endorse or promote products or services of Licensee, or any
38+
third party.
39+
40+
8. By copying, installing or otherwise using Python 2.7.12, Licensee agrees
41+
to be bound by the terms and conditions of this License Agreement.

Diff for: src/_cffi_src/openssl/callbacks.py

+72-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
#include <openssl/ssl.h>
1313
#include <openssl/x509.h>
1414
#include <openssl/x509_vfy.h>
15+
#include <openssl/crypto.h>
16+
17+
#include <pythread.h>
1518
"""
1619

1720
TYPES = """
@@ -37,6 +40,7 @@
3740
"""
3841

3942
FUNCTIONS = """
43+
int _setup_ssl_threads(void);
4044
"""
4145

4246
MACROS = """
@@ -50,4 +54,71 @@
5054
# backwards compatibility for old cffi version on PyPy
5155
# and Python >=3.5 (https://github.com/pyca/cryptography/issues/2970)
5256
TYPES = "static const long Cryptography_STATIC_CALLBACKS;"
53-
CUSTOMIZATIONS = "static const long Cryptography_STATIC_CALLBACKS = 0;"
57+
CUSTOMIZATIONS = """static const long Cryptography_STATIC_CALLBACKS = 0;
58+
"""
59+
60+
CUSTOMIZATIONS += """
61+
/* This code is derived from the locking code found in the Python _ssl module's
62+
locking callback for OpenSSL.
63+
64+
Copyright 2001-2016 Python Software Foundation; All Rights Reserved.
65+
*/
66+
67+
static unsigned int _ssl_locks_count = 0;
68+
static PyThread_type_lock *_ssl_locks = NULL;
69+
70+
static void _ssl_thread_locking_function(int mode, int n, const char *file,
71+
int line) {
72+
/* this function is needed to perform locking on shared data
73+
structures. (Note that OpenSSL uses a number of global data
74+
structures that will be implicitly shared whenever multiple
75+
threads use OpenSSL.) Multi-threaded applications will
76+
crash at random if it is not set.
77+
78+
locking_function() must be able to handle up to
79+
CRYPTO_num_locks() different mutex locks. It sets the n-th
80+
lock if mode & CRYPTO_LOCK, and releases it otherwise.
81+
82+
file and line are the file number of the function setting the
83+
lock. They can be useful for debugging.
84+
*/
85+
86+
if ((_ssl_locks == NULL) ||
87+
(n < 0) || ((unsigned)n >= _ssl_locks_count)) {
88+
return;
89+
}
90+
91+
if (mode & CRYPTO_LOCK) {
92+
PyThread_acquire_lock(_ssl_locks[n], 1);
93+
} else {
94+
PyThread_release_lock(_ssl_locks[n]);
95+
}
96+
}
97+
98+
int _setup_ssl_threads(void) {
99+
unsigned int i;
100+
101+
if (_ssl_locks == NULL) {
102+
_ssl_locks_count = CRYPTO_num_locks();
103+
_ssl_locks = PyMem_New(PyThread_type_lock, _ssl_locks_count);
104+
if (_ssl_locks == NULL) {
105+
PyErr_NoMemory();
106+
return 0;
107+
}
108+
memset(_ssl_locks, 0, sizeof(PyThread_type_lock) * _ssl_locks_count);
109+
for (i = 0; i < _ssl_locks_count; i++) {
110+
_ssl_locks[i] = PyThread_allocate_lock();
111+
if (_ssl_locks[i] == NULL) {
112+
unsigned int j;
113+
for (j = 0; j < i; j++) {
114+
PyThread_free_lock(_ssl_locks[j]);
115+
}
116+
PyMem_Free(_ssl_locks);
117+
return 0;
118+
}
119+
}
120+
CRYPTO_set_locking_callback(_ssl_thread_locking_function);
121+
}
122+
return 1;
123+
}
124+
"""

Diff for: src/cryptography/hazmat/bindings/openssl/binding.py

+2-29
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,6 @@ class Binding(object):
118118
lib = None
119119
ffi = ffi
120120
_lib_loaded = False
121-
_locks = None
122-
_lock_cb_handle = None
123121
_init_lock = threading.Lock()
124122
_lock_init_lock = threading.Lock()
125123

@@ -178,14 +176,6 @@ def _ensure_ffi_initialized(cls):
178176
def init_static_locks(cls):
179177
with cls._lock_init_lock:
180178
cls._ensure_ffi_initialized()
181-
182-
if not cls._lock_cb_handle:
183-
wrapper = ffi_callback(
184-
"void(int, int, const char *, int)",
185-
name="Cryptography_locking_cb",
186-
)
187-
cls._lock_cb_handle = wrapper(cls._lock_cb)
188-
189179
# Use Python's implementation if available, importing _ssl triggers
190180
# the setup for this.
191181
__import__("_ssl")
@@ -195,25 +185,8 @@ def init_static_locks(cls):
195185

196186
# If nothing else has setup a locking callback already, we set up
197187
# our own
198-
num_locks = cls.lib.CRYPTO_num_locks()
199-
cls._locks = [threading.Lock() for n in range(num_locks)]
200-
201-
cls.lib.CRYPTO_set_locking_callback(cls._lock_cb_handle)
202-
203-
@classmethod
204-
def _lock_cb(cls, mode, n, file, line):
205-
lock = cls._locks[n]
206-
207-
if mode & cls.lib.CRYPTO_LOCK:
208-
lock.acquire()
209-
elif mode & cls.lib.CRYPTO_UNLOCK:
210-
lock.release()
211-
else:
212-
raise RuntimeError(
213-
"Unknown lock mode {0}: lock={1}, file={2}, line={3}.".format(
214-
mode, n, file, line
215-
)
216-
)
188+
res = lib._setup_ssl_threads()
189+
_openssl_assert(cls.lib, res == 1)
217190

218191

219192
def _verify_openssl_version(version):

Diff for: tests/hazmat/bindings/test_openssl.py

-65
Original file line numberDiff line numberDiff line change
@@ -31,71 +31,6 @@ def test_crypto_lock_init(self):
3131
lock_cb = b.lib.CRYPTO_get_locking_callback()
3232
assert lock_cb != b.ffi.NULL
3333

34-
def _skip_if_not_fallback_lock(self, b):
35-
# only run this test if we are using our locking cb
36-
original_cb = b.lib.CRYPTO_get_locking_callback()
37-
if original_cb != b._lock_cb_handle:
38-
pytest.skip(
39-
"Not using the fallback Python locking callback "
40-
"implementation. Probably because import _ssl set one"
41-
)
42-
43-
def test_fallback_crypto_lock_via_openssl_api(self):
44-
b = Binding()
45-
b.init_static_locks()
46-
47-
self._skip_if_not_fallback_lock(b)
48-
49-
# check that the lock state changes appropriately
50-
lock = b._locks[b.lib.CRYPTO_LOCK_SSL]
51-
52-
# starts out unlocked
53-
assert lock.acquire(False)
54-
lock.release()
55-
56-
b.lib.CRYPTO_lock(
57-
b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ,
58-
b.lib.CRYPTO_LOCK_SSL, b.ffi.NULL, 0
59-
)
60-
61-
# becomes locked
62-
assert not lock.acquire(False)
63-
64-
b.lib.CRYPTO_lock(
65-
b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ,
66-
b.lib.CRYPTO_LOCK_SSL, b.ffi.NULL, 0
67-
)
68-
69-
# then unlocked
70-
assert lock.acquire(False)
71-
lock.release()
72-
73-
def test_fallback_crypto_lock_via_binding_api(self):
74-
b = Binding()
75-
b.init_static_locks()
76-
77-
self._skip_if_not_fallback_lock(b)
78-
79-
lock = b._locks[b.lib.CRYPTO_LOCK_SSL]
80-
81-
with pytest.raises(RuntimeError):
82-
b._lock_cb(0, b.lib.CRYPTO_LOCK_SSL, "<test>", 1)
83-
84-
# errors shouldn't cause locking
85-
assert lock.acquire(False)
86-
lock.release()
87-
88-
b._lock_cb(b.lib.CRYPTO_LOCK | b.lib.CRYPTO_READ,
89-
b.lib.CRYPTO_LOCK_SSL, "<test>", 1)
90-
# locked
91-
assert not lock.acquire(False)
92-
93-
b._lock_cb(b.lib.CRYPTO_UNLOCK | b.lib.CRYPTO_READ,
94-
b.lib.CRYPTO_LOCK_SSL, "<test>", 1)
95-
# unlocked
96-
assert lock.acquire(False)
97-
lock.release()
98-
9934
def test_add_engine_more_than_once(self):
10035
b = Binding()
10136
b._register_osrandom_engine()

0 commit comments

Comments
 (0)