Protect peer access when WOLFSSL_RW_THREADED

src/dtls.c

@@ -230,6 +230,7 @@ static int CreateDtls12Cookie(const WOLFSSL* ssl, const WolfSSL_CH* ch,
             ssl->buffers.dtlsCookieSecret.buffer,
             ssl->buffers.dtlsCookieSecret.length);
         if (ret == 0) {
+            /* peerLock not necessary. Still in handshake phase. */
             ret = wc_HmacUpdate(&cookieHmac,
                    (const byte*)ssl->buffers.dtlsCtx.peer.sa,
                                 ssl->buffers.dtlsCtx.peer.sz);

src/internal.c

@@ -7392,6 +7392,11 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
     ssl->buffers.dtlsCtx.rfd            = -1;
     ssl->buffers.dtlsCtx.wfd            = -1;
 
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_InitRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        return BAD_MUTEX_E;
+#endif
+
     ssl->IOCB_ReadCtx  = &ssl->buffers.dtlsCtx;  /* prevent invalid pointer access if not */
     ssl->IOCB_WriteCtx = &ssl->buffers.dtlsCtx;  /* correctly set */
 #else
@@ -8261,6 +8266,9 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
     }
     XFREE(ssl->buffers.dtlsCtx.peer.sa, ssl->heap, DYNAMIC_TYPE_SOCKADDR);
     ssl->buffers.dtlsCtx.peer.sa = NULL;
+#ifdef WOLFSSL_RW_THREADED
+    wc_FreeRwLock(&ssl->buffers.dtlsCtx.peerLock);
+#endif
 #ifdef WOLFSSL_DTLS_CID
     XFREE(ssl->buffers.dtlsCtx.pendingPeer.sa, ssl->heap,
             DYNAMIC_TYPE_SOCKADDR);
@@ -21384,11 +21392,11 @@ static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected)
         }
         else {
             /* Pending peer present and record deprotected. Update the peer. */
-            dtlsClearPeer(&ssl->buffers.dtlsCtx.peer);
+            (void)wolfSSL_dtls_set_peer(ssl,
-            ssl->buffers.dtlsCtx.peer = ssl->buffers.dtlsCtx.pendingPeer;
+                    &ssl->buffers.dtlsCtx.pendingPeer.sa,
-            XMEMSET(&ssl->buffers.dtlsCtx.pendingPeer, 0,
+                    ssl->buffers.dtlsCtx.pendingPeer.sz);
-                    sizeof(WOLFSSL_SOCKADDR));
             ssl->buffers.dtlsCtx.processingPendingRecord = 0;
+            dtlsClearPeer(&ssl->buffers.dtlsCtx.pendingPeer);
         }
     }
     else {

src/ssl.c

@@ -1913,12 +1913,19 @@ int wolfSSL_dtls_set_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz)
 
     if (ssl == NULL)
         return WOLFSSL_FAILURE;
-
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_LockRwLock_Wr(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        return WOLFSSL_FAILURE;
+#endif
     ret = SockAddrSet(&ssl->buffers.dtlsCtx.peer, peer, peerSz, ssl->heap);
     if (ret == WOLFSSL_SUCCESS && !(peer == NULL || peerSz == 0))
         ssl->buffers.dtlsCtx.userSet = 1;
     else
         ssl->buffers.dtlsCtx.userSet = 0;
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        ret = WOLFSSL_FAILURE;
+#endif
     return ret;
 #else
     (void)ssl;
@@ -1936,7 +1943,10 @@ int wolfSSL_dtls_set_pending_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz)
 
     if (ssl == NULL)
         return WOLFSSL_FAILURE;
-
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_LockRwLock_Rd(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        return WOLFSSL_FAILURE;
+#endif
     if (ssl->buffers.dtlsCtx.peer.sa != NULL &&
             ssl->buffers.dtlsCtx.peer.sz == peerSz &&
             sockAddrEqual((SOCKADDR_S*)ssl->buffers.dtlsCtx.peer.sa,
@@ -1959,6 +1969,10 @@ int wolfSSL_dtls_set_pending_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz)
     }
     if (ret == WOLFSSL_SUCCESS)
         ssl->buffers.dtlsCtx.processingPendingRecord = 0;
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        ret = WOLFSSL_FAILURE;
+#endif
     return ret;
 #else
     (void)ssl;
@@ -1972,18 +1986,25 @@ int wolfSSL_dtls_set_pending_peer(WOLFSSL* ssl, void* peer, unsigned int peerSz)
 int wolfSSL_dtls_get_peer(WOLFSSL* ssl, void* peer, unsigned int* peerSz)
 {
 #ifdef WOLFSSL_DTLS
-    if (ssl == NULL) {
+    int ret = WOLFSSL_FAILURE;
+    if (ssl == NULL)
         return WOLFSSL_FAILURE;
-    }
+#ifdef WOLFSSL_RW_THREADED
-
+    if (wc_LockRwLock_Rd(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        return WOLFSSL_FAILURE;
+#endif
     if (peer != NULL && peerSz != NULL
             && *peerSz >= ssl->buffers.dtlsCtx.peer.sz
             && ssl->buffers.dtlsCtx.peer.sa != NULL) {
         *peerSz = ssl->buffers.dtlsCtx.peer.sz;
         XMEMCPY(peer, ssl->buffers.dtlsCtx.peer.sa, *peerSz);
-        return WOLFSSL_SUCCESS;
+        ret = WOLFSSL_SUCCESS;
     }
-    return WOLFSSL_FAILURE;
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        ret = WOLFSSL_FAILURE;
+#endif
+    return ret;
 #else
     (void)ssl;
     (void)peer;
@@ -1995,7 +2016,7 @@ int wolfSSL_dtls_get_peer(WOLFSSL* ssl, void* peer, unsigned int* peerSz)
 int wolfSSL_dtls_get0_peer(WOLFSSL* ssl, const void** peer,
                            unsigned int* peerSz)
 {
-#ifdef WOLFSSL_DTLS
+#if defined(WOLFSSL_DTLS) && !defined(WOLFSSL_RW_THREADED)
     if (ssl == NULL)
         return WOLFSSL_FAILURE;
 

src/tls13.c

@@ -3621,6 +3621,7 @@ int CreateCookieExt(const WOLFSSL* ssl, byte* hash, word16 hashSz,
 #ifdef WOLFSSL_DTLS13
     /* Tie cookie to peer address */
     if (ret == 0) {
+        /* peerLock not necessary. Still in handshake phase. */
         if (ssl->options.dtls && ssl->buffers.dtlsCtx.peer.sz > 0) {
             ret = wc_HmacUpdate(&cookieHmac,
                 (byte*)ssl->buffers.dtlsCtx.peer.sa,
@@ -6409,6 +6410,7 @@ int TlsCheckCookie(const WOLFSSL* ssl, const byte* cookie, word16 cookieSz)
 #ifdef WOLFSSL_DTLS13
     /* Tie cookie to peer address */
     if (ret == 0) {
+        /* peerLock not necessary. Still in handshake phase. */
         if (ssl->options.dtls && ssl->buffers.dtlsCtx.peer.sz > 0) {
             ret = wc_HmacUpdate(&cookieHmac,
                 (byte*)ssl->buffers.dtlsCtx.peer.sa,

src/wolfio.c

@@ -660,8 +660,17 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
     word32 invalidPeerPackets = 0;
 #endif
     int newPeer = 0;
+    int ret = 0;
 
     WOLFSSL_ENTER("EmbedReceiveFrom");
+    (void)ret; /* possibly unused */
+
+    XMEMSET(&lclPeer, 0, sizeof(lclPeer));
+
+#ifdef WOLFSSL_RW_THREADED
+    if (wc_LockRwLock_Rd(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        return WOLFSSL_CBIO_ERR_GENERAL;
+#endif
 
     if (dtlsCtx->connected) {
         peer = NULL;
@@ -670,37 +679,32 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
 #ifndef WOLFSSL_IPV6
         if (PeerIsIpv6((SOCKADDR_S*)dtlsCtx->peer.sa, dtlsCtx->peer.sz)) {
             WOLFSSL_MSG("ipv6 dtls peer set but no ipv6 support compiled");
-            return NOT_COMPILED_IN;
+            ret = WOLFSSL_CBIO_ERR_GENERAL;
         }
 #endif
         peer = &lclPeer;
-        XMEMSET(&lclPeer, 0, sizeof(lclPeer));
         peerSz = sizeof(lclPeer);
     }
     else {
         /* Store the peer address. It is used to calculate the DTLS cookie. */
-        if (dtlsCtx->peer.sa == NULL) {
+        newPeer = dtlsCtx->peer.sa == NULL || !ssl->options.dtlsStateful;
-            dtlsCtx->peer.sa = (void*)XMALLOC(sizeof(SOCKADDR_S),
+        peer = &lclPeer;
-                    ssl->heap, DYNAMIC_TYPE_SOCKADDR);
+        if (dtlsCtx->peer.sa != NULL) {
-            dtlsCtx->peer.sz = 0;
+            XMEMCPY(peer, (SOCKADDR_S*)dtlsCtx->peer.sa, MIN(sizeof(lclPeer),
-            if (dtlsCtx->peer.sa != NULL)
+                    dtlsCtx->peer.sz));
-                dtlsCtx->peer.bufSz = sizeof(SOCKADDR_S);
-            else
-                dtlsCtx->peer.bufSz = 0;
-            newPeer = 1;
-            peer = (SOCKADDR_S*)dtlsCtx->peer.sa;
         }
-        else if (!ssl->options.dtlsStateful) {
+        peerSz = sizeof(lclPeer);
-            newPeer = 1;
-            peer = (SOCKADDR_S*)dtlsCtx->peer.sa;
-        }
-        else {
-            peer = &lclPeer;
-            XMEMCPY(peer, (SOCKADDR_S*)dtlsCtx->peer.sa, sizeof(lclPeer));
-        }
-        peerSz = dtlsCtx->peer.bufSz;
     }
 
+#ifdef WOLFSSL_RW_THREADED
+    /* We make a copy above to avoid holding the lock for the entire function */
+    if (wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+        return WOLFSSL_CBIO_ERR_GENERAL;
+#endif
+
+    if (ret != 0)
+        return ret;
+
     /* Don't use ssl->options.handShakeDone since it is true even if
      * we are in the process of renegotiation */
     doDtlsTimeout = ssl->options.handShakeState != HANDSHAKE_DONE;
@@ -709,12 +713,9 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
     if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version)) {
         doDtlsTimeout = doDtlsTimeout || ssl->dtls13Rtx.rtxRecords != NULL;
 #ifdef WOLFSSL_RW_THREADED
-        {
+        ret = wc_LockMutex(&ssl->dtls13Rtx.mutex);
-            int ret = wc_LockMutex(&ssl->dtls13Rtx.mutex);
+        if (ret != 0)
-            if (ret < 0) {
+            return ret;
-                return ret;
-            }
-        }
 #endif
         doDtlsTimeout = doDtlsTimeout ||
             (ssl->dtls13FastTimeout && ssl->dtls13Rtx.seenRecords != NULL);
@@ -785,26 +786,16 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
         }
 #endif /* !NO_ASN_TIME */
 
-        recvd = (int)DTLS_RECVFROM_FUNCTION(sd, buf, (size_t)sz, ssl->rflags,
+        {
-            (SOCKADDR*)peer, peer != NULL ? &peerSz : NULL);
+            XSOCKLENT inPeerSz = peerSz;
-
+            recvd = (int)DTLS_RECVFROM_FUNCTION(sd, buf, (size_t)sz,
-        /* From the RECV(2) man page
+                 ssl->rflags, (SOCKADDR*)peer, peer != NULL ? &inPeerSz : NULL);
-         * The returned address is truncated if the buffer provided is too
+            /* Truncate peerSz. From the RECV(2) man page
-         * small; in this case, addrlen will return a value greater than was
+             * The returned address is truncated if the buffer provided is too
-         * supplied to the call.
+             * small; in this case, addrlen will return a value greater than was
-         */
+             * supplied to the call.
-        if (dtlsCtx->connected) {
+             */
-            /* No need to sanitize the value of peerSz */
+            peerSz = MIN(peerSz, inPeerSz);
-        }
-        else if (dtlsCtx->userSet) {
-            /* Truncate peer size */
-            if (peerSz > (XSOCKLENT)sizeof(lclPeer))
-                peerSz = (XSOCKLENT)sizeof(lclPeer);
-        }
-        else {
-            /* Truncate peer size */
-            if (peerSz > (XSOCKLENT)dtlsCtx->peer.bufSz)
-                peerSz = (XSOCKLENT)dtlsCtx->peer.bufSz;
         }
 
         recvd = TranslateIoReturnCode(recvd, sd, SOCKET_RECEIVING);
@@ -833,11 +824,23 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
         }
         else if (dtlsCtx->userSet) {
             /* Check we received the packet from the correct peer */
+            int ignore = 0;
+#ifdef WOLFSSL_RW_THREADED
+            if (wc_LockRwLock_Rd(&ssl->buffers.dtlsCtx.peerLock) != 0)
+                return WOLFSSL_CBIO_ERR_GENERAL;
+#endif
             if (dtlsCtx->peer.sz > 0 &&
                 (peerSz != (XSOCKLENT)dtlsCtx->peer.sz ||
                     !sockAddrEqual(peer, peerSz, (SOCKADDR_S*)dtlsCtx->peer.sa,
                         dtlsCtx->peer.sz))) {
                 WOLFSSL_MSG("    Ignored packet from invalid peer");
+                ignore = 1;
+            }
+#ifdef WOLFSSL_RW_THREADED
+            if (wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+                return WOLFSSL_CBIO_ERR_GENERAL;
+#endif
+            if (ignore) {
 #if defined(NO_ASN_TIME) &&                                                    \
     !defined(DTLS_RECEIVEFROM_NO_TIMEOUT_ON_INVALID_PEER)
                 if (doDtlsTimeout) {
@@ -853,13 +856,27 @@ int EmbedReceiveFrom(WOLFSSL *ssl, char *buf, int sz, void *ctx)
         }
         else {
             if (newPeer) {
-                /* Store size of saved address */
+                /* Store size of saved address. Locking handled internally. */
-                dtlsCtx->peer.sz = peerSz;
+                if (wolfSSL_dtls_set_peer(ssl, peer, peerSz) != WOLFSSL_SUCCESS)
+                    return WOLFSSL_CBIO_ERR_GENERAL;
             }
 #ifndef WOLFSSL_PEER_ADDRESS_CHANGES
-            else if (!sockAddrEqual(peer, peerSz, (SOCKADDR_S*)dtlsCtx->peer.sa,
+            else {
+                ret = 0;
+#ifdef WOLFSSL_RW_THREADED
+                if (wc_LockRwLock_Rd(&ssl->buffers.dtlsCtx.peerLock) != 0)
+                    return WOLFSSL_CBIO_ERR_GENERAL;
+#endif
+                if (!sockAddrEqual(peer, peerSz, (SOCKADDR_S*)dtlsCtx->peer.sa,
                                     dtlsCtx->peer.sz)) {
-                return WOLFSSL_CBIO_ERR_GENERAL;
+                    ret = WOLFSSL_CBIO_ERR_GENERAL;
+                }
+#ifdef WOLFSSL_RW_THREADED
+                if (wc_UnLockRwLock(&ssl->buffers.dtlsCtx.peerLock) != 0)
+                    return WOLFSSL_CBIO_ERR_GENERAL;
+#endif
+                if (ret != 0)
+                    return ret;
             }
 #endif
         }

wolfssl/internal.h

@@ -2758,6 +2758,10 @@ struct WOLFSSL_SOCKADDR {
 };
 
 typedef struct WOLFSSL_DTLS_CTX {
+#ifdef WOLFSSL_RW_THREADED
+    /* Protect peer access after the handshake */
+    wolfSSL_RwLock peerLock;
+#endif
     WOLFSSL_SOCKADDR peer;
 #ifdef WOLFSSL_DTLS_CID
     WOLFSSL_SOCKADDR pendingPeer; /* When using CID's, we don't want to update