From 85a596e54accb39b67f038ebd9d021018075bf15 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 1 Sep 2023 16:38:52 +0200 Subject: [PATCH 01/22] DTLS 1.3: allow fragmenting the second ClientHello message - DTLS 1.3 pqc support - Add --enable-dtls-frag-ch option to enable CH fragmenting - Send an alert when we get an empty keyshare with a cookie present to not allow for multiple HRR in one connection - Only update the DTLS window when we have successfully processed or stored a message - Call ssl->chGoodCb as soon as we have processed a verified full or fragmented ClientHello cookie --- configure.ac | 15 +++ examples/server/server.c | 5 + src/dtls.c | 59 +++++++-- src/dtls13.c | 55 ++++++++- src/internal.c | 230 ++++++++++++++++++++++++---------- src/ssl.c | 18 +-- src/tls.c | 81 ++++++++++-- src/tls13.c | 83 ++++++++++--- tests/api.c | 232 +++++++++++++++++++++++++++++++++++ wolfssl/internal.h | 17 ++- wolfssl/ssl.h | 4 + wolfssl/wolfcrypt/settings.h | 8 ++ 12 files changed, 685 insertions(+), 122 deletions(-) diff --git a/configure.ac b/configure.ac index ddde97543..19c40fc78 100644 --- a/configure.ac +++ b/configure.ac @@ -4516,6 +4516,21 @@ then AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_DTLS_CID" fi +# DTLS 1.3 Fragment Second ClientHello +AC_ARG_ENABLE([dtls-frag-ch], + [AS_HELP_STRING([--enable-dtls-frag-ch],[Enable wolfSSL DTLS 1.3 ClientHello fragmenting (default: disabled)])], + [ ENABLED_DTLS_CH_FRAG=$enableval ], + [ ENABLED_DTLS_CH_FRAG=no ] + ) +if test "x$ENABLED_DTLS_CH_FRAG" = "xyes" +then + if test "x$ENABLED_DTLS13" != "xyes" + then + AC_MSG_ERROR([You need to enable DTLSv1.3 to use DTLS ClientHello fragmenting]) + fi + AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_DTLS_CH_FRAG" +fi + # CODING AC_ARG_ENABLE([coding], [AS_HELP_STRING([--enable-coding],[Enable Coding base 16/64 (default: enabled)])], diff --git a/examples/server/server.c b/examples/server/server.c index 504aafde6..e882b0efd 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -3322,6 +3322,11 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) } #endif /* WOLFSSL_DTLS_CID */ +#ifdef WOLFSSL_DTLS_CH_FRAG + if (doDTLS) + wolfSSL_dtls13_allow_ch_frag(ssl, 1); +#endif + #ifndef WOLFSSL_CALLBACKS if (nonBlocking) { #ifdef WOLFSSL_DTLS diff --git a/src/dtls.c b/src/dtls.c index c207f33fe..aa7ba3155 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -26,6 +26,13 @@ * will consume less bandwidth (one ClientHello and one HelloVerifyRequest * less). On the other hand, if a valid SessionID is collected, forged * clientHello messages will consume resources on the server. + * WOLFSSL_DTLS_CH_FRAG + * Allow a server to process a fragmented second/verified (one containing a + * valid cookie response) ClientHello message. The first/unverifies (one + * without a cookie extension) ClientHello MUST be unfragmented so that the + * DTLS server can process it statelessly. This is only implemented for + * DTLS 1.3. The user MUST call wolfSSL_dtls13_allow_ch_frag() on the server + * to explicitly enable this during runtime. */ #ifdef HAVE_CONFIG_H @@ -263,10 +270,13 @@ static int CheckDtlsCookie(const WOLFSSL* ssl, WolfSSL_CH* ch, return ret; } -static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch) +static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch, + byte isFirstCHFrag) { word32 idx = 0; + (void)isFirstCHFrag; + /* protocol version, random and session id length check */ if (OPAQUE16_LEN + RAN_LEN + OPAQUE8_LEN > helloSz) return BUFFER_ERROR; @@ -288,9 +298,20 @@ static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch) idx += ReadVector8(input + idx, &ch->compression); if (idx < helloSz - OPAQUE16_LEN) { /* Extensions are optional */ +#ifdef WOLFSSL_DTLS_CH_FRAG + word32 extStart = idx + OPAQUE16_LEN; +#endif idx += ReadVector16(input + idx, &ch->extension); - if (idx > helloSz) - return BUFFER_ERROR; + if (idx > helloSz) { +#ifdef WOLFSSL_DTLS_CH_FRAG + /* Allow incomplete extensions if we are parsing a fragment */ + if (isFirstCHFrag && extStart < helloSz) + ch->extension.size = helloSz - extStart; + else +#endif + return BUFFER_ERROR; + idx = helloSz; + } } if (idx != helloSz) return BUFFER_ERROR; @@ -860,17 +881,27 @@ static int ClientHelloSanityCheck(WolfSSL_CH* ch, byte isTls13) return 0; } -int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, - word32* inOutIdx, word32 helloSz) +int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, + byte isFirstCHFrag) { int ret; WolfSSL_CH ch; byte isTls13 = 0; + WOLFSSL_ENTER("DoClientHelloStateless"); + if (isFirstCHFrag) { +#ifdef WOLFSSL_DTLS_CH_FRAG + WOLFSSL_MSG("\tProcessing fragmented ClientHello"); +#else + WOLFSSL_MSG("\tProcessing fragmented ClientHello but " + "WOLFSSL_DTLS_CH_FRAG is not defined. This should not happen."); +#endif + } + XMEMSET(&ch, 0, sizeof(ch)); ssl->options.dtlsStateful = 0; - ret = ParseClientHello(input + *inOutIdx, helloSz, &ch); + ret = ParseClientHello(input, helloSz, &ch, isFirstCHFrag); if (ret != 0) return ret; @@ -894,7 +925,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, return ret; #ifdef WOLFSSL_DTLS_NO_HVR_ON_RESUME - if (!isTls13) { + if (!isTls13 && !isFirstCHFrag) { int resume = FALSE; ret = TlsResumptionIsValid(ssl, &ch, &resume); if (ret != 0) @@ -907,7 +938,13 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, #endif if (ch.cookie.size == 0 && ch.cookieExt.size == 0) { - ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); +#ifdef WOLFSSL_DTLS_CH_FRAG + /* Don't send anything here when processing fragment */ + if (isFirstCHFrag) + ret = BUFFER_ERROR; + else +#endif + ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); } else { byte cookieGood; @@ -921,6 +958,12 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, if (isTls13) ret = INVALID_PARAMETER; else +#endif +#ifdef WOLFSSL_DTLS_CH_FRAG + /* Don't send anything here when processing fragment */ + if (isFirstCHFrag) + ret = BUFFER_ERROR; + else #endif ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); } diff --git a/src/dtls13.c b/src/dtls13.c index cd84f27d6..42c7ca7bd 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -1573,6 +1573,19 @@ static int Dtls13RtxSendBuffered(WOLFSSL* ssl) return 0; } +static int Dtls13AcceptFragmented(WOLFSSL *ssl, enum HandShakeType type) +{ + if (IsEncryptionOn(ssl, 0)) + return 1; + if (ssl->options.side == WOLFSSL_CLIENT_END && type == server_hello) + return 1; +#ifdef WOLFSSL_DTLS_CH_FRAG + if (ssl->options.side == WOLFSSL_SERVER_END && type == client_hello && + ssl->options.dtls13ChFrag && ssl->options.dtlsStateful) + return 1; +#endif + return 0; +} /** * Dtls13HandshakeRecv() - process an handshake message. Deal with fragmentation if needed @@ -1646,13 +1659,33 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, isFirst = fragOff == 0; isComplete = isFirst && fragLength == messageLength; - if (!isComplete && !IsEncryptionOn(ssl, 0)) { + if (!isComplete && !Dtls13AcceptFragmented(ssl, handshakeType)) { +#ifdef WOLFSSL_DTLS_CH_FRAG + /* check if the first CH fragment contains a valid cookie */ + if (ssl->options.dtls13ChFrag && !ssl->options.dtlsStateful && + isFirst && handshakeType == client_hello && + DoClientHelloStateless(ssl, input + idx, fragLength, 1) == 0) { + /* We can save this message and continue as stateful. */ + if (ssl->chGoodCb != NULL && !IsSCR(ssl)) { + int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); + if (cbret < 0) { + ssl->error = cbret; + WOLFSSL_MSG("ClientHello Good Cb don't continue error"); + return WOLFSSL_FATAL_ERROR; + } + } + WOLFSSL_MSG("ClientHello fragment verified"); + } + else +#endif + { #ifdef WOLFSSL_DEBUG_TLS - WOLFSSL_MSG("DTLS1.3 not accepting fragmented plaintext message"); + WOLFSSL_MSG("DTLS1.3 not accepting fragmented plaintext message"); #endif /* WOLFSSL_DEBUG_TLS */ - /* ignore the message */ - *processedSize = idx + fragLength + ssl->keys.padSz; - return 0; + /* ignore the message */ + *processedSize = idx + fragLength + ssl->keys.padSz; + return 0; + } } usingAsyncCrypto = ssl->devId != INVALID_DEVID; @@ -2797,4 +2830,16 @@ int Dtls13CheckAEADFailLimit(WOLFSSL* ssl) } #endif +#ifdef WOLFSSL_DTLS_CH_FRAG +int wolfSSL_dtls13_allow_ch_frag(WOLFSSL *ssl, int enabled) +{ + if (ssl->options.side == WOLFSSL_CLIENT_END) { + return WOLFSSL_FAILURE; + } + ssl->options.dtls13ChFrag = !!enabled; + return WOLFSSL_SUCCESS; +} +#endif + + #endif /* WOLFSSL_DTLS13 */ diff --git a/src/internal.c b/src/internal.c index 638814f6f..e323acf6c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8017,6 +8017,12 @@ void SSL_ResourceFree(WOLFSSL* ssl) ssl->dtls13ClientHello = NULL; ssl->dtls13ClientHelloSz = 0; } +#ifdef WOLFSSL_DTLS_CH_FRAG + if (ssl->dtls13KSE != NULL) { + TLSX_KeyShare_FreeAll(ssl->dtls13KSE, ssl->heap); + ssl->dtls13KSE = NULL; + } +#endif #endif /* WOLFSSL_DTLS13 */ #endif /* WOLFSSL_DTLS */ @@ -16806,6 +16812,8 @@ static WC_INLINE int Dtls13CheckWindow(WOLFSSL* ssl) int wordIndex; word32 diff; + WOLFSSL_ENTER("Dtls13CheckWindow"); + if (ssl->dtls13DecryptEpoch == NULL) { WOLFSSL_MSG("Can't find decrypting epoch"); return 0; @@ -17047,14 +17055,26 @@ static WC_INLINE int Dtls13UpdateWindow(WOLFSSL* ssl) int wordOffset; int wordIndex; word32 diff; + Dtls13Epoch* e = ssl->dtls13DecryptEpoch; + + WOLFSSL_ENTER("Dtls13UpdateWindow"); if (ssl->dtls13DecryptEpoch == NULL) { WOLFSSL_MSG("Can't find decrypting Epoch"); return BAD_STATE_E; } - nextSeq = ssl->dtls13DecryptEpoch->nextPeerSeqNumber; - window = ssl->dtls13DecryptEpoch->window; + if (!w64Equal(ssl->keys.curEpoch64, ssl->dtls13DecryptEpoch->epochNumber)) { + /* ssl->dtls13DecryptEpoch has been updated since we received the msg */ + e = Dtls13GetEpoch(ssl, ssl->keys.curEpoch64); + if (e == NULL) { + WOLFSSL_MSG("Can't find decrypting Epoch"); + return BAD_STATE_E; + } + } + + nextSeq = e->nextPeerSeqNumber; + window = e->window; seq = ssl->keys.curSeq; /* seq < nextSeq */ @@ -17075,7 +17095,7 @@ static WC_INLINE int Dtls13UpdateWindow(WOLFSSL* ssl) } window[wordIndex] |= (1 << wordOffset); - return 1; + return 0; } /* seq >= nextSeq, seq - nextSeq */ @@ -17086,9 +17106,9 @@ static WC_INLINE int Dtls13UpdateWindow(WOLFSSL* ssl) _DtlsUpdateWindowGTSeq(w64GetLow32(diff64), window); w64Increment(&seq); - ssl->dtls13DecryptEpoch->nextPeerSeqNumber = seq; + e->nextPeerSeqNumber = seq; - return 1; + return 0; } #endif /* WOLFSSL_DTLS13 */ @@ -20714,32 +20734,17 @@ default: /* the record layer is here */ case runProcessingOneRecord: #ifdef WOLFSSL_DTLS13 - if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version)) { + if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version) && + !Dtls13CheckWindow(ssl)) { + /* drop packet */ + WOLFSSL_MSG("Dropping DTLS record outside receiving window"); + ssl->options.processReply = doProcessInit; + ssl->buffers.inputBuffer.idx += ssl->curSize; + if (ssl->buffers.inputBuffer.idx > + ssl->buffers.inputBuffer.length) + return BUFFER_E; - if(!Dtls13CheckWindow(ssl)) { - /* drop packet */ - WOLFSSL_MSG( - "Dropping DTLS record outside receiving window"); - ssl->options.processReply = doProcessInit; - ssl->buffers.inputBuffer.idx += ssl->curSize; - if (ssl->buffers.inputBuffer.idx > - ssl->buffers.inputBuffer.length) - return BUFFER_E; - - continue; - } - - ret = Dtls13UpdateWindow(ssl); - if (ret != 1) { - WOLFSSL_ERROR(ret); - return ret; - } - - ret = Dtls13RecordRecvd(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } + continue; } #endif /* WOLFSSL_DTLS13 */ ssl->options.processReply = runProcessingOneMessage; @@ -20773,16 +20778,16 @@ default: } else #endif - /* TLS13 plaintext limit is checked earlier before decryption */ - /* For TLS v1.1 the block size and explicit IV are added to idx, - * so it needs to be included in this limit check */ - if (!IsAtLeastTLSv1_3(ssl->version) - && ssl->curSize - ssl->keys.padSz - - (ssl->buffers.inputBuffer.idx - startIdx) - > MAX_PLAINTEXT_SZ + /* TLS13 plaintext limit is checked earlier before decryption */ + /* For TLS v1.1 the block size and explicit IV are added to idx, + * so it needs to be included in this limit check */ + if (!IsAtLeastTLSv1_3(ssl->version) + && ssl->curSize - ssl->keys.padSz - + (ssl->buffers.inputBuffer.idx - startIdx) + > MAX_PLAINTEXT_SZ #ifdef WOLFSSL_ASYNC_CRYPT - && ssl->buffers.inputBuffer.length != - ssl->buffers.inputBuffer.idx + && ssl->buffers.inputBuffer.length != + ssl->buffers.inputBuffer.idx #endif ) { WOLFSSL_MSG("Plaintext too long"); @@ -20793,17 +20798,6 @@ default: return BUFFER_ERROR; } -#ifdef WOLFSSL_DTLS - if (IsDtlsNotSctpMode(ssl) && !IsAtLeastTLSv1_3(ssl->version)) { - _DtlsUpdateWindow(ssl); - } - - if (ssl->options.dtls) { - /* Reset timeout as we have received a valid DTLS message */ - ssl->dtls_timeout = ssl->dtls_timeout_init; - } -#endif /* WOLFSSL_DTLS */ - WOLFSSL_MSG("received record layer msg"); switch (ssl->curRL.type) { @@ -20813,16 +20807,23 @@ default: if (ssl->options.dtls) { #ifdef WOLFSSL_DTLS if (!IsAtLeastTLSv1_3(ssl->version)) { - ret = DoDtlsHandShakeMsg(ssl, - ssl->buffers.inputBuffer.buffer, - &ssl->buffers.inputBuffer.idx, - ssl->buffers.inputBuffer.length); - if (ret != 0) { - if (SendFatalAlertOnly(ssl, ret) - == SOCKET_ERROR_E) { - ret = SOCKET_ERROR_E; - } + ret = DoDtlsHandShakeMsg(ssl, + ssl->buffers.inputBuffer.buffer, + &ssl->buffers.inputBuffer.idx, + ssl->buffers.inputBuffer.length); + if (ret == 0 && ssl->options.dtlsStateful) { + if (IsDtlsNotSctpMode(ssl)) + _DtlsUpdateWindow(ssl); + /* Reset timeout as we have received a valid + * DTLS handshake message */ + ssl->dtls_timeout = ssl->dtls_timeout_init; + } + if (ret != 0) { + if (SendFatalAlertOnly(ssl, ret) + == SOCKET_ERROR_E) { + ret = SOCKET_ERROR_E; } + } } #endif #ifdef WOLFSSL_DTLS13 @@ -20831,6 +20832,18 @@ default: ssl->buffers.inputBuffer.buffer, &ssl->buffers.inputBuffer.idx, ssl->buffers.inputBuffer.length); + if (ret == 0 && ssl->options.dtlsStateful) { + ret = Dtls13UpdateWindow(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + ret = Dtls13RecordRecvd(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + } #ifdef WOLFSSL_EARLY_DATA if (ret == 0 && ssl->options.side == WOLFSSL_SERVER_END && @@ -20951,6 +20964,20 @@ default: WOLFSSL_ERROR_VERBOSE(UNKNOWN_RECORD_TYPE); return UNKNOWN_RECORD_TYPE; } +#ifdef WOLFSSL_DTLS13 + if (ssl->options.dtls) { + ret = Dtls13UpdateWindow(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + ret = Dtls13RecordRecvd(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + } +#endif break; } #endif @@ -21038,6 +21065,8 @@ default: #ifdef WOLFSSL_DTLS if (ssl->options.dtls) { WOLFSSL_DTLS_PEERSEQ* peerSeq = ssl->keys.peerSeq; + if (IsDtlsNotSctpMode(ssl)) + _DtlsUpdateWindow(ssl); #ifdef WOLFSSL_MULTICAST if (ssl->options.haveMcast) { peerSeq += ssl->keys.curPeerId; @@ -21099,10 +21128,32 @@ default: return SANITY_MSG_E; } #endif - if ((ret = DoApplicationData(ssl, - ssl->buffers.inputBuffer.buffer, - &ssl->buffers.inputBuffer.idx, - NO_SNIFF)) != 0) { + ret = DoApplicationData(ssl, + ssl->buffers.inputBuffer.buffer, + &ssl->buffers.inputBuffer.idx, NO_SNIFF); +#ifdef WOLFSSL_DTLS + if (ssl->options.dtls && + (ret == 0 || ret == APP_DATA_READY)) { +#ifdef WOLFSSL_DTLS13 + if (IsAtLeastTLSv1_3(ssl->version)) { + int updateRet = Dtls13UpdateWindow(ssl); + if (updateRet != 0) { + WOLFSSL_ERROR(updateRet); + return updateRet; + } + updateRet = Dtls13RecordRecvd(ssl); + if (updateRet != 0) { + WOLFSSL_ERROR(updateRet); + return updateRet; + } + } + else +#endif + if (IsDtlsNotSctpMode(ssl)) + _DtlsUpdateWindow(ssl); + } +#endif + if (ret != 0) { WOLFSSL_ERROR(ret); return ret; } @@ -21131,6 +21182,27 @@ default: /* Reset error if we got an alert level in ret */ if (ret > 0) ret = 0; +#ifdef WOLFSSL_DTLS + if (ssl->options.dtls) { +#ifdef WOLFSSL_DTLS13 + if (IsAtLeastTLSv1_3(ssl->version)) { + ret = Dtls13UpdateWindow(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + ret = Dtls13RecordRecvd(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + } + else +#endif + if (IsDtlsNotSctpMode(ssl)) + _DtlsUpdateWindow(ssl); + } +#endif break; #ifdef WOLFSSL_DTLS13 @@ -21147,6 +21219,16 @@ default: ssl->buffers.inputBuffer.idx += ssl->keys.padSz; if (ret != 0) return ret; + ret = Dtls13UpdateWindow(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + ret = Dtls13RecordRecvd(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } break; } FALL_THROUGH; @@ -34214,8 +34296,15 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, "VerifyServerSuite() with MEMORY_E"); return 0; } - if (cs->clientKSE == NULL && searched) + if (cs->clientKSE == NULL && searched) { + #ifdef WOLFSSL_SEND_HRR_COOKIE + /* If the CH contains a cookie then we need to send an alert to + * start from scratch. */ + if (TLSX_Find(extensions, TLSX_COOKIE) != NULL) + return INVALID_PARAMETER; + #endif cs->doHelloRetry = 1; + } #ifdef WOLFSSL_ASYNC_CRYPT if (ret == WC_PENDING_E) return ret; @@ -34716,9 +34805,10 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #ifdef WOLFSSL_DTLS /* Update the ssl->options.dtlsStateful setting `if` statement in * wolfSSL_accept when changing this one. */ - if (IsDtlsNotSctpMode(ssl) && IsDtlsNotSrtpMode(ssl) && !IsSCR(ssl)) { + if (IsDtlsNotSctpMode(ssl) && IsDtlsNotSrtpMode(ssl) && !IsSCR(ssl) && + !ssl->options.dtlsStateful) { DtlsSetSeqNumForReply(ssl); - ret = DoClientHelloStateless(ssl, input, inOutIdx, helloSz); + ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0); if (ret != 0 || !ssl->options.dtlsStateful) { int alertType = TranslateErrorToAlert(ret); if (alertType != invalid_alert) { @@ -34735,6 +34825,14 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ret = 0; return ret; } + if (ssl->chGoodCb != NULL) { + int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); + if (cbret < 0) { + ssl->error = cbret; + WOLFSSL_MSG("ClientHello Good Cb don't continue error"); + return WOLFSSL_FATAL_ERROR; + } + } } ssl->options.dtlsStateful = 1; #endif /* WOLFSSL_DTLS */ diff --git a/src/ssl.c b/src/ssl.c index b0639d992..c0c562405 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -12987,17 +12987,6 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, } #endif -#ifdef WOLFSSL_DTLS - if (ssl->chGoodCb != NULL && !IsSCR(ssl)) { - int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); - if (cbret < 0) { - ssl->error = cbret; - WOLFSSL_MSG("ClientHello Good Cb don't continue error"); - return WOLFSSL_FATAL_ERROR; - } - } -#endif - ssl->options.acceptState = ACCEPT_FIRST_REPLY_DONE; WOLFSSL_MSG("accept state ACCEPT_FIRST_REPLY_DONE"); FALL_THROUGH; @@ -13275,7 +13264,6 @@ int wolfSSL_SetHsDoneCb(WOLFSSL* ssl, HandShakeDoneCb cb, void* user_ctx) ssl->hsDoneCb = cb; ssl->hsDoneCtx = user_ctx; - return WOLFSSL_SUCCESS; } @@ -19240,6 +19228,12 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, ssl->options.haveSessionId = 0; ssl->options.tls = 0; ssl->options.tls1_1 = 0; + #ifdef WOLFSSL_DTLS + ssl->options.dtlsStateful = 0; + #endif + #ifdef WOLFSSL_DTLS_CH_FRAG + ssl->options.dtlsSentEmptyKS = 0; + #endif #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) ssl->options.noPskDheKe = 0; #ifdef HAVE_SUPPORTED_CURVES diff --git a/src/tls.c b/src/tls.c index 6091cb7f9..ede90f857 100644 --- a/src/tls.c +++ b/src/tls.c @@ -67,7 +67,6 @@ #if defined(WOLFSSL_TLS13) && defined(HAVE_SUPPORTED_CURVES) static int TLSX_KeyShare_IsSupported(int namedGroup); -static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap); #endif #ifdef HAVE_SUPPORTED_CURVES @@ -7769,7 +7768,7 @@ int TLSX_KeyShare_GenKey(WOLFSSL *ssl, KeyShareEntry *kse) * list The linked list of key share entry objects. * heap The heap used for allocation. */ -static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap) +void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap) { KeyShareEntry* current; @@ -8722,7 +8721,7 @@ int TLSX_KeyShare_Parse_ClientHello(const WOLFSSL* ssl, int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length, byte msgType) { - int ret; + int ret = 0; KeyShareEntry *keyShareEntry = NULL; word16 group; @@ -8784,24 +8783,50 @@ int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length, if (ssl->error != WC_PENDING_E) #endif { - /* Check the selected group was supported by ClientHello extensions. */ + /* Check the selected group was supported by ClientHello + * extensions. */ if (!TLSX_SupportedGroups_Find(ssl, group, ssl->extensions)) { WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); return BAD_KEY_SHARE_DATA; } - /* Check if the group was sent. */ - if (TLSX_KeyShare_Find(ssl, group)) { - WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); - return BAD_KEY_SHARE_DATA; +#ifdef WOLFSSL_DTLS_CH_FRAG + /* If we sent an empty key share then we can just limit the keyshare + * to the one selected by the server. */ + if (ssl->options.dtlsSentEmptyKS) { + if (!TLSX_KeyShare_SelectGroup(ssl, group)) { + /* Clear out all groups if not found */ + ret = TLSX_KeyShare_Empty(ssl); + if (ret != 0) + return ret; + } + } + else +#endif + { + /* Check if the group was sent. */ + if (TLSX_KeyShare_Find(ssl, group)) { + WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); + return BAD_KEY_SHARE_DATA; + } + + /* Clear out unusable key shares. */ + ret = TLSX_KeyShare_Empty(ssl); + if (ret != 0) + return ret; } - /* Clear out unusable key shares. */ - ret = TLSX_KeyShare_Empty(ssl); - if (ret != 0) - return ret; } + +#ifdef WOLFSSL_DTLS_CH_FRAG + /* Check if we were able to limit the keyshare entries to one group */ + if (ssl->options.dtlsSentEmptyKS && + TLSX_KeyShare_SelectGroup(ssl, group)) { + /* Nothing to do */ + } + else +#endif #ifdef HAVE_PQC /* For post-quantum groups, do this in TLSX_PopulateExtensions(). */ if (!WOLFSSL_NAMED_GROUP_IS_PQC(group)) @@ -9102,6 +9127,38 @@ int TLSX_KeyShare_Use(const WOLFSSL* ssl, word16 group, word16 len, byte* data, return 0; } +/* Clear out all entries except for group + * + * ssl The SSL/TLS object. + * returns 1 when the group was found and 0 when it wasn't found. + * */ +int TLSX_KeyShare_SelectGroup(WOLFSSL* ssl, word16 group) +{ + TLSX* extension; + KeyShareEntry* list; + KeyShareEntry** prev; + + /* Find the KeyShare extension if it exists. */ + extension = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); + if (extension != NULL) { + for (prev = (KeyShareEntry**)&extension->data, + list = (KeyShareEntry*)extension->data; list != NULL; + prev = &list->next, list = list->next) { + if (list->group == group) { + /* Unlink it from the list */ + *prev = list->next; + list->next = NULL; + /* Free the list */ + TLSX_KeyShare_FreeAll((KeyShareEntry*)extension->data, + ssl->heap); + extension->data = list; + return 1; + } + } + } + return 0; +} + /* Set an empty Key Share extension. * * ssl The SSL/TLS object. diff --git a/src/tls13.c b/src/tls13.c index d0765a470..9a23bb2a0 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -4410,10 +4410,48 @@ int SendTls13ClientHello(WOLFSSL* ssl) } #endif - /* Include length of TLS extensions. */ - ret = TLSX_GetRequestSize(ssl, client_hello, &args->length); - if (ret != 0) - return ret; + { +#ifdef WOLFSSL_DTLS_CH_FRAG + int maxFrag = wolfSSL_GetMaxFragSize(ssl, MAX_RECORD_SIZE); + word16 lenWithoutExts = args->length; +#endif + + /* Include length of TLS extensions. */ + ret = TLSX_GetRequestSize(ssl, client_hello, &args->length); + if (ret != 0) + return ret; + +#ifdef WOLFSSL_DTLS_CH_FRAG + if (ssl->options.dtls && args->length > maxFrag && + TLSX_Find(ssl->extensions, TLSX_COOKIE) == NULL) { + /* Try again with an empty key share if we would be fragmenting + * without a cookie */ + TLSX* ks = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); + if (ks == NULL) { + WOLFSSL_MSG("No key share and CH can't fit in one fragment."); + return BUFFER_ERROR; + } + args->length = lenWithoutExts; + if (ssl->dtls13KSE != NULL) + TLSX_KeyShare_FreeAll(ssl->dtls13KSE, ssl->heap); + ssl->dtls13KSE = (KeyShareEntry*)ks->data; + ks->data = NULL; + ret = TLSX_GetRequestSize(ssl, client_hello, &args->length); + if (ret != 0) { + /* Restore key share data */ + ks->data = ssl->dtls13KSE; + ssl->dtls13KSE = NULL; + return ret; + } + if (args->length > maxFrag) { + WOLFSSL_MSG("Can't fit first CH in one fragment."); + return BUFFER_ERROR; + } + WOLFSSL_MSG("Sending empty key share so we don't fragment CH1"); + ssl->options.dtlsSentEmptyKS = 1; + } +#endif + } /* Total message size. */ args->sendSz = args->length + HANDSHAKE_HEADER_SZ + RECORD_HEADER_SZ; @@ -4653,6 +4691,19 @@ int SendTls13ClientHello(WOLFSSL* ssl) if (ret == 0) FreeAsyncCtx(ssl, 0); #endif +#ifdef WOLFSSL_DTLS_CH_FRAG + if ((ret == 0 || ret == WANT_WRITE) && ssl->dtls13KSE != NULL) { + /* Restore the keyshare */ + TLSX* ks = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); + if (ks == NULL || ks->data != NULL) { + WOLFSSL_MSG("Missing key share or key share data not NULL"); + return BUFFER_ERROR; + } + WOLFSSL_MSG("Restored key share"); + ks->data = ssl->dtls13KSE; + ssl->dtls13KSE = NULL; + } +#endif WOLFSSL_LEAVE("SendTls13ClientHello", ret); WOLFSSL_END(WC_FUNC_CLIENT_HELLO_SEND); @@ -6624,12 +6675,21 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE) /* Update the ssl->options.dtlsStateful setting `if` statement in * wolfSSL_accept_TLSv13 when changing this one. */ - if (IsDtlsNotSctpMode(ssl) && ssl->options.sendCookie) { - ret = DoClientHelloStateless(ssl, input, inOutIdx, helloSz); + if (IsDtlsNotSctpMode(ssl) && ssl->options.sendCookie && + !ssl->options.dtlsStateful) { + ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0); if (ret != 0 || !ssl->options.dtlsStateful) { *inOutIdx += helloSz; goto exit_dch; } + if (ssl->chGoodCb != NULL && !IsSCR(ssl)) { + int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); + if (cbret < 0) { + ssl->error = cbret; + WOLFSSL_MSG("ClientHello Good Cb don't continue error"); + return WOLFSSL_FATAL_ERROR; + } + } } ssl->options.dtlsStateful = 1; #endif /* WOLFSSL_DTLS */ @@ -13223,17 +13283,6 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl) case TLS13_ACCEPT_SECOND_REPLY_DONE : -#ifdef WOLFSSL_DTLS - if (ssl->chGoodCb != NULL) { - int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); - if (cbret < 0) { - ssl->error = cbret; - WOLFSSL_MSG("ClientHello Good Cb don't continue error"); - return WOLFSSL_FATAL_ERROR; - } - } -#endif - if ((ssl->error = SendTls13ServerHello(ssl, server_hello)) != 0) { WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; diff --git a/tests/api.c b/tests/api.c index 0d51fc840..4e5f6efed 100644 --- a/tests/api.c +++ b/tests/api.c @@ -65238,6 +65238,236 @@ static int test_revoked_loaded_int_cert(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS) \ + && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) +static int test_dtls_frag_ch_count_records(byte* b, int len) +{ + DtlsRecordLayerHeader* dtlsRH; + int records = 0; + size_t recordLen; + while (len > 0) { + records++; + dtlsRH = (DtlsRecordLayerHeader*)b; + recordLen = (dtlsRH->length[0] << 8) | dtlsRH->length[1]; + b += sizeof(DtlsRecordLayerHeader) + recordLen; + len -= sizeof(DtlsRecordLayerHeader) + recordLen; + } + return records; +} +#endif + +static int test_dtls_frag_ch(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) \ + && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + static unsigned int DUMMY_MTU = 256; + unsigned char four_frag_CH[] = { + 0x16, 0xfe, 0xfd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xda, 0x01, 0x00, 0x02, 0xdc, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xce, 0xfe, 0xfd, 0xf3, 0x94, 0x01, 0x33, 0x2c, 0xcf, 0x2c, 0x47, 0xb1, + 0xe5, 0xa1, 0x7b, 0x19, 0x3e, 0xac, 0x68, 0xdd, 0xe6, 0x17, 0x6b, 0x85, + 0xad, 0x5f, 0xfc, 0x7f, 0x6e, 0xf0, 0xb9, 0xe0, 0x2e, 0xca, 0x47, 0x00, + 0x00, 0x00, 0x36, 0x13, 0x01, 0x13, 0x02, 0x13, 0x03, 0xc0, 0x2c, 0xc0, + 0x2b, 0xc0, 0x30, 0xc0, 0x2f, 0x00, 0x9f, 0x00, 0x9e, 0xcc, 0xa9, 0xcc, + 0xa8, 0xcc, 0xaa, 0xc0, 0x27, 0xc0, 0x23, 0xc0, 0x28, 0xc0, 0x24, 0xc0, + 0x0a, 0xc0, 0x09, 0xc0, 0x14, 0xc0, 0x13, 0x00, 0x6b, 0x00, 0x67, 0x00, + 0x39, 0x00, 0x33, 0xcc, 0x14, 0xcc, 0x13, 0xcc, 0x15, 0x01, 0x00, 0x02, + 0x7c, 0x00, 0x2b, 0x00, 0x03, 0x02, 0xfe, 0xfc, 0x00, 0x0d, 0x00, 0x20, + 0x00, 0x1e, 0x06, 0x03, 0x05, 0x03, 0x04, 0x03, 0x02, 0x03, 0x08, 0x06, + 0x08, 0x0b, 0x08, 0x05, 0x08, 0x0a, 0x08, 0x04, 0x08, 0x09, 0x06, 0x01, + 0x05, 0x01, 0x04, 0x01, 0x03, 0x01, 0x02, 0x01, 0x00, 0x0a, 0x00, 0x0c, + 0x00, 0x0a, 0x00, 0x19, 0x00, 0x18, 0x00, 0x17, 0x00, 0x15, 0x01, 0x00, + 0x00, 0x16, 0x00, 0x00, 0x00, 0x33, 0x02, 0x39, 0x02, 0x37, 0x00, 0x17, + 0x00, 0x41, 0x04, 0x94, 0xdf, 0x36, 0xd7, 0xb3, 0x90, 0x6d, 0x01, 0xa1, + 0xe6, 0xed, 0x67, 0xf4, 0xd9, 0x9d, 0x2c, 0xac, 0x57, 0x74, 0xff, 0x19, + 0xbe, 0x5a, 0xc9, 0x30, 0x11, 0xb7, 0x2b, 0x59, 0x47, 0x80, 0x7c, 0xa9, + 0xb7, 0x31, 0x8c, 0x16, 0xfe, 0xfd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x01, 0x00, 0xda, 0x01, 0x00, 0x02, 0xdc, 0x00, 0x00, 0x00, 0x00, + 0xce, 0x00, 0x00, 0xce, 0x9e, 0x13, 0x74, 0x3b, 0x86, 0xba, 0x69, 0x1f, + 0x12, 0xf7, 0xcd, 0x78, 0x53, 0xe8, 0x50, 0x4d, 0x71, 0x3f, 0x4b, 0x4e, + 0xeb, 0x3e, 0xe5, 0x43, 0x54, 0x78, 0x17, 0x6d, 0x00, 0x18, 0x00, 0x61, + 0x04, 0xd1, 0x99, 0x66, 0x4f, 0xda, 0xc7, 0x12, 0x3b, 0xff, 0xb2, 0xd6, + 0x2f, 0x35, 0xb6, 0x17, 0x1f, 0xb3, 0xd0, 0xb6, 0x52, 0xff, 0x97, 0x8b, + 0x01, 0xe8, 0xd9, 0x68, 0x71, 0x40, 0x02, 0xd5, 0x68, 0x3a, 0x58, 0xb2, + 0x5d, 0xee, 0xa4, 0xe9, 0x5f, 0xf4, 0xaf, 0x3e, 0x30, 0x9c, 0x3e, 0x2b, + 0xda, 0x61, 0x43, 0x99, 0x02, 0x35, 0x33, 0x9f, 0xcf, 0xb5, 0xd3, 0x28, + 0x19, 0x9d, 0x1c, 0xbe, 0x69, 0x07, 0x9e, 0xfc, 0xe4, 0x8e, 0xcd, 0x86, + 0x4a, 0x1b, 0xf0, 0xfc, 0x17, 0x94, 0x66, 0x53, 0xda, 0x24, 0x5e, 0xaf, + 0xce, 0xec, 0x62, 0x4c, 0x06, 0xb4, 0x52, 0x94, 0xb1, 0x4a, 0x7a, 0x8c, + 0x4f, 0x00, 0x19, 0x00, 0x85, 0x04, 0x00, 0x27, 0xeb, 0x99, 0x49, 0x7f, + 0xcb, 0x2c, 0x46, 0x54, 0x2d, 0x93, 0x5d, 0x25, 0x92, 0x58, 0x5e, 0x06, + 0xc3, 0x7c, 0xfb, 0x9a, 0xa7, 0xec, 0xcd, 0x9f, 0xe1, 0x6b, 0x2d, 0x78, + 0xf5, 0x16, 0xa9, 0x20, 0x52, 0x48, 0x19, 0x0f, 0x1a, 0xd0, 0xce, 0xd8, + 0x68, 0xb1, 0x4e, 0x7f, 0x33, 0x03, 0x7d, 0x0c, 0x39, 0xdb, 0x9c, 0x4b, + 0xf4, 0xe7, 0xc2, 0xf5, 0xdd, 0x51, 0x9b, 0x03, 0xa8, 0x53, 0x2b, 0xe6, + 0x00, 0x15, 0x4b, 0xff, 0xd2, 0xa0, 0x16, 0xfe, 0xfd, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0xda, 0x01, 0x00, 0x02, 0xdc, 0x00, + 0x00, 0x00, 0x01, 0x9c, 0x00, 0x00, 0xce, 0x58, 0x30, 0x10, 0x3d, 0x46, + 0xcc, 0xca, 0x1a, 0x44, 0xc8, 0x58, 0x9b, 0x27, 0x17, 0x67, 0x31, 0x96, + 0x8a, 0x66, 0x39, 0xf4, 0xcc, 0xc1, 0x9f, 0x12, 0x1f, 0x01, 0x30, 0x50, + 0x16, 0xd6, 0x89, 0x97, 0xa3, 0x66, 0xd7, 0x99, 0x50, 0x09, 0x6e, 0x80, + 0x87, 0xe4, 0xa2, 0x88, 0xae, 0xb4, 0x23, 0x57, 0x2f, 0x12, 0x60, 0xe7, + 0x7d, 0x44, 0x2d, 0xad, 0xbe, 0xe9, 0x0d, 0x01, 0x00, 0x01, 0x00, 0xd5, + 0xdd, 0x62, 0xee, 0xf3, 0x0e, 0xd9, 0x30, 0x0e, 0x38, 0xf3, 0x48, 0xf4, + 0xc9, 0x8f, 0x8c, 0x20, 0xf7, 0xd3, 0xa8, 0xb3, 0x87, 0x3c, 0x98, 0x5d, + 0x70, 0xc5, 0x03, 0x76, 0xb7, 0xd5, 0x0b, 0x7b, 0x23, 0x97, 0x6b, 0xe3, + 0xb5, 0x18, 0xeb, 0x64, 0x55, 0x18, 0xb2, 0x8a, 0x90, 0x1a, 0x8f, 0x0e, + 0x15, 0xda, 0xb1, 0x8e, 0x7f, 0xee, 0x1f, 0xe0, 0x3b, 0xb9, 0xed, 0xfc, + 0x4e, 0x3f, 0x78, 0x16, 0x39, 0x95, 0x5f, 0xb7, 0xcb, 0x65, 0x55, 0x72, + 0x7b, 0x7d, 0x86, 0x2f, 0x8a, 0xe5, 0xee, 0xf7, 0x57, 0x40, 0xf3, 0xc4, + 0x96, 0x4f, 0x11, 0x4d, 0x85, 0xf9, 0x56, 0xfa, 0x3d, 0xf0, 0xc9, 0xa4, + 0xec, 0x1e, 0xaa, 0x47, 0x90, 0x53, 0xdf, 0xe1, 0xb7, 0x78, 0x18, 0xeb, + 0xdd, 0x0d, 0x89, 0xb7, 0xf6, 0x15, 0x0e, 0x55, 0x12, 0xb3, 0x23, 0x17, + 0x0b, 0x59, 0x6f, 0x83, 0x05, 0x6b, 0xa6, 0xf8, 0x6c, 0x3a, 0x9b, 0x1b, + 0x50, 0x93, 0x51, 0xea, 0x95, 0x2d, 0x99, 0x96, 0x38, 0x16, 0xfe, 0xfd, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x7e, 0x01, 0x00, + 0x02, 0xdc, 0x00, 0x00, 0x00, 0x02, 0x6a, 0x00, 0x00, 0x72, 0x2d, 0x66, + 0x3e, 0xf2, 0x36, 0x5a, 0xf2, 0x23, 0x8f, 0x28, 0x09, 0xa9, 0x55, 0x8c, + 0x8f, 0xc0, 0x0d, 0x61, 0x98, 0x33, 0x56, 0x87, 0x7a, 0xfd, 0xa7, 0x50, + 0x71, 0x84, 0x2e, 0x41, 0x58, 0x00, 0x87, 0xd9, 0x27, 0xe5, 0x7b, 0xf4, + 0x6d, 0x84, 0x4e, 0x2e, 0x0c, 0x80, 0x0c, 0xf3, 0x8a, 0x02, 0x4b, 0x99, + 0x3a, 0x1f, 0x9f, 0x18, 0x7d, 0x1c, 0xec, 0xad, 0x60, 0x54, 0xa6, 0xa3, + 0x2c, 0x82, 0x5e, 0xf8, 0x8f, 0xae, 0xe1, 0xc4, 0x82, 0x7e, 0x43, 0x43, + 0xc5, 0x99, 0x49, 0x05, 0xd3, 0xf6, 0xdf, 0xa1, 0xb5, 0x2d, 0x0c, 0x13, + 0x2f, 0x1e, 0xb6, 0x28, 0x7c, 0x5c, 0xa1, 0x02, 0x6b, 0x8d, 0xa3, 0xeb, + 0xd4, 0x58, 0xe6, 0xa0, 0x7e, 0x6b, 0xaa, 0x09, 0x43, 0x67, 0x71, 0x87, + 0xa5, 0xcb, 0x68, 0xf3 + }; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); + + /* Fragment msgs */ + ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_c, DUMMY_MTU), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_s, DUMMY_MTU), WOLFSSL_SUCCESS); + + /* Add in some key shares to make the CH long */ + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP256R1), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP384R1), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP521R1), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_FFDHE_2048), WOLFSSL_SUCCESS); + + ExpectIntEQ(wolfSSL_dtls13_allow_ch_frag(ssl_s, 1), WOLFSSL_SUCCESS); + + /* Reject fragmented first CH */ + ExpectIntEQ(test_dtls_frag_ch_count_records(four_frag_CH, + sizeof(four_frag_CH)), 4); + XMEMCPY(test_ctx.s_buff, four_frag_CH, sizeof(four_frag_CH)); + test_ctx.s_len = sizeof(four_frag_CH); + while (test_ctx.s_len > 0 && EXPECT_SUCCESS()) { + int s_len = test_ctx.s_len; + ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + /* Fail if we didn't advance the buffer to avoid infinite loops */ + ExpectIntLT(test_ctx.s_len, s_len); + } + /* Expect all fragments read */ + ExpectIntEQ(test_ctx.s_len, 0); + /* Expect quietly dropping fragmented first CH */ + ExpectIntEQ(test_ctx.c_len, 0); + + /* CH1 */ + ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + /* Count records. Expect 1 unfragmented CH */ + ExpectIntEQ(test_dtls_frag_ch_count_records(test_ctx.s_buff, + test_ctx.s_len), 1); + /* HRR */ + ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + /* CH2 */ + ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); + /* Count records. Expect fragmented CH */ + ExpectIntGT(test_dtls_frag_ch_count_records(test_ctx.s_buff, + test_ctx.s_len), 1); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + ssl_c = ssl_s = NULL; + ctx_c = ctx_s = NULL; +#endif + return EXPECT_RESULT(); +} + +static int test_dtls_empty_keyshare_with_cookie(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + unsigned char ch_empty_keyshare_with_cookie[] = { + 0x16, 0xfe, 0xfd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01, + 0x12, 0x01, 0x00, 0x01, 0x06, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x06, 0xfe, 0xfd, 0xfb, 0x8c, 0x9b, 0x28, 0xae, 0x50, 0x1c, 0x4d, 0xf3, + 0xb8, 0xcf, 0x4d, 0xd8, 0x7e, 0x93, 0x13, 0x7b, 0x9e, 0xd9, 0xeb, 0xe9, + 0x13, 0x4b, 0x0d, 0x7f, 0x2e, 0x43, 0x62, 0x8c, 0xe4, 0x57, 0x79, 0x00, + 0x00, 0x00, 0x36, 0x13, 0x01, 0x13, 0x02, 0x13, 0x03, 0xc0, 0x2c, 0xc0, + 0x2b, 0xc0, 0x30, 0xc0, 0x2f, 0x00, 0x9f, 0x00, 0x9e, 0xcc, 0xa9, 0xcc, + 0xa8, 0xcc, 0xaa, 0xc0, 0x27, 0xc0, 0x23, 0xc0, 0x28, 0xc0, 0x24, 0xc0, + 0x0a, 0xc0, 0x09, 0xc0, 0x14, 0xc0, 0x13, 0x00, 0x6b, 0x00, 0x67, 0x00, + 0x39, 0x00, 0x33, 0xcc, 0x14, 0xcc, 0x13, 0xcc, 0x15, 0x01, 0x00, 0x00, + 0xa6, 0x00, 0x2b, 0x00, 0x03, 0x02, 0xfe, 0xfc, 0x00, 0x2c, 0x00, 0x47, + 0x00, 0x45, 0x20, 0xee, 0x4b, 0x17, 0x70, 0x63, 0xa0, 0x4c, 0x82, 0xbf, + 0x43, 0x01, 0x7d, 0x8d, 0xc1, 0x1b, 0x4e, 0x9b, 0xa0, 0x3c, 0x53, 0x1f, + 0xb7, 0xd1, 0x10, 0x81, 0xa8, 0xdf, 0xdf, 0x8c, 0x7f, 0xf3, 0x11, 0x13, + 0x01, 0x02, 0x3d, 0x3b, 0x7d, 0x14, 0x2c, 0x31, 0xb3, 0x60, 0x72, 0x4d, + 0xe5, 0x1a, 0xb2, 0xa3, 0x61, 0x77, 0x73, 0x03, 0x40, 0x0e, 0x5f, 0xc5, + 0x61, 0x38, 0x43, 0x56, 0x21, 0x4a, 0x95, 0xd5, 0x35, 0xa8, 0x0d, 0x00, + 0x0d, 0x00, 0x2a, 0x00, 0x28, 0x06, 0x03, 0x05, 0x03, 0x04, 0x03, 0x02, + 0x03, 0xfe, 0x0b, 0xfe, 0x0e, 0xfe, 0xa0, 0xfe, 0xa3, 0xfe, 0xa5, 0x08, + 0x06, 0x08, 0x0b, 0x08, 0x05, 0x08, 0x0a, 0x08, 0x04, 0x08, 0x09, 0x06, + 0x01, 0x05, 0x01, 0x04, 0x01, 0x03, 0x01, 0x02, 0x01, 0x00, 0x0a, 0x00, + 0x18, 0x00, 0x16, 0x00, 0x19, 0x00, 0x18, 0x00, 0x17, 0x00, 0x15, 0x01, + 0x00, 0x02, 0x3a, 0x02, 0x3c, 0x02, 0x3d, 0x2f, 0x3a, 0x2f, 0x3c, 0x2f, + 0x3d, 0x00, 0x16, 0x00, 0x00, 0x00, 0x33, 0x00, 0x02, 0x00, 0x00 + }; + DtlsRecordLayerHeader* dtlsRH; + byte sequence_number[8]; + + XMEMSET(&sequence_number, 0, sizeof(sequence_number)); + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + XMEMCPY(test_ctx.s_buff, ch_empty_keyshare_with_cookie, + sizeof(ch_empty_keyshare_with_cookie)); + test_ctx.s_len = sizeof(ch_empty_keyshare_with_cookie); + ExpectIntEQ(test_memio_setup(&test_ctx, NULL, &ctx_s, NULL, &ssl_s, + NULL, wolfDTLSv1_3_server_method), 0); + + /* CH1 */ + ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1); + ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); + /* Expect an alert. A plaintext alert should be exactly 15 bytes. */ + ExpectIntEQ(test_ctx.c_len, 15); + dtlsRH = (DtlsRecordLayerHeader*)test_ctx.c_buff; + ExpectIntEQ(dtlsRH->type, alert); + ExpectIntEQ(dtlsRH->pvMajor, DTLS_MAJOR); + ExpectIntEQ(dtlsRH->pvMinor, DTLSv1_2_MINOR); + sequence_number[7] = 1; + ExpectIntEQ(XMEMCMP(sequence_number, dtlsRH->sequence_number, + sizeof(sequence_number)), 0); + ExpectIntEQ(dtlsRH->length[0], 0); + ExpectIntEQ(dtlsRH->length[1], 2); + ExpectIntEQ(test_ctx.c_buff[13], alert_fatal); + ExpectIntEQ(test_ctx.c_buff[14], illegal_parameter); + + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + /*----------------------------------------------------------------------------* | Main *----------------------------------------------------------------------------*/ @@ -66507,6 +66737,8 @@ TEST_CASE testCases[] = { TEST_DECL(test_dtls_dropped_ccs), TEST_DECL(test_certreq_sighash_algos), TEST_DECL(test_revoked_loaded_int_cert), + TEST_DECL(test_dtls_frag_ch), + TEST_DECL(test_dtls_empty_keyshare_with_cookie), /* This test needs to stay at the end to clean up any caches allocated. */ TEST_DECL(test_wolfSSL_Cleanup) }; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 3866fe53b..dd1aeaf3f 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3353,8 +3353,10 @@ typedef struct KeyShareEntry { struct KeyShareEntry* next; /* List pointer */ } KeyShareEntry; +WOLFSSL_LOCAL void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap); WOLFSSL_LOCAL int TLSX_KeyShare_Use(const WOLFSSL* ssl, word16 group, word16 len, byte* data, KeyShareEntry **kse, TLSX** extensions); +WOLFSSL_LOCAL int TLSX_KeyShare_SelectGroup(WOLFSSL* ssl, word16 group); WOLFSSL_LOCAL int TLSX_KeyShare_Empty(WOLFSSL* ssl); WOLFSSL_LOCAL int TLSX_KeyShare_SetSupported(const WOLFSSL* ssl, TLSX** extensions); @@ -4616,7 +4618,12 @@ struct Options { word16 tls1_3:1; /* using TLSv1.3+ ? */ word16 seenUnifiedHdr:1; /* received msg with unified header */ word16 dtls:1; /* using datagrams ? */ +#ifdef WOLFSSL_DTLS word16 dtlsStateful:1; /* allow stateful processing ? */ +#endif +#ifdef WOLFSSL_DTLS_CH_FRAG + word16 dtlsSentEmptyKS:1; /* did we send an empty key share ? */ +#endif word16 connReset:1; /* has the peer reset */ word16 isClosed:1; /* if we consider conn closed */ word16 closeNotify:1; /* we've received a close notify */ @@ -4727,6 +4734,9 @@ struct Options { #ifdef WOLFSSL_DTLS13 word16 dtls13SendMoreAcks:1; /* Send more acks during the * handshake process */ +#ifdef WOLFSSL_DTLS_CH_FRAG + word16 dtls13ChFrag:1; +#endif #endif #ifdef WOLFSSL_TLS13 word16 tls13MiddleBoxCompat:1; /* TLSv1.3 middlebox compatibility */ @@ -5417,7 +5427,7 @@ struct WOLFSSL { #endif #if defined(WOLFSSL_DTLS) && !defined(NO_WOLFSSL_SERVER) ClientHelloGoodCb chGoodCb; /* notify user we parsed a verified - * ClientHello */ + * ClientHello that passed basic tests */ void* chGoodCtx; /* user ClientHello cb context */ #endif #ifndef NO_HANDSHAKE_DONE_CB @@ -5611,6 +5621,9 @@ struct WOLFSSL { Dtls13Rtx dtls13Rtx; byte *dtls13ClientHello; word16 dtls13ClientHelloSz; +#ifdef WOLFSSL_DTLS_CH_FRAG + KeyShareEntry* dtls13KSE; +#endif #endif /* WOLFSSL_DTLS13 */ #ifdef WOLFSSL_DTLS_CID @@ -6268,7 +6281,7 @@ WOLFSSL_LOCAL int cipherExtraData(WOLFSSL* ssl); #if !defined(NO_WOLFSSL_SERVER) WOLFSSL_LOCAL int DoClientHelloStateless(WOLFSSL* ssl, - const byte* input, word32* inOutIdx, word32 helloSz); + const byte* input, word32 helloSz, byte isFirstCHFrag); #endif /* !defined(NO_WOLFSSL_SERVER) */ #endif /* WOLFSSL_DTLS */ diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index d49e73ecb..c50c8d5c2 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -5218,6 +5218,10 @@ WOLFSSL_API int wolfSSL_dtls_cid_get_tx(WOLFSSL* ssl, unsigned char* buffer, unsigned int bufferSz); #endif /* defined(WOLFSSL_DTLS_CID) */ +#ifdef WOLFSSL_DTLS_CH_FRAG + WOLFSSL_API int wolfSSL_dtls13_allow_ch_frag(WOLFSSL *ssl, int enabled); +#endif + /* */ #define SSL2_VERSION 0x0002 #define SSL3_VERSION 0x0300 diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index 0f9bb0104..54c623227 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -3014,6 +3014,14 @@ extern void uITRON4_free(void *p) ; #error Please do not define both HAVE_LIBOQS and HAVE_PQM4. #endif +#if defined(HAVE_PQC) && defined(WOLFSSL_DTLS13) && \ + !defined(WOLFSSL_DTLS_CH_FRAG) +#warning "Using DTLS 1.3 + pqc without WOLFSSL_DTLS_CH_FRAG will probably fail. Use --enable-dtls-frag-ch to enable it." +#endif +#if !defined(WOLFSSL_DTLS13) && defined(WOLFSSL_DTLS_CH_FRAG) +#error "WOLFSSL_DTLS_CH_FRAG only works with DTLS 1.3" +#endif + /* SRTP requires DTLS */ #if defined(WOLFSSL_SRTP) && !defined(WOLFSSL_DTLS) #error The SRTP extension requires DTLS From df8ee69075724ed2ebb3c6f97d4ee0198821dd94 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 7 Sep 2023 14:57:41 +0200 Subject: [PATCH 02/22] Clear the keyshare instead of storing it --- src/internal.c | 6 ---- src/tls.c | 79 ++++++----------------------------------- src/tls13.c | 36 ++++--------------- wolfcrypt/src/logging.c | 2 +- wolfssl/internal.h | 8 ----- 5 files changed, 18 insertions(+), 113 deletions(-) diff --git a/src/internal.c b/src/internal.c index e323acf6c..816301935 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8017,12 +8017,6 @@ void SSL_ResourceFree(WOLFSSL* ssl) ssl->dtls13ClientHello = NULL; ssl->dtls13ClientHelloSz = 0; } -#ifdef WOLFSSL_DTLS_CH_FRAG - if (ssl->dtls13KSE != NULL) { - TLSX_KeyShare_FreeAll(ssl->dtls13KSE, ssl->heap); - ssl->dtls13KSE = NULL; - } -#endif #endif /* WOLFSSL_DTLS13 */ #endif /* WOLFSSL_DTLS */ diff --git a/src/tls.c b/src/tls.c index ede90f857..e4f2452e2 100644 --- a/src/tls.c +++ b/src/tls.c @@ -67,6 +67,7 @@ #if defined(WOLFSSL_TLS13) && defined(HAVE_SUPPORTED_CURVES) static int TLSX_KeyShare_IsSupported(int namedGroup); +static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap); #endif #ifdef HAVE_SUPPORTED_CURVES @@ -7768,7 +7769,7 @@ int TLSX_KeyShare_GenKey(WOLFSSL *ssl, KeyShareEntry *kse) * list The linked list of key share entry objects. * heap The heap used for allocation. */ -void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap) +static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap) { KeyShareEntry* current; @@ -8783,50 +8784,24 @@ int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length, if (ssl->error != WC_PENDING_E) #endif { - /* Check the selected group was supported by ClientHello - * extensions. */ + /* Check the selected group was supported by ClientHello extensions. */ if (!TLSX_SupportedGroups_Find(ssl, group, ssl->extensions)) { WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); return BAD_KEY_SHARE_DATA; } -#ifdef WOLFSSL_DTLS_CH_FRAG - /* If we sent an empty key share then we can just limit the keyshare - * to the one selected by the server. */ - if (ssl->options.dtlsSentEmptyKS) { - if (!TLSX_KeyShare_SelectGroup(ssl, group)) { - /* Clear out all groups if not found */ - ret = TLSX_KeyShare_Empty(ssl); - if (ret != 0) - return ret; - } - } - else -#endif - { - /* Check if the group was sent. */ - if (TLSX_KeyShare_Find(ssl, group)) { - WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); - return BAD_KEY_SHARE_DATA; - } - - /* Clear out unusable key shares. */ - ret = TLSX_KeyShare_Empty(ssl); - if (ret != 0) - return ret; + /* Check if the group was sent. */ + if (TLSX_KeyShare_Find(ssl, group)) { + WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); + return BAD_KEY_SHARE_DATA; } + /* Clear out unusable key shares. */ + ret = TLSX_KeyShare_Empty(ssl); + if (ret != 0) + return ret; } - -#ifdef WOLFSSL_DTLS_CH_FRAG - /* Check if we were able to limit the keyshare entries to one group */ - if (ssl->options.dtlsSentEmptyKS && - TLSX_KeyShare_SelectGroup(ssl, group)) { - /* Nothing to do */ - } - else -#endif #ifdef HAVE_PQC /* For post-quantum groups, do this in TLSX_PopulateExtensions(). */ if (!WOLFSSL_NAMED_GROUP_IS_PQC(group)) @@ -9127,38 +9102,6 @@ int TLSX_KeyShare_Use(const WOLFSSL* ssl, word16 group, word16 len, byte* data, return 0; } -/* Clear out all entries except for group - * - * ssl The SSL/TLS object. - * returns 1 when the group was found and 0 when it wasn't found. - * */ -int TLSX_KeyShare_SelectGroup(WOLFSSL* ssl, word16 group) -{ - TLSX* extension; - KeyShareEntry* list; - KeyShareEntry** prev; - - /* Find the KeyShare extension if it exists. */ - extension = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); - if (extension != NULL) { - for (prev = (KeyShareEntry**)&extension->data, - list = (KeyShareEntry*)extension->data; list != NULL; - prev = &list->next, list = list->next) { - if (list->group == group) { - /* Unlink it from the list */ - *prev = list->next; - list->next = NULL; - /* Free the list */ - TLSX_KeyShare_FreeAll((KeyShareEntry*)extension->data, - ssl->heap); - extension->data = list; - return 1; - } - } - } - return 0; -} - /* Set an empty Key Share extension. * * ssl The SSL/TLS object. diff --git a/src/tls13.c b/src/tls13.c index 9a23bb2a0..c5fb53caa 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -4426,29 +4426,18 @@ int SendTls13ClientHello(WOLFSSL* ssl) TLSX_Find(ssl->extensions, TLSX_COOKIE) == NULL) { /* Try again with an empty key share if we would be fragmenting * without a cookie */ - TLSX* ks = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); - if (ks == NULL) { - WOLFSSL_MSG("No key share and CH can't fit in one fragment."); - return BUFFER_ERROR; - } - args->length = lenWithoutExts; - if (ssl->dtls13KSE != NULL) - TLSX_KeyShare_FreeAll(ssl->dtls13KSE, ssl->heap); - ssl->dtls13KSE = (KeyShareEntry*)ks->data; - ks->data = NULL; - ret = TLSX_GetRequestSize(ssl, client_hello, &args->length); - if (ret != 0) { - /* Restore key share data */ - ks->data = ssl->dtls13KSE; - ssl->dtls13KSE = NULL; + ret = TLSX_KeyShare_Empty(ssl); + if (ret != 0) + return ret; + args->length = lenWithoutExts; + ret = TLSX_GetRequestSize(ssl, client_hello, &args->length); + if (ret != 0) return ret; - } if (args->length > maxFrag) { WOLFSSL_MSG("Can't fit first CH in one fragment."); return BUFFER_ERROR; } WOLFSSL_MSG("Sending empty key share so we don't fragment CH1"); - ssl->options.dtlsSentEmptyKS = 1; } #endif } @@ -4691,19 +4680,6 @@ int SendTls13ClientHello(WOLFSSL* ssl) if (ret == 0) FreeAsyncCtx(ssl, 0); #endif -#ifdef WOLFSSL_DTLS_CH_FRAG - if ((ret == 0 || ret == WANT_WRITE) && ssl->dtls13KSE != NULL) { - /* Restore the keyshare */ - TLSX* ks = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); - if (ks == NULL || ks->data != NULL) { - WOLFSSL_MSG("Missing key share or key share data not NULL"); - return BUFFER_ERROR; - } - WOLFSSL_MSG("Restored key share"); - ks->data = ssl->dtls13KSE; - ssl->dtls13KSE = NULL; - } -#endif WOLFSSL_LEAVE("SendTls13ClientHello", ret); WOLFSSL_END(WC_FUNC_CLIENT_HELLO_SEND); diff --git a/wolfcrypt/src/logging.c b/wolfcrypt/src/logging.c index eacc6b09d..1adf85ae6 100644 --- a/wolfcrypt/src/logging.c +++ b/wolfcrypt/src/logging.c @@ -169,7 +169,7 @@ wolfSSL_Logging_cb wolfSSL_GetLoggingCb(void) int wolfSSL_Debugging_ON(void) { #ifdef DEBUG_WOLFSSL - loggingEnabled = 1; + loggingEnabled = 0; #if defined(WOLFSSL_APACHE_MYNEWT) log_register("wolfcrypt", &mynewt_log, &log_console_handler, NULL, LOG_SYSLEVEL); #endif /* WOLFSSL_APACHE_MYNEWT */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index dd1aeaf3f..32ccb8979 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3353,10 +3353,8 @@ typedef struct KeyShareEntry { struct KeyShareEntry* next; /* List pointer */ } KeyShareEntry; -WOLFSSL_LOCAL void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap); WOLFSSL_LOCAL int TLSX_KeyShare_Use(const WOLFSSL* ssl, word16 group, word16 len, byte* data, KeyShareEntry **kse, TLSX** extensions); -WOLFSSL_LOCAL int TLSX_KeyShare_SelectGroup(WOLFSSL* ssl, word16 group); WOLFSSL_LOCAL int TLSX_KeyShare_Empty(WOLFSSL* ssl); WOLFSSL_LOCAL int TLSX_KeyShare_SetSupported(const WOLFSSL* ssl, TLSX** extensions); @@ -4620,9 +4618,6 @@ struct Options { word16 dtls:1; /* using datagrams ? */ #ifdef WOLFSSL_DTLS word16 dtlsStateful:1; /* allow stateful processing ? */ -#endif -#ifdef WOLFSSL_DTLS_CH_FRAG - word16 dtlsSentEmptyKS:1; /* did we send an empty key share ? */ #endif word16 connReset:1; /* has the peer reset */ word16 isClosed:1; /* if we consider conn closed */ @@ -5621,9 +5616,6 @@ struct WOLFSSL { Dtls13Rtx dtls13Rtx; byte *dtls13ClientHello; word16 dtls13ClientHelloSz; -#ifdef WOLFSSL_DTLS_CH_FRAG - KeyShareEntry* dtls13KSE; -#endif #endif /* WOLFSSL_DTLS13 */ #ifdef WOLFSSL_DTLS_CID From 8da863184cd14105e560fb2516847f7527c60f63 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 7 Sep 2023 15:14:12 +0200 Subject: [PATCH 03/22] Force DTLS 1.3 when accepting fragmented CH --- src/dtls.c | 6 +++++- src/dtls13.c | 4 +++- src/internal.c | 3 ++- src/tls13.c | 2 +- wolfssl/internal.h | 2 +- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/dtls.c b/src/dtls.c index aa7ba3155..464fd1499 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -882,7 +882,7 @@ static int ClientHelloSanityCheck(WolfSSL_CH* ch, byte isTls13) } int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, - byte isFirstCHFrag) + byte isFirstCHFrag, byte* tls13) { int ret; WolfSSL_CH ch; @@ -897,6 +897,8 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, "WOLFSSL_DTLS_CH_FRAG is not defined. This should not happen."); #endif } + if (tls13 != NULL) + *tls13 = 0; XMEMSET(&ch, 0, sizeof(ch)); @@ -910,6 +912,8 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, ret = TlsCheckSupportedVersion(ssl, &ch, &isTls13); if (ret != 0) return ret; + if (tls13 != NULL) + *tls13 = isTls13; if (isTls13) { int tlsxFound; ret = FindExtByType(&ch.cookieExt, TLSX_COOKIE, ch.extension, diff --git a/src/dtls13.c b/src/dtls13.c index 42c7ca7bd..c7df75f61 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -1661,10 +1661,12 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, if (!isComplete && !Dtls13AcceptFragmented(ssl, handshakeType)) { #ifdef WOLFSSL_DTLS_CH_FRAG + byte tls13 = 0; /* check if the first CH fragment contains a valid cookie */ if (ssl->options.dtls13ChFrag && !ssl->options.dtlsStateful && isFirst && handshakeType == client_hello && - DoClientHelloStateless(ssl, input + idx, fragLength, 1) == 0) { + DoClientHelloStateless(ssl, input + idx, fragLength, 1, &tls13) + == 0 && tls13) { /* We can save this message and continue as stateful. */ if (ssl->chGoodCb != NULL && !IsSCR(ssl)) { int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); diff --git a/src/internal.c b/src/internal.c index 816301935..dd4c26e40 100644 --- a/src/internal.c +++ b/src/internal.c @@ -34802,7 +34802,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (IsDtlsNotSctpMode(ssl) && IsDtlsNotSrtpMode(ssl) && !IsSCR(ssl) && !ssl->options.dtlsStateful) { DtlsSetSeqNumForReply(ssl); - ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0); + ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0, + NULL); if (ret != 0 || !ssl->options.dtlsStateful) { int alertType = TranslateErrorToAlert(ret); if (alertType != invalid_alert) { diff --git a/src/tls13.c b/src/tls13.c index c5fb53caa..a4c794b28 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6653,7 +6653,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, * wolfSSL_accept_TLSv13 when changing this one. */ if (IsDtlsNotSctpMode(ssl) && ssl->options.sendCookie && !ssl->options.dtlsStateful) { - ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0); + ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0, NULL); if (ret != 0 || !ssl->options.dtlsStateful) { *inOutIdx += helloSz; goto exit_dch; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 32ccb8979..1e069b380 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -6273,7 +6273,7 @@ WOLFSSL_LOCAL int cipherExtraData(WOLFSSL* ssl); #if !defined(NO_WOLFSSL_SERVER) WOLFSSL_LOCAL int DoClientHelloStateless(WOLFSSL* ssl, - const byte* input, word32 helloSz, byte isFirstCHFrag); + const byte* input, word32 helloSz, byte isFirstCHFrag, byte* tls13); #endif /* !defined(NO_WOLFSSL_SERVER) */ #endif /* WOLFSSL_DTLS */ From ada785e115201ead98ae47c392832942c2bbdf81 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 7 Sep 2023 16:26:42 +0200 Subject: [PATCH 04/22] Address code review --- src/dtls.c | 7 ++++--- src/dtls13.c | 2 +- src/tls13.c | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/dtls.c b/src/dtls.c index 464fd1499..30af15c69 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -28,7 +28,7 @@ * clientHello messages will consume resources on the server. * WOLFSSL_DTLS_CH_FRAG * Allow a server to process a fragmented second/verified (one containing a - * valid cookie response) ClientHello message. The first/unverifies (one + * valid cookie response) ClientHello message. The first/unverified (one * without a cookie extension) ClientHello MUST be unfragmented so that the * DTLS server can process it statelessly. This is only implemented for * DTLS 1.3. The user MUST call wolfSSL_dtls13_allow_ch_frag() on the server @@ -895,6 +895,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, #else WOLFSSL_MSG("\tProcessing fragmented ClientHello but " "WOLFSSL_DTLS_CH_FRAG is not defined. This should not happen."); + return BAD_STATE_E; #endif } if (tls13 != NULL) @@ -945,7 +946,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, #ifdef WOLFSSL_DTLS_CH_FRAG /* Don't send anything here when processing fragment */ if (isFirstCHFrag) - ret = BUFFER_ERROR; + ret = COOKIE_ERROR; else #endif ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); @@ -966,7 +967,7 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, #ifdef WOLFSSL_DTLS_CH_FRAG /* Don't send anything here when processing fragment */ if (isFirstCHFrag) - ret = BUFFER_ERROR; + ret = COOKIE_ERROR; else #endif ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); diff --git a/src/dtls13.c b/src/dtls13.c index c7df75f61..3ea608215 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -1668,7 +1668,7 @@ static int _Dtls13HandshakeRecv(WOLFSSL* ssl, byte* input, word32 size, DoClientHelloStateless(ssl, input + idx, fragLength, 1, &tls13) == 0 && tls13) { /* We can save this message and continue as stateful. */ - if (ssl->chGoodCb != NULL && !IsSCR(ssl)) { + if (ssl->chGoodCb != NULL) { int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); if (cbret < 0) { ssl->error = cbret; diff --git a/src/tls13.c b/src/tls13.c index a4c794b28..1c4919e4c 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6658,7 +6658,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, *inOutIdx += helloSz; goto exit_dch; } - if (ssl->chGoodCb != NULL && !IsSCR(ssl)) { + if (ssl->chGoodCb != NULL) { int cbret = ssl->chGoodCb(ssl, ssl->chGoodCtx); if (cbret < 0) { ssl->error = cbret; From f640fdf91fb070104a5c564bd2414c59d2485d64 Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Thu, 7 Sep 2023 15:45:40 -0400 Subject: [PATCH 05/22] Adding a post-quantum DTLS 1.3 test. This exercises the fragmenting of ClientHello via large post-quantum key share. ./configure --enable-dtls-mtu --enable-dtls-frag-ch --enable-dtls \ --enable-dtls13 --with-liboqs --- tests/api.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/api.c b/tests/api.c index 4e5f6efed..8cebd4f8c 100644 --- a/tests/api.c +++ b/tests/api.c @@ -65256,6 +65256,50 @@ static int test_dtls_frag_ch_count_records(byte* b, int len) } #endif +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) \ + && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) \ + && defined(HAVE_LIBOQS) +static int test_dtls13_frag_ch_pq(void) +{ + EXPECT_DECLS; + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char *test_str = "test"; + int test_str_size; + byte buf[255]; + static unsigned int DUMMY_MTU = 256; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); +wolfSSL_Debugging_ON(); + /* Fragment msgs */ + ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_c, DUMMY_MTU), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_s, DUMMY_MTU), WOLFSSL_SUCCESS); + /* Add in a large post-quantum key share to make the CH long. */ + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_KYBER_LEVEL5), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_dtls13_allow_ch_frag(ssl_s, 1), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + test_str_size = XSTRLEN("test") + 1; + ExpectIntEQ(wolfSSL_write(ssl_c, test_str, test_str_size), test_str_size); + ExpectIntEQ(wolfSSL_read(ssl_s, buf, sizeof(buf)), test_str_size); + ExpectIntEQ(XSTRCMP((char*)buf, test_str), 0); + ExpectIntEQ(wolfSSL_write(ssl_c, test_str, test_str_size), test_str_size); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + return EXPECT_RESULT(); +} +#else +static int test_dtls13_frag_ch_pq(void) +{ + return TEST_SKIPPED; +} +#endif static int test_dtls_frag_ch(void) { EXPECT_DECLS; @@ -66738,6 +66782,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_certreq_sighash_algos), TEST_DECL(test_revoked_loaded_int_cert), TEST_DECL(test_dtls_frag_ch), + TEST_DECL(test_dtls13_frag_ch_pq), TEST_DECL(test_dtls_empty_keyshare_with_cookie), /* This test needs to stay at the end to clean up any caches allocated. */ TEST_DECL(test_wolfSSL_Cleanup) From 230f81712c16047fcdf4a7fadbd429e20402229d Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 8 Sep 2023 14:00:44 +0200 Subject: [PATCH 06/22] fixup! Clear the keyshare instead of storing it --- wolfcrypt/src/logging.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/logging.c b/wolfcrypt/src/logging.c index 1adf85ae6..eacc6b09d 100644 --- a/wolfcrypt/src/logging.c +++ b/wolfcrypt/src/logging.c @@ -169,7 +169,7 @@ wolfSSL_Logging_cb wolfSSL_GetLoggingCb(void) int wolfSSL_Debugging_ON(void) { #ifdef DEBUG_WOLFSSL - loggingEnabled = 0; + loggingEnabled = 1; #if defined(WOLFSSL_APACHE_MYNEWT) log_register("wolfcrypt", &mynewt_log, &log_console_handler, NULL, LOG_SYSLEVEL); #endif /* WOLFSSL_APACHE_MYNEWT */ From 0dbf97c867f34ae25c4724e7d1004c59b714136e Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 8 Sep 2023 14:35:10 +0200 Subject: [PATCH 07/22] fixup! Clear the keyshare instead of storing it --- src/ssl.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index c0c562405..04bb5a6fc 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -19231,9 +19231,6 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, #ifdef WOLFSSL_DTLS ssl->options.dtlsStateful = 0; #endif - #ifdef WOLFSSL_DTLS_CH_FRAG - ssl->options.dtlsSentEmptyKS = 0; - #endif #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) ssl->options.noPskDheKe = 0; #ifdef HAVE_SUPPORTED_CURVES From c802193119e16840edb9d78268f1336c1e8fd91c Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 8 Sep 2023 14:35:33 +0200 Subject: [PATCH 08/22] Simplify the pqc keyshare handling --- src/tls.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/tls.c b/src/tls.c index e4f2452e2..3ceb7e0a7 100644 --- a/src/tls.c +++ b/src/tls.c @@ -8802,12 +8802,7 @@ int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length, return ret; } -#ifdef HAVE_PQC - /* For post-quantum groups, do this in TLSX_PopulateExtensions(). */ - if (!WOLFSSL_NAMED_GROUP_IS_PQC(group)) -#endif - ret = TLSX_KeyShare_Use(ssl, group, 0, NULL, NULL, &ssl->extensions); - + ret = TLSX_KeyShare_Use(ssl, group, 0, NULL, NULL, &ssl->extensions); if (ret == 0) ssl->session->namedGroup = ssl->namedGroup = group; } @@ -12968,19 +12963,8 @@ int TLSX_PopulateExtensions(WOLFSSL* ssl, byte isServer) namedGroup = kse->group; } if (namedGroup != WOLFSSL_NAMED_GROUP_INVALID) { -#ifdef HAVE_PQC - /* For KEMs, the key share has already been generated, but not - * if we are resuming. */ - if (!WOLFSSL_NAMED_GROUP_IS_PQC(namedGroup) -#ifdef HAVE_SESSION_TICKET - || ssl->options.resuming -#endif /* HAVE_SESSION_TICKET */ - ) -#endif /* HAVE_PQC */ - { - ret = TLSX_KeyShare_Use(ssl, namedGroup, 0, NULL, NULL, - &ssl->extensions); - } + ret = TLSX_KeyShare_Use(ssl, namedGroup, 0, NULL, NULL, + &ssl->extensions); if (ret != 0) return ret; } From 37c0d52fa892b224fc304fd78786f6276076a6eb Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 8 Sep 2023 14:35:51 +0200 Subject: [PATCH 09/22] Dump manual memio stream with WOLFSSL_DUMP_MEMIO_STREAM --- tests/utils.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/utils.h b/tests/utils.h index f22295745..46b16e2c8 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -170,6 +170,15 @@ static WC_INLINE int test_memio_write_cb(WOLFSSL *ssl, char *data, int sz, if ((unsigned)(*len + sz) > TEST_MEMIO_BUF_SZ) return WOLFSSL_CBIO_ERR_WANT_WRITE; +#ifdef WOLFSSL_DUMP_MEMIO_STREAM + { + WOLFSSL_BIO *dump_file = wolfSSL_BIO_new_file("test_memio.dump", "a"); + if (dump_file != NULL) { + (void)wolfSSL_BIO_write(dump_file, data, sz); + wolfSSL_BIO_free(dump_file); + } + } +#endif XMEMCPY(buf + *len, data, sz); *len += sz; From 2c6c52078a2ab0a4c4cc3b0269a6eedbe75e744a Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 8 Sep 2023 14:51:49 +0200 Subject: [PATCH 10/22] test_dtls13_frag_ch_pq: make sure kyber5 is used --- src/tls.c | 1 + tests/api.c | 83 ++++++++++++++++++++++++---------------------------- tests/unit.h | 2 +- 3 files changed, 41 insertions(+), 45 deletions(-) diff --git a/src/tls.c b/src/tls.c index 3ceb7e0a7..86dd392ec 100644 --- a/src/tls.c +++ b/src/tls.c @@ -9627,6 +9627,7 @@ int TLSX_KeyShare_Setup(WOLFSSL *ssl, KeyShareEntry* clientKSE) serverKSE->keLen = clientKSE->keLen; clientKSE->ke = NULL; clientKSE->keLen = 0; + ssl->namedGroup = serverKSE->group; TLSX_KeyShare_FreeAll((KeyShareEntry*)extension->data, ssl->heap); extension->data = (void *)serverKSE; diff --git a/tests/api.c b/tests/api.c index 8cebd4f8c..921b4258c 100644 --- a/tests/api.c +++ b/tests/api.c @@ -65238,6 +65238,45 @@ static int test_revoked_loaded_int_cert(void) return EXPECT_RESULT(); } +static int test_dtls13_frag_ch_pq(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) \ + && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) \ + && defined(HAVE_LIBOQS) + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *ssl_s = NULL; + struct test_memio_ctx test_ctx; + const char *test_str = "test"; + int test_str_size; + byte buf[255]; + int group = WOLFSSL_KYBER_LEVEL5; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); + /* Add in a large post-quantum key share to make the CH long. */ + ExpectIntEQ(wolfSSL_set_groups(ssl_c, &group, 1), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, group), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_dtls13_allow_ch_frag(ssl_s, 1), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectStrEQ(wolfSSL_get_curve_name(ssl_c), "KYBER_LEVEL5"); + ExpectStrEQ(wolfSSL_get_curve_name(ssl_s), "KYBER_LEVEL5"); + test_str_size = XSTRLEN("test") + 1; + ExpectIntEQ(wolfSSL_write(ssl_c, test_str, test_str_size), test_str_size); + ExpectIntEQ(wolfSSL_read(ssl_s, buf, sizeof(buf)), test_str_size); + ExpectIntEQ(XSTRCMP((char*)buf, test_str), 0); + ExpectIntEQ(wolfSSL_write(ssl_c, test_str, test_str_size), test_str_size); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + #if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS) \ && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) static int test_dtls_frag_ch_count_records(byte* b, int len) @@ -65256,50 +65295,6 @@ static int test_dtls_frag_ch_count_records(byte* b, int len) } #endif -#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) \ - && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) \ - && defined(HAVE_LIBOQS) -static int test_dtls13_frag_ch_pq(void) -{ - EXPECT_DECLS; - WOLFSSL_CTX *ctx_c = NULL; - WOLFSSL_CTX *ctx_s = NULL; - WOLFSSL *ssl_c = NULL; - WOLFSSL *ssl_s = NULL; - struct test_memio_ctx test_ctx; - const char *test_str = "test"; - int test_str_size; - byte buf[255]; - static unsigned int DUMMY_MTU = 256; - - XMEMSET(&test_ctx, 0, sizeof(test_ctx)); - ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, - wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0); -wolfSSL_Debugging_ON(); - /* Fragment msgs */ - ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_c, DUMMY_MTU), WOLFSSL_SUCCESS); - ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_s, DUMMY_MTU), WOLFSSL_SUCCESS); - /* Add in a large post-quantum key share to make the CH long. */ - ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_KYBER_LEVEL5), WOLFSSL_SUCCESS); - ExpectIntEQ(wolfSSL_dtls13_allow_ch_frag(ssl_s, 1), WOLFSSL_SUCCESS); - ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); - test_str_size = XSTRLEN("test") + 1; - ExpectIntEQ(wolfSSL_write(ssl_c, test_str, test_str_size), test_str_size); - ExpectIntEQ(wolfSSL_read(ssl_s, buf, sizeof(buf)), test_str_size); - ExpectIntEQ(XSTRCMP((char*)buf, test_str), 0); - ExpectIntEQ(wolfSSL_write(ssl_c, test_str, test_str_size), test_str_size); - wolfSSL_free(ssl_c); - wolfSSL_free(ssl_s); - wolfSSL_CTX_free(ctx_c); - wolfSSL_CTX_free(ctx_s); - return EXPECT_RESULT(); -} -#else -static int test_dtls13_frag_ch_pq(void) -{ - return TEST_SKIPPED; -} -#endif static int test_dtls_frag_ch(void) { EXPECT_DECLS; diff --git a/tests/unit.h b/tests/unit.h index b18174327..185fc22df 100644 --- a/tests/unit.h +++ b/tests/unit.h @@ -174,7 +174,7 @@ if (_ret != TEST_FAIL) { \ const char* _x = (const char*)(x); \ const char* _y = (const char*)(y); \ - int _z = (_x && _y) ? strcmp(_x, _y) : -1; \ + int _z = (_x && _y) ? XSTRCMP(_x, _y) : -1; \ Expect(_z op 0, ("%s " #op " %s", #x, #y), \ ("\"%s\" " #er " \"%s\"", _x, _y));\ } \ From 3a881079d38f53f62cf4553dad6183aa6869ea34 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 11 Sep 2023 16:20:28 +0200 Subject: [PATCH 11/22] Fix async --- src/dtls13.c | 27 ++++++++-------- src/internal.c | 86 ++++++++++++++++++++++++++------------------------ 2 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index 3ea608215..2a2e543cd 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -2404,7 +2404,11 @@ static int Dtls13WriteAckMessage(WOLFSSL* ssl, c16toa(msgSz, ackMessage); ackMessage += OPAQUE16_LEN; + WOLFSSL_MSG("write ack records"); + while (recordNumberList != NULL) { + WOLFSSL_MSG_EX("epoch %d seq %d", recordNumberList->epoch, + recordNumberList->seq); c64toa(&recordNumberList->epoch, ackMessage); ackMessage += OPAQUE64_LEN; c64toa(&recordNumberList->seq, ackMessage); @@ -2596,10 +2600,13 @@ int DoDtls13Ack(WOLFSSL* ssl, const byte* input, word32 inputSize, if (length % (DTLS13_RN_SIZE) != 0) return PARSE_ERROR; + WOLFSSL_MSG("read ack records"); + ackMessage = input + OPAQUE16_LEN; for (i = 0; i < length; i += DTLS13_RN_SIZE) { ato64(ackMessage + i, &epoch); ato64(ackMessage + i + OPAQUE64_LEN, &seq); + WOLFSSL_MSG_EX("epoch %d seq %d", epoch, seq); Dtls13RtxRemoveRecord(ssl, epoch, seq); } @@ -2670,14 +2677,13 @@ int SendDtls13Ack(WOLFSSL* ssl) if (ret != 0) return ret; + ret = Dtls13WriteAckMessage(ssl, ssl->dtls13Rtx.seenRecords, &length); + if (ret != 0) + return ret; + + output = GetOutputBuffer(ssl); + if (w64IsZero(ssl->dtls13EncryptEpoch->epochNumber)) { - - ret = Dtls13WriteAckMessage(ssl, ssl->dtls13Rtx.seenRecords, &length); - if (ret != 0) - return ret; - - output = GetOutputBuffer(ssl); - ret = Dtls13RlAddPlaintextHeader(ssl, output, ack, (word16)length); if (ret != 0) return ret; @@ -2685,13 +2691,6 @@ int SendDtls13Ack(WOLFSSL* ssl) ssl->buffers.outputBuffer.length += length + DTLS_RECORD_HEADER_SZ; } else { - - ret = Dtls13WriteAckMessage(ssl, ssl->dtls13Rtx.seenRecords, &length); - if (ret != 0) - return ret; - - output = GetOutputBuffer(ssl); - outputSize = ssl->buffers.outputBuffer.bufferSize - ssl->buffers.outputBuffer.idx - ssl->buffers.outputBuffer.length; diff --git a/src/internal.c b/src/internal.c index dd4c26e40..138d65667 100644 --- a/src/internal.c +++ b/src/internal.c @@ -17040,8 +17040,23 @@ static int _DtlsUpdateWindow(WOLFSSL* ssl) next_hi, next_lo, window); } +static WC_INLINE int DtlsShouldUpdateWindow(int ret) +{ + switch (ret) { + case 0: +#ifdef WOLFSSL_ASYNC_CRYPT + case WC_PENDING_E: +#endif + case APP_DATA_READY: + return 1; + default: + return 0; + } +} + #ifdef WOLFSSL_DTLS13 -static WC_INLINE int Dtls13UpdateWindow(WOLFSSL* ssl) + +static int Dtls13UpdateWindow(WOLFSSL* ssl) { w64wrapper nextSeq, seq; w64wrapper diff64; @@ -17104,6 +17119,14 @@ static WC_INLINE int Dtls13UpdateWindow(WOLFSSL* ssl) return 0; } + +static WC_INLINE int Dtls13UpdateWindowRecordRecvd(WOLFSSL* ssl) +{ + int ret = Dtls13UpdateWindow(ssl); + if (ret != 0) + return ret; + return Dtls13RecordRecvd(ssl); +} #endif /* WOLFSSL_DTLS13 */ int DtlsMsgDrain(WOLFSSL* ssl) @@ -20805,7 +20828,8 @@ default: ssl->buffers.inputBuffer.buffer, &ssl->buffers.inputBuffer.idx, ssl->buffers.inputBuffer.length); - if (ret == 0 && ssl->options.dtlsStateful) { + if (DtlsShouldUpdateWindow(ret) && + ssl->options.dtlsStateful) { if (IsDtlsNotSctpMode(ssl)) _DtlsUpdateWindow(ssl); /* Reset timeout as we have received a valid @@ -20826,16 +20850,13 @@ default: ssl->buffers.inputBuffer.buffer, &ssl->buffers.inputBuffer.idx, ssl->buffers.inputBuffer.length); - if (ret == 0 && ssl->options.dtlsStateful) { - ret = Dtls13UpdateWindow(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } - ret = Dtls13RecordRecvd(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; + if (DtlsShouldUpdateWindow(ret) && + ssl->options.dtlsStateful) { + int updateRet = + Dtls13UpdateWindowRecordRecvd(ssl); + if (updateRet != 0) { + WOLFSSL_ERROR(updateRet); + return updateRet; } } #ifdef WOLFSSL_EARLY_DATA @@ -20960,12 +20981,7 @@ default: } #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { - ret = Dtls13UpdateWindow(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } - ret = Dtls13RecordRecvd(ssl); + ret = Dtls13UpdateWindowRecordRecvd(ssl); if (ret != 0) { WOLFSSL_ERROR(ret); return ret; @@ -21126,16 +21142,10 @@ default: ssl->buffers.inputBuffer.buffer, &ssl->buffers.inputBuffer.idx, NO_SNIFF); #ifdef WOLFSSL_DTLS - if (ssl->options.dtls && - (ret == 0 || ret == APP_DATA_READY)) { + if (ssl->options.dtls && DtlsShouldUpdateWindow(ret)) { #ifdef WOLFSSL_DTLS13 if (IsAtLeastTLSv1_3(ssl->version)) { - int updateRet = Dtls13UpdateWindow(ssl); - if (updateRet != 0) { - WOLFSSL_ERROR(updateRet); - return updateRet; - } - updateRet = Dtls13RecordRecvd(ssl); + int updateRet = Dtls13UpdateWindowRecordRecvd(ssl); if (updateRet != 0) { WOLFSSL_ERROR(updateRet); return updateRet; @@ -21180,12 +21190,7 @@ default: if (ssl->options.dtls) { #ifdef WOLFSSL_DTLS13 if (IsAtLeastTLSv1_3(ssl->version)) { - ret = Dtls13UpdateWindow(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } - ret = Dtls13RecordRecvd(ssl); + ret = Dtls13UpdateWindowRecordRecvd(ssl); if (ret != 0) { WOLFSSL_ERROR(ret); return ret; @@ -21211,18 +21216,15 @@ default: ssl->keys.padSz, &processedSize); ssl->buffers.inputBuffer.idx += processedSize; ssl->buffers.inputBuffer.idx += ssl->keys.padSz; + if (DtlsShouldUpdateWindow(ret)) { + int updateRet = Dtls13UpdateWindowRecordRecvd(ssl); + if (updateRet != 0) { + WOLFSSL_ERROR(updateRet); + return updateRet; + } + } if (ret != 0) return ret; - ret = Dtls13UpdateWindow(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } - ret = Dtls13RecordRecvd(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } break; } FALL_THROUGH; From bec87e525fb7f7dc6bedd5e1e19988d411c2d9bd Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 11 Sep 2023 17:06:30 +0200 Subject: [PATCH 12/22] PQC TLS 1.3: test setting pqc with wolfSSL_CTX_set_groups --- tests/api.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/api.c b/tests/api.c index 921b4258c..32da35abf 100644 --- a/tests/api.c +++ b/tests/api.c @@ -65242,8 +65242,7 @@ static int test_dtls13_frag_ch_pq(void) { EXPECT_DECLS; #if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13) \ - && defined(WOLFSSL_DTLS_MTU) && defined(WOLFSSL_DTLS_CH_FRAG) \ - && defined(HAVE_LIBOQS) + && defined(WOLFSSL_DTLS_CH_FRAG) && defined(HAVE_LIBOQS) WOLFSSL_CTX *ctx_c = NULL; WOLFSSL_CTX *ctx_s = NULL; WOLFSSL *ssl_c = NULL; @@ -65507,6 +65506,45 @@ static int test_dtls_empty_keyshare_with_cookie(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_LIBOQS) +static void test_tls13_pq_groups_ctx_ready(WOLFSSL_CTX* ctx) +{ + int group = WOLFSSL_KYBER_LEVEL5; + AssertIntEQ(wolfSSL_CTX_set_groups(ctx, &group, 1), WOLFSSL_SUCCESS); +} + +static void test_tls13_pq_groups_on_result(WOLFSSL* ssl) +{ + AssertStrEQ(wolfSSL_get_curve_name(ssl), "KYBER_LEVEL5"); +} +#endif + +static int test_tls13_pq_groups(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \ + defined(HAVE_LIBOQS) + callback_functions func_cb_client; + callback_functions func_cb_server; + + XMEMSET(&func_cb_client, 0, sizeof(callback_functions)); + XMEMSET(&func_cb_server, 0, sizeof(callback_functions)); + + func_cb_client.method = wolfTLSv1_3_client_method; + func_cb_server.method = wolfTLSv1_3_server_method; + func_cb_client.ctx_ready = test_tls13_pq_groups_ctx_ready; + func_cb_client.on_result = test_tls13_pq_groups_on_result; + func_cb_server.on_result = test_tls13_pq_groups_on_result; + + test_wolfSSL_client_server_nofail(&func_cb_client, &func_cb_server); + + ExpectIntEQ(func_cb_client.return_code, TEST_SUCCESS); + ExpectIntEQ(func_cb_server.return_code, TEST_SUCCESS); +#endif + return EXPECT_RESULT(); +} + /*----------------------------------------------------------------------------* | Main *----------------------------------------------------------------------------*/ @@ -66779,6 +66817,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_dtls_frag_ch), TEST_DECL(test_dtls13_frag_ch_pq), TEST_DECL(test_dtls_empty_keyshare_with_cookie), + TEST_DECL(test_tls13_pq_groups), /* This test needs to stay at the end to clean up any caches allocated. */ TEST_DECL(test_wolfSSL_Cleanup) }; From c1a49fef9925379ecdc4e2e7ab659ca36f8b5348 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 11 Sep 2023 17:17:19 +0200 Subject: [PATCH 13/22] Fix unreachable code error --- src/dtls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dtls.c b/src/dtls.c index 30af15c69..9475bd2ed 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -304,13 +304,13 @@ static int ParseClientHello(const byte* input, word32 helloSz, WolfSSL_CH* ch, idx += ReadVector16(input + idx, &ch->extension); if (idx > helloSz) { #ifdef WOLFSSL_DTLS_CH_FRAG + idx = helloSz; /* Allow incomplete extensions if we are parsing a fragment */ if (isFirstCHFrag && extStart < helloSz) ch->extension.size = helloSz - extStart; else #endif return BUFFER_ERROR; - idx = helloSz; } } if (idx != helloSz) From 948d7ae7611dc728c38e56fdf2b7e63dfa7c3856 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 11 Sep 2023 19:34:24 +0200 Subject: [PATCH 14/22] keyLog_callback: flush the descriptor to make sure it is written out --- tests/api.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/api.c b/tests/api.c index 32da35abf..1aae40f47 100644 --- a/tests/api.c +++ b/tests/api.c @@ -34625,6 +34625,7 @@ static void keyLog_callback(const WOLFSSL* ssl, const char* line ) fp = XFOPEN("./MyKeyLog.txt", "a"); XFWRITE( line, 1, strlen(line),fp); XFWRITE( (void*)&lf,1,1,fp); + XFFLUSH(fp); XFCLOSE(fp); } From 8ac72750bce608b1b2a0a7f8854127637265387f Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 11 Sep 2023 19:40:37 +0200 Subject: [PATCH 15/22] Fix linting issues --- tests/api.c | 18 +++++++++++------- wolfssl/internal.h | 12 ++++++------ wolfssl/wolfcrypt/settings.h | 3 ++- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/api.c b/tests/api.c index 1aae40f47..56590b9a6 100644 --- a/tests/api.c +++ b/tests/api.c @@ -65388,10 +65388,14 @@ static int test_dtls_frag_ch(void) ExpectIntEQ(wolfSSL_dtls_set_mtu(ssl_s, DUMMY_MTU), WOLFSSL_SUCCESS); /* Add in some key shares to make the CH long */ - ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP256R1), WOLFSSL_SUCCESS); - ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP384R1), WOLFSSL_SUCCESS); - ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP521R1), WOLFSSL_SUCCESS); - ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_FFDHE_2048), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP256R1), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP384R1), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ECC_SECP521R1), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_FFDHE_2048), + WOLFSSL_SUCCESS); ExpectIntEQ(wolfSSL_dtls13_allow_ch_frag(ssl_s, 1), WOLFSSL_SUCCESS); @@ -65413,16 +65417,16 @@ static int test_dtls_frag_ch(void) ExpectIntEQ(test_ctx.c_len, 0); /* CH1 */ - ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); + ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); /* Count records. Expect 1 unfragmented CH */ ExpectIntEQ(test_dtls_frag_ch_count_records(test_ctx.s_buff, test_ctx.s_len), 1); /* HRR */ - ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1); + ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1); ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ); /* CH2 */ - ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); + ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1); ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ); /* Count records. Expect fragmented CH */ ExpectIntGT(test_dtls_frag_ch_count_records(test_ctx.s_buff, diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 1e069b380..ac7ca5a82 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -4730,7 +4730,7 @@ struct Options { word16 dtls13SendMoreAcks:1; /* Send more acks during the * handshake process */ #ifdef WOLFSSL_DTLS_CH_FRAG - word16 dtls13ChFrag:1; + word16 dtls13ChFrag:1; #endif #endif #ifdef WOLFSSL_TLS13 @@ -5421,13 +5421,13 @@ struct WOLFSSL { WOLFSSL_HEAP_HINT heap_hint; #endif #if defined(WOLFSSL_DTLS) && !defined(NO_WOLFSSL_SERVER) - ClientHelloGoodCb chGoodCb; /* notify user we parsed a verified - * ClientHello that passed basic tests */ - void* chGoodCtx; /* user ClientHello cb context */ + ClientHelloGoodCb chGoodCb; /* notify user we parsed a verified + * ClientHello that passed basic tests */ + void* chGoodCtx; /* user ClientHello cb context */ #endif #ifndef NO_HANDSHAKE_DONE_CB - HandShakeDoneCb hsDoneCb; /* notify user handshake done */ - void* hsDoneCtx; /* user handshake cb context */ + HandShakeDoneCb hsDoneCb; /* notify user handshake done */ + void* hsDoneCtx; /* user handshake cb context */ #endif #ifdef WOLFSSL_ASYNC_IO #ifdef WOLFSSL_ASYNC_CRYPT diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index 54c623227..b9b5e5f60 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -3016,7 +3016,8 @@ extern void uITRON4_free(void *p) ; #if defined(HAVE_PQC) && defined(WOLFSSL_DTLS13) && \ !defined(WOLFSSL_DTLS_CH_FRAG) -#warning "Using DTLS 1.3 + pqc without WOLFSSL_DTLS_CH_FRAG will probably fail. Use --enable-dtls-frag-ch to enable it." +#warning "Using DTLS 1.3 + pqc without WOLFSSL_DTLS_CH_FRAG will probably" \ + "fail.Use --enable-dtls-frag-ch to enable it." #endif #if !defined(WOLFSSL_DTLS13) && defined(WOLFSSL_DTLS_CH_FRAG) #error "WOLFSSL_DTLS_CH_FRAG only works with DTLS 1.3" From 275c0a0838035cbce5311954e8272ac6deebd1c9 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 3 Oct 2023 16:46:32 +0200 Subject: [PATCH 16/22] Update window in one place only when stateful --- src/dtls.c | 10 +++- src/internal.c | 129 ++++++++++++--------------------------------- wolfssl/internal.h | 2 + 3 files changed, 45 insertions(+), 96 deletions(-) diff --git a/src/dtls.c b/src/dtls.c index 9475bd2ed..8b5234fee 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -972,8 +972,16 @@ int DoClientHelloStateless(WOLFSSL* ssl, const byte* input, word32 helloSz, #endif ret = SendStatelessReply((WOLFSSL*)ssl, &ch, isTls13); } - else + else { ssl->options.dtlsStateful = 1; + /* Update the window now that we enter the stateful parsing */ +#ifdef WOLFSSL_DTLS13 + if (isTls13) + ret = Dtls13UpdateWindowRecordRecvd(ssl); + else +#endif + DtlsUpdateWindow(ssl); + } } return ret; diff --git a/src/internal.c b/src/internal.c index 138d65667..f530f9683 100644 --- a/src/internal.c +++ b/src/internal.c @@ -215,7 +215,6 @@ WOLFSSL_CALLBACKS needs LARGE_STATIC_BUFFERS, please add LARGE_STATIC_BUFFERS #ifdef WOLFSSL_DTLS static int _DtlsCheckWindow(WOLFSSL* ssl); - static int _DtlsUpdateWindow(WOLFSSL* ssl); #endif #ifdef WOLFSSL_DTLS13 @@ -16975,7 +16974,7 @@ int wolfSSL_DtlsUpdateWindow(word16 cur_hi, word32 cur_lo, return 1; } -static int _DtlsUpdateWindow(WOLFSSL* ssl) +int DtlsUpdateWindow(WOLFSSL* ssl) { WOLFSSL_DTLS_PEERSEQ* peerSeq = ssl->keys.peerSeq; word16 *next_hi; @@ -17040,20 +17039,6 @@ static int _DtlsUpdateWindow(WOLFSSL* ssl) next_hi, next_lo, window); } -static WC_INLINE int DtlsShouldUpdateWindow(int ret) -{ - switch (ret) { - case 0: -#ifdef WOLFSSL_ASYNC_CRYPT - case WC_PENDING_E: -#endif - case APP_DATA_READY: - return 1; - default: - return 0; - } -} - #ifdef WOLFSSL_DTLS13 static int Dtls13UpdateWindow(WOLFSSL* ssl) @@ -17120,7 +17105,7 @@ static int Dtls13UpdateWindow(WOLFSSL* ssl) return 0; } -static WC_INLINE int Dtls13UpdateWindowRecordRecvd(WOLFSSL* ssl) +int Dtls13UpdateWindowRecordRecvd(WOLFSSL* ssl) { int ret = Dtls13UpdateWindow(ssl); if (ret != 0) @@ -20751,17 +20736,33 @@ default: /* the record layer is here */ case runProcessingOneRecord: #ifdef WOLFSSL_DTLS13 - if (ssl->options.dtls && IsAtLeastTLSv1_3(ssl->version) && - !Dtls13CheckWindow(ssl)) { - /* drop packet */ - WOLFSSL_MSG("Dropping DTLS record outside receiving window"); - ssl->options.processReply = doProcessInit; - ssl->buffers.inputBuffer.idx += ssl->curSize; - if (ssl->buffers.inputBuffer.idx > - ssl->buffers.inputBuffer.length) - return BUFFER_E; + if (ssl->options.dtls) { + if (IsAtLeastTLSv1_3(ssl->version)) { + if (!Dtls13CheckWindow(ssl)) { + /* drop packet */ + WOLFSSL_MSG("Dropping DTLS record outside receiving " + "window"); + ssl->options.processReply = doProcessInit; + ssl->buffers.inputBuffer.idx += ssl->curSize; + if (ssl->buffers.inputBuffer.idx > + ssl->buffers.inputBuffer.length) + return BUFFER_E; - continue; + continue; + } + + /* Only update the window once we enter stateful parsing */ + if (ssl->options.dtlsStateful) { + ret = Dtls13UpdateWindowRecordRecvd(ssl); + if (ret != 0) { + WOLFSSL_ERROR(ret); + return ret; + } + } + } + else if (IsDtlsNotSctpMode(ssl)) { + DtlsUpdateWindow(ssl); + } } #endif /* WOLFSSL_DTLS13 */ ssl->options.processReply = runProcessingOneMessage; @@ -20828,15 +20829,12 @@ default: ssl->buffers.inputBuffer.buffer, &ssl->buffers.inputBuffer.idx, ssl->buffers.inputBuffer.length); - if (DtlsShouldUpdateWindow(ret) && - ssl->options.dtlsStateful) { - if (IsDtlsNotSctpMode(ssl)) - _DtlsUpdateWindow(ssl); + if (ret == 0 || ret == WC_PENDING_E) { /* Reset timeout as we have received a valid * DTLS handshake message */ ssl->dtls_timeout = ssl->dtls_timeout_init; } - if (ret != 0) { + else { if (SendFatalAlertOnly(ssl, ret) == SOCKET_ERROR_E) { ret = SOCKET_ERROR_E; @@ -20850,15 +20848,6 @@ default: ssl->buffers.inputBuffer.buffer, &ssl->buffers.inputBuffer.idx, ssl->buffers.inputBuffer.length); - if (DtlsShouldUpdateWindow(ret) && - ssl->options.dtlsStateful) { - int updateRet = - Dtls13UpdateWindowRecordRecvd(ssl); - if (updateRet != 0) { - WOLFSSL_ERROR(updateRet); - return updateRet; - } - } #ifdef WOLFSSL_EARLY_DATA if (ret == 0 && ssl->options.side == WOLFSSL_SERVER_END && @@ -20979,15 +20968,6 @@ default: WOLFSSL_ERROR_VERBOSE(UNKNOWN_RECORD_TYPE); return UNKNOWN_RECORD_TYPE; } -#ifdef WOLFSSL_DTLS13 - if (ssl->options.dtls) { - ret = Dtls13UpdateWindowRecordRecvd(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } - } -#endif break; } #endif @@ -21075,8 +21055,6 @@ default: #ifdef WOLFSSL_DTLS if (ssl->options.dtls) { WOLFSSL_DTLS_PEERSEQ* peerSeq = ssl->keys.peerSeq; - if (IsDtlsNotSctpMode(ssl)) - _DtlsUpdateWindow(ssl); #ifdef WOLFSSL_MULTICAST if (ssl->options.haveMcast) { peerSeq += ssl->keys.curPeerId; @@ -21138,26 +21116,10 @@ default: return SANITY_MSG_E; } #endif - ret = DoApplicationData(ssl, - ssl->buffers.inputBuffer.buffer, - &ssl->buffers.inputBuffer.idx, NO_SNIFF); -#ifdef WOLFSSL_DTLS - if (ssl->options.dtls && DtlsShouldUpdateWindow(ret)) { -#ifdef WOLFSSL_DTLS13 - if (IsAtLeastTLSv1_3(ssl->version)) { - int updateRet = Dtls13UpdateWindowRecordRecvd(ssl); - if (updateRet != 0) { - WOLFSSL_ERROR(updateRet); - return updateRet; - } - } - else -#endif - if (IsDtlsNotSctpMode(ssl)) - _DtlsUpdateWindow(ssl); - } -#endif - if (ret != 0) { + if ((ret = DoApplicationData(ssl, + ssl->buffers.inputBuffer.buffer, + &ssl->buffers.inputBuffer.idx, + NO_SNIFF)) != 0) { WOLFSSL_ERROR(ret); return ret; } @@ -21186,22 +21148,6 @@ default: /* Reset error if we got an alert level in ret */ if (ret > 0) ret = 0; -#ifdef WOLFSSL_DTLS - if (ssl->options.dtls) { -#ifdef WOLFSSL_DTLS13 - if (IsAtLeastTLSv1_3(ssl->version)) { - ret = Dtls13UpdateWindowRecordRecvd(ssl); - if (ret != 0) { - WOLFSSL_ERROR(ret); - return ret; - } - } - else -#endif - if (IsDtlsNotSctpMode(ssl)) - _DtlsUpdateWindow(ssl); - } -#endif break; #ifdef WOLFSSL_DTLS13 @@ -21216,13 +21162,6 @@ default: ssl->keys.padSz, &processedSize); ssl->buffers.inputBuffer.idx += processedSize; ssl->buffers.inputBuffer.idx += ssl->keys.padSz; - if (DtlsShouldUpdateWindow(ret)) { - int updateRet = Dtls13UpdateWindowRecordRecvd(ssl); - if (updateRet != 0) { - WOLFSSL_ERROR(updateRet); - return updateRet; - } - } if (ret != 0) return ret; break; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index ac7ca5a82..10c395551 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -6479,6 +6479,7 @@ WOLFSSL_LOCAL word32 nid2oid(int nid, int grp); #ifdef WOLFSSL_DTLS WOLFSSL_API int wolfSSL_DtlsUpdateWindow(word16 cur_hi, word32 cur_lo, word16* next_hi, word32* next_lo, word32 *window); +WOLFSSL_LOCAL int DtlsUpdateWindow(WOLFSSL* ssl); WOLFSSL_LOCAL void DtlsResetState(WOLFSSL *ssl); WOLFSSL_LOCAL int DtlsIgnoreError(int err); WOLFSSL_LOCAL void DtlsSetSeqNumForReply(WOLFSSL* ssl); @@ -6547,6 +6548,7 @@ WOLFSSL_LOCAL void Dtls13RtxFlushBuffered(WOLFSSL* ssl, WOLFSSL_LOCAL int Dtls13RtxTimeout(WOLFSSL* ssl); WOLFSSL_LOCAL int Dtls13ProcessBufferedMessages(WOLFSSL* ssl); WOLFSSL_LOCAL int Dtls13CheckAEADFailLimit(WOLFSSL* ssl); +WOLFSSL_LOCAL int Dtls13UpdateWindowRecordRecvd(WOLFSSL* ssl); #endif /* WOLFSSL_DTLS13 */ #ifdef WOLFSSL_STATIC_EPHEMERAL From 64ed7d57eb5da349183b210738354e28ea930743 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 9 Oct 2023 12:54:07 +0200 Subject: [PATCH 17/22] Add comment --- src/internal.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal.c b/src/internal.c index f530f9683..60c5f0e7a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -17041,6 +17041,11 @@ int DtlsUpdateWindow(WOLFSSL* ssl) #ifdef WOLFSSL_DTLS13 +/* Update DTLS 1.3 window + * Return + * 0 on successful update + * <0 on error + */ static int Dtls13UpdateWindow(WOLFSSL* ssl) { w64wrapper nextSeq, seq; From 365fae8ac0f8833789001b0fe2a53e1aa95dc866 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 10 Oct 2023 11:37:48 +0200 Subject: [PATCH 18/22] Add curl test dep --- .github/workflows/curl.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/curl.yml b/.github/workflows/curl.yml index 16430bafd..e447571e2 100644 --- a/.github/workflows/curl.yml +++ b/.github/workflows/curl.yml @@ -13,6 +13,7 @@ jobs: run: | sudo apt-get update sudo apt-get install nghttp2 + sudo pip install impacket - name: Build wolfSSL uses: wolfSSL/actions-build-autotools-project@v1 From cb912219e4f0666d1d58a712d6173aec3f27cd3e Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 10 Oct 2023 14:19:03 +0200 Subject: [PATCH 19/22] Run only stable curl tests --- .github/workflows/curl.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/curl.yml b/.github/workflows/curl.yml index e447571e2..009deb599 100644 --- a/.github/workflows/curl.yml +++ b/.github/workflows/curl.yml @@ -4,7 +4,7 @@ on: workflow_call: jobs: - build: + build-and-test: runs-on: ubuntu-latest # This should be a safe limit for the tests to run. timeout-minutes: 14 @@ -22,10 +22,14 @@ jobs: configure: --enable-curl install: true - - name: Build and test curl + - name: Build curl uses: wolfSSL/actions-build-autotools-project@v1 with: repository: curl/curl path: curl configure: --with-wolfssl=$GITHUB_WORKSPACE/build-dir - check: true + check: false + + - name: Test curl + working-directory: curl + run: make -j test-ci From 53f14206d155d4c48e55799cd60c93ecfe765144 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 10 Oct 2023 14:36:00 +0200 Subject: [PATCH 20/22] Increase curl timeout --- .github/workflows/curl.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/curl.yml b/.github/workflows/curl.yml index 009deb599..5f1612ed5 100644 --- a/.github/workflows/curl.yml +++ b/.github/workflows/curl.yml @@ -7,7 +7,7 @@ jobs: build-and-test: runs-on: ubuntu-latest # This should be a safe limit for the tests to run. - timeout-minutes: 14 + timeout-minutes: 25 steps: - name: Install test dependencies run: | From 5372cd50267676146c446fa5adb959c084383eb6 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 10 Oct 2023 15:07:09 +0200 Subject: [PATCH 21/22] Update openwrt script --- Docker/OpenWrt/runTests.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Docker/OpenWrt/runTests.sh b/Docker/OpenWrt/runTests.sh index fc79d27f7..e69125696 100755 --- a/Docker/OpenWrt/runTests.sh +++ b/Docker/OpenWrt/runTests.sh @@ -1,11 +1,12 @@ #!/bin/sh runCMD() { # usage: runCMD "" "" - eval $1 >/dev/null 2>&1 + TMP_FILE=$(mktemp) + eval $1 > $TMP_FILE 2>&1 RETVAL=$? if [ "$RETVAL" != "$2" ]; then - echo "Command ($1) returned ${RETVAL}, but expected $2. Rerunning with output to terminal:" - eval $1 + echo "Command ($1) returned ${RETVAL}, but expected $2. Error output:" + cat $TMP_FILE exit 1 fi } From ca73a311cf70dda10b89ccbe94821b783a24828d Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 10 Oct 2023 17:31:45 +0200 Subject: [PATCH 22/22] Don't use /dev/null --- Docker/OpenWrt/runTests.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Docker/OpenWrt/runTests.sh b/Docker/OpenWrt/runTests.sh index e69125696..1585da5d9 100755 --- a/Docker/OpenWrt/runTests.sh +++ b/Docker/OpenWrt/runTests.sh @@ -17,11 +17,11 @@ runCMD "ldd /lib/libustream-ssl.so" 0 # Remove after fixed upstream. runCMD "sed '\/src\/gz openwrt_kmods https:\/\/downloads.openwrt.org\/releases\/21.02-SNAPSHOT\/targets\/x86\/64\/kmods\/5.4.238-1-5a722da41bc36de95a7195be6fce1b45/s//#&/' -i /etc/opkg/distfeeds.conf" 0 runCMD "opkg update" 0 -runCMD "uclient-fetch -O /dev/null 'https://letsencrypt.org'" 0 +runCMD "uclient-fetch 'https://letsencrypt.org'" 0 # Negative tests -runCMD "uclient-fetch --ca-certificate=/dev/null -O /dev/null 'https://letsencrypt.org'" 5 -runCMD "uclient-fetch -O /dev/null 'https://self-signed.badssl.com/'" 5 -runCMD "uclient-fetch -O /dev/null 'https://untrusted-root.badssl.com/'" 5 -runCMD "uclient-fetch -O /dev/null 'https://expired.badssl.com/'" 5 +runCMD "uclient-fetch --ca-certificate=/dev/null 'https://letsencrypt.org'" 5 +runCMD "uclient-fetch 'https://self-signed.badssl.com/'" 5 +runCMD "uclient-fetch 'https://untrusted-root.badssl.com/'" 5 +runCMD "uclient-fetch 'https://expired.badssl.com/'" 5 echo "All tests passed."