Fenrir fixes for AES-GCM/GMAC and RSA-PSS JNI wrappers - #256
Open
cconlon wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Improves JNI wrapper behavior and docs so operational/native errors throw exceptions while cryptographic mismatches return false, and tightens native input/initialization safety.
Changes:
- Align AES-GCM encrypt JNI validation and initialize GMAC debug-size locals to prevent uninitialized reads in debug builds.
- Update AES-GMAC and RSA-PSS verify semantics to throw on operational errors but return
falsefor authentication/signature mismatches; add tests to pin behavior. - Refine Javadoc contracts for updated verify behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/wolfssl/wolfcrypt/test/RsaTest.java | Adds tests to ensure RSA-PSS operational errors throw and mismatches return false. |
| src/test/java/com/wolfssl/wolfcrypt/test/AesGmacTest.java | Adds tests asserting AesGmac.verify() throws on invalid args. |
| src/main/java/com/wolfssl/wolfcrypt/Rsa.java | Updates verify Javadoc contract to distinguish mismatch vs operational error. |
| src/main/java/com/wolfssl/wolfcrypt/AesGmac.java | Implements “throw on operational error, false on tag mismatch” policy. |
| jni/jni_rsa.c | Adjusts RSA-PSS verify wrapper error handling to throw except BAD_PADDING_E. |
| jni/jni_aesgmac.c | Initializes size locals to avoid uninitialized reads on validation/debug paths. |
| jni/jni_aesgcm.c | Adds missing (inLen != 0 && in == NULL) validation in encrypt wrapper. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes five Fenrir fixes:
F-3767: Add
(inLen != 0 && in == NULL)input validation to the AES-GCM encrypt wrapper, matching the existing check in the decrypt wrapper.F-3768: Make
AesGmac.verify()throwWolfCryptExceptionfor operational errors while still returning false for an authentication tag mismatch, matching its documented contract. Adds tests for the invalid argument cases.F-3995: Apply the same policy to the RSA-PSS
VerifyCheckwrapper. ABAD_PADDING_Esignature mismatch returns false in both salt length branches, all other errors throw, including those detected before the native verify call. Updates thersaPssVerifyWithDigest()javadoc and adds tests pinning both sides of the boundary.F-4435 / F-4436: Initialize the GMAC size locals read by debug logging in
wc_Gmacandwc_GmacVerify, soWOLFCRYPT_JNI_DEBUG_ONbuilds no longer read uninitialized values on the argument validation path.