Dtls err handling (#1832)

* Improve DTLS error handling * Add check for SSL_do_handshake too
awslabs · Dec 5, 2023 · a0c6e76 · a0c6e76
1 parent 594e28b
commit a0c6e76
Showing 1 changed file with 42 additions and 12 deletions.
diff --git a/src/source/Crypto/Dtls_openssl.c b/src/source/Crypto/Dtls_openssl.c
@@ -448,9 +448,9 @@ STATUS dtlsSessionHandshakeInThread(PDtlsSession pDtlsSession, BOOL isServer)
     BOOL locked = FALSE;
     INT32 sslRet, sslErr;
     struct timeval timeout;
-    LONG dtlsTimeoutRet = 0;
+    int dtlsTimeoutRet = 0, dtlsHandleTimeoutRet = 0;
     BOOL firstMsg = TRUE;
-    UINT64 waitTime = INFINITE_TIME_VALUE;
+    UINT64 waitTime = 1 * HUNDREDS_OF_NANOS_IN_A_SECOND;
     BOOL dtlsHandshakeErrored = FALSE;
     BOOL timedOut = FALSE;
     MEMSET(&timeout, 0x00, SIZEOF(struct timeval));
@@ -464,11 +464,6 @@ STATUS dtlsSessionHandshakeInThread(PDtlsSession pDtlsSession, BOOL isServer)
     CHK(!ATOMIC_LOAD_BOOL(&pDtlsSession->isCleanUp), STATUS_DTLS_SESSION_ALREADY_FREED);
     CHK_STATUS(beginHandshakeProcess(pDtlsSession, isServer, &sslRet));
     while (!(ATOMIC_LOAD_BOOL(&pDtlsSession->sslInitFinished)) && !dtlsHandshakeErrored && !(ATOMIC_LOAD_BOOL(&pDtlsSession->isCleanUp))) {
-        dtlsTimeoutRet = DTLSv1_get_timeout(pDtlsSession->pSsl, &timeout);
-        if (dtlsTimeoutRet > 0) {
-            waitTime = timeout.tv_sec * HUNDREDS_OF_NANOS_IN_A_SECOND + timeout.tv_usec * HUNDREDS_OF_NANOS_IN_A_MICROSECOND;
-        }
-
         switch (pDtlsSession->handshakeState) {
             case DTLS_STATE_HANDSHAKE_NEW:
                 if (sslRet <= 0) {
@@ -482,6 +477,10 @@ STATUS dtlsSessionHandshakeInThread(PDtlsSession pDtlsSession, BOOL isServer)
                         LOG_OPENSSL_ERROR("SSL_do_handshake");
                     }
                     pDtlsSession->handshakeState = DTLS_STATE_HANDSHAKE_IN_PROGRESS;
+                } else {
+                    pDtlsSession->handshakeState = DTLS_STATE_HANDSHAKE_COMPLETED;
+                    ATOMIC_STORE_BOOL(&pDtlsSession->sslInitFinished, TRUE);
+                    CHK_STATUS(dtlsSessionChangeState(pDtlsSession, RTC_DTLS_TRANSPORT_STATE_CONNECTED));
                 }
                 break;
             case DTLS_STATE_HANDSHAKE_IN_PROGRESS:
@@ -490,16 +489,47 @@ STATUS dtlsSessionHandshakeInThread(PDtlsSession pDtlsSession, BOOL isServer)
                     ATOMIC_STORE_BOOL(&pDtlsSession->sslInitFinished, TRUE);
                     CHK_STATUS(dtlsSessionChangeState(pDtlsSession, RTC_DTLS_TRANSPORT_STATE_CONNECTED));
                 } else {
-                    if (dtlsTimeoutRet < 0) {
-                        pDtlsSession->handshakeState = DTLS_STATE_HANDSHAKE_ERROR;
-                        dtlsHandshakeErrored = TRUE;
+                    // We check for timeout here. If the timeout is 0, it is likely it
+                    // is in Server mode at which point it is basically waiting on the first message
+                    // from DTLS client. So, we need to wait on the CVAR. Even if timeout is 0, there is no
+                    // guarantee that handshake was complete. It just means that no retransmission is required
+                    // We always rely on sslInitFinished to be set to truly confirm handshake was complete
+
+                    // DTLSv1_handle_timeout: https://www.openssl.org/docs/manmaster/man3/DTLSv1_handle_timeout.html
+                    // DTLSv1_get_timeout: https://www.openssl.org/docs/manmaster/man3/DTLSv1_get_timeout.html
+                    dtlsTimeoutRet = DTLSv1_get_timeout(pDtlsSession->pSsl, &timeout);
+                    if (dtlsTimeoutRet == 0) {
+                        // Listening in on fatal errors only: https://www.openssl.org/docs/man1.1.1/man3/SSL_get_error.html
+                        if (sslErr == SSL_ERROR_SYSCALL || sslErr == SSL_ERROR_SSL) {
+                            DLOGW("FATAL ERROR encountered while getting timeout");
+                            pDtlsSession->handshakeState = DTLS_STATE_HANDSHAKE_ERROR;
+                            dtlsHandshakeErrored = TRUE;
+                        } else {
+                            DLOGI("No timeout is active, no retransmissions to handle");
+                        }
                     } else {
+                        waitTime = timeout.tv_sec * HUNDREDS_OF_NANOS_IN_A_SECOND + timeout.tv_usec * HUNDREDS_OF_NANOS_IN_A_MICROSECOND;
+                    }
+                    if (!dtlsHandshakeErrored) {
                         timedOut = (CVAR_WAIT(pDtlsSession->receivePacketCvar, pDtlsSession->sslLock, waitTime) == STATUS_OPERATION_TIMED_OUT);
                         if (timedOut) {
                             DLOGD("DTLS handshake timeout event occurred, going to retransmit");
-                            DTLSv1_handle_timeout(pDtlsSession->pSsl);
+                            dtlsHandleTimeoutRet = DTLSv1_handle_timeout(pDtlsSession->pSsl);
+                            if (dtlsHandleTimeoutRet > 0) {
+                                DLOGI("Timeout handled successfully, packet retransmitted");
+                            } else if (dtlsHandleTimeoutRet == 0) {
+                                DLOGI("No pending timeout event to handle");
+                            } else {
+                                sslErr = SSL_get_error(pDtlsSession->pSsl, sslRet);
+                                if (sslErr == SSL_ERROR_SYSCALL || sslErr == SSL_ERROR_SSL) {
+                                    DLOGE("A fatal error was encountered while handling timeout");
+                                    pDtlsSession->handshakeState = DTLS_STATE_HANDSHAKE_ERROR;
+                                    dtlsHandshakeErrored = TRUE;
+                                } else {
+                                    DLOGW("Non fatal error while handling timeout, will retry next time");
+                                }
+                            }
                         }
-
                         // We start calculating start of handshake DTLS handshake time taken in server mode only after clientHello
                         // is received, until then, we are only waiting, so we should not count that time into handshake latency
                         // calculation