ResourceWarning in ssl.SSLSocket connected detection

[graingert]

Bug report

Bug description:

from __future__ import annotations

import sys
import contextlib
import socket
import ssl
import typing

import threading
import os

ALPN_PROTOCOLS = ["http/1.1"]

CERTS_PATH = os.path.join(os.path.dirname(__file__), "certs")
DEFAULT_CERTS: dict[str, typing.Any] = {
    "certfile": os.path.join(CERTS_PATH, "server.crt"),
    "keyfile": os.path.join(CERTS_PATH, "server.key"),
    "cert_reqs": ssl.CERT_OPTIONAL,
    "ca_certs": os.path.join(CERTS_PATH, "cacert.pem"),
    "alpn_protocols": ALPN_PROTOCOLS,
}
DEFAULT_CA = os.path.join(CERTS_PATH, "cacert.pem")
DEFAULT_CA_KEY = os.path.join(CERTS_PATH, "cacert.key")


class SocketServerThread(threading.Thread):
    """
    :param socket_handler: Callable which receives a socket argument for one
        request.
    :param ready_event: Event which gets set when the socket handler is
        ready to receive requests.
    """

    USE_IPV6 = False

    def __init__(
        self,
        socket_handler: typing.Callable[[socket.socket], None],
        host: str = "localhost",
        ready_event: threading.Event | None = None,
    ) -> None:
        super().__init__()
        self.daemon = True

        self.socket_handler = socket_handler
        self.host = host
        self.ready_event = ready_event

    def _start_server(self) -> None:
        sock = socket.socket(socket.AF_INET)
        if sys.platform != "win32":
            sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)

        with sock:
            sock.bind((self.host, 0))
            self.port = sock.getsockname()[1]

            # Once listen() returns, the server socket is ready
            sock.listen(1)

            if self.ready_event:
                self.ready_event.set()

            self.socket_handler(sock)

    def run(self) -> None:
        self._start_server()


class NewConnectionError(Exception):
    pass


def original_ssl_wrap_socket(
    sock: socket.socket,
    keyfile: StrOrBytesPath | None = None,
    certfile: StrOrBytesPath | None = None,
    server_side: bool = False,
    cert_reqs: ssl.VerifyMode = ssl.CERT_NONE,
    ssl_version: int = ssl.PROTOCOL_TLS_SERVER,
    ca_certs: str | None = None,
    do_handshake_on_connect: bool = True,
    suppress_ragged_eofs: bool = True,
    ciphers: str | None = None,
) -> ssl.SSLSocket:
    if server_side and not certfile:
        raise ValueError("certfile must be specified for server-side operations")
    if keyfile and not certfile:
        raise ValueError("certfile must be specified")
    context = ssl.SSLContext(ssl_version)
    context.verify_mode = cert_reqs
    if ca_certs:
        context.load_verify_locations(ca_certs)
    if certfile:
        context.load_cert_chain(certfile, keyfile)
    if ciphers:
        context.set_ciphers(ciphers)
    return context.wrap_socket(
        sock=sock,
        server_side=server_side,
        do_handshake_on_connect=do_handshake_on_connect,
        suppress_ragged_eofs=suppress_ragged_eofs,
    )


@contextlib.contextmanager
def _socket_server(handler):
    ready_event = threading.Event()
    server_thread = SocketServerThread(
        socket_handler=handler, ready_event=ready_event, host="localhost"
    )
    server_thread.start()
    ready_event.wait(5)
    if not ready_event.is_set():
        raise Exception("timeout")
    try:
        yield server_thread.port
    finally:
        server_thread.join()


def _test_ssl_failed_fingerprint_verification() -> None:
    def socket_handler(listener: socket.socket) -> None:
        with listener.accept()[0] as sock:
            try:
                ssl_sock = original_ssl_wrap_socket(
                    sock,
                    server_side=True,
                    keyfile=DEFAULT_CERTS["keyfile"],
                    certfile=DEFAULT_CERTS["certfile"],
                    ca_certs=DEFAULT_CA,
                )
            except (ssl.SSLError, ConnectionResetError):
                pass
            else:
                with ssl_sock:
                    ssl_sock.send(
                        b"HTTP/1.1 200 OK\r\n"
                        b"Content-Type: text/plain\r\n"
                        b"Content-Length: 5\r\n\r\n"
                        b"Hello"
                    )

    with _socket_server(socket_handler) as port:
        # GitHub's fingerprint. Valid, but not matching.
        def request() -> None:
            try:
                try:
                    sock = socket.create_connection(
                        ("localhost", port),
                        source_address=None,
                    )
                except OSError as e:
                    raise NewConnectionError(None, None)
                ssl.create_default_context().wrap_socket(
                    sock, server_hostname="localhost"
                ).close()
            except BaseException as e:
                err = e
                raise

        with contextlib.suppress(ssl.SSLCertVerificationError):
            request()
        # Should not hang, see https://github.com/urllib3/urllib3/issues/529
        with contextlib.suppress(NewConnectionError):
            request()


def test_foo():
    for i in range(100):
        print(i)
        _test_ssl_failed_fingerprint_verification()


def test_gc():
    import gc

    for i in range(5):
        gc.collect()


def main():
    test_foo()
    test_gc()


if __name__ == "__main__":
    sys.exit(main())

run like this:

python -W error test_socketlevel.py
0
1
2
3
4
Traceback (most recent call last):
  File "/usr/lib/python3.12/ssl.py", line 992, in _create
    self.getpeername()
OSError: [Errno 107] Transport endpoint is not connected

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/graingert/projects/demo-resource-warning/test_socketlevel.py", line 188, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/graingert/projects/demo-resource-warning/test_socketlevel.py", line 183, in main
    test_foo()
  File "/home/graingert/projects/demo-resource-warning/test_socketlevel.py", line 172, in test_foo
    _test_ssl_failed_fingerprint_verification()
  File "/home/graingert/projects/demo-resource-warning/test_socketlevel.py", line 166, in _test_ssl_failed_fingerprint_verification
    request()
  File "/home/graingert/projects/demo-resource-warning/test_socketlevel.py", line 155, in request
    ssl.create_default_context().wrap_socket(
  File "/usr/lib/python3.12/ssl.py", line 455, in wrap_socket
    return self.sslsocket_class._create(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/ssl.py", line 1004, in _create
    notconn_pre_handshake_data = self.recv(1)
                                 ^^^^^^^^^^^^
  File "/usr/lib/python3.12/ssl.py", line 1237, in recv
    return super().recv(buflen, flags)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ConnectionResetError: [Errno 104] Connection reset by peer
Exception ignored in: <ssl.SSLSocket fd=5, family=2, type=1, proto=6, laddr=('127.0.0.1', 54864)>
ResourceWarning: unclosed <ssl.SSLSocket fd=5, family=2, type=1, proto=6, laddr=('127.0.0.1', 54864)>

certs from here https://github.com/graingert/demo-resource-warning

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

• GH-113280: Fix ResourceWarning in ssl.SSLSocket connected detection #113282
• gh-113280: Always close socket if SSLSocket creation failed #114659
• [3.12] gh-113280: Always close socket if SSLSocket creation failed (GH-114659) #114995
• [3.11] gh-113280: Always close socket if SSLSocket creation failed (GH-114659) #114996

[graingert]

sorry the reproducer is so long, it's extracted from urllib3's test_ssl_failed_fingerprint_verification. The issue is a missing self.close() before the raise here

cpython/Lib/ssl.py

Lines 1030 to 1033 in d71fcde

	except OSError as e:
	# EINVAL occurs for recv(1) on non-connected on unix sockets.
	if e.errno not in (errno.ENOTCONN, errno.EINVAL):
	raise

[hugovk]

Thanks for hunting this down! Does it also happen in 3.11?

[graingert]

I can reproduce this on ubuntu in 3.8, 3.9, 3.10 and 3.11

[mnot]

I'm still seeing this in 3.12.3, FWIW.