Address review feedback

DtlsTimersRunning and DtlsTimerLeft returned either the timer state or
BAD_MUTEX_E. They now return 0 or BAD_MUTEX_E and report the timers
through an out parameter.

Rename DtlsTimerLowRes to DtlsTimerIsLowRes, since the old name read
like a variant of LowResTimer.

Move the handShakeDone comment in EmbedReceiveFrom back above the check
it describes.

Author: Frauschi

src/internal.c

@@ -12082,7 +12082,7 @@ word32 DtlsTimerElapsedMs(word32 start)
     word32 elapsed = DtlsTimerNowMs() - start;
 
     /* Both readings were truncated to whole seconds. */
-    if (DtlsTimerLowRes())
+    if (DtlsTimerIsLowRes())
         elapsed = (elapsed >= 1000) ? elapsed - 1000 : 0;
     return elapsed;
 }
@@ -12134,43 +12134,44 @@ static int DtlsMsgPoolWaiting(WOLFSSL* ssl)
 /* Get the DTLS timers that run.
  *
  * @param [in]  ssl       SSL/TLS object.
+ * @param [out] running   DTLS_TIMER_RTX and DTLS_TIMER_ACK bits, 0 when none
+ *                        runs.
  * @param [out] ackStart  Start of the DTLS 1.3 ack timer. May be NULL.
- * @return  DTLS_TIMER_RTX and DTLS_TIMER_ACK bits, 0 when none runs.
+ * @return  0 on success.
  * @return  BAD_MUTEX_E when locking fails.
  */
-int DtlsTimersRunning(WOLFSSL* ssl, word32* ackStart)
+int DtlsTimersRunning(WOLFSSL* ssl, int* running, word32* ackStart)
 {
-    int running = 0;
-
     (void)ackStart;
 
+    *running = 0;
 #ifdef WOLFSSL_DTLS13
     if (IsAtLeastTLSv1_3(ssl->version)) {
         if (ssl->dtls13Rtx.rtxRecords != NULL)
-            running |= DTLS_TIMER_RTX;
+            *running |= DTLS_TIMER_RTX;
     #ifdef WOLFSSL_RW_THREADED
         if (wc_LockMutex(&ssl->dtls13Rtx.mutex) != 0)
             return BAD_MUTEX_E;
     #endif
         if (ssl->dtls13FastTimeout && Dtls13AckSendable(ssl))
-            running |= DTLS_TIMER_ACK;
+            *running |= DTLS_TIMER_ACK;
     #ifndef NO_ASN_TIME
         if (ackStart != NULL)
             *ackStart = ssl->dtls_ack_timer_start;
     #endif
     #ifdef WOLFSSL_RW_THREADED
         wc_UnLockMutex(&ssl->dtls13Rtx.mutex);
     #endif
-        return running;
+        return 0;
     }
 #endif /* WOLFSSL_DTLS13 */
 
     if (ssl->options.handShakeState != HANDSHAKE_DONE &&
             (ssl->dtls_tx_msg_list != NULL || ssl->dtls_tx_msg != NULL) &&
             DtlsMsgPoolWaiting(ssl))
-        running |= DTLS_TIMER_RTX;
+        *running |= DTLS_TIMER_RTX;
 
-    return running;
+    return 0;
 }
 
 
@@ -12939,7 +12940,7 @@ word32 DtlsTimerNowMs(void)
 }
 
 /* Return 1 when the DTLS timer clock counts whole seconds. */
-int DtlsTimerLowRes(void)
+int DtlsTimerIsLowRes(void)
 {
 #if defined(MS_TIME_LOW_RES) || defined(MS_TIME_FALLBACK)
     return 1;

src/ssl_api_dtls.c

@@ -1222,39 +1222,44 @@ static int Dtls13GotTimeout(WOLFSSL* ssl, int expired)
 /* Get the time left on the DTLS timers.
  *
  * @param [in]  ssl      SSL/TLS object.
+ * @param [out] running  1 when a timer runs, 0 otherwise.
  * @param [out] left     Milliseconds until the first timer expires.
  * @param [out] ackOnly  1 when only the DTLS 1.3 ack timer expired.
- * @return  1 when a timer runs.
- * @return  0 when no timer runs.
+ * @return  0 on success.
  * @return  BAD_MUTEX_E when locking fails.
  */
-static int DtlsTimerLeft(WOLFSSL* ssl, word32* left, int* ackOnly)
+static int DtlsTimerLeft(WOLFSSL* ssl, int* running, word32* left,
+    int* ackOnly)
 {
 #ifndef NO_ASN_TIME
-    int running;
+    int ret;
+    int timers = 0;
     word32 timeout;
     word32 elapsed;
 #ifdef WOLFSSL_DTLS13
     word32 ackLeft;
 #endif
     word32 ackStart = 0;
 
+    *running = 0;
     if (!IsDtlsNotSctpMode(ssl) || ssl->dtls_timeout <= 0 ||
             ssl->buffers.outputBuffer.length > 0)
         return 0;
-    running = DtlsTimersRunning(ssl, &ackStart);
+    ret = DtlsTimersRunning(ssl, &timers, &ackStart);
+    if (ret != 0)
+        return ret;
 #ifdef WOLFSSL_HAVE_HS_SUSPEND
     /* A suspended handshake has not finished sending its flight. */
-    if (running > 0 && IsHsSuspendErr(ssl->error))
-        running &= ~DTLS_TIMER_RTX;
+    if (IsHsSuspendErr(ssl->error))
+        timers &= ~DTLS_TIMER_RTX;
 #endif
-    if (running <= 0)
-        return running;
+    if (timers == 0)
+        return 0;
 
     timeout = DtlsTimerTimeoutMs(ssl);
     *left = timeout;
     *ackOnly = 1;
-    if (running & DTLS_TIMER_RTX) {
+    if (timers & DTLS_TIMER_RTX) {
         elapsed = DtlsTimerElapsedMs(ssl->dtls_timer_start);
         *left = (elapsed < timeout) ? timeout - elapsed : 0;
 #ifdef WOLFSSL_DTLS13
@@ -1264,16 +1269,18 @@ static int DtlsTimerLeft(WOLFSSL* ssl, word32* left, int* ackOnly)
         *ackOnly = (*left > 0);
     }
 #ifdef WOLFSSL_DTLS13
-    if (running & DTLS_TIMER_ACK) {
+    if (timers & DTLS_TIMER_ACK) {
         elapsed = DtlsTimerNowMs() - ackStart;
         ackLeft = (elapsed < timeout / 4) ? timeout / 4 - elapsed : 0;
-        if (ackLeft < *left || !(running & DTLS_TIMER_RTX))
+        if (ackLeft < *left || !(timers & DTLS_TIMER_RTX))
             *left = ackLeft;
     }
 #endif
-    return 1;
+    *running = 1;
+    return 0;
 #else
     (void)ssl;
+    *running = 0;
     (void)left;
     (void)ackOnly;
     return 0;
@@ -1291,16 +1298,17 @@ int wolfSSL_DTLSv1_get_timeout(WOLFSSL* ssl, WOLFSSL_TIMEVAL* timeleft)
 {
     word32 left = 0;
     int ackOnly = 0;
+    int running = 0;
     int ret;
 
     if (ssl == NULL || timeleft == NULL)
         return 0;
 
     XMEMSET(timeleft, 0, sizeof(WOLFSSL_TIMEVAL));
-    ret = DtlsTimerLeft(ssl, &left, &ackOnly);
-    if (ret < 0)
+    ret = DtlsTimerLeft(ssl, &running, &left, &ackOnly);
+    if (ret != 0)
         WOLFSSL_MSG("Getting the DTLS timers failed");
-    if (ret <= 0)
+    if (ret != 0 || !running)
         return 0;
 
     timeleft->tv_sec = left / 1000;
@@ -1322,18 +1330,19 @@ int wolfSSL_DTLSv1_handle_timeout(WOLFSSL* ssl)
 {
     word32 left = 0;
     int ackOnly = 0;
+    int running = 0;
     int ret;
 
     if (ssl == NULL)
         return WOLFSSL_FATAL_ERROR;
 
-    ret = DtlsTimerLeft(ssl, &left, &ackOnly);
-    if (ret < 0) {
+    ret = DtlsTimerLeft(ssl, &running, &left, &ackOnly);
+    if (ret != 0) {
         ssl->error = ret;
         WOLFSSL_ERROR(ret);
         return WOLFSSL_FATAL_ERROR;
     }
-    if (ret == 0 || left > 0)
+    if (!running || left > 0)
         return 0;
 
 #ifdef WOLFSSL_DTLS13

src/wolfio.c

@@ -683,6 +683,7 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
     int sd = dtlsCtx->rfd;
     int dtls_timeout = wolfSSL_dtls_get_current_timeout(ssl);
     byte doDtlsTimeout;
+    int running = 0;
     SOCKADDR_S lclPeer;
     SOCKADDR_S* peer;
     XSOCKLENT peerSz = 0;
@@ -736,12 +737,13 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
     if (ret != 0)
         return ret;
 
+    ret = DtlsTimersRunning(ssl, &running, NULL);
+    if (ret != 0)
+        return ret;
     /* Don't use ssl->options.handShakeDone since it is true even if
      * we are in the process of renegotiation */
-    ret = DtlsTimersRunning(ssl, NULL);
-    if (ret < 0)
-        return ret;
-    doDtlsTimeout = ssl->options.handShakeState != HANDSHAKE_DONE || ret != 0;
+    doDtlsTimeout = ssl->options.handShakeState != HANDSHAKE_DONE ||
+        running != 0;
 
     do {
         int ignore = 0;

tests/api/test_dtls.c

@@ -13861,7 +13861,7 @@ static int test_DTLSv1_timer(method_provider method_c,
     ExpectIntEQ(wolfSSL_DTLSv1_handle_timeout(ssl_c), 0);
     ExpectIntEQ(test_ctx.s_msg_count, 1);
 
-    if (DtlsTimerLowRes()) {
+    if (DtlsTimerIsLowRes()) {
         /* A start one tick ago may be under a second, so 1 s hasn't passed. */
         for (i = 0; i < 3 && EXPECT_SUCCESS(); i++) {
             now = DtlsTimerNowMs();

wolfssl/internal.h

@@ -8123,7 +8123,7 @@ WOLFSSL_LOCAL int Tls13KeyUpdateLimitReached(WOLFSSL* ssl);
         #define DtlsMsgFind wolfSSL_DtlsMsgFind
         #define DtlsMsgStore wolfSSL_DtlsMsgStore
         #define DtlsTimerNowMs wolfSSL_DtlsTimerNowMs
-        #define DtlsTimerLowRes wolfSSL_DtlsTimerLowRes
+        #define DtlsTimerIsLowRes wolfSSL_DtlsTimerIsLowRes
     #endif /* WOLFSSL_API_PREFIX_MAP */
     WOLFSSL_LOCAL DtlsMsg* DtlsMsgNew(word32 sz, byte tx, void* heap);
     WOLFSSL_LOCAL void DtlsMsgDelete(DtlsMsg* item, void* heap);
@@ -8154,10 +8154,11 @@ WOLFSSL_LOCAL int Tls13KeyUpdateLimitReached(WOLFSSL* ssl);
     WOLFSSL_LOCAL int  DtlsMsgPoolTimeout(WOLFSSL* ssl);
     WOLFSSL_LOCAL void DtlsTimerStart(WOLFSSL* ssl);
     WOLFSSL_LOCAL void DtlsTimeoutReset(WOLFSSL* ssl);
-    WOLFSSL_LOCAL int  DtlsTimersRunning(WOLFSSL* ssl, word32* ackStart);
+    WOLFSSL_LOCAL int  DtlsTimersRunning(WOLFSSL* ssl, int* running,
+                                         word32* ackStart);
 #ifndef NO_ASN_TIME
     WOLFSSL_TEST_VIS word32 DtlsTimerNowMs(void);
-    WOLFSSL_TEST_VIS int DtlsTimerLowRes(void);
+    WOLFSSL_TEST_VIS int DtlsTimerIsLowRes(void);
     WOLFSSL_LOCAL word32 DtlsTimerElapsedMs(word32 start);
     WOLFSSL_LOCAL word32 DtlsTimerTimeoutMs(const WOLFSSL* ssl);
 #endif