Fenrir fixes#236
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a set of Fenrir-tracked hardening and correctness issues across the wolfcrypt JNI bindings, primarily by tightening size/NULL validation in native wrappers, fixing an authTag buffer handling issue in AES-GCM/CCM encrypt wrappers, and adding/expanding regression tests. It also includes small build/packaging improvements (Ant classpath pinning, Gradle wrapper script updates) and a minor ML-KEM compile-time gating cleanup.
Changes:
- Add/extend JNI parameter validation (buffer sizes, optional NULL args) and adjust RSA-PSS verify to honor caller-provided
saltLen. - Fix AES-GCM/CCM encrypt wrapper handling of
authTagarray release/commit to avoid leaking a native buffer/copy. - Add new JUnit4 regression tests and wire them into the test suite; tighten Ant classpath and update Android Gradle wrapper scripts.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/wolfssl/wolfcrypt/test/WolfSSLCertManagerTest.java | New regression tests for CertManager CA loading behavior and size handling. |
| src/test/java/com/wolfssl/wolfcrypt/test/WolfCryptTestSuite.java | Registers the new CertManager test class in the suite. |
| src/test/java/com/wolfssl/wolfcrypt/test/RsaTest.java | Adds regression tests for RSA raw key size validation and RSA-PSS verify saltLen handling. |
| src/test/java/com/wolfssl/wolfcrypt/test/AsnTest.java | Adds regression tests ensuring encodeSignature rejects invalid sizes (byte[] and ByteBuffer). |
| jni/jni_wolfssl_cert_manager.c | Makes file/dir args optional and honors caller-provided buffer sizes for CertManager APIs. |
| jni/jni_rsa.c | Adds size validation for raw public key decode/export and updates RSA-PSS verify path for non-default salt lengths. |
| jni/jni_mlkem.c | Gates ML-KEM JNI compilation on WOLFSSL_HAVE_MLKEM only. |
| jni/jni_asn.c | Adds hash/output buffer size validation in encodeSignature JNI wrappers. |
| jni/jni_aesgcm.c | Fixes authTag array release mode in AES-GCM encrypt wrapper to avoid buffer leak. |
| jni/jni_aesccm.c | Fixes authTag array release mode in AES-CCM encrypt wrapper to avoid buffer leak. |
| IDE/Android/gradlew.bat | Updates Windows Gradle wrapper script. |
| IDE/Android/gradlew | Updates POSIX Gradle wrapper script. |
| build.xml | Pins Ant test/provider classpath to wolfcrypt-jni.jar directly (no lib/*.jar glob). |
Comments suppressed due to low confidence (4)
jni/jni_wolfssl_cert_manager.c:429
GetByteArrayElementscan return NULL (e.g., OOM), and the current code will still call into wolfSSL and thenReleaseByteArrayElementswith a NULL pointer. That risks a native crash and can also mask the pending Java exception. Add a NULL check and returnMEMORY_E(matching other OCSP paths in this file) before using or releasingbuff.
buff = (byte*)(*env)->GetByteArrayElements(env, in, NULL);
buffSz = (word32)sz;
ret = wolfSSL_CertManagerLoadCABuffer(cm, buff, buffSz, format);
(*env)->ReleaseByteArrayElements(env, in, (jbyte*)buff, JNI_ABORT);
jni/jni_wolfssl_cert_manager.c:453
GetByteArrayElementsmay return NULL (e.g., OOM). The code currently passesbuffdirectly towolfSSL_CertManagerLoadCABuffer_exand then callsReleaseByteArrayElementsunconditionally, which can crash ifbuffis NULL. Add a NULL check and returnMEMORY_Ebefore using or releasingbuff.
buff = (byte*)(*env)->GetByteArrayElements(env, in, NULL);
buffSz = (word32)sz;
ret = wolfSSL_CertManagerLoadCABuffer_ex(cm, buff, buffSz, format, 0,
(word32)flags);
(*env)->ReleaseByteArrayElements(env, in, (jbyte*)buff, JNI_ABORT);
jni/jni_wolfssl_cert_manager.c:494
- If
GetByteArrayElementsreturns NULL, this path will callwolfSSL_CertManagerVerifyBufferand thenReleaseByteArrayElementswith a NULL pointer, risking a crash. Add a NULL check and returnMEMORY_Ebefore using or releasingbuff.
buff = (byte*)(*env)->GetByteArrayElements(env, in, NULL);
buffSz = (word32)sz;
ret = wolfSSL_CertManagerVerifyBuffer(cm, buff, buffSz, format);
(*env)->ReleaseByteArrayElements(env, in, (jbyte*)buff, JNI_ABORT);
jni/jni_wolfssl_cert_manager.c:562
GetByteArrayElementscan return NULL (e.g., OOM), but this code unconditionally usesbuffand then releases it. That can crash ifbuffis NULL. Add a NULL check and returnMEMORY_Ebefore using or releasingbuff.
buff = (byte*)(*env)->GetByteArrayElements(env, in, NULL);
buffSz = (word32)sz;
ret = wolfSSL_CertManagerLoadCRLBuffer(cm, buff, buffSz, type);
(*env)->ReleaseByteArrayElements(env, in, (jbyte*)buff, JNI_ABORT);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s in Rsa JNI wrappers
This PR includes fixes for 11 Fenrir items:
saltLeninRsa.wc_RsaPSS_VerifyCheck().authTagbuffer leak in the encrypt wrappers.CertManagerLoadCA()and honor caller-provided buffer sizes in theWolfSSLCertManagerwrappers.Rsawrappers.WOLFSSL_HAVE_MLKEMonly, removing the unusedHAVE_MLKEMmacro.Asn.encodeSignature()wrappers.wolfcrypt-jni.jarinstead of alib/*.jarglob.