From 60befc82c5957a0e91e434e2f332d03bffc69682 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 29 Aug 2019 16:53:15 +1000 Subject: [PATCH] Fixes for fuzz testing Changes - Don't ignore decryption errors when doing TLS 1.3 and after Client Finished. - Put out an alert when TLS 1.3 decryption fails. - Properly ignore RSA pss_pss algorithms when checking for matching cipher suite. - Check X25519 public value before import in TLS v1.2- - REcognise TLS 1.3 integrity-only cipher suites as not negotiable with TLS 1.2-. - Send decode_error alert when bad message data in CertificateVerify. - Negotiate protocol version in TLS 1.3 using extension and keep decision when using TLS 1.2 parsing. - Must have a signature algorithms extension in TLS 1.3 if not doing PSK. - More TLS v1.3 alerts. - MAX_PSK_ID_LEN needs to be modified at compile time for tlsfuzzer to work. - change the good ecc public key to be a real public key when compiled to check imported public keys - Fix early data in TLS 1.3 - Make max early data size able to be changed at compile time - default 4K but fuzzer sends 16K - Fix HRR, PSK and message hashes: Don't initialize hashes in parsing ClientHello as need to keep hash state from previous ClientHello and HelloRetryRequest --- src/internal.c | 129 ++++++++++++++++++++++++++++++++++++--------- src/tls.c | 5 +- src/tls13.c | 51 ++++++++++-------- tests/api.c | 16 ++++++ wolfssl/internal.h | 19 ++++--- 5 files changed, 166 insertions(+), 54 deletions(-) diff --git a/src/internal.c b/src/internal.c index beb06738f..560709a44 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8187,9 +8187,9 @@ static int BuildFinished(WOLFSSL* ssl, Hashes* hashes, const byte* sender) #if defined(WOLFSSL_TLS13) && defined(HAVE_NULL_CIPHER) case TLS_SHA256_SHA256: - return 0; + break; case TLS_SHA384_SHA384: - return 0; + break; #endif default: @@ -13220,9 +13220,19 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx) #endif #ifdef WOLFSSL_EARLY_DATA - if (ssl->earlyData != no_early_data) { + if (ssl->options.tls1_3 && ssl->options.handShakeDone == 0) { + if (ssl->options.side == WOLFSSL_SERVER_END && + ssl->earlyData != no_early_data && + ssl->options.clientState < CLIENT_FINISHED_COMPLETE) { + ssl->earlyDataSz += ssl->curSize; + if (ssl->earlyDataSz <= ssl->options.maxEarlyDataSz) { + WOLFSSL_MSG("Ignoring EarlyData!"); + *inOutIdx = ssl->buffers.inputBuffer.length; + return 0; + } + WOLFSSL_MSG("Too much EarlyData!"); + } } - else #endif if (ssl->options.handShakeDone == 0) { WOLFSSL_MSG("Received App data before a handshake completed"); @@ -13253,7 +13263,7 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx) return BUFFER_ERROR; } #ifdef WOLFSSL_EARLY_DATA - if (ssl->earlyData != no_early_data) { + if (ssl->earlyData > early_data_ext) { if (ssl->earlyDataSz + dataSz > ssl->options.maxEarlyDataSz) { SendAlert(ssl, alert_fatal, unexpected_message); return WOLFSSL_FATAL_ERROR; @@ -13926,15 +13936,24 @@ int ProcessReply(WOLFSSL* ssl) WOLFSSL_ERROR(ret); #ifdef WOLFSSL_EARLY_DATA if (ssl->options.tls1_3) { - ssl->earlyDataSz += ssl->curSize; - if (ssl->earlyDataSz <= ssl->options.maxEarlyDataSz) { - if (ssl->keys.peer_sequence_number_lo-- == 0) - ssl->keys.peer_sequence_number_hi--; - ssl->options.processReply = doProcessInit; - ssl->buffers.inputBuffer.idx = - ssl->buffers.inputBuffer.length; - return 0; + if (ssl->options.side == WOLFSSL_SERVER_END && + ssl->earlyData != no_early_data && + ssl->options.clientState < + CLIENT_FINISHED_COMPLETE) { + ssl->earlyDataSz += ssl->curSize; + if (ssl->earlyDataSz <= + ssl->options.maxEarlyDataSz) { + WOLFSSL_MSG("Ignoring EarlyData!"); + if (ssl->keys.peer_sequence_number_lo-- == 0) + ssl->keys.peer_sequence_number_hi--; + ssl->options.processReply = doProcessInit; + ssl->buffers.inputBuffer.idx = + ssl->buffers.inputBuffer.length; + return 0; + } + WOLFSSL_MSG("Too much EarlyData!"); } + SendAlert(ssl, alert_fatal, bad_record_mac); } #endif #ifdef WOLFSSL_DTLS @@ -13949,7 +13968,6 @@ int ProcessReply(WOLFSSL* ssl) #endif /* WOLFSSL_DTLS_DROP_STATS */ } #endif /* WOLFSSL_DTLS */ - return DECRYPT_ERROR; } } @@ -14076,7 +14094,7 @@ int ProcessReply(WOLFSSL* ssl) if (ret != 0) return ret; if (ssl->options.side == WOLFSSL_SERVER_END && - ssl->earlyData && + ssl->earlyData > early_data_ext && ssl->options.handShakeState == HANDSHAKE_DONE) { ssl->earlyData = no_early_data; ssl->options.processReply = doProcessInit; @@ -14246,7 +14264,7 @@ int ProcessReply(WOLFSSL* ssl) #endif if ((ret = DoApplicationData(ssl, ssl->buffers.inputBuffer.buffer, - &ssl->buffers.inputBuffer.idx)) + &ssl->buffers.inputBuffer.idx)) != 0) { WOLFSSL_ERROR(ret); return ret; @@ -17517,7 +17535,10 @@ int SetCipherList(WOLFSSL_CTX* ctx, Suites* suites, const char* list) /* The suites are either ECDSA, RSA, PSK, or Anon. The RSA * suites don't necessarily have RSA in the name. */ #ifdef WOLFSSL_TLS13 - if (cipher_names[i].cipherSuite0 == TLS13_BYTE) { + if (cipher_names[i].cipherSuite0 == TLS13_BYTE || + (cipher_names[i].cipherSuite0 == ECC_BYTE && + (cipher_names[i].cipherSuite == TLS_SHA256_SHA256 || + cipher_names[i].cipherSuite == TLS_SHA384_SHA384))) { #ifndef NO_RSA haveRSAsig = 1; #endif @@ -17648,8 +17669,14 @@ int PickHashSigAlgo(WOLFSSL* ssl, const byte* hashSigAlgo, word32 hashSigAlgoSz) else #endif #ifdef WC_RSA_PSS - if (sigAlgo == ssl->suites->sigAlgo || (sigAlgo == rsa_pss_sa_algo && - ssl->suites->sigAlgo == rsa_sa_algo)) + if (IsAtLeastTLSv1_3(ssl->version) && + ssl->suites->sigAlgo == rsa_sa_algo && + sigAlgo != rsa_pss_sa_algo) { + continue; + } + else if (sigAlgo == ssl->suites->sigAlgo || + (sigAlgo == rsa_pss_sa_algo && + (ssl->suites->sigAlgo == rsa_sa_algo))) #else if (sigAlgo == ssl->suites->sigAlgo) #endif @@ -17684,7 +17711,7 @@ int PickHashSigAlgo(WOLFSSL* ssl, const byte* hashSigAlgo, word32 hashSigAlgoSz) ret = 0; break; } -#if !defined(WOLFSSL_TLS13) || !defined(HAVE_NULL_CIPHER) +#if defined(WOLFSSL_TLS13) else if (ssl->specs.sig_algo == 0 && IsAtLeastTLSv1_3(ssl->version)) { } #endif @@ -17695,6 +17722,7 @@ int PickHashSigAlgo(WOLFSSL* ssl, const byte* hashSigAlgo, word32 hashSigAlgoSz) } } + return ret; } #endif /* !defined(NO_WOLFSSL_SERVER) || !defined(NO_CERTS) */ @@ -19421,6 +19449,21 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, } } + if ((ret = wc_curve25519_check_public( + input + args->idx, length, + EC25519_LITTLE_ENDIAN)) != 0) { + #ifdef WOLFSSL_EXTRA_ALERTS + if (ret == BUFFER_E) + SendAlert(ssl, alert_fatal, decode_error); + else if (ret == ECC_OUT_OF_RANGE_E) + SendAlert(ssl, alert_fatal, bad_record_mac); + else { + SendAlert(ssl, alert_fatal, illegal_parameter); + } + #endif + ERROR_OUT(ECC_PEERKEY_ERROR, exit_dske); + } + if (wc_curve25519_import_public_ex(input + args->idx, length, ssl->peerX25519Key, EC25519_LITTLE_ENDIAN) != 0) { @@ -19559,6 +19602,21 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, } } + if ((ret = wc_curve25519_check_public( + input + args->idx, length, + EC25519_LITTLE_ENDIAN)) != 0) { + #ifdef WOLFSSL_EXTRA_ALERTS + if (ret == BUFFER_E) + SendAlert(ssl, alert_fatal, decode_error); + else if (ret == ECC_OUT_OF_RANGE_E) + SendAlert(ssl, alert_fatal, bad_record_mac); + else { + SendAlert(ssl, alert_fatal, illegal_parameter); + } + #endif + ERROR_OUT(ECC_PEERKEY_ERROR, exit_dske); + } + if (wc_curve25519_import_public_ex(input + args->idx, length, ssl->peerX25519Key, EC25519_LITTLE_ENDIAN) != 0) { @@ -24133,7 +24191,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, else if (ret != 0) return 0; } - else if (first == TLS13_BYTE) { + else if (first == TLS13_BYTE || (first == ECC_BYTE && + (second == TLS_SHA256_SHA256 || second == TLS_SHA384_SHA384))) { /* Can't negotiate TLS 1.3 cipher suites with lower protocol * version. */ return 0; @@ -25419,7 +25478,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } #endif /* WOLFSSL_ASYNC_CRYPT */ #ifdef WOLFSSL_EXTRA_ALERTS - if (ret == SIG_VERIFY_E) + if (ret == BUFFER_ERROR) + SendAlert(ssl, alert_fatal, decode_error); + else if (ret == SIG_VERIFY_E) SendAlert(ssl, alert_fatal, decrypt_error); else if (ret != 0) SendAlert(ssl, alert_fatal, bad_certificate); @@ -26357,6 +26418,22 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } } + if ((ret = wc_curve25519_check_public( + input + args->idx, args->length, + EC25519_LITTLE_ENDIAN)) != 0) { + #ifdef WOLFSSL_EXTRA_ALERTS + if (ret == BUFFER_E) + SendAlert(ssl, alert_fatal, decode_error); + else if (ret == ECC_OUT_OF_RANGE_E) + SendAlert(ssl, alert_fatal, bad_record_mac); + else { + SendAlert(ssl, alert_fatal, + illegal_parameter); + } + #endif + ERROR_OUT(ECC_PEERKEY_ERROR, exit_dcke); + } + if (wc_curve25519_import_public_ex( input + args->idx, args->length, ssl->peerX25519Key, @@ -26404,8 +26481,12 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } } - if (wc_ecc_import_x963_ex(input + args->idx, args->length, - ssl->peerEccKey, private_key->dp->id)) { + if (wc_ecc_import_x963_ex(input + args->idx, + args->length, ssl->peerEccKey, + private_key->dp->id)) { + #ifdef WOLFSSL_EXTRA_ALERTS + SendAlert(ssl, alert_fatal, illegal_parameter); + #endif ERROR_OUT(ECC_PEERKEY_ERROR, exit_dcke); } diff --git a/src/tls.c b/src/tls.c index e362a5a8c..72d30915e 100644 --- a/src/tls.c +++ b/src/tls.c @@ -8698,7 +8698,10 @@ static int TLSX_EarlyData_Parse(WOLFSSL* ssl, byte* input, word16 length, if (length != 0) return BUFFER_E; - return TLSX_EarlyData_Use(ssl, 0); + if (ssl->earlyData == expecting_early_data) + return TLSX_EarlyData_Use(ssl, 0); + ssl->earlyData = early_data_ext; + return 0; } if (msgType == encrypted_extensions) { if (length != 0) diff --git a/src/tls13.c b/src/tls13.c index 1c4123b92..aa8a969b8 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -3488,8 +3488,11 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz, WOLFSSL_ENTER("DoPreSharedKeys"); ext = TLSX_Find(ssl->extensions, TLSX_PRE_SHARED_KEY); - if (ext == NULL) - return 0; + if (ext == NULL) { + /* Hash data up to binders for deriving binders in PSK extension. */ + ret = HashInput(ssl, input, helloSz); + return ret; + } /* Extensions pushed on stack/list and PSK must be last. */ if (ssl->extensions != ext) @@ -3668,6 +3671,11 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz, break; } + /* Hash the rest of the ClientHello. */ + ret = HashInputRaw(ssl, input + helloSz - bindersLen, bindersLen); + if (ret != 0) + return ret; + if (current == NULL) { #ifdef WOLFSSL_PSK_ID_PROTECTION #ifndef NO_CERTS @@ -3680,11 +3688,6 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz, #endif } - /* Hash the rest of the ClientHello. */ - ret = HashInputRaw(ssl, input + helloSz - bindersLen, bindersLen); - if (ret != 0) - return ret; - #ifdef WOLFSSL_EARLY_DATA extEarlyData = TLSX_Find(ssl->extensions, TLSX_EARLY_DATA); if (extEarlyData != NULL) { @@ -4068,13 +4071,16 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* Legacy protocol version cannot negotiate TLS 1.3 or higher. */ if (pv.major > SSLv3_MAJOR || (pv.major == SSLv3_MAJOR && pv.minor >= TLSv1_3_MINOR)) { - pv.minor = SSLv3_MAJOR; + pv.major = SSLv3_MAJOR; pv.minor = TLSv1_2_MINOR; wantDowngrade = 1; + ssl->version.minor = pv.minor; } /* Legacy version must be [ SSLv3_MAJOR, TLSv1_2_MINOR ] for TLS v1.3 */ - else if (pv.major == SSLv3_MAJOR && pv.minor < TLSv1_2_MINOR) + else if (pv.major == SSLv3_MAJOR && pv.minor < TLSv1_2_MINOR) { wantDowngrade = 1; + ssl->version.minor = pv.minor; + } else { ret = DoTls13SupportedVersions(ssl, input + begin, i - begin, helloSz, &wantDowngrade); @@ -4091,7 +4097,6 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (pv.minor < ssl->options.minDowngrade) return VERSION_ERROR; - ssl->version.minor = pv.minor; if ((ret = HashInput(ssl, input + begin, helloSz)) != 0) return ret; @@ -4240,11 +4245,6 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #if (defined(HAVE_SESSION_TICKET) || !defined(NO_PSK)) && \ defined(HAVE_TLS_EXTENSIONS) if (TLSX_Find(ssl->extensions, TLSX_PRE_SHARED_KEY) != NULL) { - if (ssl->options.downgrade) { - if ((ret = InitHandshakeHashes(ssl)) != 0) - return ret; - } - /* Refine list for PSK processing. */ RefineSuites(ssl, &clSuites); @@ -4253,12 +4253,16 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (ret != 0) return ret; } - else { + else +#endif + { #ifdef WOLFSSL_EARLY_DATA ssl->earlyData = no_early_data; #endif + if ((ret = HashInput(ssl, input + begin, helloSz)) != 0) + return ret; + } -#endif if (!usingPSK) { if (TLSX_Find(ssl->extensions, TLSX_KEY_SHARE) == NULL) { @@ -4266,9 +4270,15 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, SendAlert(ssl, alert_fatal, missing_extension); return INCOMPLETE_DATA; } + if (TLSX_Find(ssl->extensions, TLSX_SIGNATURE_ALGORITHMS) == NULL) { + WOLFSSL_MSG("Client did not send a SignatureAlgorithms extension"); + SendAlert(ssl, alert_fatal, missing_extension); + return INCOMPLETE_DATA; + } if ((ret = MatchSuite(ssl, &clSuites)) < 0) { WOLFSSL_MSG("Unsupported cipher suite, ClientHello"); + SendAlert(ssl, alert_fatal, handshake_failure); return ret; } @@ -4284,6 +4294,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (ssl->options.cipherSuite0 != TLS13_BYTE) { WOLFSSL_MSG("Negotiated ciphersuite from lesser version than " "TLS v1.3"); + SendAlert(ssl, alert_fatal, handshake_failure); return VERSION_ERROR; } @@ -4291,15 +4302,9 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (ssl->options.resuming) { ssl->options.resuming = 0; XMEMSET(ssl->arrays->psk_key, 0, ssl->specs.hash_size); - /* May or may not have done any hashing. */ - if ((ret = InitHandshakeHashes(ssl)) != 0) - return ret; } #endif - if ((ret = HashInput(ssl, input + begin, helloSz)) != 0) - return ret; - /* Derive early secret for handshake secret. */ if ((ret = DeriveEarlySecret(ssl)) != 0) return ret; diff --git a/tests/api.c b/tests/api.c index 1ed7e1ea3..d205f1c11 100644 --- a/tests/api.c +++ b/tests/api.c @@ -2026,6 +2026,12 @@ static THREAD_RETURN WOLFSSL_THREAD test_server_nofail(void* args) ctx = wolfSSL_CTX_new(method); } +#if defined(HAVE_SESSION_TICKET) && defined(HAVE_CHACHA) && \ + defined(HAVE_POLY1305) + TicketInit(); + wolfSSL_CTX_set_TicketEncCb(ctx, myTicketEncCb); +#endif + #if defined(USE_WINDOWS_API) port = ((func_args*)args)->signal->port; #elif defined(NO_MAIN_DRIVER) && !defined(WOLFSSL_SNIFFER) && \ @@ -2188,6 +2194,11 @@ done: wc_ecc_fp_free(); /* free per thread cache */ #endif +#if defined(HAVE_SESSION_TICKET) && defined(HAVE_CHACHA) && \ + defined(HAVE_POLY1305) + TicketCleanup(); +#endif + #ifndef WOLFSSL_TIRTOS return 0; #endif @@ -21905,6 +21916,7 @@ static void test_wolfSSL_SESSION(void) tcp_ready ready; func_args server_args; THREAD_TYPE serverThread; + char msg[80]; printf(testingFmt, "wolfSSL_SESSION()"); AssertNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method())); @@ -21953,6 +21965,10 @@ static void test_wolfSSL_SESSION(void) } } while (ret != SSL_SUCCESS && err == WC_PENDING_E); AssertIntEQ(ret, SSL_SUCCESS); + + AssertIntEQ(wolfSSL_write(ssl, "GET", 3), 3); + AssertIntEQ(wolfSSL_read(ssl, msg, sizeof(msg)), 23); + sess = wolfSSL_get_session(ssl); wolfSSL_shutdown(ssl); wolfSSL_free(ssl); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index a71a77b8a..813ae7729 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1156,7 +1156,19 @@ enum { #endif #define MAX_DHKEY_SZ (WOLFSSL_MAX_DHKEY_BITS / 8) +#ifndef MAX_PSK_ID_LEN + /* max psk identity/hint supported */ + #if defined(WOLFSSL_TLS13) + #define MAX_PSK_ID_LEN 256 + #else + #define MAX_PSK_ID_LEN 128 + #endif +#endif +#ifndef MAX_EARLY_DATA_SZ + /* maximum early data size */ + #define MAX_EARLY_DATA_SZ 4096 +#endif enum Misc { CIPHER_BYTE = 0x00, /* Default ciphers */ @@ -1199,11 +1211,6 @@ enum Misc { HELLO_EXT_EXTMS = 0x0017, /* ID for the extended master secret ext */ SECRET_LEN = WOLFSSL_MAX_MASTER_KEY_LENGTH, /* pre RSA and all master */ -#if defined(WOLFSSL_TLS13) - MAX_PSK_ID_LEN = 256, /* max psk identity/hint supported */ -#else - MAX_PSK_ID_LEN = 128, /* max psk identity/hint supported */ -#endif #if defined(WOLFSSL_MYSQL_COMPATIBLE) || \ (defined(USE_FAST_MATH) && defined(FP_MAX_BITS) && FP_MAX_BITS > 8192) #ifndef NO_PSK @@ -1260,7 +1267,6 @@ enum Misc { DEF_TICKET_NONCE_SZ = 1, /* Default ticket nonce size */ MAX_TICKET_NONCE_SZ = 8, /* maximum ticket nonce size */ MAX_LIFETIME = 604800, /* maximum ticket lifetime */ - MAX_EARLY_DATA_SZ = 4096, /* maximum early data size */ RAN_LEN = 32, /* random length */ SEED_LEN = RAN_LEN * 2, /* tls prf seed length */ @@ -3738,6 +3744,7 @@ struct CertReqCtx { #ifdef WOLFSSL_EARLY_DATA typedef enum EarlyDataState { no_early_data, + early_data_ext, expecting_early_data, process_early_data, done_early_data