extmod/modtls_mbedtls: Close transport on handshake failure - #19630

Summary

The existing MicroPython mbedtls implementation doesn't correctly close the underlying transport sockets on fatal SSL handshake errors that occur during read/write operations. It only deletes the reference to them, making future cleanup attempts no-ops.

This bug affects web server projects (Such as those leveraging Microdot) which target chromium-based browsers.

Unlike Firefox - when chromium encounters a self-signed certificate during the TLS handshake - It closes the connection before re-opening a new one even if the browser user explicitly accepted the warning prompt. This sub-optimal behavior is admitted in chromium sources as well (see the spoiler below).

Chrome's behavior combined with MicroPython's clean-up bug resulted in the device running out of free sockets to use, because all got stuck waiting for their time-outs, causing all requests to be dropped with ERR_CONNECTION_RESET errors. The server would become responsive once more after the underlying transports timed out and cleaned-up on their own.

Chromium isn't to blame for this bug - it merely triggers aggressively and makes it apparent.

This bug presumably affects all ports relying on mbedtls.

Testing

I was able to replicate this bug on ESP32 S2 and ESP32 C3 boards using MicroPython version 1.28.0 through actual socket exhaustion. And on the Unix port as well using the updated ssl_poll.py coverage test.

The Microdot-utilizing project in which I discovered this bug no longer experiences the ERR_CONNECTION_RESET errors anymore after the fix either, but it did before the code changes included in this PR.

I have included an additional check in the ssl_poll.py that ensures the socket was closed exactly once. In the bug-affected builds, this test caught the problem. After testing the code in this PR the test passes cleanly.

NOTE:

I suspect the original ssl_poll.py had an unrelated bug:

def assert_raises(cb, *args, **kwargs):
    try:
        cb(*args, **kwargs)
        raise AssertionError("should have raised")
    except Exception as exc:
        pass

The AssertionError("should have raised") is raised in a way where it immediately gets caught and discarded. I assume the intent was to raise an AssertionError() only when cb() doesn't raise anything. So I changed this function to look like this:

def assert_raises(cb, *args, **kwargs):
    try:
        cb(*args, **kwargs)
    except Exception:
        return
    raise AssertionError("should have raised")

Returning plainly if the cb() function raises as expected, raising an AssertionError() if not. Please let me know if the intent behind the code was different than what I took it to be.

Trade-offs and Alternatives

I am not aware of any worthwhile alternative approach that wouldn't only be mitigating the symptoms of this bug - or moving the burden of cleaning up to the Python code (which feels like inappropriately making Python code responsible for C-level lifecycles).

Generative AI

I used generative AI tools when creating this PR, but a human has checked the
code and is responsible for the code and the description above.

This PR was created after a long ai-assisted debugging session examining why self-signed certificates caused odd ERR_CONNECTION_RESET failures in chromium-based browsers and not others (like Firefox).

The code included in it was manually adjusted by me and manually reviewed by me and my colleagues.