Skip to content

Commit 3bf3309

Browse files
authored
Merge pull request #151 from cconlon/aesCcmFix
Fix ArrayIndexOutOfBoundsException in AES-GCM/CCM, correct CCM arg sanitization
2 parents da42515 + 2508cd2 commit 3bf3309

File tree

4 files changed

+180
-43
lines changed

4 files changed

+180
-43
lines changed

jni/jni_aesccm.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,9 @@ JNIEXPORT jbyteArray JNICALL Java_com_wolfssl_wolfcrypt_AesCcm_wc_1AesCcmEncrypt
192192
authInSz = (*env)->GetArrayLength(env, authInArr);
193193
}
194194

195-
/* authIn can be null */
196-
if (in == NULL || inLen == 0 || nonce == NULL || nonceSz == 0 ||
197-
authTag == NULL || authTagSz == 0) {
195+
/* in can be NULL if inLen is 0 - case with only AAD to gen tag */
196+
if ((inLen != 0 && in == NULL) || nonce == NULL || authTag == NULL ||
197+
nonceSz < 7 || nonceSz > 13 || authTagSz > WC_AES_BLOCK_SIZE) {
198198
ret = BAD_FUNC_ARG;
199199
}
200200

@@ -325,8 +325,9 @@ JNIEXPORT jbyteArray JNICALL Java_com_wolfssl_wolfcrypt_AesCcm_wc_1AesCcmDecrypt
325325
authInSz = (*env)->GetArrayLength(env, authInArr);
326326
}
327327

328-
if (in == NULL || inLen == 0 || nonce == NULL || nonceSz == 0 ||
329-
authTag == NULL || authTagSz == 0) {
328+
/* in can be NULL if inLen is 0 - case with only AAD to verify tag */
329+
if ((inLen != 0 && in == NULL) || nonce == NULL || authTag == NULL ||
330+
nonceSz < 7 || nonceSz > 13 || authTagSz > WC_AES_BLOCK_SIZE) {
330331
ret = BAD_FUNC_ARG;
331332
}
332333

src/main/java/com/wolfssl/provider/jce/WolfCryptCipher.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,19 @@ protected int engineGetOutputSize(int inputLen)
379379
if (isBlockCipher()) {
380380
if (buffered != null && buffered.length > 0) {
381381
totalSz = inputLen + buffered.length;
382+
} else {
383+
totalSz = inputLen;
384+
}
385+
386+
/* For block ciphers that require block boundaries, round
387+
* to next block size. GCM, CCM, CTR, and OFB do not require block
388+
* boundaries. */
389+
if (cipherMode != CipherMode.WC_GCM &&
390+
cipherMode != CipherMode.WC_CCM &&
391+
cipherMode != CipherMode.WC_CTR &&
392+
cipherMode != CipherMode.WC_OFB) {
382393
totalBlocks = totalSz / blockSize;
383394
totalSz = totalBlocks * blockSize;
384-
} else {
385-
totalBlocks = inputLen / blockSize;
386395
}
387396
}
388397

@@ -1061,6 +1070,15 @@ private byte[] wolfCryptFinal(byte[] input, int inputOffset, int len)
10611070
tmpOut = totalOut;
10621071
}
10631072
else {
1073+
/* Case where input is only the authentication tag,
1074+
* zero-length plaintext */
1075+
if (tmpIn.length < this.gcmTagLen) {
1076+
throw new AEADBadTagException(
1077+
"Input too short for GCM tag, got " +
1078+
tmpIn.length + " bytes, need at least " +
1079+
this.gcmTagLen);
1080+
}
1081+
10641082
/* Get auth tag from end of ciphertext */
10651083
byte[] tag = Arrays.copyOfRange(tmpIn,
10661084
tmpIn.length - this.gcmTagLen,
@@ -1099,6 +1117,15 @@ else if (cipherMode == CipherMode.WC_CCM) {
10991117
tmpOut = totalOut;
11001118
}
11011119
else {
1120+
/* Case where input is only the authentication tag,
1121+
* zero-length plaintext */
1122+
if (tmpIn.length < this.gcmTagLen) {
1123+
throw new AEADBadTagException(
1124+
"Input too short for CCM tag, got " +
1125+
tmpIn.length + " bytes, need at least " +
1126+
this.gcmTagLen);
1127+
}
1128+
11021129
/* Get auth tag from end of ciphertext */
11031130
byte[] tag = Arrays.copyOfRange(tmpIn,
11041131
tmpIn.length - this.gcmTagLen,

src/test/java/com/wolfssl/provider/jce/test/WolfCryptCipherTest.java

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6918,5 +6918,102 @@ public void testPKCS5PaddingDecryptBehavior()
69186918
assertArrayEquals("Decrypted result should match original plaintext",
69196919
plaintext, fullResult);
69206920
}
6921+
6922+
/*
6923+
* Test AES-GCM and AES-CCM with zero-length plaintext.
6924+
*/
6925+
@Test
6926+
public void testAesGcmCcmZeroLengthPlaintext()
6927+
throws NoSuchAlgorithmException, NoSuchProviderException,
6928+
NoSuchPaddingException, InvalidKeyException,
6929+
InvalidAlgorithmParameterException,
6930+
IllegalBlockSizeException, BadPaddingException {
6931+
6932+
/* Test AES-GCM with zero-length plaintext */
6933+
if (enabledJCEAlgos.contains("AES/GCM/NoPadding")) {
6934+
byte[] key = new byte[16];
6935+
byte[] iv = new byte[12];
6936+
byte[] aad = new byte[100];
6937+
6938+
secureRandom.nextBytes(key);
6939+
secureRandom.nextBytes(iv);
6940+
for (int i = 0; i < aad.length; i++) {
6941+
aad[i] = (byte) (i % 256);
6942+
}
6943+
6944+
/* Test zero-length plaintext */
6945+
byte[] plaintext = new byte[0];
6946+
6947+
/* Encrypt */
6948+
Cipher encCipher =
6949+
Cipher.getInstance("AES/GCM/NoPadding", jceProvider);
6950+
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
6951+
encCipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"),
6952+
gcmSpec);
6953+
encCipher.updateAAD(aad);
6954+
byte[] ciphertext = encCipher.doFinal(plaintext);
6955+
6956+
/* Should have only the authentication tag (16 bytes) */
6957+
assertEquals("GCM ciphertext should be tag length only",
6958+
16, ciphertext.length);
6959+
6960+
/* Decrypt */
6961+
Cipher decCipher =
6962+
Cipher.getInstance("AES/GCM/NoPadding", jceProvider);
6963+
decCipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"),
6964+
gcmSpec);
6965+
decCipher.updateAAD(aad);
6966+
byte[] decrypted = decCipher.doFinal(ciphertext);
6967+
6968+
/* Should decrypt to zero-length plaintext */
6969+
assertEquals("Decrypted plaintext should be zero length",
6970+
0, decrypted.length);
6971+
assertArrayEquals("Decrypted should match original plaintext",
6972+
plaintext, decrypted);
6973+
}
6974+
6975+
/* Test AES-CCM with zero-length plaintext */
6976+
if (enabledJCEAlgos.contains("AES/CCM/NoPadding")) {
6977+
byte[] key = new byte[16];
6978+
byte[] nonce = new byte[11]; /* 88-bit nonce for CCM */
6979+
byte[] aad = new byte[50];
6980+
6981+
secureRandom.nextBytes(key);
6982+
secureRandom.nextBytes(nonce);
6983+
for (int i = 0; i < aad.length; i++) {
6984+
aad[i] = (byte) (i % 256);
6985+
}
6986+
6987+
/* Test zero-length plaintext */
6988+
byte[] plaintext = new byte[0];
6989+
6990+
/* Encrypt */
6991+
Cipher encCipher =
6992+
Cipher.getInstance("AES/CCM/NoPadding", jceProvider);
6993+
GCMParameterSpec ccmSpec = new GCMParameterSpec(128, nonce);
6994+
encCipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"),
6995+
ccmSpec);
6996+
encCipher.updateAAD(aad);
6997+
byte[] ciphertext = encCipher.doFinal(plaintext);
6998+
6999+
/* Should have only the authentication tag (16 bytes) */
7000+
assertEquals("CCM ciphertext should be tag length only",
7001+
16, ciphertext.length);
7002+
7003+
/* Decrypt */
7004+
Cipher decCipher =
7005+
Cipher.getInstance("AES/CCM/NoPadding", jceProvider);
7006+
decCipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"),
7007+
ccmSpec);
7008+
decCipher.updateAAD(aad);
7009+
byte[] decrypted = decCipher.doFinal(ciphertext);
7010+
7011+
/* Should decrypt to zero-length plaintext */
7012+
assertEquals("Decrypted plaintext should be zero length",
7013+
0, decrypted.length);
7014+
assertArrayEquals("Decrypted should match original plaintext",
7015+
plaintext, decrypted);
7016+
}
7017+
}
69217018
}
69227019

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

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -257,12 +257,14 @@ public void testAesCcm128() throws WolfCryptException {
257257
plain = dec.decrypt(cipher, iv3, tag, a3);
258258
assertArrayEquals(p3, plain);
259259

260-
/* bad encrypt arguments: null input */
261-
try {
262-
enc.encrypt(null, iv3, tag, a3);
263-
fail("encrypt() with null input should fail");
264-
} catch (WolfCryptException e) {
265-
/* expected */
260+
/* success case: null input with aad. FIPSv2 incorrectly returned
261+
* BAD_FUNC_ARG when in buffer was null. Skip this test for v2 */
262+
if (Fips.fipsVersion != 2) {
263+
try {
264+
enc.encrypt(null, iv3, tag, a3);
265+
} catch (WolfCryptException e) {
266+
fail("encrypt() with null input should pass");
267+
}
266268
}
267269

268270
/* bad encrypt arguments: null iv */
@@ -281,12 +283,14 @@ public void testAesCcm128() throws WolfCryptException {
281283
/* expected */
282284
}
283285

284-
/* bad decrypt arguments: null input */
285-
try {
286-
enc.decrypt(null, iv3, tag, a3);
287-
fail("decrypt() with null input should fail");
288-
} catch (WolfCryptException e) {
289-
/* expected */
286+
/* success case: null input with aad. FIPSv2 incorrectly returned
287+
* BAD_FUNC_ARG when in buffer was null. Skip this test for v2 */
288+
if (Fips.fipsVersion != 2) {
289+
try {
290+
enc.decrypt(null, iv3, tag, a3);
291+
} catch (WolfCryptException e) {
292+
fail("decrypt() with null input should pass");
293+
}
290294
}
291295

292296
/* bad decrypt arguments: null iv */
@@ -336,12 +340,14 @@ public void testAesCcm192() throws WolfCryptException {
336340
assertNotNull(plain);
337341
assertArrayEquals(p2, plain);
338342

339-
/* bad encrypt arguments: null input */
340-
try {
341-
enc.encrypt(null, iv2, tag, a2);
342-
fail("encrypt() with null input should fail");
343-
} catch (WolfCryptException e) {
344-
/* expected */
343+
/* success case: null input with aad. FIPSv2 incorrectly returned
344+
* BAD_FUNC_ARG when in buffer was null. Skip this test for v2 */
345+
if (Fips.fipsVersion != 2) {
346+
try {
347+
enc.encrypt(null, iv2, tag, a2);
348+
} catch (WolfCryptException e) {
349+
fail("encrypt() with null input should pass");
350+
}
345351
}
346352

347353
/* bad encrypt arguments: null iv */
@@ -360,12 +366,14 @@ public void testAesCcm192() throws WolfCryptException {
360366
/* expected */
361367
}
362368

363-
/* bad decrypt arguments: null input */
364-
try {
365-
enc.decrypt(null, iv2, tag, a2);
366-
fail("decrypt() with null input should fail");
367-
} catch (WolfCryptException e) {
368-
/* expected */
369+
/* success case: null input with aad. FIPSv2 incorrectly returned
370+
* BAD_FUNC_ARG when in buffer was null. Skip this test for v2 */
371+
if (Fips.fipsVersion != 2) {
372+
try {
373+
enc.decrypt(null, iv2, tag, a2);
374+
} catch (WolfCryptException e) {
375+
fail("decrypt() with null input should pass");
376+
}
369377
}
370378

371379
/* bad decrypt arguments: null iv */
@@ -415,12 +423,14 @@ public void testAesCcm256() throws WolfCryptException {
415423
assertNotNull(plain);
416424
assertArrayEquals(p1, plain);
417425

418-
/* bad encrypt arguments: null input */
419-
try {
420-
enc.encrypt(null, iv1, tag, a1);
421-
fail("encrypt() with null input should fail");
422-
} catch (WolfCryptException e) {
423-
/* expected */
426+
/* success case: null input with aad. FIPSv2 incorrectly returned
427+
* BAD_FUNC_ARG when in buffer was null. Skip this test for v2 */
428+
if (Fips.fipsVersion != 2) {
429+
try {
430+
enc.encrypt(null, iv1, tag, a1);
431+
} catch (WolfCryptException e) {
432+
fail("encrypt() with null input should pass");
433+
}
424434
}
425435

426436
/* bad encrypt arguments: null iv */
@@ -439,12 +449,14 @@ public void testAesCcm256() throws WolfCryptException {
439449
/* expected */
440450
}
441451

442-
/* bad decrypt arguments: null input */
443-
try {
444-
enc.decrypt(null, iv1, tag, a1);
445-
fail("decrypt() with null input should fail");
446-
} catch (WolfCryptException e) {
447-
/* expected */
452+
/* success case: null input with aad. FIPSv2 incorrectly returned
453+
* BAD_FUNC_ARG when in buffer was null. Skip this test for v2 */
454+
if (Fips.fipsVersion != 2) {
455+
try {
456+
enc.decrypt(null, iv1, tag, a1);
457+
} catch (WolfCryptException e) {
458+
fail("decrypt() with null input should pass");
459+
}
448460
}
449461

450462
/* bad decrypt arguments: null iv */

0 commit comments

Comments
 (0)