Add refcounts on key, keyMask, altKey, altKeyMask to prevent UAF

Author: padelsbach

src/internal.c

@@ -7194,6 +7194,9 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
     }
 #else
     ssl->buffers.key      = ctx->privateKey;
+    /* Hold a reference on the shared key so reloading it on the CTX cannot
+     * free it while this SSL still aliases it. */
+    RefDer(ssl->buffers.key);
 #endif
 #else
     if (ctx->privateKey != NULL) {
@@ -7224,6 +7227,8 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
 #ifdef WOLFSSL_DUAL_ALG_CERTS
 #ifndef WOLFSSL_BLIND_PRIVATE_KEY
     ssl->buffers.altKey   = ctx->altPrivateKey;
+    /* Hold a reference on the shared alternative key. */
+    RefDer(ssl->buffers.altKey);
 #else
     if (ctx->altPrivateKey != NULL) {
         ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer,
@@ -9211,12 +9216,25 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
 #ifndef NO_CERTS
     ssl->keepCert = 0; /* make sure certificate is free'd */
     wolfSSL_UnloadCertsKeys(ssl);
-    /* Release references on cert buffers aliased from the context. Owned
-     * buffers were already freed and cleared by wolfSSL_UnloadCertsKeys. */
+    /* Release references on cert and key buffers aliased from the context.
+     * Owned buffers were already freed and cleared by
+     * wolfSSL_UnloadCertsKeys. */
     FreeDer(&ssl->buffers.certificate);
     ssl->buffers.weOwnCert = 0;
     FreeDer(&ssl->buffers.certChain);
     ssl->buffers.weOwnCertChain = 0;
+    FreeDer(&ssl->buffers.key);
+    ssl->buffers.weOwnKey = 0;
+#ifdef WOLFSSL_BLIND_PRIVATE_KEY
+    FreeDer(&ssl->buffers.keyMask);
+#endif
+#ifdef WOLFSSL_DUAL_ALG_CERTS
+    FreeDer(&ssl->buffers.altKey);
+    ssl->buffers.weOwnAltKey = 0;
+#ifdef WOLFSSL_BLIND_PRIVATE_KEY
+    FreeDer(&ssl->buffers.altKeyMask);
+#endif
+#endif /* WOLFSSL_DUAL_ALG_CERTS */
 #endif
 #ifndef NO_RSA
     FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey);

src/ssl.c

@@ -12711,33 +12711,23 @@ void wolfSSL_certs_clear(WOLFSSL* ssl)
 #ifdef WOLFSSL_TLS13
     ssl->buffers.certChainCnt = 0;
 #endif
-    if (ssl->buffers.weOwnKey) {
-        FreeDer(&ssl->buffers.key);
-    #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-        FreeDer(&ssl->buffers.keyMask);
-    #endif
-        ssl->buffers.weOwnKey = 0;
-    }
-    ssl->buffers.key      = NULL;
+    /* Release our reference to the key (owned copy or CTX alias). */
+    FreeDer(&ssl->buffers.key);
+    ssl->buffers.weOwnKey = 0;
 #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-    ssl->buffers.keyMask  = NULL;
+    FreeDer(&ssl->buffers.keyMask);
 #endif
     ssl->buffers.keyType  = 0;
     ssl->buffers.keyId    = 0;
     ssl->buffers.keyLabel = 0;
     ssl->buffers.keySz    = 0;
     ssl->buffers.keyDevId = 0;
 #ifdef WOLFSSL_DUAL_ALG_CERTS
-    if (ssl->buffers.weOwnAltKey) {
-        FreeDer(&ssl->buffers.altKey);
-    #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-        FreeDer(&ssl->buffers.altKeyMask);
-    #endif
-        ssl->buffers.weOwnAltKey = 0;
-    }
-    ssl->buffers.altKey     = NULL;
+    /* Release our reference to the alternative key (owned copy or CTX alias). */
+    FreeDer(&ssl->buffers.altKey);
+    ssl->buffers.weOwnAltKey = 0;
 #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-    ssl->buffers.altKeyMask = NULL;
+    FreeDer(&ssl->buffers.altKeyMask);
 #endif
 #endif /* WOLFSSL_DUAL_ALG_CERTS */
 }
@@ -13201,7 +13191,11 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
         ssl->buffers.key      = ctx->privateKey;
     }
 #else
+    /* Release our previous key reference before aliasing the new CTX's. */
+    FreeDer(&ssl->buffers.key);
+    ssl->buffers.weOwnKey = 0;
     ssl->buffers.key      = ctx->privateKey;
+    RefDer(ssl->buffers.key);
 #endif
 #else
     if (ctx->privateKey != NULL) {
@@ -13238,7 +13232,11 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
     ssl->options.haveMlDsaSig = ctx->haveMlDsaSig;
 #ifdef WOLFSSL_DUAL_ALG_CERTS
 #ifndef WOLFSSL_BLIND_PRIVATE_KEY
+    /* Release our previous alt key reference before aliasing the new CTX's. */
+    FreeDer(&ssl->buffers.altKey);
+    ssl->buffers.weOwnAltKey = 0;
     ssl->buffers.altKey   = ctx->altPrivateKey;
+    RefDer(ssl->buffers.altKey);
 #else
     if (ctx->altPrivateKey != NULL) {
         ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer,

src/ssl_load.c

@@ -1317,13 +1317,12 @@ static int ProcessBufferPrivKeyHandleDer(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
     else
 #endif /* WOLFSSL_DUAL_ALG_CERTS */
     if (ssl != NULL) {
-        /* Dispose of previous key if not context's. */
-        if (ssl->buffers.weOwnKey) {
-            FreeDer(&ssl->buffers.key);
-        #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-            FreeDer(&ssl->buffers.keyMask);
-        #endif
-        }
+        /* Release our reference to the previous key (owned copy or CTX
+         * alias) before storing the new one. */
+        FreeDer(&ssl->buffers.key);
+    #ifdef WOLFSSL_BLIND_PRIVATE_KEY
+        FreeDer(&ssl->buffers.keyMask);
+    #endif
         ssl->buffers.keyId = 0;
         ssl->buffers.keyLabel = 0;
         ssl->buffers.keyDevId = INVALID_DEVID;
@@ -4621,12 +4620,11 @@ int wolfSSL_use_PrivateKey_Id(WOLFSSL* ssl, const unsigned char* id,
     }
 
     /* Dispose of old private key if owned and allocate and copy in id. */
-    if (ssl->buffers.weOwnKey) {
-        FreeDer(&ssl->buffers.key);
-    #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-        FreeDer(&ssl->buffers.keyMask);
-    #endif
-    }
+    /* Release our reference to the old key (owned copy or CTX alias). */
+    FreeDer(&ssl->buffers.key);
+#ifdef WOLFSSL_BLIND_PRIVATE_KEY
+    FreeDer(&ssl->buffers.keyMask);
+#endif
     if (AllocCopyDer(&ssl->buffers.key, id, (word32)sz, PRIVATEKEY_TYPE,
             ssl->heap) != 0) {
         ret = 0;
@@ -4697,12 +4695,11 @@ int wolfSSL_use_PrivateKey_Label(WOLFSSL* ssl, const char* label, int devId)
     sz = (word32)XSTRLEN(label) + 1;
 
     /* Dispose of old private key if owned and allocate and copy in label. */
-    if (ssl->buffers.weOwnKey) {
-        FreeDer(&ssl->buffers.key);
-    #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-        FreeDer(&ssl->buffers.keyMask);
-    #endif
-    }
+    /* Release our reference to the old key (owned copy or CTX alias). */
+    FreeDer(&ssl->buffers.key);
+#ifdef WOLFSSL_BLIND_PRIVATE_KEY
+    FreeDer(&ssl->buffers.keyMask);
+#endif
     if (AllocCopyDer(&ssl->buffers.key, (const byte*)label, (word32)sz,
             PRIVATEKEY_TYPE, ssl->heap) != 0) {
         ret = 0;

src/tls13.c

@@ -10142,20 +10142,22 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl)
                     }
                     ssl->buffers.keyType = ssl->buffers.altKeyType;
                     ssl->buffers.keySz = ssl->buffers.altKeySz;
-                    /* If we own it, free key before overriding it. */
-                    if (ssl->buffers.weOwnKey) {
-                        FreeDer(&ssl->buffers.key);
-                    #ifdef WOLFSSL_BLIND_PRIVATE_KEY
-                        FreeDer(&ssl->buffers.keyMask);
-                    #endif
-                    }
+                    /* Release our reference to the current key before
+                     * overriding it (owned copy or CTX alias). */
+                    FreeDer(&ssl->buffers.key);
+                #ifdef WOLFSSL_BLIND_PRIVATE_KEY
+                    FreeDer(&ssl->buffers.keyMask);
+                #endif
 
-                    /* Swap keys */
+                    /* Swap keys. key and altKey now reference the same buffer,
+                     * so take a reference for the new alias. */
                     ssl->buffers.key     = ssl->buffers.altKey;
                     ssl->buffers.weOwnKey = ssl->buffers.weOwnAltKey;
+                    RefDer(ssl->buffers.key);
 
                 #ifdef WOLFSSL_BLIND_PRIVATE_KEY
                     ssl->buffers.keyMask = ssl->buffers.altKeyMask;
+                    RefDer(ssl->buffers.keyMask);
                     /* Unblind the alternative key before decoding */
                     wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
                 #endif

tests/api/test_tls13.c

@@ -3687,6 +3687,69 @@ int test_tls13_cert_alias_uaf_sni(void)
 }
 
 
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
+    defined(HAVE_SNI) && !defined(WOLFSSL_COPY_KEY) && \
+    !defined(WOLFSSL_BLIND_PRIVATE_KEY) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \
+    !defined(NO_RSA) && !defined(NO_CERTS)
+/* React to the client SNI by reloading the private key on the existing CTX.
+ * Without WOLFSSL_COPY_KEY the key buffer is shared between the CTX and SSL,
+ * so freeing it on the CTX would dangle the SSL's alias. The server signs its
+ * CertificateVerify with that key, so a dangling alias is a heap-use-after-free
+ * on the signing path; the handshake must still complete. */
+static int test_key_alias_sni_cb(WOLFSSL* ssl, int* ad, void* arg)
+{
+    WOLFSSL_CTX* ctx = wolfSSL_get_SSL_CTX(ssl);
+    (void)ad;
+    (void)arg;
+    (void)wolfSSL_CTX_use_PrivateKey_file(ctx, svrKeyFile,
+        WOLFSSL_FILETYPE_PEM);
+    /* 0 means ack the servername and continue the handshake. */
+    return 0;
+}
+#endif
+
+int test_tls13_key_alias_uaf_sni(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
+    defined(HAVE_SNI) && !defined(WOLFSSL_COPY_KEY) && \
+    !defined(WOLFSSL_BLIND_PRIVATE_KEY) && \
+    !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \
+    !defined(NO_RSA) && !defined(NO_CERTS)
+    WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
+    WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
+    struct test_memio_ctx test_ctx;
+    const char* host = "example.com";
+
+    XMEMSET(&test_ctx, 0, sizeof(test_ctx));
+    /* Server cert/key are loaded on ctx_s, so ssl_s->buffers.key aliases
+     * ctx_s->privateKey. */
+    ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
+        wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);
+
+    /* Server swaps its CTX private key when the SNI arrives. */
+    wolfSSL_CTX_set_servername_callback(ctx_s, test_key_alias_sni_cb);
+
+    /* Client offers SNI so the server callback fires while parsing the
+     * ClientHello. */
+    ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, host,
+        (word16)XSTRLEN(host)), WOLFSSL_SUCCESS);
+
+    /* The callback frees the aliased key mid-handshake; the server then signs
+     * CertificateVerify with ssl->buffers.key. The handshake must complete
+     * without a UAF. */
+    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);
+#endif
+    return EXPECT_RESULT();
+}
+
+
 #if defined(HAVE_IO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
     defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \
     !defined(WOLFSSL_MLKEM_NO_DECAPSULATE) && \

tests/api/test_tls13.h

@@ -31,6 +31,7 @@ int test_tls13_rpk_handshake(void);
 int test_tls13_rpk_handshake_no_negotiation(void);
 int test_tls13_pha(void);
 int test_tls13_cert_alias_uaf_sni(void);
+int test_tls13_key_alias_uaf_sni(void);
 int test_tls13_pq_groups(void);
 int test_tls13_multi_pqc_key_share(void);
 int test_tls13_early_data(void);
@@ -109,6 +110,7 @@ int test_tls13_AEAD_limit_KU_aes128_ccm_8_sha256(void);
     TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake_no_negotiation), \
     TEST_DECL_GROUP("tls13", test_tls13_pha),                   \
     TEST_DECL_GROUP("tls13", test_tls13_cert_alias_uaf_sni),    \
+    TEST_DECL_GROUP("tls13", test_tls13_key_alias_uaf_sni),     \
     TEST_DECL_GROUP("tls13", test_tls13_pq_groups),             \
     TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share),   \
     TEST_DECL_GROUP("tls13", test_tls13_early_data),            \