diff --git a/src/dtls13.c b/src/dtls13.c index 50d999a1c6..9eb304923e 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -2388,7 +2388,10 @@ static Dtls13Epoch* Dtls13NewEpochSlot(WOLFSSL* ssl) WOLFSSL_MSG_EX("Delete epoch: %d", e->epochNumber); #endif /* WOLFSSL_DEBUG_TLS */ - XMEMSET(e, 0, sizeof(*e)); + /* The slot we are reusing holds the previous epoch's symmetric keys, IVs, + * and sn-keys; use ForceZero so the wipe cannot be elided by the + * optimizer when the slot is later overwritten. */ + ForceZero(e, sizeof(*e)); return e; } @@ -3053,11 +3056,17 @@ int SendDtls13Ack(WOLFSSL* ssl) static int Dtls13RtxRecordMatchesReqCtx(Dtls13RtxRecord* r, byte* ctx, byte ctxLen) { + /* r->data points at the 12-byte Dtls13HandshakeHeader (set by + * Dtls13RtxNewRecord from message + recordHeaderLength); the + * CertificateRequest body starts at r->data[DTLS13_HANDSHAKE_HEADER_SZ] + * with a 1-byte request_context length followed by ctxLength bytes. */ if (r->handshakeType != certificate_request) return 0; - if (r->length <= ctxLen + 1) + if (r->length < (word16)(DTLS13_HANDSHAKE_HEADER_SZ + 1 + ctxLen)) return 0; - return XMEMCMP(ctx, r->data + 1, ctxLen) == 0; + if (r->data[DTLS13_HANDSHAKE_HEADER_SZ] != ctxLen) + return 0; + return XMEMCMP(ctx, r->data + DTLS13_HANDSHAKE_HEADER_SZ + 1, ctxLen) == 0; } int Dtls13RtxProcessingCertificate(WOLFSSL* ssl, byte* input, word32 inputSize) diff --git a/src/internal.c b/src/internal.c index 4706275f0c..ebdbd9a1ea 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9126,6 +9126,11 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) #ifdef WOLFSSL_DTLS13 Dtls13FreeFsmResources(ssl); + /* Zero per-epoch symmetric keys / IVs / sn-keys so they are not left + * resident in the heap after FreeSSL releases the SSL struct. Mirrors + * the existing ForceZero on ssl->keys and ssl->clientSecret/serverSecret. */ + ForceZero(ssl->dtls13Epochs, sizeof(ssl->dtls13Epochs)); + #ifdef WOLFSSL_RW_THREADED wc_FreeMutex(&ssl->dtls13Rtx.mutex); #endif @@ -19604,7 +19609,14 @@ static int Dtls13UpdateWindow(WOLFSSL* ssl) /* zero based index */ w64Decrement(&diff64); - /* FIXME: check that diff64 < DTLS_WORDS_BITS */ + /* If the high 32 bits are non-zero, the gap is >= 2^32 which is far + * beyond the replay window; truncating via w64GetLow32 would set the + * wrong bit. Reject such packets as out-of-window. */ + if (w64GetHigh32(diff64) != 0) { + WOLFSSL_MSG("Invalid sequence number to Dtls13UpdateWindow"); + return BAD_STATE_E; + } + diff = w64GetLow32(diff64); wordIndex = ((int)diff) / DTLS_WORD_BITS; wordOffset = ((int)diff) % DTLS_WORD_BITS; @@ -19623,6 +19635,13 @@ static int Dtls13UpdateWindow(WOLFSSL* ssl) /* as we are considering nextSeq inside the window, we should add + 1 */ w64Increment(&diff64); + /* Same truncation hazard as the seq < nextSeq branch above: if the high + * 32 bits are non-zero the gap is >= 2^32, beyond anything the window + * can represent. Reject as out-of-window before truncating. */ + if (w64GetHigh32(diff64) != 0) { + WOLFSSL_MSG("Invalid sequence number to Dtls13UpdateWindow"); + return BAD_STATE_E; + } _DtlsUpdateWindowGTSeq(w64GetLow32(diff64), window); w64Increment(&seq); @@ -27342,7 +27361,7 @@ static const char* wolfSSL_ERR_reason_error_string_OpenSSL(unsigned long e) return "certificate has expired"; case WOLFSSL_X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD: - return "certificate signature failure"; + return "format error in certificate's notBefore field"; case WOLFSSL_X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD: return "format error in certificate's notAfter field"; diff --git a/src/keys.c b/src/keys.c index 74b094fc59..3a980b9ce3 100644 --- a/src/keys.c +++ b/src/keys.c @@ -4138,6 +4138,8 @@ static int MakeSslMasterSecret(WOLFSSL* ssl) ENCRYPT_LEN + WC_SHA_DIGEST_SIZE); wc_MemZero_Add("MakeSslMasterSecret shaInput", shaInput, PREFIX + ENCRYPT_LEN + 2 * RAN_LEN); + wc_MemZero_Add("MakeSslMasterSecret shaOutput", shaOutput, + WC_SHA_DIGEST_SIZE); #endif XMEMSET(shaOutput, 0, WC_SHA_DIGEST_SIZE); @@ -4200,9 +4202,11 @@ static int MakeSslMasterSecret(WOLFSSL* ssl) ForceZero(md5Input, ENCRYPT_LEN + WC_SHA_DIGEST_SIZE); ForceZero(shaInput, PREFIX + ENCRYPT_LEN + 2 * RAN_LEN); + ForceZero(shaOutput, WC_SHA_DIGEST_SIZE); #ifdef WOLFSSL_CHECK_MEM_ZERO wc_MemZero_Check(md5Input, ENCRYPT_LEN + WC_SHA_DIGEST_SIZE); wc_MemZero_Check(shaInput, PREFIX + ENCRYPT_LEN + 2 * RAN_LEN); + wc_MemZero_Check(shaOutput, WC_SHA_DIGEST_SIZE); #endif WC_FREE_VAR_EX(shaOutput, NULL, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/src/sniffer.c b/src/sniffer.c index 8218131435..256e9b24fc 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -7591,11 +7591,15 @@ static int parseKeyLogFile(const char* fileName, char* error) if (ret != 0) { fclose(file); + ForceZero(secret, SECRET_LENGTH); + ForceZero(secretHex, sizeof(secretHex)); return ret; } } fclose(file); + ForceZero(secret, SECRET_LENGTH); + ForceZero(secretHex, sizeof(secretHex)); return 0; } @@ -7613,6 +7617,7 @@ static void freeSecretList(void) while (current != NULL) { next = current->next; + ForceZero(current, sizeof(SecretNode)); XFREE(current, NULL, DYNAMIC_TYPE_SNIFFER_KEYLOG_NODE); current = next; } diff --git a/src/ssl.c b/src/ssl.c index 9b694806e1..8deb85591d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -20130,17 +20130,41 @@ int wolfSSL_RAND_poll(void) { int ret = WOLFSSL_SUCCESS; #ifndef WOLFSSL_NO_OPENSSL_RAND_CB + int customMethodChecked = 0; if (wolfSSL_RAND_InitMutex() == 0 && wc_LockMutex(&gRandMethodMutex) == 0) { - if (gRandMethods && gRandMethods->status) + if (gRandMethods && gRandMethods->status) { ret = gRandMethods->status(); + customMethodChecked = 1; + } wc_UnLockMutex(&gRandMethodMutex); } else { - ret = WOLFSSL_FAILURE; + return WOLFSSL_FAILURE; + } + if (customMethodChecked) + return ret; + #endif + + /* Default path: exercise the global RNG so RNG init / DRBG state + * failures (mutex acquisition, reseed required, corrupted state) + * are surfaced rather than masked by an unconditional success + * return. This does not directly probe the entropy source; DRBG + * output is deterministic between reseeds. */ + #ifdef HAVE_GLOBAL_RNG + if (wolfSSL_RAND_Init() != WOLFSSL_SUCCESS) + return WOLFSSL_FAILURE; + { + byte b = 0; + int genRet; + if (wc_LockMutex(&globalRNGMutex) != 0) + return WOLFSSL_FAILURE; + genRet = wc_RNG_GenerateBlock(&globalRNG, &b, 1); + wc_UnLockMutex(&globalRNGMutex); + ForceZero(&b, 1); + if (genRet != 0) + return WOLFSSL_FAILURE; } - #else - /* wolfCrypt provides enough seed internally, so return success */ #endif return ret; } @@ -20175,12 +20199,96 @@ void wolfSSL_RAND_screen(void) int wolfSSL_RAND_load_file(const char* fname, long len) { +#if !defined(NO_FILESYSTEM) && defined(HAVE_HASHDRBG) + XFILE f; + long maxBytes; + long readSoFar = 0; + int ret = 0; +#ifndef WOLFSSL_SMALL_STACK + unsigned char buf[256]; +#else + unsigned char* buf; +#endif + + WOLFSSL_ENTER("wolfSSL_RAND_load_file"); + + if (fname == NULL) + return WOLFSSL_FATAL_ERROR; + + /* OpenSSL semantics: RAND_load_file(file, -1) reads up to an + * implementation-defined maximum. Match OpenSSL's RAND_LOAD_BUF_SIZE + * (1 MB) so callers passing -1 to ingest a multi-KB seed file aren't + * silently truncated. */ + maxBytes = (len < 0) ? (1L << 20) : len; + if (maxBytes == 0) + return 0; + + f = XFOPEN(fname, "rb"); + if (f == XBADFILE) { + WOLFSSL_MSG("RAND_load_file: cannot open file"); + return WOLFSSL_FATAL_ERROR; + } + +#ifdef WOLFSSL_SMALL_STACK + buf = (unsigned char*)XMALLOC(256, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (buf == NULL) { + XFCLOSE(f); + return WOLFSSL_FATAL_ERROR; + } +#endif +#ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Add("wolfSSL_RAND_load_file buf", buf, 256); +#endif + + if (initGlobalRNG == 0 && wolfSSL_RAND_Init() != WOLFSSL_SUCCESS) { + WOLFSSL_MSG("RAND_load_file: global RNG not available"); + ret = WOLFSSL_FATAL_ERROR; + goto cleanup; + } + + while (readSoFar < maxBytes) { + size_t toRead = (size_t)((maxBytes - readSoFar) < 256 + ? (maxBytes - readSoFar) : 256); + size_t n = XFREAD(buf, 1, toRead, f); + if (n == 0) + break; + if (wc_LockMutex(&globalRNGMutex) != 0) { + ret = WOLFSSL_FATAL_ERROR; + break; + } + if (wc_RNG_DRBG_Reseed(&globalRNG, buf, (word32)n) != 0) { + wc_UnLockMutex(&globalRNGMutex); + WOLFSSL_MSG("RAND_load_file: DRBG reseed failed"); + ret = WOLFSSL_FATAL_ERROR; + break; + } + wc_UnLockMutex(&globalRNGMutex); + readSoFar += (long)n; + } + +cleanup: + XFCLOSE(f); + ForceZero(buf, 256); +#ifdef WOLFSSL_CHECK_MEM_ZERO + /* Balance the wc_MemZero_Add above for both stack/heap paths. */ + wc_MemZero_Check(buf, 256); +#endif +#ifdef WOLFSSL_SMALL_STACK + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif + + if (ret < 0) + return WOLFSSL_FATAL_ERROR; + return (int)readSoFar; +#else + /* Without HAVE_HASHDRBG / filesystem support there is no way to feed + * external entropy to the wolfCrypt RNG; preserve the legacy return + * to avoid breaking callers in those configurations. */ (void)fname; - /* wolfCrypt provides enough entropy internally or will report error */ if (len == -1) return 1024; - else - return (int)len; + return (int)len; +#endif } #endif /* OPENSSL_EXTRA */ diff --git a/src/ssl_crypto.c b/src/ssl_crypto.c index 81f17c8dda..3393f25fef 100644 --- a/src/ssl_crypto.c +++ b/src/ssl_crypto.c @@ -2659,7 +2659,7 @@ void wolfSSL_DES_cbc_encrypt(const unsigned char* input, unsigned char* output, WOLFSSL_ENTER("wolfSSL_DES_cbc_encrypt"); #ifdef WOLFSSL_SMALL_STACK - des = (Des*)XMALLOC(sizeof(Des3), NULL, DYNAMIC_TYPE_CIPHER); + des = (Des*)XMALLOC(sizeof(Des), NULL, DYNAMIC_TYPE_CIPHER); if (des == NULL) { WOLFSSL_MSG("Failed to allocate memory for Des object"); } diff --git a/src/ssl_p7p12.c b/src/ssl_p7p12.c index a07f018b2a..167083554d 100644 --- a/src/ssl_p7p12.c +++ b/src/ssl_p7p12.c @@ -988,7 +988,7 @@ int wolfSSL_PEM_write_bio_PKCS7(WOLFSSL_BIO* bio, PKCS7* p7) outputHead = (byte*)XMALLOC(outputHeadSz, bio->heap, DYNAMIC_TYPE_TMP_BUFFER); if (outputHead == NULL) - return MEMORY_E; + return WOLFSSL_FAILURE; outputFoot = (byte*)XMALLOC(outputFootSz, bio->heap, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/src/tls13.c b/src/tls13.c index 794e3e068d..775e7921ff 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -1038,22 +1038,28 @@ int Tls13_Exporter(WOLFSSL* ssl, unsigned char *out, size_t outLen, protocol, protocolLen, (byte*)label, (word32)labelLen, emptyHash, hashLen, (int)hashType); if (ret != 0) - return ret; + goto cleanup; /* Sanity check contextLen to prevent truncation when cast to word32. */ if (contextLen > WOLFSSL_MAX_32BIT) { - return BAD_FUNC_ARG; + ret = BAD_FUNC_ARG; + goto cleanup; } /* Hash(context_value) */ ret = wc_Hash(hashType, context, (word32)contextLen, hashOut, WC_MAX_DIGEST_SIZE); if (ret != 0) - return ret; + goto cleanup; ret = Tls13HKDFExpandLabel(ssl, out, (word32)outLen, firstExpand, hashLen, protocol, protocolLen, exporterLabel, EXPORTER_LABEL_SZ, hashOut, hashLen, (int)hashType); +cleanup: + /* firstExpand is the per-label Derive-Secret PRK and hashOut holds + * Hash(context_value); wipe both before the stack frame is reclaimed. */ + ForceZero(firstExpand, sizeof(firstExpand)); + ForceZero(hashOut, sizeof(hashOut)); return ret; } #endif @@ -6066,8 +6072,16 @@ static int DoTls13CertificateRequest(WOLFSSL* ssl, const byte* input, len = input[(*inOutIdx)++]; if ((*inOutIdx - begin) + len > size) return BUFFER_ERROR; - if (ssl->options.connectState < FINISHED_DONE && len > 0) - return BUFFER_ERROR; + /* RFC 8446 §4.3.2: certificate_request_context SHALL be zero length in + * the initial CertificateRequest. The wire is well-formed but the value + * is incorrect, so the right alert is illegal_parameter (§6.2). + * INVALID_PARAMETER does not map to illegal_parameter in + * SendFatalAlertOnly's switch, so emit the alert explicitly. */ + if (ssl->options.connectState < FINISHED_DONE && len > 0) { + SendAlert(ssl, alert_fatal, illegal_parameter); + WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); + return INVALID_PARAMETER; + } #ifdef WOLFSSL_POST_HANDSHAKE_AUTH /* CertReqCtx has one byte at end for context value. diff --git a/tests/api/test_certman.c b/tests/api/test_certman.c index 1ac5c6b81c..c227f4bc7d 100644 --- a/tests/api/test_certman.c +++ b/tests/api/test_certman.c @@ -3115,3 +3115,22 @@ int test_wolfSSL_CertManagerRejectMD5Cert(void) #endif return EXPECT_RESULT(); } + +int test_wolfSSL_X509_V_ERR_strings(void) +{ + EXPECT_DECLS; +#if !defined(NO_ERROR_STRINGS) && (defined(OPENSSL_EXTRA) || \ + defined(OPENSSL_EXTRA_X509_SMALL) || \ + defined(HAVE_WEBSERVER) || defined(HAVE_MEMCACHED)) + /* Spot-check the OpenSSL-compat reason strings for X509 verify errors. + * NOT_BEFORE_FIELD previously returned the wrong string (a copy-paste + * of "certificate signature failure"). */ + ExpectStrEQ(wolfSSL_ERR_reason_error_string( + WOLFSSL_X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD), + "format error in certificate's notBefore field"); + ExpectStrEQ(wolfSSL_ERR_reason_error_string( + WOLFSSL_X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD), + "format error in certificate's notAfter field"); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_certman.h b/tests/api/test_certman.h index ca0d83653f..7a9dfa3564 100644 --- a/tests/api/test_certman.h +++ b/tests/api/test_certman.h @@ -46,6 +46,7 @@ int test_wolfSSL_CRL_unknown_critical_entry_ext(void); int test_wolfSSL_CertManagerCheckOCSPResponse(void); int test_various_pathlen_chains(void); int test_wolfSSL_CertManagerRejectMD5Cert(void); +int test_wolfSSL_X509_V_ERR_strings(void); #define TEST_CERTMAN_DECLS \ TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerAPI), \ @@ -69,7 +70,8 @@ int test_wolfSSL_CertManagerRejectMD5Cert(void); TEST_DECL_GROUP("certman", test_wolfSSL_CRL_unknown_critical_entry_ext), \ TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerCheckOCSPResponse), \ TEST_DECL_GROUP("certman", test_various_pathlen_chains), \ - TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerRejectMD5Cert) + TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerRejectMD5Cert), \ + TEST_DECL_GROUP("certman", test_wolfSSL_X509_V_ERR_strings) #endif /* WOLFCRYPT_TEST_CERTMAN_H */ diff --git a/tests/api/test_ossl_rand.c b/tests/api/test_ossl_rand.c index 799e248d4b..caa73b743c 100644 --- a/tests/api/test_ossl_rand.c +++ b/tests/api/test_ossl_rand.c @@ -249,6 +249,39 @@ int test_wolfSSL_RAND_bytes(void) return EXPECT_RESULT(); } +int test_wolfSSL_RAND_load_file(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && \ + defined(HAVE_HASHDRBG) + /* Pre-fix RAND_load_file was a stub that returned len without opening + * the file. Post-fix it opens the file, reads up to len bytes, and + * feeds them into the global DRBG via wc_RNG_DRBG_Reseed. */ + + /* NULL fname must fail. */ + ExpectIntEQ(RAND_load_file(NULL, 32), -1); + + /* A non-existent path must fail (was returning 32 pre-fix). */ + ExpectIntEQ(RAND_load_file("/no/such/file/wolfssl_rand_load_file_test", + 32), -1); + +#if defined(__linux__) || defined(__FreeBSD__) + /* Reading from the OS entropy source returns the requested byte count. */ + ExpectIntEQ(RAND_load_file("/dev/urandom", 32), 32); + + /* len < 0 caps at the implementation default (1 MB, matching OpenSSL's + * RAND_LOAD_BUF_SIZE). */ + ExpectIntEQ(RAND_load_file("/dev/urandom", -1), 1L << 20); + + /* len == 0 short-circuits to 0. */ + ExpectIntEQ(RAND_load_file("/dev/urandom", 0), 0); +#endif + + RAND_cleanup(); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_RAND(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_rand.h b/tests/api/test_ossl_rand.h index f41ae6c3b9..e0ace20eb0 100644 --- a/tests/api/test_ossl_rand.h +++ b/tests/api/test_ossl_rand.h @@ -26,12 +26,14 @@ int test_wolfSSL_RAND_set_rand_method(void); int test_wolfSSL_RAND_bytes(void); +int test_wolfSSL_RAND_load_file(void); int test_wolfSSL_RAND(void); int test_wolfSSL_RAND_poll(void); #define TEST_OSSL_RAND_DECLS \ TEST_DECL_GROUP("ossl_rand", test_wolfSSL_RAND_set_rand_method), \ TEST_DECL_GROUP("ossl_rand", test_wolfSSL_RAND_bytes), \ + TEST_DECL_GROUP("ossl_rand", test_wolfSSL_RAND_load_file), \ TEST_DECL_GROUP("ossl_rand", test_wolfSSL_RAND), \ TEST_DECL_GROUP("ossl_rand", test_wolfSSL_RAND_poll)