From 335722c586d894c49c5317333a25b67e101d61c6 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 22 Feb 2023 09:46:02 +0100 Subject: [PATCH] Async fixes --- src/dtls.c | 2 +- src/dtls13.c | 6 +++-- src/internal.c | 2 +- src/tls.c | 60 +++++++++++++++++++++++++++++-------------- src/tls13.c | 64 ++++++++++++++++++++++++++++++++++------------ tests/api.c | 6 +++++ wolfssl/internal.h | 8 ++++-- 7 files changed, 107 insertions(+), 41 deletions(-) diff --git a/src/dtls.c b/src/dtls.c index c517bf895..fda5a8957 100644 --- a/src/dtls.c +++ b/src/dtls.c @@ -744,7 +744,7 @@ static int SendStatelessReplyDtls13(const WOLFSSL* ssl, WolfSSL_CH* ch) /* TLSX_KeyShare_Choose is done deep inside MatchSuite_ex */ ret = MatchSuite_ex(ssl, &suites, &cs, parsedExts); if (ret < 0) { - WOLFSSL_MSG("Unsupported cipher suite, ClientHello"); + WOLFSSL_MSG("Unsupported cipher suite, ClientHello DTLS 1.3"); ERROR_OUT(INCOMPLETE_DATA, dtls13_cleanup); } } diff --git a/src/dtls13.c b/src/dtls13.c index aa0da54b0..6149031f5 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -363,8 +363,10 @@ int Dtls13ProcessBufferedMessages(WOLFSSL* ssl) msg->sz, msg->sz); /* processing certificate_request triggers a connect. The error came - * from there, the message can be considered processed successfully */ - if (ret == 0 || (msg->type == certificate_request && + * from there, the message can be considered processed successfully. + * WANT_WRITE means that we are done with processing the msg and we are + * waiting to flush the output buffer. */ + if ((ret == 0 || ret == WANT_WRITE) || (msg->type == certificate_request && ssl->options.handShakeDone && ret == WC_PENDING_E)) { Dtls13MsgWasProcessed(ssl, (enum HandShakeType)msg->type); diff --git a/src/internal.c b/src/internal.c index 35ba423bd..aad2ae532 100644 --- a/src/internal.c +++ b/src/internal.c @@ -20125,7 +20125,7 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) ) { /* Shrink input buffer when we successfully finish record * processing */ - if (ret == 0 && ssl->buffers.inputBuffer.dynamicFlag) + if ((ret == 0) && ssl->buffers.inputBuffer.dynamicFlag) ShrinkInputBuffer(ssl, NO_FORCED_FREE); return ret; } diff --git a/src/tls.c b/src/tls.c index 9a68fe13d..541f60900 100644 --- a/src/tls.c +++ b/src/tls.c @@ -7393,7 +7393,7 @@ static int TLSX_KeyShare_GenPqcKey(WOLFSSL *ssl, KeyShareEntry* kse) * ssl The SSL/TLS object. * kse The key share entry holding peer data. */ -static int TLSX_KeyShare_GenKey(WOLFSSL *ssl, KeyShareEntry *kse) +int TLSX_KeyShare_GenKey(WOLFSSL *ssl, KeyShareEntry *kse) { int ret; /* Named FFDHE groups have a bit set to identify them. */ @@ -9027,6 +9027,7 @@ int TLSX_KeyShare_SetSupported(const WOLFSSL* ssl, TLSX** extensions) TLSX* extension; SupportedCurve* curve = NULL; SupportedCurve* preferredCurve = NULL; + KeyShareEntry* kse = NULL; int preferredRank = WOLFSSL_MAX_GROUP_COUNT; int rank; @@ -9055,27 +9056,32 @@ int TLSX_KeyShare_SetSupported(const WOLFSSL* ssl, TLSX** extensions) return BAD_KEY_SHARE_DATA; } - /* Delete the old key share data list. */ + #ifdef WOLFSSL_ASYNC_CRYPT + /* Check the old key share data list. */ extension = TLSX_Find(*extensions, TLSX_KEY_SHARE); if (extension != NULL) { - KeyShareEntry* kse = (KeyShareEntry*)extension->data; - #ifdef WOLFSSL_ASYNC_CRYPT - /* for async don't free, call `TLSX_KeyShare_Use` again */ - if (kse && kse->lastRet != WC_PENDING_E) - #endif - { - TLSX_KeyShare_FreeAll(kse, ssl->heap); - extension->data = NULL; + kse = (KeyShareEntry*)extension->data; + /* We should not be computing keys if we are only going to advertise + * our choice here. */ + if (kse != NULL && kse->lastRet == WC_PENDING_E) { + WOLFSSL_ERROR_VERBOSE(BAD_KEY_SHARE_DATA); + return BAD_KEY_SHARE_DATA; } } + #endif - /* Add in the chosen group. */ - ret = TLSX_KeyShare_Use(ssl, curve->name, 0, NULL, NULL, extensions); - if (ret != 0 && ret != WC_PENDING_E) + /* Push new KeyShare extension. This will also free the old one */ + ret = TLSX_Push(extensions, TLSX_KEY_SHARE, NULL, ssl->heap); + if (ret != 0) + return ret; + /* Extension got pushed to head */ + extension = *extensions; + /* Push the selected curve */ + ret = TLSX_KeyShare_New((KeyShareEntry**)&extension->data, curve->name, + ssl->heap, &kse); + if (ret != 0) return ret; - /* Set extension to be in response. */ - extension = TLSX_Find(*extensions, TLSX_KEY_SHARE); extension->resp = 1; #else @@ -9114,7 +9120,7 @@ int TLSX_KeyShare_Choose(const WOLFSSL *ssl, TLSX* extensions, int ret = INCOMPLETE_DATA; #ifdef WOLFSSL_ASYNC_CRYPT /* in async case make sure key generation is finalized */ - serverKSE = (KeyShareEntry*)extension->data; + KeyShareEntry* serverKSE = (KeyShareEntry*)extension->data; if (serverKSE && serverKSE->lastRet == WC_PENDING_E) { if (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE) *searched = 1; @@ -9168,14 +9174,30 @@ int TLSX_KeyShare_Setup(WOLFSSL *ssl, KeyShareEntry* clientKSE) KeyShareEntry* serverKSE; KeyShareEntry* list = NULL; - if (ssl == NULL || ssl->options.side != WOLFSSL_SERVER_END || - clientKSE == NULL) + if (ssl == NULL || ssl->options.side != WOLFSSL_SERVER_END) return BAD_FUNC_ARG; extension = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); if (extension == NULL) return BAD_STATE_E; + if (clientKSE == NULL) { +#ifdef WOLFSSL_ASYNC_CRYPT + /* Not necessarily an error. The key may have already been setup. */ + if (extension != NULL && extension->resp == 1) { + serverKSE = (KeyShareEntry*)extension->data; + if (serverKSE != NULL) { + /* in async case make sure key generation is finalized */ + if (serverKSE->lastRet == WC_PENDING_E) + return TLSX_KeyShare_GenKey((WOLFSSL*)ssl, serverKSE); + else if (serverKSE->lastRet == 0) + return 0; + } + } +#endif + return BAD_FUNC_ARG; + } + /* Generate a new key pair except in the case of OQS KEM because we * are going to encapsulate and that does not require us to generate a * key pair. @@ -9230,7 +9252,7 @@ int TLSX_KeyShare_Setup(WOLFSSL *ssl, KeyShareEntry* clientKSE) extension->data = (void *)serverKSE; extension->resp = 1; - return 0; + return ret; } /* Ensure there is a key pair that can be used for key exchange. diff --git a/src/tls13.c b/src/tls13.c index 44b0ec687..78da84a58 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6581,6 +6581,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ssl->options.sendVerify = SEND_CERT; #if defined(WOLFSSL_SEND_HRR_COOKIE) + ssl->options.cookieGood = 0; if (ssl->options.sendCookie && (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE #ifdef WOLFSSL_DTLS13 @@ -6599,22 +6600,17 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ret = RestartHandshakeHashWithCookie(ssl, (Cookie*)ext->data); if (ret != 0) goto exit_dch; - ssl->options.serverState = SERVER_HELLO_COMPLETE; + /* Don't change state here as we may want to enter + * DoTls13ClientHello again. */ + ssl->options.cookieGood = 1; } else { -#ifdef WOLFSSL_DTLS13 - if (ssl->options.dtls) - ssl->options.serverState = NULL_STATE; - else -#endif - ERROR_OUT(HRR_COOKIE_ERROR, exit_dch); + ERROR_OUT(HRR_COOKIE_ERROR, exit_dch); } } - else -#ifdef WOLFSSL_DTLS13 - if (!ssl->options.dtls) -#endif - ERROR_OUT(HRR_COOKIE_ERROR, exit_dch); + else { + ERROR_OUT(HRR_COOKIE_ERROR, exit_dch); + } } #endif @@ -6659,12 +6655,14 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if ((ret = ALPN_Select(ssl)) != 0) goto exit_dch; #endif - /* Advance state and proceed */ - ssl->options.asyncState = TLS_ASYNC_BUILD; } /* case TLS_ASYNC_BEGIN */ FALL_THROUGH; case TLS_ASYNC_BUILD: + /* Advance state and proceed */ + ssl->options.asyncState = TLS_ASYNC_DO; + FALL_THROUGH; + case TLS_ASYNC_DO: { #ifndef NO_CERTS @@ -6673,7 +6671,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #ifdef WOLFSSL_ASYNC_CRYPT if (ret != WC_PENDING_E) #endif - WOLFSSL_MSG("Unsupported cipher suite, ClientHello"); + WOLFSSL_MSG("Unsupported cipher suite, ClientHello 1.3"); goto exit_dch; } } @@ -6695,10 +6693,31 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #endif /* Advance state and proceed */ - ssl->options.asyncState = TLS_ASYNC_FINALIZE; + ssl->options.asyncState = TLS_ASYNC_VERIFY; } /* case TLS_ASYNC_BUILD || TLS_ASYNC_DO */ FALL_THROUGH; + case TLS_ASYNC_VERIFY: + { +#if defined(WOLFSSL_ASYNC_CRYPT) && defined(HAVE_SUPPORTED_CURVES) + /* Check if the KeyShare calculations from the previous state are complete. + * wolfSSL_AsyncPop advances ssl->options.asyncState so we may end up here + * with a pending calculation. */ + TLSX* extension = TLSX_Find(ssl->extensions, TLSX_KEY_SHARE); + if (extension != NULL && extension->resp == 1) { + KeyShareEntry* serverKSE = (KeyShareEntry*)extension->data; + if (serverKSE != NULL && serverKSE->lastRet == WC_PENDING_E) { + ret = TLSX_KeyShare_GenKey(ssl, serverKSE); + if (ret != 0) + goto exit_dch; + } + } +#endif + /* Advance state and proceed */ + ssl->options.asyncState = TLS_ASYNC_FINALIZE; + } + FALL_THROUGH; + case TLS_ASYNC_FINALIZE: { *inOutIdx = args->idx; @@ -6742,6 +6761,19 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ret = INPUT_CASE_ERROR; } /* switch (ssl->options.asyncState) */ +#if defined(WOLFSSL_SEND_HRR_COOKIE) + if (ret == 0 && ssl->options.sendCookie && ssl->options.cookieGood && + (ssl->options.serverState == SERVER_HELLO_RETRY_REQUEST_COMPLETE +#ifdef WOLFSSL_DTLS13 + /* DTLS cookie exchange should be done in stateless code in + * DoClientHelloStateless. If we verified the cookie then + * always advance the state. */ + || ssl->options.dtls +#endif + )) + ssl->options.serverState = SERVER_HELLO_COMPLETE; +#endif + #if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE) if (ret == 0 && ssl->options.dtls && ssl->options.sendCookie && ssl->options.serverState <= SERVER_HELLO_RETRY_REQUEST_COMPLETE) { diff --git a/tests/api.c b/tests/api.c index 5f02f9f61..80e731d8e 100644 --- a/tests/api.c +++ b/tests/api.c @@ -59356,6 +59356,12 @@ static word32 test_wolfSSL_dtls_stateless_HashWOLFSSL(const WOLFSSL* ssl) sslCopy.keys.dtls_peer_handshake_number = 0; XMEMSET(&sslCopy.alert_history, 0, sizeof(sslCopy.alert_history)); sslCopy.hsHashes = NULL; +#ifdef WOLFSSL_ASYNC_IO +#ifdef WOLFSSL_ASYNC_CRYPT + sslCopy.asyncDev = NULL; +#endif + sslCopy.async = NULL; +#endif AssertIntEQ(wc_HashInit(&hash, hashType), 0); AssertIntEQ(wc_HashUpdate(&hash, hashType, (byte*)&sslCopy, sizeof(sslCopy)), 0); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 3b0bc62c8..9708d17d7 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3084,6 +3084,7 @@ WOLFSSL_LOCAL int TLSX_KeyShare_Use(const WOLFSSL* ssl, word16 group, WOLFSSL_LOCAL int TLSX_KeyShare_Empty(WOLFSSL* ssl); WOLFSSL_LOCAL int TLSX_KeyShare_SetSupported(const WOLFSSL* ssl, TLSX** extensions); +WOLFSSL_LOCAL int TLSX_KeyShare_GenKey(WOLFSSL *ssl, KeyShareEntry *kse); WOLFSSL_LOCAL int TLSX_KeyShare_Choose(const WOLFSSL *ssl, TLSX* extensions, KeyShareEntry** kse, byte* searched); WOLFSSL_LOCAL int TLSX_KeyShare_Setup(WOLFSSL *ssl, KeyShareEntry* clientKSE); @@ -4389,10 +4390,13 @@ typedef struct Options { word16 tls13MiddleBoxCompat:1; /* TLSv1.3 middlebox compatibility */ #endif #ifdef WOLFSSL_DTLS_CID - byte useDtlsCID:1; + word16 useDtlsCID:1; #endif /* WOLFSSL_DTLS_CID */ #if defined(HAVE_ECH) - byte useEch:1; + word16 useEch:1; +#endif +#ifdef WOLFSSL_SEND_HRR_COOKIE + word16 cookieGood:1; #endif /* need full byte values for this section */