[accumulo] branch master updated: Added additional crypto unit tests (#756)

[mm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mmiller pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
     new 95214d4  Added additional crypto unit tests (#756)
95214d4 is described below

commit 95214d4255e96a40601c3162a6f79cb956e1df5a
Author: Nick Felts <31...@users.noreply.github.com>
AuthorDate: Thu Nov 8 13:41:07 2018 -0500

    Added additional crypto unit tests (#756)
    
    * Test for missing KEK uri file
    * Test for empty KEK uri file
    * Test for unwrapping with an incorrect KEK
    * Added IllegalArgumentException to catch in AESKeyUtils.loadKekFromUri to provide more useful error message
---
 .../core/security/crypto/impl/AESKeyUtils.java     |  2 +-
 .../apache/accumulo/core/file/rfile/RFileTest.java |  2 +-
 .../accumulo/core/security/crypto/CryptoTest.java  | 43 +++++++++++++++++++---
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESKeyUtils.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESKeyUtils.java
index 6e6c838..16eb8e4 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESKeyUtils.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESKeyUtils.java
@@ -83,7 +83,7 @@ public class AESKeyUtils {
     try {
       uri = new URI(keyId);
       key = new SecretKeySpec(Files.readAllBytes(Paths.get(uri.getPath())), "AES");
-    } catch (URISyntaxException | IOException e) {
+    } catch (URISyntaxException | IOException | IllegalArgumentException e) {
       throw new CryptoException("Unable to load key encryption key.", e);
     }
 
diff --git a/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java b/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
index 8917469..c51d091 100644
--- a/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
@@ -128,7 +128,7 @@ public class RFileTest {
 
   @BeforeClass
   public static void setupCryptoKeyFile() throws Exception {
-    CryptoTest.setupKeyFile();
+    CryptoTest.setupKeyFiles();
   }
 
   static class SeekableByteArrayInputStream extends ByteArrayInputStream
diff --git a/core/src/test/java/org/apache/accumulo/core/security/crypto/CryptoTest.java b/core/src/test/java/org/apache/accumulo/core/security/crypto/CryptoTest.java
index 3c05df7..23d788f 100644
--- a/core/src/test/java/org/apache/accumulo/core/security/crypto/CryptoTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/security/crypto/CryptoTest.java
@@ -62,6 +62,7 @@ import org.apache.accumulo.core.security.crypto.streams.NoFlushOutputStream;
 import org.apache.accumulo.core.spi.crypto.CryptoEnvironment;
 import org.apache.accumulo.core.spi.crypto.CryptoEnvironment.Scope;
 import org.apache.accumulo.core.spi.crypto.CryptoService;
+import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException;
 import org.apache.accumulo.core.spi.crypto.FileDecrypter;
 import org.apache.accumulo.core.spi.crypto.FileEncrypter;
 import org.apache.accumulo.core.util.CachedConfiguration;
@@ -87,17 +88,20 @@ public class CryptoTest {
   public static final String CRYPTO_OFF_CONF = "OFF";
   public static final String keyPath = System.getProperty("user.dir")
       + "/target/CryptoTest-testkeyfile";
+  public static final String emptyKeyPath = System.getProperty("user.dir")
+      + "/target/CryptoTest-emptykeyfile";
 
   @Rule
   public ExpectedException exception = ExpectedException.none();
 
   @BeforeClass
-  public static void setupKeyFile() throws Exception {
+  public static void setupKeyFiles() throws Exception {
     FileSystem fs = FileSystem.getLocal(CachedConfiguration.getInstance());
     Path aesPath = new Path(keyPath);
     try (FSDataOutputStream out = fs.create(aesPath)) {
       out.writeUTF("sixteenbytekey"); // 14 + 2 from writeUTF
     }
+    FSDataOutputStream out = fs.create(new Path(emptyKeyPath));
   }
 
   @Test
@@ -278,8 +282,8 @@ public class CryptoTest {
 
   @SuppressFBWarnings(value = "CIPHER_INTEGRITY", justification = "CBC is being tested")
   @Test
-  public void testKeyManagerGeneratesKey() throws NoSuchAlgorithmException, NoSuchProviderException,
-      NoSuchPaddingException, InvalidKeyException {
+  public void testAESKeyUtilsGeneratesKey() throws NoSuchAlgorithmException,
+      NoSuchProviderException, NoSuchPaddingException, InvalidKeyException {
     SecureRandom sr = SecureRandom.getInstance("SHA1PRNG", "SUN");
     java.security.Key key;
     key = AESKeyUtils.generateKey(sr, 16);
@@ -294,7 +298,7 @@ public class CryptoTest {
   }
 
   @Test
-  public void testKeyManagerWrapAndUnwrap()
+  public void testAESKeyUtilsWrapAndUnwrap()
       throws NoSuchAlgorithmException, NoSuchProviderException {
     SecureRandom sr = SecureRandom.getInstance("SHA1PRNG", "SUN");
     java.security.Key kek = AESKeyUtils.generateKey(sr, 16);
@@ -306,7 +310,22 @@ public class CryptoTest {
   }
 
   @Test
-  public void testKeyManagerLoadKekFromUri() throws IOException {
+  public void testAESKeyUtilsFailUnwrapWithWrongKEK()
+      throws NoSuchAlgorithmException, NoSuchProviderException {
+    SecureRandom sr = SecureRandom.getInstance("SHA1PRNG", "SUN");
+    java.security.Key kek = AESKeyUtils.generateKey(sr, 16);
+    java.security.Key fek = AESKeyUtils.generateKey(sr, 16);
+    byte[] wrongBytes = kek.getEncoded();
+    wrongBytes[0]++;
+    java.security.Key wrongKek = new SecretKeySpec(wrongBytes, "AES");
+
+    byte[] wrapped = AESKeyUtils.wrapKey(fek, kek);
+    exception.expect(CryptoException.class);
+    java.security.Key unwrapped = AESKeyUtils.unwrapKey(wrapped, wrongKek);
+  }
+
+  @Test
+  public void testAESKeyUtilsLoadKekFromUri() throws IOException {
     SecretKeySpec fileKey = AESKeyUtils.loadKekFromUri(keyPath);
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
     DataOutputStream dos = new DataOutputStream(baos);
@@ -315,6 +334,20 @@ public class CryptoTest {
     assertEquals(fileKey, handKey);
   }
 
+  @Test
+  public void testAESKeyUtilsLoadKekFromUriInvalidUri() {
+    exception.expect(CryptoException.class);
+    SecretKeySpec fileKey = AESKeyUtils.loadKekFromUri(
+        System.getProperty("user.dir") + "/target/CryptoTest-testkeyfile-doesnt-exist");
+  }
+
+  @Test
+  public void testAESKeyUtilsLoadKekFromEmptyFile() {
+    exception.expect(CryptoException.class);
+    SecretKeySpec fileKey = AESKeyUtils.loadKekFromUri(emptyKeyPath);
+
+  }
+
   private ArrayList<Key> testData() {
     ArrayList<Key> keys = new ArrayList<>();
     keys.add(new Key("a", "cf", "cq"));