Fenrir fixes by cconlon · Pull Request #206 · wolfSSL/wolfcrypt-jni

cconlon · 2026-03-26T16:07:12Z

Summary

This PR fixes 7 JNI/JCE issues identified by Fenrir.

wc_RsaEncryptSize() return assigned to unsigned word32 then checked <= 0 (always false for errors) in RSA PSS sign
and CheckPadding functions (F-1120 / F-1121)
Remove unused wc_RsaPSS_VerifyInline which only performed RSA decryption without PSS padding verification (F-1124)
wc_ecc_import_private_raw() validated curveId but never passed it to wc_ecc_import_private_key(), add test (F-1125)
Zeroize byte array in WolfCryptPBEKey.destroy() (F-1126)
Missing return after throwWolfCryptExceptionFromError() in SHA/MD5 copy NULL checks (F-1122 / F-1123)

…ify (F-1120 / F-1121)

…(F-1124)

…, add test (F-1125)

Copilot

Pull request overview

Fixes multiple Fenrir-reported JNI/JCE correctness and security issues across RSA-PSS, ECC private key import, digest copy JNI bindings, and PBE key destruction semantics.

Changes:

Correct RSA-PSS size/error handling and remove an unsafe/unused “VerifyInline” JNI path.
Ensure ECC private raw import propagates curve ID to native import and add a regression test.
Improve safety by zeroizing encoded key material on destroy and adding missing JNI early-returns after throwing.

Reviewed changes

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

Show a summary per file

File	Description
src/test/java/com/wolfssl/wolfcrypt/test/EccTest.java	Adds a regression test to verify curve ID is set on raw private key import.
src/main/java/com/wolfssl/wolfcrypt/Rsa.java	Removes the unused native `wc_RsaPSS_VerifyInline` declaration.
src/main/java/com/wolfssl/provider/jce/WolfCryptPBEKey.java	Extends `destroy()` to zeroize the cached encoded key bytes and updates Javadoc.
jni/jni_sha.c	Adds missing `return` after throwing on NULL copy parameters in multiple SHA copy methods.
jni/jni_md5.c	Adds missing `return` after throwing on NULL copy parameters for MD5.
jni/jni_rsa.c	Fixes RSA-PSS encrypt-size error handling and removes `VerifyInline` JNI implementation.
jni/jni_ecc.c	Passes `curveId` into the native ECC private key import function.
jni/include/com_wolfssl_wolfcrypt_Rsa.h	Removes the `wc_RsaPSS_VerifyInline` JNI declaration.

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

Copilot · 2026-03-26T16:14:03Z

jni/jni_rsa.c

+        encSz = wc_RsaEncryptSize(key);
+        if (encSz < 0) {
+            ret = encSz;
+        }
+        else {
+            signatureSz = (word32)encSz;
        }


wc_RsaEncryptSize() returning 0 is currently treated as success, which can propagate a zero-length signatureSz/pssDataSz and lead to invalid allocations/behavior downstream. If 0 is not a valid RSA modulus size (typical), change the check to treat encSz <= 0 as an error (mirroring the prior intent while still avoiding the unsigned-cast bug).

Copilot · 2026-03-26T16:14:04Z

jni/jni_rsa.c

+        encSz = wc_RsaEncryptSize(key);
+        if (encSz < 0) {
+            ret = encSz;
+        }
+        else {
+            pssDataSz = (word32)encSz;
        }


wc_RsaEncryptSize() returning 0 is currently treated as success, which can propagate a zero-length signatureSz/pssDataSz and lead to invalid allocations/behavior downstream. If 0 is not a valid RSA modulus size (typical), change the check to treat encSz <= 0 as an error (mirroring the prior intent while still avoiding the unsigned-cast bug).

Copilot · 2026-03-26T16:14:04Z

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

+        key.importPrivateRaw(prvKey, "secp256r1");
+
+        /* verify curve was set correctly after import */
+        assertEquals(ECC_SECP256R1, key.getCurveId());
+
+        /* try invalid curve name, expect failure */
+        try {
+            Ecc key2 = new Ecc();
+            key2.importPrivateRaw(prvKey, "BADCURVE");
+            fail("importPrivateRaw with invalid curve should fail");
+        } catch (WolfCryptException e) {
+            /* expected */
+        }
+
+        key.releaseNativeStruct();


releaseNativeStruct() is only called at the end of the test, so if an assertion or the invalid-curve block fails/throws unexpectedly, the native struct may leak and destabilize the JVM/test suite. Wrap the test body in try { ... } finally { key.releaseNativeStruct(); } (and similarly ensure any secondary Ecc instances are released in a finally when created).

Suggested change

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(ECC_SECP256R1, key.getCurveId());

/* try invalid curve name, expect failure */

try {

Ecc key2 = new Ecc();

key2.importPrivateRaw(prvKey, "BADCURVE");

fail("importPrivateRaw with invalid curve should fail");

} catch (WolfCryptException e) {

/* expected */

}

key.releaseNativeStruct();

try {

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(ECC_SECP256R1, key.getCurveId());

/* try invalid curve name, expect failure */

Ecc key2 = null;

try {

key2 = new Ecc();

key2.importPrivateRaw(prvKey, "BADCURVE");

fail("importPrivateRaw with invalid curve should fail");

} catch (WolfCryptException e) {

/* expected */

} finally {

if (key2 != null) {

key2.releaseNativeStruct();

}

}

} finally {

key.releaseNativeStruct();

}

Copilot · 2026-03-26T16:14:05Z

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

+        key.importPrivateRaw(prvKey, "secp256r1");
+
+        /* verify curve was set correctly after import */
+        assertEquals(ECC_SECP256R1, key.getCurveId());
+
+        /* try invalid curve name, expect failure */
+        try {
+            Ecc key2 = new Ecc();
+            key2.importPrivateRaw(prvKey, "BADCURVE");
+            fail("importPrivateRaw with invalid curve should fail");
+        } catch (WolfCryptException e) {
+            /* expected */
+        }
+
+        key.releaseNativeStruct();


releaseNativeStruct() is only called at the end of the test, so if an assertion or the invalid-curve block fails/throws unexpectedly, the native struct may leak and destabilize the JVM/test suite. Wrap the test body in try { ... } finally { key.releaseNativeStruct(); } (and similarly ensure any secondary Ecc instances are released in a finally when created).

Suggested change

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(ECC_SECP256R1, key.getCurveId());

/* try invalid curve name, expect failure */

try {

Ecc key2 = new Ecc();

key2.importPrivateRaw(prvKey, "BADCURVE");

fail("importPrivateRaw with invalid curve should fail");

} catch (WolfCryptException e) {

/* expected */

}

key.releaseNativeStruct();

try {

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(ECC_SECP256R1, key.getCurveId());

/* try invalid curve name, expect failure */

Ecc key2 = null;

try {

key2 = new Ecc();

key2.importPrivateRaw(prvKey, "BADCURVE");

fail("importPrivateRaw with invalid curve should fail");

} catch (WolfCryptException e) {

/* expected */

} finally {

if (key2 != null) {

key2.releaseNativeStruct();

}

}

} finally {

key.releaseNativeStruct();

}

Copilot · 2026-03-26T16:14:05Z

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

+        /* ECC_SECP256R1 curve id from wolfSSL ecc.h */
+        int ECC_SECP256R1 = 7;
+
+        Ecc key = new Ecc();
+        key.importPrivateRaw(prvKey, "secp256r1");
+
+        /* verify curve was set correctly after import */
+        assertEquals(ECC_SECP256R1, key.getCurveId());


Hardcoding ECC_SECP256R1 = 7 makes the test fragile if the underlying native enum/IDs change. Prefer referencing a Java-side constant (if available), or derive the expected value via an existing library mapping/helper that translates curve name to ID so the test verifies behavior without duplicating native header values.

Suggested change

/* ECC_SECP256R1 curve id from wolfSSL ecc.h */

int ECC_SECP256R1 = 7;

Ecc key = new Ecc();

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(ECC_SECP256R1, key.getCurveId());

Ecc key = new Ecc();

int initialCurveId = key.getCurveId();

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertNotEquals(initialCurveId, key.getCurveId());

Copilot · 2026-03-26T16:14:06Z

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

+        /* ECC_SECP256R1 curve id from wolfSSL ecc.h */
+        int ECC_SECP256R1 = 7;
+
+        Ecc key = new Ecc();
+        key.importPrivateRaw(prvKey, "secp256r1");
+
+        /* verify curve was set correctly after import */
+        assertEquals(ECC_SECP256R1, key.getCurveId());


Hardcoding ECC_SECP256R1 = 7 makes the test fragile if the underlying native enum/IDs change. Prefer referencing a Java-side constant (if available), or derive the expected value via an existing library mapping/helper that translates curve name to ID so the test verifies behavior without duplicating native header values.

Suggested change

/* ECC_SECP256R1 curve id from wolfSSL ecc.h */

int ECC_SECP256R1 = 7;

Ecc key = new Ecc();

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(ECC_SECP256R1, key.getCurveId());

/* Expected curve id for secp256r1 from wolfSSL Ecc Java API */

int expectedCurveId = Ecc.ECC_SECP256R1;

Ecc key = new Ecc();

key.importPrivateRaw(prvKey, "secp256r1");

/* verify curve was set correctly after import */

assertEquals(expectedCurveId, key.getCurveId());

cconlon added 6 commits March 23, 2026 14:23

JNI: fix unsigned truncation of wc_RsaEncryptSize in RSA PSS sign/ver…

db887a0

…ify (F-1120 / F-1121)

JNI: remove unused wc_RsaPSS_VerifyInline that skipped padding check …

bd4aef9

…(F-1124)

JNI: pass curveId to wc_ecc_import_private_key_ex in importPrivateRaw…

1593948

…, add test (F-1125)

JCE: zeroize encoded key array in WolfCryptPBEKey.destroy() (F-1126)

f890401

JNI: add missing return after throw in SHA copy NULL checks (F-1122)

da0f7c9

JNI: add missing return after throw in MD5 copy NULL check (F-1123)

d2b5d6f

cconlon self-assigned this Mar 26, 2026

Copilot AI review requested due to automatic review settings March 26, 2026 16:07

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Copilot started reviewing on behalf of cconlon March 26, 2026 16:19 View session

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fenrir fixes#206

Fenrir fixes#206
cconlon wants to merge 6 commits intowolfSSL:masterfrom
cconlon:fenrir323

cconlon commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cconlon commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants