Skip to content

Fix global ref lifecycle for verify and missing-CRL callbacks#369

Open
cconlon wants to merge 1 commit intowolfSSL:masterfrom
cconlon:crlVerifyCbRefLifecycle
Open

Fix global ref lifecycle for verify and missing-CRL callbacks#369
cconlon wants to merge 1 commit intowolfSSL:masterfrom
cconlon:crlVerifyCbRefLifecycle

Conversation

@cconlon
Copy link
Copy Markdown
Member

@cconlon cconlon commented May 7, 2026

This PR fixes a JNI callback global reference lifecycle bug where setVerify() / setCRLCb() on one WolfSSLContext or WolfSSLSession could delete a still-live callback reference held by another instance.

  • Moves per-context verify callback jobject from a process-wide static to per-WOLFSSL_CTX ex_data, allocated in Java_com_wolfssl_WolfSSL_init() after wolfSSL_Init().
  • NativeVerifyCallback() now resolves the callback via store -> ssl -> ctx -> ex_data, so each context fires its own user callback.
  • Keeps CTX-level and session-level missing-CRL callback refs as process-globals (wolfSSL CbMissingCRL has no SSL/CTX argument), but stops deleting them in freeContext() / freeSSL(). Only setCRLCb() overwrites and JNI_OnUnload() release them.
  • Adds NativeWolfSSLContextCleanup() / NativeWolfSSLSessionCleanup() helpers called from JNI_OnUnload() for library-level cleanup of globals.
  • Adds test_WolfSSLContext_setVerifyMultipleContexts() exercising the multi-context install / replace / unregister / interleaved-free lifecycle.

Fixes F-1024.

@cconlon cconlon self-assigned this May 7, 2026
Copilot AI review requested due to automatic review settings May 7, 2026 22:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes JNI global-ref lifecycle issues by making the verify callback reference per-WOLFSSL_CTX (via ex_data) and adjusting missing-CRL callback cleanup to only happen at library unload.

Changes:

  • Store per-context verify callback jobject in WOLFSSL_CTX ex_data and resolve it in NativeVerifyCallback().
  • Move missing-CRL callback global-ref cleanup to JNI_OnUnload() via new cleanup helpers.
  • Add a JUnit test covering multi-context setVerify() install/replace/unregister/free flows.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/test/com/wolfssl/test/WolfSSLContextTest.java Adds a multi-context setVerify() lifecycle test intended to surface global-ref mishandling.
native/com_wolfssl_globals.h Declares the ex_data index global and new cleanup helper prototypes.
native/com_wolfssl_WolfSSLSession.c Adds JNI_OnUnload() cleanup for the process-global session missing-CRL callback; stops freeing it in freeSSL().
native/com_wolfssl_WolfSSLContext.c Moves verify callback storage to per-CTX ex_data, adds CTX-level missing-CRL cleanup helper, and adjusts freeContext() cleanup.
native/com_wolfssl_WolfSSL.c Allocates the ex_data index during WolfSSL_init() and calls cleanup helpers in JNI_OnUnload().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread native/com_wolfssl_WolfSSLContext.c Outdated
Comment thread native/com_wolfssl_WolfSSLContext.c
Comment thread src/test/com/wolfssl/test/WolfSSLContextTest.java Outdated
Comment thread src/test/com/wolfssl/test/WolfSSLContextTest.java
Comment thread native/com_wolfssl_WolfSSL.c Outdated
@cconlon cconlon force-pushed the crlVerifyCbRefLifecycle branch from 8e1a905 to b0b710f Compare May 7, 2026 23:53
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Comment thread native/com_wolfssl_WolfSSL.c Outdated
Comment thread native/com_wolfssl_WolfSSLContext.c
Comment thread src/test/com/wolfssl/test/WolfSSLContextTest.java Outdated
Comment thread src/test/com/wolfssl/test/WolfSSLContextTest.java Outdated
Comment thread src/test/com/wolfssl/test/WolfSSLContextTest.java Outdated
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #369

Scan targets checked: wolfssljni-bugs, wolfssljni-src

No new issues found in the changed files. ✅

@cconlon cconlon force-pushed the crlVerifyCbRefLifecycle branch from b0b710f to 3935575 Compare May 8, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants