[GitHub] milleruntime closed pull request #756: Add additional crypto unit tests

[GitBox <gi...@apache.org>]

milleruntime closed pull request #756: Add additional crypto unit tests
URL: https://github.com/apache/accumulo/pull/756
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 6e6c838c01..16eb8e4d6a 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 static SecretKeySpec loadKekFromUri(String keyId) {
     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 8917469ef8..c51d091fb4 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 SamplerConfiguration getSamplerConfiguration() {
 
   @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 3c05df791e..23d788f8bc 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.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 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 void testMissingConfigProperties()
 
   @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 void testKeyManagerGeneratesKey() throws NoSuchAlgorithmException, NoSuch
   }
 
   @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 void testKeyManagerWrapAndUnwrap()
   }
 
   @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 void testKeyManagerLoadKekFromUri() throws IOException {
     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"));


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services