diff --git a/jni/jni_fips.c b/jni/jni_fips.c index 581b36dd..f670bbe0 100644 --- a/jni/jni_fips.c +++ b/jni/jni_fips.c @@ -1928,6 +1928,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_Fips_wc_1RNG_1GenerateBlock_1f RNG* rng = NULL; byte* buf = NULL; + jlong bufCap = 0; rng = (RNG*) getNativeStruct(env, rng_object); if ((*env)->ExceptionOccurred(env)) { @@ -1936,9 +1937,13 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_Fips_wc_1RNG_1GenerateBlock_1f } buf = getDirectBufferAddress(env, buf_buffer); + bufCap = (*env)->GetDirectBufferCapacity(env, buf_buffer); - if (!rng || !buf) + /* reject NULL, negative, or bufSz beyond the buffer capacity, the + * capacity ceiling also keeps the word32 cast from wrapping */ + if (rng == NULL || buf == NULL || bufSz < 0 || bufSz > bufCap) { return BAD_FUNC_ARG; + } #if FIPS_VERSION_GT(5,0) ret = wc_RNG_GenerateBlock_fips(rng, buf, (word32)bufSz); @@ -1948,7 +1953,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_Fips_wc_1RNG_1GenerateBlock_1f LogStr("RNG_GenerateBlock_fips(rng=%p, buf, bufSz) = %d\n", rng, ret); LogStr("output[%u]: [%p]\n", (word32)bufSz, buf); - LogHex(buf, 0, bufSz); + LogHex(buf, 0, (word32)bufSz); #endif @@ -1965,6 +1970,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_Fips_wc_1RNG_1GenerateBlock_1f RNG* rng = NULL; byte* buf = NULL; + word32 bufLen = 0; rng = (RNG*) getNativeStruct(env, rng_object); if ((*env)->ExceptionOccurred(env)) { @@ -1973,8 +1979,10 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_Fips_wc_1RNG_1GenerateBlock_1f } buf = getByteArray(env, buf_buffer); + bufLen = getByteArrayLength(env, buf_buffer); - if (rng == NULL || buf == NULL) { + /* reject NULL, negative, or bufSz beyond the backing array */ + if (rng == NULL || buf == NULL || bufSz < 0 || bufSz > (jlong)bufLen) { ret = BAD_FUNC_ARG; } else { @@ -1986,8 +1994,10 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_wolfcrypt_Fips_wc_1RNG_1GenerateBlock_1f } LogStr("RNG_GenerateBlock_fips(rng=%p, buf, bufSz) = %d\n", rng, ret); - LogStr("output[%u]: [%p]\n", (word32)bufSz, buf); - LogHex(buf, 0, bufSz); + if (buf != NULL && ret == 0) { + LogStr("output[%u]: [%p]\n", (word32)bufSz, buf); + LogHex(buf, 0, (word32)bufSz); + } releaseByteArray(env, buf_buffer, buf, ret); diff --git a/jni/jni_sha.c b/jni/jni_sha.c index b2a0d802..d724262f 100644 --- a/jni/jni_sha.c +++ b/jni/jni_sha.c @@ -390,16 +390,22 @@ Java_com_wolfssl_wolfcrypt_Sha_native_1final_1internal___3B( hash = getByteArray(env, hash_buffer); - ret = (!sha || !hash) - ? BAD_FUNC_ARG - : wc_ShaFinal(sha, hash); + if (sha == NULL || hash == NULL || + getByteArrayLength(env, hash_buffer) < SHA_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_ShaFinal(sha, hash); + } if (ret != 0) throwWolfCryptExceptionFromError(env, ret); LogStr("wc_ShaFinal(sha=%p, hash) = %d\n", sha, ret); - LogStr("hash[%u]: [%p]\n", (word32)SHA_DIGEST_SIZE, hash); - LogHex(hash, 0, SHA_DIGEST_SIZE); + if (ret == 0) { + LogStr("hash[%u]: [%p]\n", (word32)SHA_DIGEST_SIZE, hash); + LogHex(hash, 0, SHA_DIGEST_SIZE); + } releaseByteArray(env, hash_buffer, hash, ret); #else @@ -619,7 +625,8 @@ JNIEXPORT void JNICALL Java_com_wolfssl_wolfcrypt_Sha224_native_1final_1internal hash = getByteArray(env, hash_buffer); - if (sha == NULL || hash == NULL) { + if (sha == NULL || hash == NULL || + getByteArrayLength(env, hash_buffer) < SHA224_DIGEST_SIZE) { ret = BAD_FUNC_ARG; } else { @@ -631,8 +638,10 @@ JNIEXPORT void JNICALL Java_com_wolfssl_wolfcrypt_Sha224_native_1final_1internal } LogStr("wc_Sha224Final(sha=%p, hash) = %d\n", sha, ret); - LogStr("hash[%u]: [%p]\n", (word32)SHA224_DIGEST_SIZE, hash); - LogHex(hash, 0, SHA224_DIGEST_SIZE); + if (ret == 0) { + LogStr("hash[%u]: [%p]\n", (word32)SHA224_DIGEST_SIZE, hash); + LogHex(hash, 0, SHA224_DIGEST_SIZE); + } releaseByteArray(env, hash_buffer, hash, ret); #else @@ -835,16 +844,22 @@ Java_com_wolfssl_wolfcrypt_Sha256_native_1final_1internal___3B( hash = getByteArray(env, hash_buffer); - ret = (!sha || !hash) - ? BAD_FUNC_ARG - : wc_Sha256Final(sha, hash); + if (sha == NULL || hash == NULL || + getByteArrayLength(env, hash_buffer) < SHA256_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha256Final(sha, hash); + } if (ret != 0) throwWolfCryptExceptionFromError(env, ret); LogStr("wc_Sha256Final(sha=%p, hash) = %d\n", sha, ret); - LogStr("hash[%u]: [%p]\n", (word32)SHA256_DIGEST_SIZE, hash); - LogHex(hash, 0, SHA256_DIGEST_SIZE); + if (ret == 0) { + LogStr("hash[%u]: [%p]\n", (word32)SHA256_DIGEST_SIZE, hash); + LogHex(hash, 0, SHA256_DIGEST_SIZE); + } releaseByteArray(env, hash_buffer, hash, ret); #else @@ -1041,16 +1056,22 @@ Java_com_wolfssl_wolfcrypt_Sha384_native_1final_1internal___3B( hash = getByteArray(env, hash_buffer); - ret = (!sha || !hash) - ? BAD_FUNC_ARG - : wc_Sha384Final(sha, hash); + if (sha == NULL || hash == NULL || + getByteArrayLength(env, hash_buffer) < SHA384_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha384Final(sha, hash); + } if (ret != 0) throwWolfCryptExceptionFromError(env, ret); LogStr("wc_Sha384Final(sha=%p, hash) = %d\n", sha, ret); - LogStr("hash[%u]: [%p]\n", (word32)SHA384_DIGEST_SIZE, hash); - LogHex(hash, 0, SHA384_DIGEST_SIZE); + if (ret == 0) { + LogStr("hash[%u]: [%p]\n", (word32)SHA384_DIGEST_SIZE, hash); + LogHex(hash, 0, SHA384_DIGEST_SIZE); + } releaseByteArray(env, hash_buffer, hash, ret); #else @@ -1248,16 +1269,22 @@ Java_com_wolfssl_wolfcrypt_Sha512_native_1final_1internal___3B( hash = getByteArray(env, hash_buffer); - ret = (!sha || !hash) - ? BAD_FUNC_ARG - : wc_Sha512Final(sha, hash); + if (sha == NULL || hash == NULL || + getByteArrayLength(env, hash_buffer) < SHA512_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha512Final(sha, hash); + } if (ret != 0) throwWolfCryptExceptionFromError(env, ret); LogStr("wc_Sha512Final(sha=%p, hash) = %d\n", sha, ret); - LogStr("hash[%u]: [%p]\n", (word32)SHA512_DIGEST_SIZE, hash); - LogHex(hash, 0, SHA512_DIGEST_SIZE); + if (ret == 0) { + LogStr("hash[%u]: [%p]\n", (word32)SHA512_DIGEST_SIZE, hash); + LogHex(hash, 0, SHA512_DIGEST_SIZE); + } releaseByteArray(env, hash_buffer, hash, ret); #else @@ -1579,6 +1606,7 @@ JNIEXPORT void JNICALL Java_com_wolfssl_wolfcrypt_Sha3_native_1final_1internal__ int ret = 0; wc_Sha3* sha = NULL; byte* hash = NULL; + word32 hashSz = 0; sha = (wc_Sha3*) getNativeStruct(env, this); if ((*env)->ExceptionOccurred(env)) { @@ -1593,18 +1621,39 @@ JNIEXPORT void JNICALL Java_com_wolfssl_wolfcrypt_Sha3_native_1final_1internal__ } if (ret == 0) { + hashSz = getByteArrayLength(env, hash_buffer); switch(hashType) { case WC_HASH_TYPE_SHA3_224: - ret = wc_Sha3_224_Final(sha, hash); + if (hashSz < WC_SHA3_224_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha3_224_Final(sha, hash); + } break; case WC_HASH_TYPE_SHA3_256: - ret = wc_Sha3_256_Final(sha, hash); + if (hashSz < WC_SHA3_256_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha3_256_Final(sha, hash); + } break; case WC_HASH_TYPE_SHA3_384: - ret = wc_Sha3_384_Final(sha, hash); + if (hashSz < WC_SHA3_384_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha3_384_Final(sha, hash); + } break; case WC_HASH_TYPE_SHA3_512: - ret = wc_Sha3_512_Final(sha, hash); + if (hashSz < WC_SHA3_512_DIGEST_SIZE) { + ret = BAD_FUNC_ARG; + } + else { + ret = wc_Sha3_512_Final(sha, hash); + } break; default: ret = BAD_FUNC_ARG; diff --git a/src/main/java/com/wolfssl/wolfcrypt/AesCmac.java b/src/main/java/com/wolfssl/wolfcrypt/AesCmac.java index 533e2b43..67d8102b 100644 --- a/src/main/java/com/wolfssl/wolfcrypt/AesCmac.java +++ b/src/main/java/com/wolfssl/wolfcrypt/AesCmac.java @@ -110,7 +110,8 @@ public synchronized void setKey(byte[] key) wc_CmacSetKey(key); } - /* Store key for reset functionality */ + /* Store key for reset, zeroize any prior key clone */ + zeroizeKey(); this.key = new byte[key.length]; System.arraycopy(key, 0, this.key, 0, key.length); diff --git a/src/test/java/com/wolfssl/wolfcrypt/test/AesCmacTest.java b/src/test/java/com/wolfssl/wolfcrypt/test/AesCmacTest.java index e9677976..bedd4a7c 100644 --- a/src/test/java/com/wolfssl/wolfcrypt/test/AesCmacTest.java +++ b/src/test/java/com/wolfssl/wolfcrypt/test/AesCmacTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.*; import java.nio.ByteBuffer; +import java.lang.reflect.Field; import org.junit.Assume; import org.junit.BeforeClass; @@ -574,6 +575,48 @@ public void aesCmacShouldHandleObjectReuse() { } } + @Test + public void aesCmacShouldZeroizePriorKeyOnReKey() throws Exception { + + String key1 = "2b7e151628aed2a6abf7158809cf4f3c"; + String key2 = "603deb1015ca71be2b73aef0857d7781" + + "1f352c073b6108d72d9810a30914dff4"; + + byte[] key1Bytes = Util.h2b(key1); + byte[] key2Bytes = Util.h2b(key2); + + AesCmac cmac; + try { + cmac = new AesCmac(); + cmac.setKey(key1Bytes); + } catch (WolfCryptException e) { + if (e.getError() == WolfCryptError.NOT_COMPILED_IN) { + System.out.println("AesCmac re-key zeroize test skipped: " + + e.getError()); + return; + } + throw e; + } + + /* Capture reference to the internal key clone stored by setKey. + * Zeroization has no public observable, so this deliberately couples + * to the private AesCmac.key field name and must be updated if that + * field is renamed. */ + Field keyField = AesCmac.class.getDeclaredField("key"); + keyField.setAccessible(true); + byte[] priorClone = (byte[]) keyField.get(cmac); + + assertNotNull("Internal key clone should be stored", priorClone); + assertFalse("Internal key clone should be non-zero before re-key", + Arrays.equals(priorClone, new byte[priorClone.length])); + + /* Re-keying must zeroize the prior clone before replacing it */ + cmac.setKey(key2Bytes); + + assertArrayEquals("Prior key clone should be zeroized after re-key", + new byte[priorClone.length], priorClone); + } + @Test public void aesCmacStaticMethodsShouldHandleEdgeCases() { try { diff --git a/src/test/java/com/wolfssl/wolfcrypt/test/fips/RngFipsTest.java b/src/test/java/com/wolfssl/wolfcrypt/test/fips/RngFipsTest.java index 528fa4b2..7c30f9d1 100644 --- a/src/test/java/com/wolfssl/wolfcrypt/test/fips/RngFipsTest.java +++ b/src/test/java/com/wolfssl/wolfcrypt/test/fips/RngFipsTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.*; import java.nio.ByteBuffer; +import java.util.Arrays; import org.junit.BeforeClass; import org.junit.Test; @@ -34,6 +35,7 @@ import com.wolfssl.wolfcrypt.Rng; import com.wolfssl.wolfcrypt.WolfCrypt; +import com.wolfssl.wolfcrypt.WolfCryptError; import com.wolfssl.wolfcrypt.Fips; import com.wolfssl.wolfcrypt.test.Util; @@ -60,6 +62,65 @@ public void initShouldReturnZero() { assertEquals(WolfCrypt.SUCCESS, Fips.FreeRng_fips(rng)); } + @Test + public void GenerateBlockShouldRejectBadBufSzUsingByteBuffer() { + Rng rng = new Rng(); + ByteBuffer buf = ByteBuffer.allocateDirect(32); + + assertEquals(WolfCrypt.SUCCESS, Fips.InitRng_fips(rng)); + + /* valid request within capacity fills the buffer */ + assertEquals(WolfCrypt.SUCCESS, + Fips.RNG_GenerateBlock_fips(rng, buf, 32)); + + byte[] filled = new byte[32]; + buf.duplicate().get(filled); + assertFalse("buffer should not be all zeros after generate", + Arrays.equals(filled, new byte[32])); + + /* zero length is admitted by the guard and handled by the RNG */ + assertEquals(WolfCrypt.SUCCESS, + Fips.RNG_GenerateBlock_fips(rng, buf, 0)); + + /* negative bufSz is rejected before the RNG call */ + assertEquals(WolfCryptError.BAD_FUNC_ARG.getCode(), + Fips.RNG_GenerateBlock_fips(rng, buf, -1)); + + /* bufSz larger than capacity is rejected before the RNG call */ + assertEquals(WolfCryptError.BAD_FUNC_ARG.getCode(), + Fips.RNG_GenerateBlock_fips(rng, buf, 33)); + + assertEquals(WolfCrypt.SUCCESS, Fips.FreeRng_fips(rng)); + } + + @Test + public void GenerateBlockShouldRejectBadBufSzUsingByteArray() { + Rng rng = new Rng(); + byte[] buf = new byte[32]; + + assertEquals(WolfCrypt.SUCCESS, Fips.InitRng_fips(rng)); + + /* valid request within length fills the buffer */ + assertEquals(WolfCrypt.SUCCESS, + Fips.RNG_GenerateBlock_fips(rng, buf, 32)); + assertFalse("buffer should not be all zeros after generate", + Arrays.equals(buf, new byte[32])); + + /* zero length is admitted by the guard and handled by the RNG */ + assertEquals(WolfCrypt.SUCCESS, + Fips.RNG_GenerateBlock_fips(rng, buf, 0)); + + /* negative bufSz is rejected before the RNG call */ + assertEquals(WolfCryptError.BAD_FUNC_ARG.getCode(), + Fips.RNG_GenerateBlock_fips(rng, buf, -1)); + + /* bufSz larger than the array is rejected before the RNG call */ + assertEquals(WolfCryptError.BAD_FUNC_ARG.getCode(), + Fips.RNG_GenerateBlock_fips(rng, buf, 33)); + + assertEquals(WolfCrypt.SUCCESS, Fips.FreeRng_fips(rng)); + } + @Test public void HeathTestShouldReturnZeroUsingByteBuffer() { String[] inputA = new String[] {