This is an automated email from the ASF dual-hosted git repository. vanzin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-crypto.git
The following commit(s) were added to refs/heads/master by this push: new 218b3db Additional unit tests for JNA, Cipher, Random, Utils testing error inputs (#97) 218b3db is described below commit 218b3db4149a53397f1e726d37cc3928e2e2135c Author: Geoffrey Blake <blakg...@amazon.com> AuthorDate: Thu Apr 23 22:34:42 2020 -0500 Additional unit tests for JNA, Cipher, Random, Utils testing error inputs (#97) Add unit tests that exercise error paths for JNA, Cipher, Random, and Utils parts of the package. Raises coverage overall by a few percentage points. These tests touch some of the error checking deep in the library as much as possible. Tested on MacOS AMD64, RHEL8 x86 and RHEL8 aarch64. --- .../commons/crypto/jna/OpenSslJnaCipher.java | 9 ++- .../commons/crypto/cipher/AbstractCipherTest.java | 66 +++++++++++++++++++++- .../commons/crypto/cipher/OpenSslCipherTest.java | 9 +++ .../commons/crypto/jna/OpenSslJnaCipherTest.java | 6 +- .../crypto/random/CryptoRandomFactoryTest.java | 16 ++++++ .../org/apache/commons/crypto/utils/UtilsTest.java | 12 ++++ 6 files changed, 112 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/apache/commons/crypto/jna/OpenSslJnaCipher.java b/src/main/java/org/apache/commons/crypto/jna/OpenSslJnaCipher.java index bc63d11..1c68add 100644 --- a/src/main/java/org/apache/commons/crypto/jna/OpenSslJnaCipher.java +++ b/src/main/java/org/apache/commons/crypto/jna/OpenSslJnaCipher.java @@ -51,6 +51,7 @@ class OpenSslJnaCipher implements CryptoCipher { private final AlgorithmMode algMode; private final int padding; private final String transformation; + private final int IV_LENGTH = 16; /** * Constructs a {@link CryptoCipher} using JNA into OpenSSL @@ -104,7 +105,13 @@ class OpenSslJnaCipher implements CryptoCipher { throw new InvalidAlgorithmParameterException("Illegal parameters"); } - if(algMode == AlgorithmMode.AES_CBC) { + if ((algMode == AlgorithmMode.AES_CBC || + algMode == AlgorithmMode.AES_CTR) + && iv.length != IV_LENGTH) { + throw new InvalidAlgorithmParameterException("Wrong IV length: must be 16 bytes long"); + } + + if(algMode == AlgorithmMode.AES_CBC) { switch (key.getEncoded().length) { case 16: algo = OpenSslNativeJna.EVP_aes_128_cbc(); break; case 24: algo = OpenSslNativeJna.EVP_aes_192_cbc(); break; diff --git a/src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java b/src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java index f14804a..b87c6d5 100644 --- a/src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java +++ b/src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java @@ -19,7 +19,11 @@ package org.apache.commons.crypto.cipher; import static org.junit.Assert.assertNotNull; +import java.lang.reflect.InvocationTargetException; import java.nio.ByteBuffer; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Properties; import java.util.Random; @@ -27,6 +31,7 @@ import java.util.Random; import javax.crypto.Cipher; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; +import javax.crypto.spec.GCMParameterSpec; import javax.xml.bind.DatatypeConverter; import org.apache.commons.crypto.utils.ReflectionUtils; @@ -51,7 +56,8 @@ public abstract class AbstractCipherTest { // cipher static final byte[] KEY = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, - 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16 }; + 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, + 0x17, 0x18, 0x19, 0x20, 0x21, 0x22, 0x23, 0x24}; static final byte[] IV = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 }; private CryptoCipher enc, dec; @@ -147,6 +153,62 @@ public abstract class AbstractCipherTest { } } + @Test(expected = RuntimeException.class) + public void testNullTransform() throws Exception { + getCipher(null); + } + + @Test(expected = RuntimeException.class) + public void testInvalidTransform() throws Exception { + getCipher("AES/CBR/NoPadding/garbage/garbage"); + } + + @Test + public void testInvalidKey() throws Exception { + for (String transform : transformations) { + try { + final CryptoCipher cipher = getCipher(transform); + Assert.assertNotNull(cipher); + + final byte[] invalidKey = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, + 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 }; + cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(invalidKey, "AES"), new IvParameterSpec(IV)); + Assert.fail("Expected InvalidKeyException"); + } catch (final InvalidKeyException ike) { + } + } + } + + @Test + public void testInvalidIV() throws Exception { + for (String transform : transformations) { + try { + final CryptoCipher cipher = getCipher(transform); + Assert.assertNotNull(cipher); + + final byte[] invalidIV = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, + 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 }; + cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(KEY, "AES"), new IvParameterSpec(invalidIV)); + Assert.fail("Expected InvalidAlgorithmParameterException"); + } catch (final InvalidAlgorithmParameterException iape) { + } + } + } + + @Test + public void testInvalidIVClass() throws Exception { + for (String transform : transformations) { + try { + final CryptoCipher cipher = getCipher(transform); + Assert.assertNotNull(cipher); + + cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(KEY, "AES"), new GCMParameterSpec(IV.length, IV)); + Assert.fail("Should have caught an InvalidAlgorithmParameterException"); + } catch (final InvalidAlgorithmParameterException iape) { + } + } + } + private void byteBufferTest(final String transformation, final byte[] key, final byte[] iv, final ByteBuffer input, final ByteBuffer output) throws Exception { @@ -288,4 +350,4 @@ public abstract class AbstractCipherTest { ReflectionUtils.getClassByName(cipherClass), props, transformation); } -} \ No newline at end of file +} diff --git a/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java b/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java index 7578206..279ff3a 100644 --- a/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java +++ b/src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java @@ -28,6 +28,7 @@ import javax.crypto.NoSuchPaddingException; import javax.crypto.ShortBufferException; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; +import javax.crypto.spec.GCMParameterSpec; import org.junit.Assert; import org.junit.Assume; @@ -136,6 +137,14 @@ public class OpenSslCipherTest extends AbstractCipherTest { cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new IvParameterSpec(invalidIV)); } + @Test(expected = InvalidAlgorithmParameterException.class, timeout = 120000) + public void testInvalidIVClass() throws Exception { + final OpenSsl cipher = OpenSsl.getInstance("AES/CTR/NoPadding"); + Assert.assertNotNull(cipher); + + cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new GCMParameterSpec(IV.length, IV)); + } + @Test public void testCipherLifecycle() throws Exception { try (OpenSslCipher cipher = new OpenSslCipher(new Properties(), "AES/CTR/NoPadding")) { diff --git a/src/test/java/org/apache/commons/crypto/jna/OpenSslJnaCipherTest.java b/src/test/java/org/apache/commons/crypto/jna/OpenSslJnaCipherTest.java index d935e8b..b90e8a0 100644 --- a/src/test/java/org/apache/commons/crypto/jna/OpenSslJnaCipherTest.java +++ b/src/test/java/org/apache/commons/crypto/jna/OpenSslJnaCipherTest.java @@ -18,10 +18,11 @@ package org.apache.commons.crypto.jna; -import org.apache.commons.crypto.cipher.OpenSslCipherTest; +import org.apache.commons.crypto.cipher.AbstractCipherTest; import org.junit.Assume; +import org.junit.Test; -public class OpenSslJnaCipherTest extends OpenSslCipherTest { +public class OpenSslJnaCipherTest extends AbstractCipherTest { @Override public void init() { @@ -33,5 +34,4 @@ public class OpenSslJnaCipherTest extends OpenSslCipherTest { }; cipherClass = OpenSslJnaCipher.class.getName(); } - } diff --git a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java index e6747cb..26041f7 100644 --- a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java +++ b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java @@ -88,6 +88,22 @@ public class CryptoRandomFactoryTest { } @Test + public void testInvalidRandomClass() throws GeneralSecurityException { + final Properties properties = new Properties(); + properties.setProperty( + "org.apache.commons.crypto.cipher", + "OpenSsl"); + final CryptoRandom rand = CryptoRandomFactory.getCryptoRandom(properties); + Assert.assertEquals(OpenSslCryptoRandom.class.getName(), rand.getClass().getName()); + } + + @Test + public void testDefaultRandomClass() throws GeneralSecurityException { + final CryptoRandom rand = CryptoRandomFactory.getCryptoRandom(); + Assert.assertEquals(OpenSslCryptoRandom.class.getName(), rand.getClass().getName()); + } + + @Test public void testAbstractRandom() { final Properties props = new Properties(); props.setProperty(CryptoRandomFactory.CLASSES_KEY, AbstractRandom.class.getName()); diff --git a/src/test/java/org/apache/commons/crypto/utils/UtilsTest.java b/src/test/java/org/apache/commons/crypto/utils/UtilsTest.java index c35f2f6..5a60c55 100644 --- a/src/test/java/org/apache/commons/crypto/utils/UtilsTest.java +++ b/src/test/java/org/apache/commons/crypto/utils/UtilsTest.java @@ -20,6 +20,7 @@ package org.apache.commons.crypto.utils; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Properties; import org.junit.Assert; import org.junit.Test; @@ -37,8 +38,19 @@ public class UtilsTest { clazzNames = Utils.splitClassNames("a, b,", ","); Assert.assertEquals(Arrays.asList("a", "b"), clazzNames); } + @Test public void testSplitNull() { Assert.assertEquals(Collections.<String> emptyList(), Utils.splitClassNames(null, ",")); } + + @Test + public void testGetProperties() { + final Properties props = new Properties(); + props.setProperty( + "garbage.in", + "out"); + final Properties allprops = Utils.getProperties(props); + Assert.assertEquals(allprops.getProperty("garbage.in"), "out"); + } }