ssl: Fix "timed out" exceptions

jepler · jepler · commit 3e029a9d0df2 · 2024-03-04T20:55:32.000-06:00
Incorrect error handling in send/recv would raise an OSError with an incorrect (negative) code. It's likely that this bug was always happening in the Pico W implementation, which became the basis of the current shared implementation. Push handling of WANT_{READ,WRITE} down into mbedtls_raise_error and use it in recv_into and send. Tested by connecting to google.com:443, sending nothing, and trying to read a byte: ```py import socketpool, ssl, time, wifi socket = socketpool.SocketPool(wifi.radio) ctx = ssl.SSLContext() with ctx.wrap_socket(socket.socket()) as ss: ss.connect(("google.com", 443)) ss.settimeout(1) b = bytearray(1) try: t0 = time.monotonic() ss.recv_into(b) except Exception as ee: t1 = time.monotonic() exc = ee print(t1-t0) raise exc ``` As desired, an exception `OSError: [Errno 116] ETIMEDOUT` occurred and the time delta value was 1.0 seconds. (tested on pycamera) Closes: adafruit#8988
diff --git a/shared-module/ssl/SSLSocket.c b/shared-module/ssl/SSLSocket.c
@@ -71,6 +71,10 @@ STATIC NORETURN void mbedtls_raise_error(int err) {
         mp_raise_OSError(-err);
     }
 
+    if (err == MBEDTLS_ERR_SSL_WANT_WRITE || err == MBEDTLS_ERR_SSL_WANT_READ) {
+        mp_raise_OSError(MP_EWOULDBLOCK);
+    }
+
     #if defined(MBEDTLS_ERROR_C)
     // Including mbedtls_strerror takes about 1.5KB due to the error strings.
     // MBEDTLS_ERROR_C is the define used by mbedtls to conditionally include mbedtls_strerror.
@@ -271,16 +275,8 @@ mp_uint_t common_hal_ssl_sslsocket_recv_into(ssl_sslsocket_obj_t *self, uint8_t
         DEBUG_PRINT("returning %d\n", ret);
         return ret;
     }
-    if (ret == MBEDTLS_ERR_SSL_WANT_READ) {
-        ret = MP_EWOULDBLOCK;
-    } else if (ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
-        // If handshake is not finished, read attempt may end up in protocol
-        // wanting to write next handshake message. The same may happen with
-        // renegotiation.
-        ret = MP_EWOULDBLOCK;
-    }
     DEBUG_PRINT("raising errno [error case] %d\n", ret);
-    mp_raise_OSError(ret);
+    mbedtls_raise_error(ret);
 }
 
 mp_uint_t common_hal_ssl_sslsocket_send(ssl_sslsocket_obj_t *self, const uint8_t *buf, uint32_t len) {
@@ -290,16 +286,8 @@ mp_uint_t common_hal_ssl_sslsocket_send(ssl_sslsocket_obj_t *self, const uint8_t
         DEBUG_PRINT("returning %d\n", ret);
         return ret;
     }
-    if (ret == MBEDTLS_ERR_SSL_WANT_WRITE) {
-        ret = MP_EWOULDBLOCK;
-    } else if (ret == MBEDTLS_ERR_SSL_WANT_READ) {
-        // If handshake is not finished, write attempt may end up in protocol
-        // wanting to read next handshake message. The same may happen with
-        // renegotiation.
-        ret = MP_EWOULDBLOCK;
-    }
     DEBUG_PRINT("raising errno [error case] %d\n", ret);
-    mp_raise_OSError(ret);
+    mbedtls_raise_error(ret);
 }
 
 size_t common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, const char *host, size_t hostlen, uint32_t port) {
