QPID-6017 : [Java Broker] add tests for AESKeyFileEncrypter

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1618840 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 224 insertions, 10 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypter.java
index c0c92f0389..b094ea96f9 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypter.java
@@ -36,17 +36,25 @@ import javax.crypto.SecretKey;
 import javax.crypto.spec.IvParameterSpec;
 import javax.xml.bind.DatatypeConverter;
 
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
-
 class AESKeyFileEncrypter implements ConfigurationSecretEncrypter
 {
     private static final String CIPHER_NAME = "AES/CBC/PKCS5Padding";
     private static final int AES_INITIALIZATION_VECTOR_LENGTH = 16;
+    private static final String AES_ALGORITHM = "AES";
     private final SecretKey _secretKey;
     private final SecureRandom _random = new SecureRandom();
 
     AESKeyFileEncrypter(SecretKey secretKey)
     {
+        if(secretKey == null)
+        {
+            throw new NullPointerException("A non null secret key must be supplied");
+        }
+        if(!AES_ALGORITHM.equals(secretKey.getAlgorithm()))
+        {
+            throw new IllegalArgumentException("Provided secret key was for the algorithm: " + secretKey.getAlgorithm()
+                                                + "when" + AES_ALGORITHM + "was needed.");
+        }
         _secretKey = secretKey;
     }
 
@@ -68,19 +76,26 @@ class AESKeyFileEncrypter implements ConfigurationSecretEncrypter
         }
         catch (IOException | InvalidAlgorithmParameterException | InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException e)
         {
-            throw new IllegalConfigurationException("Unable to encrypt secret", e);
+            throw new IllegalArgumentException("Unable to encrypt secret", e);
         }
     }
 
     @Override
     public String decrypt(final String encrypted)
     {
+        if(!isValidBase64(encrypted))
+        {
+            throw new IllegalArgumentException("Encrypted value is not valid Base 64 data: '" + encrypted + "'");
+        }
         byte[] encryptedBytes = DatatypeConverter.parseBase64Binary(encrypted);
         try
         {
             Cipher cipher = Cipher.getInstance(CIPHER_NAME);
-            cipher.init(Cipher.DECRYPT_MODE, _secretKey, new IvParameterSpec(encryptedBytes, 0,
-                                                                             AES_INITIALIZATION_VECTOR_LENGTH));
+
+            IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptedBytes, 0, AES_INITIALIZATION_VECTOR_LENGTH);
+
+            cipher.init(Cipher.DECRYPT_MODE, _secretKey, ivParameterSpec);
+
             return new String(readFromCipherStream(encryptedBytes,
                                                    AES_INITIALIZATION_VECTOR_LENGTH,
                                                    encryptedBytes.length - AES_INITIALIZATION_VECTOR_LENGTH,
@@ -88,10 +103,15 @@ class AESKeyFileEncrypter implements ConfigurationSecretEncrypter
         }
         catch (IOException | InvalidAlgorithmParameterException | InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException e)
         {
-            throw new IllegalConfigurationException("Unable to encrypt secret", e);
+            throw new IllegalArgumentException("Unable to encrypt secret", e);
         }
     }
 
+    private boolean isValidBase64(final String encrypted)
+    {
+        return encrypted.matches("^([\\w\\d+/]{4})*([\\w\\d+/]{2}==|[\\w\\d+/]{3}=)?$");
+    }
+
 
     private byte[] readFromCipherStream(final byte[] unencryptedBytes, final Cipher cipher) throws IOException
     {
@@ -106,16 +126,16 @@ class AESKeyFileEncrypter implements ConfigurationSecretEncrypter
                                                                                                   offset,
                                                                                                   length), cipher))
         {
-            byte[] buf = new byte[1024];
+            byte[] buf = new byte[512];
             int pos = 0;
             int read;
             while ((read = cipherInputStream.read(buf, pos, buf.length - pos)) != -1)
             {
                 pos += read;
-                if (pos == buf.length - 1)
+                if (pos == buf.length)
                 {
                     byte[] tmp = buf;
-                    buf = new byte[buf.length + 1024];
+                    buf = new byte[buf.length + 512];
                     System.arraycopy(tmp, 0, buf, 0, tmp.length);
                 }
             }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypterFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypterFactory.java
index 447f19b7ce..7a4394de1e 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypterFactory.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/encryption/AESKeyFileEncrypterFactory.java
@@ -52,7 +52,7 @@ public class AESKeyFileEncrypterFactory implements ConfigurationSecretEncrypterF
     private static final int AES_KEY_SIZE_BYTES = AES_KEY_SIZE_BITS / 8;
     private static final String AES_ALGORITHM = "AES";
 
-    public static String TYPE = "AESKeyFile";
+    public static final String TYPE = "AESKeyFile";
 
     @Override
     public ConfigurationSecretEncrypter createEncrypter(final ConfiguredObject<?> object)
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESFileEncrypterTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESFileEncrypterTest.java
new file mode 100644
index 0000000000..4105924507
--- /dev/null
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/encryption/AESFileEncrypterTest.java
@@ -0,0 +1,194 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.qpid.server.security.encryption;
+
+import java.nio.charset.StandardCharsets;
+import java.security.SecureRandom;
+import java.util.HashSet;
+import java.util.Random;
+import java.util.Set;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.qpid.test.utils.QpidTestCase;
+
+public class AESFileEncrypterTest extends QpidTestCase
+{
+    private final SecureRandom _random = new SecureRandom();
+    public static final String PLAINTEXT = "notaverygoodpassword";
+
+    public void testSimpleEncryptDecrypt() throws Exception
+    {
+        doTestSimpleEncryptDecrypt(PLAINTEXT);
+    }
+
+
+    public void testRepeatedEncryptionsReturnDifferentValues()
+    {
+        SecretKeySpec secretKey = createSecretKey();
+        AESKeyFileEncrypter encrypter = new AESKeyFileEncrypter(secretKey);
+
+        Set<String> encryptions = new HashSet<>();
+
+        int iterations = 100;
+
+        for(int i = 0; i < iterations; i++)
+        {
+            encryptions.add(encrypter.encrypt(PLAINTEXT));
+        }
+
+        assertEquals("Not all encryptions were distinct", iterations, encryptions.size());
+
+        for(String encrypted : encryptions)
+        {
+            assertEquals("Not all encryptions decrypt correctly", PLAINTEXT, encrypter.decrypt(encrypted));
+        }
+    }
+
+    public void testCreationFailsOnInvalidSecret() throws Exception
+    {
+        try
+        {
+            new AESKeyFileEncrypter(null);
+            fail("An encrypter should not be creatable from a null key");
+        }
+        catch(NullPointerException e)
+        {
+            // pass
+        }
+
+        try
+        {
+            PBEKeySpec keySpec = new PBEKeySpec("password".toCharArray());
+            SecretKeyFactory factory = SecretKeyFactory.getInstance("PBEWithMD5AndDES");
+            new AESKeyFileEncrypter(factory.generateSecret(keySpec));
+            fail("An encrypter should not be creatable from the wrong type of secret key");
+        }
+        catch (IllegalArgumentException e)
+        {
+            // pass
+        }
+    }
+
+    public void testEncryptionOfEmptyString()
+    {
+        String text = "";
+        doTestSimpleEncryptDecrypt(text);
+    }
+
+    private void doTestSimpleEncryptDecrypt(final String text)
+    {
+        SecretKeySpec secretKey = createSecretKey();
+        AESKeyFileEncrypter encrypter = new AESKeyFileEncrypter(secretKey);
+
+        String encrypted = encrypter.encrypt(text);
+        assertNotNull("Encrypter did not return a result from encryption", encrypted);
+        assertFalse("Plain text and encrypted version are equal", text.equals(encrypted));
+        String decrypted = encrypter.decrypt(encrypted);
+        assertNotNull("Encrypter did not return a result from decryption",decrypted);
+        assertTrue("Encryption was not reversible", text.equals(decrypted));
+    }
+
+    public void testEncryptingNullFails()
+    {
+        try
+        {
+            SecretKeySpec secretKey = createSecretKey();
+            AESKeyFileEncrypter encrypter = new AESKeyFileEncrypter(secretKey);
+
+            String encrypted = encrypter.encrypt(null);
+            fail("Attempting to encrypt null should fail");
+        }
+        catch(NullPointerException e)
+        {
+            // pass
+        }
+    }
+
+    public void testEncryptingVeryLargeSecret()
+    {
+        Random random = new Random();
+        byte[] data = new byte[4096];
+        random.nextBytes(data);
+        for(int i = 0; i < data.length; i++)
+        {
+            data[i] = (byte)(data[i] & 0xEF);
+        }
+        doTestSimpleEncryptDecrypt(new String(data, StandardCharsets.US_ASCII));
+    }
+
+    public void testDecryptNonsense()
+    {
+
+        SecretKeySpec secretKey = createSecretKey();
+        AESKeyFileEncrypter encrypter = new AESKeyFileEncrypter(secretKey);
+
+
+        try
+        {
+            encrypter.decrypt(null);
+            fail("Should not decrypt a null value");
+        }
+        catch(NullPointerException e)
+        {
+            // pass
+        }
+
+        try
+        {
+            encrypter.decrypt("");
+            fail("Should not decrypt the empty String");
+        }
+        catch(IllegalArgumentException e)
+        {
+            // pass
+        }
+
+        try
+        {
+            encrypter.decrypt("thisisnonsense");
+            fail("Should not decrypt a small amount of nonsense");
+        }
+        catch(IllegalArgumentException e)
+        {
+            // pass
+        }
+
+        try
+        {
+            String answer = encrypter.decrypt("thisisn'tvalidBase64!soitshouldfailwithanIllegalArgumentException");
+            fail("Should not decrypt a larger amount of nonsense");
+        }
+        catch(IllegalArgumentException e)
+        {
+            // pass
+        }
+    }
+
+    private SecretKeySpec createSecretKey()
+    {
+        final byte[] keyData = new byte[32];
+        _random.nextBytes(keyData);
+        return new SecretKeySpec(keyData, "AES");
+    }
+}