From 07afc594a8d1e0fbaf29f0cd567418293d8173f6 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 13 Jul 2022 11:22:23 +0200 Subject: [PATCH 1/6] dtls13: aesthetic only changes --- src/dtls13.c | 53 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index e31d2c699..56904a5c3 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -796,7 +796,8 @@ static int Dtls13RtxMsgRecvd(WOLFSSL* ssl, enum HandShakeType hs, Dtls13RtxRemoveCurAck(ssl); } - if (ssl->options.dtls13SendMoreAcks && Dtls13DetectDisruption(ssl, fragOffset)) { + if (ssl->options.dtls13SendMoreAcks && + Dtls13DetectDisruption(ssl, fragOffset)) { WOLFSSL_MSG("Disruption detected"); ssl->dtls13Rtx.sendAcks = 1; } @@ -1416,40 +1417,40 @@ static int Dtls13RtxSendBuffered(WOLFSSL* ssl) static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, word32* processedSize) { - word32 frag_off, frag_length; + word32 fragOff, fragLength; byte isComplete, isFirst; - word32 message_length; - byte handshake_type; + word32 messageLength; + byte handshakeType; word32 idx; int ret; idx = 0; - ret = GetDtlsHandShakeHeader(ssl, input, &idx, &handshake_type, - &message_length, &frag_off, &frag_length, size); + ret = GetDtlsHandShakeHeader(ssl, input, &idx, &handshakeType, + &messageLength, &fragOff, &fragLength, size); if (ret != 0) return PARSE_ERROR; - if (idx + frag_length > size) { + if (idx + fragLength > size) { WOLFSSL_ERROR(INCOMPLETE_DATA); return INCOMPLETE_DATA; } - if (frag_off + frag_length > message_length) + if (fragOff + fragLength > messageLength) return BUFFER_ERROR; - if (handshake_type == client_hello && - /* Only when receiving an unverified ClientHello */ - ssl->options.serverState < SERVER_HELLO_COMPLETE) { + if (handshakeType == client_hello && + /* Only when receiving an unverified ClientHello */ + ssl->options.serverState < SERVER_HELLO_COMPLETE) { /* To be able to operate in stateless mode, we assume the ClientHello * is in order and we use its Handshake Message number and Sequence * Number for our Tx. */ ssl->keys.dtls_expected_peer_handshake_number = - ssl->keys.dtls_handshake_number = - ssl->keys.dtls_peer_handshake_number; + ssl->keys.dtls_handshake_number = + ssl->keys.dtls_peer_handshake_number; ssl->dtls13Epochs[0].nextSeqNumber = ssl->keys.curSeq; } - ret = Dtls13RtxMsgRecvd(ssl, (enum HandShakeType)handshake_type, frag_off); + ret = Dtls13RtxMsgRecvd(ssl, (enum HandShakeType)handshakeType, fragOff); if (ret != 0) return ret; @@ -1462,40 +1463,34 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, #endif /* WOLFSSL_DEBUG_TLS */ /* ignore the message */ - *processedSize = idx + frag_length; - - *processedSize += ssl->keys.padSz; + *processedSize = idx + fragLength + ssl->keys.padSz; return 0; } - isFirst = frag_off == 0; - isComplete = isFirst && frag_length == message_length; + isFirst = fragOff == 0; + isComplete = isFirst && fragLength == messageLength; if (!isComplete || ssl->keys.dtls_peer_handshake_number > ssl->keys.dtls_expected_peer_handshake_number) { DtlsMsgStore(ssl, w64GetLow32(ssl->keys.curEpoch64), ssl->keys.dtls_peer_handshake_number, - input + DTLS_HANDSHAKE_HEADER_SZ, message_length, handshake_type, - frag_off, frag_length, ssl->heap); - - *processedSize = idx + frag_length; - - *processedSize += ssl->keys.padSz; + input + DTLS_HANDSHAKE_HEADER_SZ, messageLength, handshakeType, + fragOff, fragLength, ssl->heap); + *processedSize = idx + fragLength + ssl->keys.padSz; if (Dtls13NextMessageComplete(ssl)) return Dtls13ProcessBufferedMessages(ssl); return 0; } - ret = DoTls13HandShakeMsgType(ssl, input, &idx, handshake_type, - message_length, size); + ret = DoTls13HandShakeMsgType(ssl, input, &idx, handshakeType, + messageLength, size); if (ret != 0) return ret; - Dtls13MsgWasProcessed(ssl, (enum HandShakeType)handshake_type); - + Dtls13MsgWasProcessed(ssl, (enum HandShakeType)handshakeType); *processedSize = idx; /* check if we have buffered some message */ From dce63fdfb3307154214e07b47bd6fbc701ecafa1 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 13 Jul 2022 11:17:38 +0200 Subject: [PATCH 2/6] async: fix issue with DTLSv1.3 --- src/dtls13.c | 15 ++++++++++++--- src/internal.c | 16 +++++++++++++++- src/tls13.c | 5 ++++- wolfssl/internal.h | 1 + 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index 56904a5c3..3f50dae18 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -351,7 +351,7 @@ static void Dtls13MsgWasProcessed(WOLFSSL* ssl, enum HandShakeType hs) ssl->dtls13Rtx.sendAcks = Dtls13RtxMsgNeedsAck(ssl, hs); } -static int Dtls13ProcessBufferedMessages(WOLFSSL* ssl) +int Dtls13ProcessBufferedMessages(WOLFSSL* ssl) { DtlsMsg* msg = ssl->dtls_rx_msg_list; word32 idx = 0; @@ -1419,6 +1419,7 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, { word32 fragOff, fragLength; byte isComplete, isFirst; + byte usingAsyncCrypto; word32 messageLength; byte handshakeType; word32 idx; @@ -1470,9 +1471,17 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, isFirst = fragOff == 0; isComplete = isFirst && fragLength == messageLength; + usingAsyncCrypto = ssl->devId != INVALID_DEVID; - if (!isComplete || ssl->keys.dtls_peer_handshake_number > - ssl->keys.dtls_expected_peer_handshake_number) { + /* store the message if any of the following: (a) incomplete message, (b) + * out of order message or (c) if using async crypto. In (c) the processing + * of the message can return WC_PENDING_E, it's easier to handle this error + * if the message is stored in the buffer. + */ + if (!isComplete || + ssl->keys.dtls_peer_handshake_number > + ssl->keys.dtls_expected_peer_handshake_number || + usingAsyncCrypto) { DtlsMsgStore(ssl, w64GetLow32(ssl->keys.curEpoch64), ssl->keys.dtls_peer_handshake_number, input + DTLS_HANDSHAKE_HEADER_SZ, messageLength, handshakeType, diff --git a/src/internal.c b/src/internal.c index f57457492..c0f7a9fa1 100644 --- a/src/internal.c +++ b/src/internal.c @@ -18113,11 +18113,25 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) #if defined(WOLFSSL_DTLS) && defined(WOLFSSL_ASYNC_CRYPT) /* process any pending DTLS messages - this flow can happen with async */ if (ssl->dtls_rx_msg_list != NULL) { - ret = DtlsMsgDrain(ssl); + word32 pendingMsg = ssl->dtls_rx_msg_list_sz; + if(IsAtLeastTLSv1_3(ssl->version)) { +#ifdef WOLFSSL_DTLS13 + ret = Dtls13ProcessBufferedMessages(ssl); +#elif + ret = NOT_COMPILED_IN; +#endif /* WOLFSSL_DTLS13 */ + } + else { + ret = DtlsMsgDrain(ssl); + } if (ret != 0) { WOLFSSL_ERROR(ret); return ret; } + /* we processed some messages, return so connect/accept can make + progress */ + if (ssl->dtls_rx_msg_list_sz != pendingMsg) + return ret; } #endif diff --git a/src/tls13.c b/src/tls13.c index 4353ddc3d..716685ae5 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -9475,8 +9475,11 @@ int DoTls13HandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, #if defined(WOLFSSL_ASYNC_CRYPT) || defined(WOLFSSL_ASYNC_IO) /* if async, offset index so this msg will be processed again */ - /* NOTE: check this now before other calls can overwirte ret */ + /* NOTE: check this now before other calls can overwrite ret */ if ((ret == WC_PENDING_E || ret == OCSP_WANT_READ) && *inOutIdx > 0) { + /* DTLS always stores a message in a buffer when async is enable, so we + * don't need to adjust for the extra bytes here (*inOutIdx is always + * == 0) */ *inOutIdx -= HANDSHAKE_HEADER_SZ; } #endif diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 33daeaa7c..567b08a23 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -5479,6 +5479,7 @@ WOLFSSL_LOCAL int Dtls13HashHandshake(WOLFSSL* ssl, const byte* output, word16 length); WOLFSSL_LOCAL void Dtls13FreeFsmResources(WOLFSSL* ssl); WOLFSSL_LOCAL int Dtls13RtxTimeout(WOLFSSL* ssl); +WOLFSSL_LOCAL int Dtls13ProcessBufferedMessages(WOLFSSL* ssl); #endif /* WOLFSSL_DTLS13 */ #ifdef WOLFSSL_STATIC_EPHEMERAL From 964ea85d3def3ea639713c2ae868bf8bc9577a05 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 13 Jul 2022 08:28:50 -0700 Subject: [PATCH 3/6] Fix typos for dynamic types in dtls13.c. --- src/dtls13.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index 3f50dae18..e6e7fb412 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -523,7 +523,7 @@ static int Dtls13SendFragment(WOLFSSL* ssl, byte* output, word16 output_size, static void Dtls13FreeFragmentsBuffer(WOLFSSL* ssl) { XFREE(ssl->dtls13FragmentsBuffer.buffer, ssl->heap, - DYNAMIC_TYPE_TEMP_BUFFER); + DYNAMIC_TYPE_TMP_BUFFER); ssl->dtls13FragmentsBuffer.buffer = NULL; ssl->dtls13SendingFragments = 0; ssl->dtls13MessageLength = ssl->dtls13FragOffset = 0; @@ -664,7 +664,7 @@ static void Dtls13RtxFlushAcks(WOLFSSL* ssl) while (list != NULL) { rn = list; list = rn->next; - XFREE(rn, ssl->heap, DYNAMIC_TYEP_DTLS_MSG); + XFREE(rn, ssl->heap, DYNAMIC_TYPE_DTLS_MSG); } ssl->dtls13Rtx.seenRecords = NULL; @@ -710,7 +710,7 @@ static void Dtls13RtxRemoveCurAck(WOLFSSL* ssl) if (w64Equal(rn->epoch, ssl->keys.curEpoch64) && w64Equal(rn->seq, ssl->keys.curSeq)) { *prevNext = rn->next; - XFREE(rn, ssl->heap, DYNAMIC_TYEP_DTLS_MSG); + XFREE(rn, ssl->heap, DYNAMIC_TYPE_DTLS_MSG); return; } From 53dde1dafe90dcf3c34d1aca297cad656a70c77e Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Thu, 14 Jul 2022 10:25:29 +0200 Subject: [PATCH 4/6] dtls12: async: store the message only if async is really used --- src/internal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index c0f7a9fa1..ae470ae2e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -15848,7 +15848,8 @@ static int DoDtlsHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, /* In async mode always store the message and process it with * DtlsMsgDrain because in case of a WC_PENDING_E it will be * easier this way. */ - if (ssl->dtls_rx_msg_list_sz < DTLS_POOL_SZ) { + if (ssl->devId != INVALID_DEVID && + ssl->dtls_rx_msg_list_sz < DTLS_POOL_SZ) { DtlsMsgStore(ssl, ssl->keys.curEpoch, ssl->keys.dtls_peer_handshake_number, input + idx, size, type, From aca83b42d78d2e56469191609584cd2788953f4a Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 20 Jul 2022 17:38:32 +0200 Subject: [PATCH 5/6] fix: dtls13: send immediately post-handshake certificate request --- src/dtls13.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dtls13.c b/src/dtls13.c index e6e7fb412..7cb32fa64 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -432,7 +432,9 @@ static int Dtls13SendNow(WOLFSSL* ssl, enum HandShakeType handshakeType) if (handshakeType == client_hello || handshakeType == hello_retry_request || handshakeType == finished || handshakeType == session_ticket || - handshakeType == session_ticket || handshakeType == key_update) + handshakeType == session_ticket || handshakeType == key_update || + (handshakeType == certificate_request && + ssl->options.handShakeState == HANDSHAKE_DONE)) return 1; return 0; From 163acb89afb23a95e15c27277c25dc8a2cd31585 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 20 Jul 2022 19:57:00 +0200 Subject: [PATCH 6/6] dtls13: consider certificate_request processed on WC_PENDING_E The error is due to the message triggered by the processing of the message (Connect()->SendTls13Certificate/SendTls13CertificateVerify/SendTls13Verify). Consider the message processed to avoid double processing. --- src/dtls13.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index 7cb32fa64..824bcc622 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -372,15 +372,21 @@ int Dtls13ProcessBufferedMessages(WOLFSSL* ssl) ret = DoTls13HandShakeMsgType(ssl, msg->msg, &idx, msg->type, msg->sz, msg->sz); + + /* processing certificate_request triggers a connect. The error came + * from there, the message can be considered processed successfully */ + if (ret == 0 || (msg->type == certificate_request && + ssl->options.handShakeDone && ret == WC_PENDING_E)) { + Dtls13MsgWasProcessed(ssl, (enum HandShakeType)msg->type); + + ssl->dtls_rx_msg_list = msg->next; + DtlsMsgDelete(msg, ssl->heap); + msg = ssl->dtls_rx_msg_list; + ssl->dtls_rx_msg_list_sz--; + } + if (ret != 0) break; - - Dtls13MsgWasProcessed(ssl, (enum HandShakeType)msg->type); - - ssl->dtls_rx_msg_list = msg->next; - DtlsMsgDelete(msg, ssl->heap); - msg = ssl->dtls_rx_msg_list; - ssl->dtls_rx_msg_list_sz--; } WOLFSSL_LEAVE("dtls13_process_buffered_messages()", ret);