Wycheproof: streaming AES-GCM, Cipher security fixes, ECDH/DH validation#239
Open
MarkAtwood wants to merge 6 commits into
Open
Wycheproof: streaming AES-GCM, Cipher security fixes, ECDH/DH validation#239MarkAtwood wants to merge 6 commits into
MarkAtwood wants to merge 6 commits into
Conversation
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.
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>
MarkAtwood
force-pushed
the
fix/wycheproof-v2
branch
from
c74aa28 to
eb0c522
Compare
Closed
6 tasks
There was a problem hiding this comment.
Pull request overview
This PR hardens wolfJCE against several Wycheproof findings by adding AES-GCM streaming encryption support (JNI + Java), tightening Cipher lifecycle/nonce-reuse behavior, strengthening DH/ECDH validation/exception mapping, and adding OAEP AlgorithmParameters support.
Changes:
- Add AES-GCM streaming encrypt JNI wrappers and Java APIs, and integrate streaming encryption into
WolfCryptCipherwhen available. - Improve key agreement and key factory behavior (DH
{p,g}matching + pubkey validation; ECDH point/curve validation; wrap parameter validation errors as JCE exceptions). - Add RSA CRT consistency checking/zeroing and implement/register
AlgorithmParameters.OAEP.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/wolfssl/wolfcrypt/AesGcm.java | Adds Java streaming AES-GCM init/update/final API with a small state machine. |
| jni/jni_aesgcm.c | Implements streaming AES-GCM JNI bindings to wolfCrypt init/update/final APIs. |
| jni/include/com_wolfssl_wolfcrypt_AesGcm.h | Declares new JNI methods for streaming AES-GCM encrypt. |
| src/main/java/com/wolfssl/provider/jce/WolfCryptCipher.java | Integrates AES-GCM streaming encrypt, adds AEAD size limits, and adds a post-doFinal lifecycle guard. |
| src/main/java/com/wolfssl/provider/jce/WolfCryptKeyAgreement.java | Adds DH parameter matching, ECDH point/curve validation, and improves exception propagation. |
| src/main/java/com/wolfssl/provider/jce/WolfCryptECKeyFactory.java | Wraps unsupported-curve IllegalArgumentException as InvalidKeySpecException. |
| src/main/java/com/wolfssl/provider/jce/WolfCryptSignature.java | Adds RSA CRT consistency check and zeroes exported CRT components. |
| src/main/java/com/wolfssl/provider/jce/WolfCryptOaepParameters.java | Adds AlgorithmParametersSpi implementation for OAEP parameters. |
| src/main/java/com/wolfssl/provider/jce/WolfCryptProvider.java | Registers AlgorithmParameters.OAEP when RSA-OAEP is enabled. |
Files not reviewed (1)
- jni/include/com_wolfssl_wolfcrypt_AesGcm.h: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+300
to
+308
| checkStateAndInitialize(); | ||
| throwIfKeyNotLoaded(); | ||
|
|
||
| /* Allowed from IDLE or STREAMING (re-init resets a prior stream) */ | ||
| streamingState = StreamingState.STREAMING; | ||
|
|
||
| synchronized (pointerLock) { | ||
| wc_AesGcmEncryptInitStreaming(iv); | ||
| } |
Comment on lines
+66
to
+70
| String mgfDigest = ((MGF1ParameterSpec) mgfParams).getDigestAlgorithm(); | ||
| if (!isDigestSupported(mgfDigest)) { | ||
| throw new InvalidParameterSpecException( | ||
| "Unsupported MGF digest: " + mgfDigest); | ||
| } |
Comment on lines
+72
to
+79
| PSource pSource = spec.getPSource(); | ||
| if (pSource != null && pSource instanceof PSource.PSpecified) { | ||
| byte[] label = ((PSource.PSpecified) pSource).getValue(); | ||
| if (label != null && label.length > 0) { | ||
| throw new InvalidParameterSpecException( | ||
| "OAEP label (PSource) must be empty"); | ||
| } | ||
| } |
Comment on lines
+439
to
+458
| if (ivArr != NULL) { | ||
| iv = (const byte*)(*env)->GetByteArrayElements(env, ivArr, NULL); | ||
| ivSz = (*env)->GetArrayLength(env, ivArr); | ||
| } | ||
|
|
||
| if (iv == NULL || ivSz == 0) { | ||
| ret = BAD_FUNC_ARG; | ||
| } | ||
|
|
||
| /* | ||
| * Pass NULL key (key already loaded via wc_AesGcmSetKey). | ||
| * wc_AesGcmEncryptInit only sets key when key != NULL. | ||
| */ | ||
| if (ret == 0) { | ||
| ret = wc_AesGcmEncryptInit(aes, NULL, 0, iv, ivSz); | ||
| } | ||
|
|
||
| if (ivArr != NULL) { | ||
| (*env)->ReleaseByteArrayElements(env, ivArr, (jbyte*)iv, JNI_ABORT); | ||
| } |
Comment on lines
1780
to
1782
| this.operationStarted = false; | ||
| this.cipherInitialized = true; | ||
|
|
Comment on lines
+498
to
+512
| if (inputArr != NULL) { | ||
| in = (const byte*)(*env)->GetByteArrayElements(env, inputArr, NULL); | ||
| inLen = (*env)->GetArrayLength(env, inputArr); | ||
| if ((inLen > 0) && (in == NULL)) { | ||
| ret = BAD_FUNC_ARG; | ||
| } | ||
| } | ||
| if ((ret == 0) && (authInArr != NULL)) { | ||
| authIn = (const byte*)(*env)->GetByteArrayElements(env, | ||
| authInArr, NULL); | ||
| authInSz = (*env)->GetArrayLength(env, authInArr); | ||
| if ((authInSz > 0) && (authIn == NULL)) { | ||
| ret = BAD_FUNC_ARG; | ||
| } | ||
| } |
Comment on lines
+546
to
+552
| if (inputArr != NULL) { | ||
| (*env)->ReleaseByteArrayElements(env, inputArr, (jbyte*)in, JNI_ABORT); | ||
| } | ||
| if (authInArr != NULL) { | ||
| (*env)->ReleaseByteArrayElements(env, authInArr, (jbyte*)authIn, | ||
| JNI_ABORT); | ||
| } |
Comment on lines
+536
to
+542
| if ((*env)->ExceptionOccurred(env)) { | ||
| (*env)->ExceptionDescribe(env); | ||
| (*env)->ExceptionClear(env); | ||
| (*env)->DeleteLocalRef(env, outArr); | ||
| outArr = NULL; | ||
| ret = -1; | ||
| } |
Comment on lines
+560
to
+563
| if (ret != 0) { | ||
| throwWolfCryptExceptionFromError(env, ret); | ||
| return NULL; | ||
| } |
Comment on lines
+609
to
+635
| if (ret == 0) { | ||
| tagArr = (*env)->NewByteArray(env, tagLen); | ||
| if (tagArr == NULL) { | ||
| ret = MEMORY_E; | ||
| } | ||
| else { | ||
| (*env)->SetByteArrayRegion(env, tagArr, 0, tagLen, (jbyte*)tag); | ||
| if ((*env)->ExceptionOccurred(env)) { | ||
| (*env)->ExceptionDescribe(env); | ||
| (*env)->ExceptionClear(env); | ||
| (*env)->DeleteLocalRef(env, tagArr); | ||
| tagArr = NULL; | ||
| ret = -1; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| XFREE(tag, NULL, DYNAMIC_TYPE_TMP_BUFFER); | ||
|
|
||
| LogStr("wc_AesGcmEncryptFinal(aes = %p, tagLen = %d)\n", aes, tagLen); | ||
|
|
||
| if (ret != 0) { | ||
| throwWolfCryptExceptionFromError(env, ret); | ||
| return NULL; | ||
| } | ||
|
|
||
| return tagArr; |
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-tripsOAEPParameterSpecRebase notes (supersedes #224)
This PR replaces #224 (same content, moved from an in-repo branch to a fork branch and rebased onto current master). Conflict resolutions against the July Fenrir fixes:
WolfCryptCipher.java: master's GCM key+IV-reuse guard is now applied before both the new streaming path and the one-shot path, and the (key, IV) pair is recorded after either path completes.WolfCryptCipher.java: kept master's generic"Decryption error"bad-padding message (padding-oracle hygiene) over this branch's more specific text.WolfCryptKeyAgreement.java: kept both master's SP 800-56A validation comment and this branch's DH parameter-match + EC curve-match checks.Dh.java: dropped this branch'scheckPublicKey()— master gained an identical method in the meantime (net-zero diff).Verified: full
ant testJCE suite green (981 tests, 0 failures) on the rebased tip against wolfSSL master built with--enable-jni.