Requests v2.32.0-v2.32.3 caused a segmentation fault with multiple simultaneous requests with a certificate.

[alloni]

We are having intermittent segmentation faults on requests (using requests v2.32.0-v2.32.3, urllib3 v1.26.19). On investigation with faulthandler, they occur when a certificate is passed in, and two separate threads are trying to access the same ssl_context at the same time - one updating it with the given cert, one reading:

Current thread 0x00007f46c114d700 (most recent call first):
  File "<library path>/site-packages/urllib3/util/ssl_.py", line 418 in ssl_wrap_socket
  File "<library path>/site-packages/urllib3/connection.py", line 419 in connect
  File "<library path>/site-packages/urllib3/connectionpool.py", line 1060 in _validate_conn
  File "<library path>/site-packages/urllib3/connectionpool.py", line 404 in _make_request
  File "<library path>/site-packages/urllib3/connectionpool.py", line 715 in urlopen
  File "<library path>/site-packages/requests/adapters.py", line 667 in send
  File "<library path>/site-packages/requests/sessions.py", line 703 in send
  File "<library path>/site-packages/requests/sessions.py", line 589 in request
  File "<library path>/site-packages/requests/sessions.py", line 602 in get
  ...

Thread 0x00007f469e7fc700 (most recent call first):
  File "/usr/ssl.py", line 1382 in do_handshake
  File "/usr/ssl.py", line 1104 in _create
  File "/usr/ssl.py", line 517 in wrap_socket
  File "<library path>/site-packages/urllib3/util/ssl_.py", line 493 in _ssl_wrap_socket_impl
  File "<library path>/site-packages/urllib3/util/ssl_.py", line 449 in ssl_wrap_socket
  File "<library path>/site-packages/urllib3/connection.py", line 419 in connect
  File "<library path>/site-packages/urllib3/connectionpool.py", line 1060 in _validate_conn
  File "<library path>/site-packages/urllib3/connectionpool.py", line 404 in _make_request
  File "<library path>/site-packages/urllib3/connectionpool.py", line 715 in urlopen
  File "<library path>/site-packages/requests/adapters.py", line 667 in send
  File "<library path>/site-packages/requests/sessions.py", line 703 in send
  File "<library path>/site-packages/requests/sessions.py", line 589 in request
  File "<library path>/site-packages/requests/sessions.py", line 602 in get
  ...

This seems to happen because requests v2.32 switched from having a request without an SSL context create a new one to using the same preloaded SSL context across multiple requests. (This is #6745 with more information as to what exactly is going on - opening a new issue as the original issue creator has left the company.)

Expected Result

No segmentation faults.

Actual Result

Got intermittent segmentation faults.

Reproduction Steps

As per the original issue - and note that it only crashes if a generated certificate is passed in - openssl req -x509 -newkey rsa:4096 -keyout client.key -out client.crt -days 365 -nodes:

import concurrent.futures
import random
import uuid
from threading import Thread
from time import time

import requests


def do_request():
    start = time()
    random_id = uuid.uuid4()
    delay = random.randint(1, 5)
    print("start {} delay {} seconds".format(random_id, delay))
    endpoints = []
    endpoints.append('https://httpbin.org/delay/' + str(delay))
    delay = str(random.randint(1, 5)) + 's'
    endpoints.append('https://run.mocky.io/v3/0432e9f0-674f-45bd-9c18-628b861c2258?mocky-delay=' + str(delay))
    random.shuffle(endpoints)
    response = None
    for endpoint in endpoints:
        try:
            print("start {} delay {} seconds".format(random_id, endpoint))
            if 'run' in endpoint:
                cert = './client.crt', './client.key'
                response = requests.get(endpoint, timeout=random.randint(1, 5), cert=cert)
            else:
                response = requests.get(endpoint, timeout=random.randint(1, 5))
        except Exception as e:
            print(e)
    end = time()


    print("finished {} in {} seconds".format(random_id, end - start))
    return response


def measure():
    cnt = 20
    with concurrent.futures.ThreadPoolExecutor(max_workers=10) as executor:
        futures = []
        for server in range(1, cnt):
            futures.append(executor.submit(do_request))
        for future in concurrent.futures.as_completed(futures):
            pass


for i in range(1, 500):
    threads = [Thread(target=measure, args=()) for _ in range(5)]

    for t in threads: t.start()
    for t in threads: t.join()

System Information

{
  "chardet": {
    "version": null
  },
  "charset_normalizer": {
    "version": "3.4.1"
  },
  "cryptography": {
    "version": "43.0.1"
  },
  "idna": {
    "version": "3.10"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.11.11"
  },
  "platform": {
    "release": "4.18.0-553.34.1.el8_10.x86_64",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "30300020",
    "version": "24.2.1"
  },
  "requests": {
    "version": "2.32.3"
  },
  "system_ssl": {
    "version": "101010bf"
  },
  "urllib3": {
    "version": "1.26.19"
  },
  "using_charset_normalizer": true,
  "using_pyopenssl": true
}

(Attempted with requests v2.32.0, v2.32.2, v2.32.3, urllib3 v1.26.18, v1.26.19.)

[alloni]

@saksham-sak - that's fine for a workaround - and I will be implementing that - but it would still be a good idea to add a mutex around the SSL context read/update to prevent this happening to anyone else.

[sigmavirus24]

If you're finding that an SSLContext is not thread safe you should instead file that bug with CPython, with the most minimal reproduction case possible in order to get a real fix. I know for a fact it disables the GIL in _ssl.c and uses openssl's locking logic, so it's also possible this an OpenSSL issue although I imagine something like this would have already been found.

[alloni]

@sigmavirus24 - Looks like this is documented in CPython - as per https://docs.python.org/3/library/ssl.html#ssl.SSLContext:

Note - SSLContext is designed to be shared and used by multiple connections. Thus, it is thread-safe as long as it is not reconfigured after being used by a connection.

So it's explicitly documented as not thread-safe under these circumstances.

[sigmavirus24]

Ah, I see this is effectively a duplicate of #6730 and #6726