F-3571: replace existing verify callback context in CertManagerSetVerify

jni/jni_wolfssl_cert_manager.c

@@ -138,6 +138,29 @@ static int addCallbackCtx(WOLFSSL_CERT_MANAGER* cm, VerifyCallbackCtx* ctx)
     return 0;
 }
 
+/* Swap newCtx into the CallbackNode matching cm, in place.
+ *
+ * Caller must hold g_callbackMutex.
+ *
+ * Returns the displaced ctx, or NULL if no node matched. */
+static VerifyCallbackCtx* swapCallbackCtx(WOLFSSL_CERT_MANAGER* cm,
+    VerifyCallbackCtx* newCtx)
+{
+    CallbackNode* node = g_callbackList;
+    VerifyCallbackCtx* oldCtx = NULL;
+
+    while (node != NULL) {
+        if (node->cm == cm) {
+            oldCtx = node->ctx;
+            node->ctx = newCtx;
+            return oldCtx;
+        }
+        node = node->next;
+    }
+
+    return NULL;
+}
+
 /* Remove CallbackNode from global g_callbackList.
  *
  * Caller must hold g_callbackMutex. */
@@ -959,6 +982,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_WolfSSLCertManager_CertManager
     int ret = 0;
     WOLFSSL_CERT_MANAGER* cm = (WOLFSSL_CERT_MANAGER*)(uintptr_t)cmPtr;
     VerifyCallbackCtx* ctx = NULL;
+    VerifyCallbackCtx* oldCtx = NULL;
     JavaVM* jvm = NULL;
     (void)jcl;
 
@@ -987,18 +1011,31 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_WolfSSLCertManager_CertManager
     }
     ctx->jvm = jvm;
 
-    /* Add context to global list */
+    /* Swap into any existing entry for this cm so repeated SetVerify does
+     * not leak the prior context */
     if (wc_LockMutex(&g_callbackMutex) != 0) {
         (*env)->DeleteGlobalRef(env, ctx->callback);
         XFREE(ctx, NULL, DYNAMIC_TYPE_TMP_BUFFER);
         return BAD_MUTEX_E;
     }
 
-    ret = addCallbackCtx(cm, ctx);
+    oldCtx = swapCallbackCtx(cm, ctx);
+    if (oldCtx == NULL) {
+        /* No existing entry, add new node */
+        ret = addCallbackCtx(cm, ctx);
+    }
 
     wc_UnLockMutex(&g_callbackMutex);
 
+    if (oldCtx != NULL) {
+        if (oldCtx->callback != NULL) {
+            (*env)->DeleteGlobalRef(env, oldCtx->callback);
+        }
+        XFREE(oldCtx, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    }
+
     if (ret != 0) {
+        /* Registration failed, no callback was registered */
         (*env)->DeleteGlobalRef(env, ctx->callback);
         XFREE(ctx, NULL, DYNAMIC_TYPE_TMP_BUFFER);
         return ret;

src/test/java/com/wolfssl/wolfcrypt/test/WolfSSLCertManagerVerifyCallbackTest.java

@@ -224,6 +224,95 @@ public class WolfSSLCertManagerVerifyCallbackTest {
         }
     }
 
+    /* Register a first callback on cm then replace with a second.
+     *
+     * Return a WeakReference to the replaced first callback. */
+    private static WeakReference<WolfSSLCertManagerVerifyCallback>
+        registerThenReplaceCallback(WolfSSLCertManager cm,
+            final AtomicBoolean firstInvoked,
+            final AtomicBoolean secondInvoked) throws Exception {
+
+        WolfSSLCertManagerVerifyCallback first =
+            new WolfSSLCertManagerVerifyCallback() {
+                public int verify(int preverify, int error, int errorDepth) {
+                    firstInvoked.set(true);
+                    return 1;
+                }
+            };
+
+        WeakReference<WolfSSLCertManagerVerifyCallback> firstRef =
+            new WeakReference<WolfSSLCertManagerVerifyCallback>(first);
+
+        cm.setVerifyCallback(first);
+
+        cm.setVerifyCallback(new WolfSSLCertManagerVerifyCallback() {
+            public int verify(int preverify, int error, int errorDepth) {
+                secondInvoked.set(true);
+                return 1;
+            }
+        });
+
+        return firstRef;
+    }
+
+    /**
+     * Test that a second setVerifyCallback() replaces the first callback and
+     * releases the native global reference to it.
+     */
+    @Test
+    public void testVerifyCallbackReplaceReleasesPrevious() throws Exception {
+
+        final AtomicBoolean firstInvoked = new AtomicBoolean(false);
+        final AtomicBoolean secondInvoked = new AtomicBoolean(false);
+        WolfSSLCertManager cm = new WolfSSLCertManager();
+        byte[] gcPressure = null;
+        WeakReference<WolfSSLCertManagerVerifyCallback> firstRef = null;
+
+        byte[] caDer = readFile(caCertDer);
+        byte[] serverDer = readFile(serverCertDer);
+
+        try {
+            cm.CertManagerLoadCABuffer(caDer, caDer.length,
+                WolfCrypt.SSL_FILETYPE_ASN1);
+
+            firstRef = registerThenReplaceCallback(cm, firstInvoked,
+                secondInvoked);
+
+            try {
+                cm.CertManagerVerifyBuffer(serverDer, serverDer.length,
+                    WolfCrypt.SSL_FILETYPE_ASN1);
+            } catch (WolfCryptException e) {
+                /* Verification result itself is not what we assert here */
+            }
+
+            assertTrue("second callback should have been invoked",
+                secondInvoked.get());
+            assertFalse("first callback should not have been invoked",
+                firstInvoked.get());
+
+            /* Android ART does not reliably collect within the retry
+             * window even after the native global reference is deleted,
+             * so the collectibility assert runs on non-Android JVMs only.
+             * The dispatch asserts above still run everywhere. */
+            if (!isAndroid()) {
+                /* Retry GC up to ~500ms, small allocations encourage
+                 * collectors that treat System.gc() as advisory */
+                for (int i = 0; i < 50 && firstRef.get() != null; i++) {
+                    gcPressure = new byte[4096];
+                    gcPressure[0] = (byte)i;
+                    System.gc();
+                    Thread.sleep(10);
+                }
+
+                assertNull("replaced callback not collected, native " +
+                    "global reference leaked", firstRef.get());
+            }
+
+        } finally {
+            cm.free();
+        }
+    }
+
     /**
      * Test each CertManager verification invokes its own callback, not
      * the most recently registered one from another CertManager.