Wycheproof: streaming AES-GCM, Cipher security fixes, ECDH/DH validation, RSA CRT hardening#224
Open
MarkAtwood wants to merge 5 commits into
Open
Wycheproof: streaming AES-GCM, Cipher security fixes, ECDH/DH validation, RSA CRT hardening#224MarkAtwood wants to merge 5 commits into
MarkAtwood wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens wolfJCE behavior against Wycheproof findings by adding AES-GCM streaming encryption support, tightening key validation, improving JCE exception behavior, and adding OAEP parameter support.
Changes:
- Adds AES-GCM streaming JNI/Java APIs and integrates them into
WolfCryptCipher. - Strengthens DH/ECDH and RSA private key validation paths.
- Adds OAEP
AlgorithmParameterssupport and improves exception mapping for padding/key-spec failures.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/wolfssl/wolfcrypt/Dh.java |
Adds DH public key validation wrapper. |
src/main/java/com/wolfssl/wolfcrypt/AesGcm.java |
Adds streaming AES-GCM state and Java wrappers. |
src/main/java/com/wolfssl/provider/jce/WolfCryptSignature.java |
Adds RSA CRT modulus consistency validation and zeroing. |
src/main/java/com/wolfssl/provider/jce/WolfCryptProvider.java |
Registers OAEP AlgorithmParameters. |
src/main/java/com/wolfssl/provider/jce/WolfCryptOaepParameters.java |
Adds OAEP AlgorithmParametersSpi implementation. |
src/main/java/com/wolfssl/provider/jce/WolfCryptKeyAgreement.java |
Adds DH parameter/public-key checks and ECDH validation improvements. |
src/main/java/com/wolfssl/provider/jce/WolfCryptECKeyFactory.java |
Wraps unsupported EC parameter errors as InvalidKeySpecException. |
src/main/java/com/wolfssl/provider/jce/WolfCryptCipher.java |
Integrates streaming GCM, IV-reuse prevention, OAEP parameter return, and padding exception fixes. |
jni/jni_aesgcm.c |
Adds native AES-GCM streaming init/update/final wrappers. |
jni/include/com_wolfssl_wolfcrypt_AesGcm.h |
Declares new AES-GCM streaming JNI functions. |
Files not reviewed (1)
- jni/include/com_wolfssl_wolfcrypt_AesGcm.h: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+302
to
+307
| /* Allowed from IDLE or STREAMING (re-init resets a prior stream) */ | ||
| streamingState = StreamingState.STREAMING; | ||
|
|
||
| synchronized (pointerLock) { | ||
| wc_AesGcmEncryptInitStreaming(iv); | ||
| } |
Comment on lines
+183
to
+191
| /** | ||
| * Set private key | ||
| * | ||
| * @param priv private key array | ||
| * | ||
| * @throws IllegalStateException if object fails to initialize, or if | ||
| * releaseNativeStruct() has been called and object has been | ||
| * released. | ||
| */ |
Comment on lines
+52
to
+58
| OAEPParameterSpec spec = (OAEPParameterSpec) paramSpec; | ||
|
|
||
| if (!"MGF1".equals(spec.getMGFAlgorithm())) { | ||
| throw new InvalidParameterSpecException( | ||
| "Only MGF1 supported for OAEP, got: " + | ||
| spec.getMGFAlgorithm()); | ||
| } |
Comment on lines
+120
to
+126
| protected byte[] engineGetEncoded() throws IOException { | ||
| throw new IOException("Encoded OAEP parameters not supported"); | ||
| } | ||
|
|
||
| @Override | ||
| protected byte[] engineGetEncoded(String format) throws IOException { | ||
| throw new IOException("Encoded OAEP parameters not supported"); |
Comment on lines
+454
to
+456
| if (ivArr != NULL) { | ||
| (*env)->ReleaseByteArrayElements(env, ivArr, (jbyte*)iv, JNI_ABORT); | ||
| } |
Comment on lines
+542
to
+547
| if (inputArr != NULL) { | ||
| (*env)->ReleaseByteArrayElements(env, inputArr, (jbyte*)in, JNI_ABORT); | ||
| } | ||
| if (authInArr != NULL) { | ||
| (*env)->ReleaseByteArrayElements(env, authInArr, (jbyte*)authIn, | ||
| JNI_ABORT); |
Add JNI wrappers for wc_AesGcmEncryptInit/Update/Final and expose them through AesGcm.java. Includes: - Three new JNI functions in jni_aesgcm.c with null-pointer guard on GetByteArrayElements to handle OOM gracefully - Corresponding JNI declarations in com_wolfssl_wolfcrypt_AesGcm.h - Java wrappers with IDLE/STREAMING state machine enforcing that encryptUpdateStreaming and encryptFinalStreaming cannot be called before encryptInitStreaming, and that state resets to IDLE after encryptFinalStreaming even if the native call throws
WolfCryptCipher.java — four independent improvements: 1. Streaming AES-GCM encrypt path: when WOLFSSL_AESGCM_STREAM is compiled in (FeatureDetect.AesGcmStreamEnabled()), encrypting with AES/GCM/NoPadding uses the streaming API so plaintext is never fully buffered. Enforces the NIST SP 800-38D plaintext limit (2^32-2 blocks) via gcmBytesEncrypted counter. AAD is passed on the first update call, or in doFinal if no update was called, always before any plaintext. 2. IV-reuse prevention: adds 'finalized' flag set in the finally block of engineDoFinal. engineUpdate (both overloads), engineWrap, and engineUnwrap all throw IllegalStateException if called after doFinal without re-initializing, closing the window where an attacker could reuse a GCM IV by calling wrap after doFinal on the same cipher instance. 3. BadPaddingException on PKCS5 unpadding failure: AES-CBC and 3DES decrypt no longer propagates WolfCryptException from unPadPKCS7; it is caught and rethrown as BadPaddingException so callers receive the standard JCE exception type. 4. OAEP AlgorithmParameters: stores the active OAEPParameterSpec and returns it from engineGetParameters(), allowing callers to retrieve the hash and MGF parameters used during init.
Three related hardening changes:
1. DH public key validation (WolfCryptKeyAgreement, Dh.java):
- Store dhParamP/dhParamG from the private key at init time
- In engineDoPhase compare peer's {p,g} against stored values;
throw InvalidKeyException on mismatch (prevents cross-group
attacks)
- Call new Dh.checkPublicKey() wrapping wc_DhCheckPubKey to
reject out-of-range public key values
2. ECDH hardening (WolfCryptKeyAgreement):
- Release and recreate ecPrivate/ecPublic native structs in
wcInitECDHParams so engineInit() can be called multiple times
on the same object without leaking the prior session
- Wrap publicKeyDecode WolfCryptException as InvalidKeyException
- Call ecPublic.checkKey() and wrap failure as InvalidKeyException
- Compare getCurveId() for public and private keys; throw
InvalidKeyException on curve mismatch before makeSharedSecret
- Wrap makeSharedSecret WolfCryptException as IllegalStateException
3. WolfCryptECKeyFactory: wrap IllegalArgumentException from
validateParameters() as InvalidKeySpecException at both the
generatePublic and generatePrivate call sites
Two independent improvements to WolfCryptSignature and a new
AlgorithmParametersSpi for OAEP:
1. RSA CRT consistency check (WolfCryptSignature): in
wolfCryptInitPrivateKey, export n/p/q via exportRawPrivateKey
and verify n == p*q before accepting the key. A fault-injection
or corrupt key that has mismatched CRT components could expose
the private key via differential fault analysis; this check
rejects such keys at init time.
2. CRT component zeroing (WolfCryptSignature): d, p, q, dP, dQ,
and qInv (u) are exported into byte[] locals during validation.
A try-finally zeroes all six arrays with zeroArray() regardless
of outcome so private key material does not linger on the heap.
3. WolfCryptOaepParameters: new AlgorithmParametersSpi implementing
the "OAEP" algorithm name, supporting init/getEncoded/getSpec
for OAEPParameterSpec. Registered in WolfCryptProvider so that
AlgorithmParameters.getInstance("OAEP") resolves to this class.
MarkAtwood
force-pushed
the
fix/wycheproof-v2
branch
from
eacd2a7 to
279d16c
Compare
Remove try/finally { finalized = true } from both engineDoFinal()
overloads. wolfCryptFinal() already resets cipher state for reuse,
matching the JCE contract that doFinal() allows subsequent
update()/doFinal() without re-init.
Wrap long lines in JNI declarations, Javadoc, and Java source
to stay within the 80-character project limit.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Fix: doFinal() state reset + 80-char limit (c74aa28)Root cause of all test failures: Fix: Removed the 80-char violations: Wrapped 13 lines across 6 files (JNI declarations, Javadoc, Java source). Test results (local): 1218 run, 0 new failures. 3 pre-existing GCM streaming failures unchanged ( |
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.
Summary
Four commits addressing findings from running the Wycheproof test suite against wolfJCE, plus a new Google Wycheproof runner context that integrates the test suite with CI.
Commit 1 —
feat(aesgcm): streaming encrypt via WOLFSSL_AESGCM_STREAMAdds JNI wrappers for
wc_AesGcmEncryptInit/Update/Finaland exposes them throughAesGcm.java.jni_aesgcm.c; null-pointer guard onGetByteArrayElementsprotects against OOM during streaming updateAesGcm.javaJava wrappers withIDLE/STREAMINGstate machine —encryptUpdateStreamingandencryptFinalStreamingthrowIllegalStateExceptionif called out of order; state resets toIDLEin afinallyblock afterencryptFinalStreamingThe
WolfCryptCipherintegration (commit 2) uses this to avoid buffering entire GCM plaintexts.Commit 2 —
fix: Cipher security and AES-GCM streaming integrationFour independent improvements to
WolfCryptCipher.java:Streaming AES-GCM path: when
FeatureDetect.AesGcmStreamEnabled()is true,AES/GCM/NoPaddingencrypt uses the new streaming API instead of buffering the entire plaintext. Enforces the NIST SP 800-38D plaintext limit (2³²−2 blocks) via agcmBytesEncryptedcounter. AAD is always passed before any plaintext.IV-reuse prevention: adds a
finalizedflag set in thefinallyblock ofengineDoFinal.engineUpdate(both overloads),engineWrap, andengineUnwrapall throwIllegalStateExceptionif called afterdoFinalwithout re-initializing — closes the window where callingwrapafterdoFinalon the same instance could reuse a GCM nonce.BadPaddingExceptionon PKCS5 unpadding failure: AES-CBC and 3DES decrypt now throwBadPaddingException(notWolfCryptException) when unpadding fails, matching the standard JCE contract.OAEP
AlgorithmParameters: stores the activeOAEPParameterSpecand returns it fromengineGetParameters(), so callers can retrieve the hash and MGF parameters used at init time.Commit 3 —
fix: ECDH/DH key validation and exception propagationDH public key validation (
WolfCryptKeyAgreement,Dh.java):dhParamP/dhParamGfrom the private key atengineInittimeengineDoPhase, compare peer{p,g}against stored values; throwInvalidKeyExceptionon mismatch (prevents small-subgroup / cross-group attacks)Dh.checkPublicKey()wrappingwc_DhCheckPubKeyrejects out-of-range public key valuesECDH hardening (
WolfCryptKeyAgreement):ecPrivate/ecPublicnative structs inwcInitECDHParamssoengineInit()can be called multiple times on the same objectpublicKeyDecodeWolfCryptException→InvalidKeyExceptionecPublic.checkKey()validates the peer point; failure →InvalidKeyExceptiongetCurveId()comparison rejects curve-mismatch keys before callingmakeSharedSecretmakeSharedSecretWolfCryptException→IllegalStateExceptionWolfCryptECKeyFactory: wrapsIllegalArgumentExceptionfromvalidateParameters()asInvalidKeySpecExceptionat bothgeneratePublicandgeneratePrivatecall sites.Commit 4 —
fix: RSA CRT key validation and private key material zeroingRSA CRT consistency check (
WolfCryptSignature): inwolfCryptInitPrivateKey, exportn/p/qviaexportRawPrivateKeyand verifyn == p*qbefore accepting the key. A corrupt or fault-injected key with mismatched CRT components can expose the private key via differential fault analysis — this rejects such keys at init time.CRT component zeroing (
WolfCryptSignature):d,p,q,dP,dQ, andqInvare zeroed withzeroArray()in afinallyblock after the consistency check, regardless of outcome.WolfCryptOaepParameters: newAlgorithmParametersSpiimplementing the"OAEP"algorithm name. SupportsOAEPParameterSpecinit,getEncoded, andgetParameterSpec. Registered inWolfCryptProvider.Test plan
WOLFSSL_AESGCM_STREAMdefined and verify streaming path is taken forAES/GCM/NoPaddingencryptWOLFSSL_AESGCM_STREAMand verify buffered path still worksmvn test)engineDoFinalfollowed byengineUpdatethrowsIllegalStateExceptionAlgorithmParameters.getInstance("OAEP")resolves and round-tripsOAEPParameterSpec