Skip to content

Commit

Permalink
Protect peer access when WOLFSSL_RW_THREADED
Browse files Browse the repository at this point in the history
  • Loading branch information
@julek-wolfssl
julek-wolfssl committed Dec 16, 2024
1 parent 5977bdb commit 69ed5e4
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 63 deletions.
1 change: 1 addition & 0 deletions src/dtls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 12 additions & 4 deletions src/internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
37 changes: 29 additions & 8 deletions src/ssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions src/tls13.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
119 changes: 68 additions & 51 deletions src/wolfio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions wolfssl/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 69ed5e4

Please sign in to comment.