diff --git a/src/dtls.c b/src/dtls.c index 75b5e84dcb..fceb376030 100644 --- a/src/dtls.c +++ b/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); diff --git a/src/internal.c b/src/internal.c index 14b6683c3a..6bce578f03 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7390,6 +7390,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 @@ -8257,6 +8262,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); @@ -21376,11 +21384,11 @@ static void dtlsProcessPendingPeer(WOLFSSL* ssl, int deprotected) } else { /* Pending peer present and record deprotected. Update the peer. */ - dtlsClearPeer(&ssl->buffers.dtlsCtx.peer); - ssl->buffers.dtlsCtx.peer = ssl->buffers.dtlsCtx.pendingPeer; - XMEMSET(&ssl->buffers.dtlsCtx.pendingPeer, 0, - sizeof(WOLFSSL_SOCKADDR)); + (void)wolfSSL_dtls_set_peer(ssl, + &ssl->buffers.dtlsCtx.pendingPeer.sa, + ssl->buffers.dtlsCtx.pendingPeer.sz); ssl->buffers.dtlsCtx.processingPendingRecord = 0; + dtlsClearPeer(&ssl->buffers.dtlsCtx.pendingPeer); } } else { diff --git a/src/ssl.c b/src/ssl.c index 5248b38d30..93556657d5 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1904,12 +1904,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; @@ -1927,7 +1934,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, @@ -1950,6 +1960,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; @@ -1963,18 +1977,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; @@ -1986,7 +2007,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; diff --git a/src/tls13.c b/src/tls13.c index ffc106afed..481df4ff41 100644 --- a/src/tls13.c +++ b/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, @@ -6405,6 +6406,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, diff --git a/src/wolfio.c b/src/wolfio.c index 1e31f7304f..9769be4ec9 100644 --- a/src/wolfio.c +++ b/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) { - dtlsCtx->peer.sa = (void*)XMALLOC(sizeof(SOCKADDR_S), - ssl->heap, DYNAMIC_TYPE_SOCKADDR); - dtlsCtx->peer.sz = 0; - if (dtlsCtx->peer.sa != NULL) - dtlsCtx->peer.bufSz = sizeof(SOCKADDR_S); - else - dtlsCtx->peer.bufSz = 0; - newPeer = 1; - peer = (SOCKADDR_S*)dtlsCtx->peer.sa; - } - else if (!ssl->options.dtlsStateful) { - newPeer = 1; - peer = (SOCKADDR_S*)dtlsCtx->peer.sa; - } - else { - peer = &lclPeer; - XMEMCPY(peer, (SOCKADDR_S*)dtlsCtx->peer.sa, sizeof(lclPeer)); + newPeer = dtlsCtx->peer.sa == NULL || !ssl->options.dtlsStateful; + peer = &lclPeer; + if (dtlsCtx->peer.sa != NULL) { + XMEMCPY(peer, (SOCKADDR_S*)dtlsCtx->peer.sa, MIN(sizeof(lclPeer), + dtlsCtx->peer.sz)); } - peerSz = dtlsCtx->peer.bufSz; + peerSz = sizeof(lclPeer); } +#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 - { - int ret = wc_LockMutex(&ssl->dtls13Rtx.mutex); - if (ret < 0) { - return ret; - } - } + ret = wc_LockMutex(&ssl->dtls13Rtx.mutex); + if (ret != 0) + 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); - - /* From the RECV(2) man page - * The returned address is truncated if the buffer provided is too - * 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 */ - } - 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; + { + XSOCKLENT inPeerSz = peerSz; + recvd = (int)DTLS_RECVFROM_FUNCTION(sd, buf, (size_t)sz, ssl->rflags, + (SOCKADDR*)peer, peer != NULL ? &inPeerSz : NULL); + /* Truncate peerSz. From the RECV(2) man page + * The returned address is truncated if the buffer provided is too + * small; in this case, addrlen will return a value greater than was + * supplied to the call. + */ + peerSz = MIN(peerSz, inPeerSz); } 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 */ - dtlsCtx->peer.sz = peerSz; + /* Store size of saved address. Locking handled internally. */ + 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 } diff --git a/wolfssl/internal.h b/wolfssl/internal.h index b01c1e7c2e..af8e4a9bd8 100644 --- a/wolfssl/internal.h +++ b/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