Merge pull request #10426 from JeremiahM37/fenrir-8

protocol correctness, OpenSSL-compat hardening, and sensitive-memory zeroization

Author: SparkiDev

src/dtls13.c

@@ -2393,7 +2393,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;
 }
@@ -3058,11 +3061,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)

src/internal.c

@@ -9193,6 +9193,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
@@ -19657,15 +19662,21 @@ static int Dtls13UpdateWindow(WOLFSSL* ssl)
         /* zero based index */
         w64Decrement(&diff64);
 
-        /* FIXME: check that diff64 < DTLS_WORDS_BITS */
-        diff = w64GetLow32(diff64);
-        wordIndex = ((int)diff) / DTLS_WORD_BITS;
-        wordOffset = ((int)diff) % DTLS_WORD_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;
+        }
 
-        if (wordIndex >= WOLFSSL_DTLS_WINDOW_WORDS) {
+        diff = w64GetLow32(diff64);
+        if (diff >= (word32)(WOLFSSL_DTLS_WINDOW_WORDS * DTLS_WORD_BITS)) {
             WOLFSSL_MSG("Invalid sequence number to Dtls13UpdateWindow");
             return BAD_STATE_E;
         }
+        wordIndex = (int)(diff / DTLS_WORD_BITS);
+        wordOffset = (int)(diff % DTLS_WORD_BITS);
 
         window[wordIndex] |= (1 << wordOffset);
         return 0;
@@ -19676,6 +19687,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);
@@ -27381,7 +27399,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";

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);

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;
         }

src/ssl.c

@@ -20219,19 +20219,45 @@ int wolfSSL_RAND_poll(void)
     int wolfSSL_RAND_status(void)
     {
         int ret = WOLFSSL_SUCCESS;
+        int useGlobalRng = 1;
     #ifndef WOLFSSL_NO_OPENSSL_RAND_CB
         if (wolfSSL_RAND_InitMutex() == 0 &&
                 wc_LockMutex(&gRandMethodMutex) == 0) {
-            if (gRandMethods && gRandMethods->status)
+            if (gRandMethods && gRandMethods->status) {
                 ret = gRandMethods->status();
+                useGlobalRng = 0;
+            }
             wc_UnLockMutex(&gRandMethodMutex);
         }
         else {
             ret = WOLFSSL_FAILURE;
+            useGlobalRng = 0;
+        }
+    #endif
+
+        /* Drive the global RNG so init / DRBG state failures (mutex
+         * acquisition, reseed required, corrupted state) surface to the
+         * caller. DRBG output is deterministic between reseeds, so this
+         * does not directly probe the entropy source. */
+    #ifdef HAVE_GLOBAL_RNG
+        if (useGlobalRng) {
+            if (wolfSSL_RAND_Init() != WOLFSSL_SUCCESS) {
+                ret = WOLFSSL_FAILURE;
+            }
+            else if (wc_LockMutex(&globalRNGMutex) != 0) {
+                ret = WOLFSSL_FAILURE;
+            }
+            else {
+                byte b = 0;
+                int genRet = wc_RNG_GenerateBlock(&globalRNG, &b, 1);
+                wc_UnLockMutex(&globalRNGMutex);
+                ForceZero(&b, 1);
+                if (genRet != 0)
+                    ret = WOLFSSL_FAILURE;
+            }
         }
-    #else
-        /* wolfCrypt provides enough seed internally, so return success */
     #endif
+        (void)useGlobalRng;
         return ret;
     }
 
@@ -20263,14 +20289,106 @@ void wolfSSL_RAND_screen(void)
 }
 #endif
 
+#ifndef WOLFSSL_RAND_LOAD_FILE_BUF_SZ
+#define WOLFSSL_RAND_LOAD_FILE_BUF_SZ 256
+#endif
+#ifndef WOLFSSL_RAND_LOAD_FILE_MAX_BYTES
+#define WOLFSSL_RAND_LOAD_FILE_MAX_BYTES (1L << 20)
+#endif
+
 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[WOLFSSL_RAND_LOAD_FILE_BUF_SZ];
+#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. WOLFSSL_RAND_LOAD_FILE_MAX_BYTES
+     * caps the read so callers passing -1 to ingest a seed file aren't
+     * silently truncated at a small default. */
+    maxBytes = (len < 0) ? WOLFSSL_RAND_LOAD_FILE_MAX_BYTES : 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(WOLFSSL_RAND_LOAD_FILE_BUF_SZ, 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,
+        WOLFSSL_RAND_LOAD_FILE_BUF_SZ);
+#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) <
+                WOLFSSL_RAND_LOAD_FILE_BUF_SZ
+            ? (maxBytes - readSoFar) : WOLFSSL_RAND_LOAD_FILE_BUF_SZ);
+        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, WOLFSSL_RAND_LOAD_FILE_BUF_SZ);
+#ifdef WOLFSSL_SMALL_STACK
+    XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+#elif defined(WOLFSSL_CHECK_MEM_ZERO)
+    wc_MemZero_Check(buf, WOLFSSL_RAND_LOAD_FILE_BUF_SZ);
+#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; return success so callers
+     * in those configurations are not broken. */
     (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 */

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");
     }

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);

src/tls13.c

@@ -1051,17 +1051,22 @@ 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;
 
     /* 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
@@ -6093,8 +6098,13 @@ 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;
+    /* INVALID_PARAMETER does not map to illegal_parameter in the central
+     * alert path, so emit the alert explicitly before returning. */
+    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
     /* Remember the request context bytes; the CertReqCtx allocation and

tests/api/test_certman.c

@@ -3519,3 +3519,19 @@ 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))
+    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();
+}

tests/api/test_certman.h

@@ -50,6 +50,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),                \
@@ -78,7 +79,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 */