Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-124984: Fix ssl thread safety #124993

Merged
merged 60 commits into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from 59 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
132a2dd
Make SSL objects thread safe.
ZeroIntensity Oct 5, 2024
572e48f
Fix silly syntax errors.
ZeroIntensity Oct 5, 2024
b0cd005
Fix thread safety for set_alpn_protocols
ZeroIntensity Oct 5, 2024
0629acd
Fix thread safety for configure_hostname
ZeroIntensity Oct 5, 2024
3712d05
Make some more operations atomic.
ZeroIntensity Oct 5, 2024
eb06e05
Fix thread safety with hostnames.
ZeroIntensity Oct 5, 2024
30fce36
Fix thread safety for BIO.
ZeroIntensity Oct 5, 2024
0d63bfb
Add blurb.
ZeroIntensity Oct 5, 2024
7268ebb
Add test.
ZeroIntensity Oct 5, 2024
28004f1
Fix deadlock.
ZeroIntensity Oct 5, 2024
caa205a
Remove useless locks in constructor.
ZeroIntensity Oct 5, 2024
578d40a
Zero-out locks upon initialization.
ZeroIntensity Oct 5, 2024
c601906
Wrap lock initializations with #ifdef Py_GIL_DISABLED
ZeroIntensity Oct 5, 2024
014a95a
Fix lock ordering deadlock.
ZeroIntensity Oct 5, 2024
ebb8b40
Remove lock for do_handshake()
ZeroIntensity Oct 5, 2024
a3ff154
Fix typo.
ZeroIntensity Oct 5, 2024
591535f
Make tests better.
ZeroIntensity Oct 5, 2024
53e6d59
Fix tests.
ZeroIntensity Oct 5, 2024
cd047ba
Fix missing global.
ZeroIntensity Oct 5, 2024
3f3715b
Update comment.
ZeroIntensity Oct 5, 2024
d808932
Update Modules/_ssl.c
ZeroIntensity Oct 5, 2024
df26ffe
Update Modules/_ssl.c
ZeroIntensity Oct 5, 2024
288c9a3
Merge branch 'fix-ssl-threadsafety' of https://github.com/ZeroIntensi…
ZeroIntensity Oct 5, 2024
c8df918
Don't lock Py* functions.
ZeroIntensity Oct 5, 2024
23aa4f9
Clarify comment.
ZeroIntensity Oct 5, 2024
92d9525
Remove useless functions in threaded tests and lower number of threads.
ZeroIntensity Oct 5, 2024
eff01db
Remove more useless functions.
ZeroIntensity Oct 5, 2024
f678df1
Add note about thread count
ZeroIntensity Oct 5, 2024
e96bf31
Clarify news entry.
ZeroIntensity Oct 7, 2024
028bb6a
Remove lock for deallocator.
ZeroIntensity Oct 7, 2024
f578c8e
Remove PySSL_LOCK() and PySSL_UNLOCK()
ZeroIntensity Oct 10, 2024
80aea32
Use argument clinic for critical sections.
ZeroIntensity Oct 10, 2024
7d76621
Switch reused_session to clinic
ZeroIntensity Oct 10, 2024
44a8c48
Switch session to argument clinic.
ZeroIntensity Oct 10, 2024
6e13fed
Switch owner to argument clinic.
ZeroIntensity Oct 10, 2024
2424f67
Switch server_hostname to argument clinic.
ZeroIntensity Oct 10, 2024
9b91bc0
Switch context to argument clinic.
ZeroIntensity Oct 10, 2024
c0377eb
Switch check_hostname to argument clinic.
ZeroIntensity Oct 10, 2024
be02857
Switch host_flags to argument clinic.
ZeroIntensity Oct 10, 2024
fe362d4
Switch minimum_version to argument clinic.
ZeroIntensity Oct 10, 2024
fe088d6
Switch maximum_version to argument clinic.
ZeroIntensity Oct 10, 2024
f9ae852
Switch sni_callback to argument clinic.
ZeroIntensity Oct 10, 2024
eb61a84
Switch num_tickets to clinic.
ZeroIntensity Oct 10, 2024
b9c2732
Switch options to argument clinic.
ZeroIntensity Oct 10, 2024
ae25f65
Switch protocol to argument clinic.
ZeroIntensity Oct 10, 2024
4950f45
Switch verify_flags to argument clinic.
ZeroIntensity Oct 10, 2024
fee2d4f
Switch verify_mode to argument clinic.
ZeroIntensity Oct 10, 2024
6449c95
Switch security_level to argument clinic.
ZeroIntensity Oct 10, 2024
a0fc838
Switch MemoryBIO to argument clinic.
ZeroIntensity Oct 10, 2024
8c4e20d
Switch has_ticket to argument clinic.
ZeroIntensity Oct 10, 2024
44f1782
Switch session_id to argument clinic.
ZeroIntensity Oct 10, 2024
216c318
Switch ticket_lifetime_hint to argument clinic.
ZeroIntensity Oct 10, 2024
701bf00
Switch time to argument clinic.
ZeroIntensity Oct 10, 2024
ffc4879
Switch timeout to argument clinic.
ZeroIntensity Oct 10, 2024
ebd621b
Fix name inconsistency.
ZeroIntensity Oct 10, 2024
3b81e78
Fix build complaints.
ZeroIntensity Oct 10, 2024
e7e0f47
Fix more build complaints.
ZeroIntensity Oct 10, 2024
8cf4992
Fix failing builds.
ZeroIntensity Oct 11, 2024
d9152af
Clarify comment and use catch_threading_exception()
ZeroIntensity Oct 18, 2024
07f848c
Check for SkipTest in exc_value.
ZeroIntensity Oct 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import unittest
import unittest.mock
from ast import literal_eval
from threading import Thread
from test import support
from test.support import import_helper
from test.support import os_helper
Expand Down Expand Up @@ -277,11 +278,19 @@ def test_wrap_socket(sock, *,
return context.wrap_socket(sock, **kwargs)


USE_SAME_TEST_CONTEXT = False
_TEST_CONTEXT = None

def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True):
"""Create context

client_context, server_context, hostname = testing_context()
"""
global _TEST_CONTEXT
if USE_SAME_TEST_CONTEXT:
if _TEST_CONTEXT is not None:
return _TEST_CONTEXT

if server_cert == SIGNED_CERTFILE:
hostname = SIGNED_CERTFILE_HOSTNAME
elif server_cert == SIGNED_CERTFILE2:
Expand All @@ -299,6 +308,10 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True):
if server_chain:
server_context.load_verify_locations(SIGNING_CA)

if USE_SAME_TEST_CONTEXT:
if _TEST_CONTEXT is not None:
_TEST_CONTEXT = client_context, server_context, hostname

return client_context, server_context, hostname


Expand Down Expand Up @@ -2800,6 +2813,41 @@ def test_echo(self):
'Cannot create a client socket with a PROTOCOL_TLS_SERVER context',
str(e.exception))

@unittest.skipUnless(support.Py_GIL_DISABLED, "test is only useful if the GIL is disabled")
def test_ssl_in_multiple_threads(self):
# See GH-124984: OpenSSL is not thread safe.
threads = []

global USE_SAME_TEST_CONTEXT
USE_SAME_TEST_CONTEXT = True
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
try:
for func in (
self.test_echo,
self.test_alpn_protocols,
self.test_getpeercert,
self.test_crl_check,
self.test_check_hostname_idn,
self.test_wrong_cert_tls12,
self.test_wrong_cert_tls13,
):
# Be careful with the number of threads here.
# Too many can result in failing tests.
for num in range(5):
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
with self.subTest(func=func, num=num):
threads.append(Thread(target=func))
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved

with threading_helper.catch_threading_exception() as cm:
for thread in threads:
with self.subTest(thread=thread):
thread.start()

for thread in threads:
with self.subTest(thread=thread):
thread.join()
self.assertIsNone(cm.exc_value)
finally:
USE_SAME_TEST_CONTEXT = False

def test_getpeercert(self):
if support.verbose:
sys.stdout.write("\n")
Expand Down
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed thread safety in :mod:`ssl` in the free-threaded build. OpenSSL operations are now protected by a per-object lock.
Loading
Loading