accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] 01/02: ACCUMULO-4731 Improved handling of key load failure (#315)
Date Sat, 20 Jan 2018 00:16:29 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 518c1093de504de77e237b8fdaca28067ce17174
Author: Nick Felts <31989480+PircDef@users.noreply.github.com>
AuthorDate: Fri Jan 19 19:09:50 2018 -0500

    ACCUMULO-4731 Improved handling of key load failure (#315)
    
    CachingHDFSSecretKeyEncryptionStrategy will now throw an exception of a key encryption
key cannot be loaded from file.
    The key encryption key file size is checked to verify the key length is correct.
    Sample key encryption key files were added for a unit test.
---
 .../CachingHDFSSecretKeyEncryptionStrategy.java    | 29 +++++++--
 .../crypto/SecretKeyEncryptionStrategy.java        |  4 +-
 .../accumulo/core/security/crypto/CryptoTest.java  | 73 +++++++++++++++++++++-
 3 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/CachingHDFSSecretKeyEncryptionStrategy.java
b/core/src/main/java/org/apache/accumulo/core/security/crypto/CachingHDFSSecretKeyEncryptionStrategy.java
index 1fa659a..58f1010 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/CachingHDFSSecretKeyEncryptionStrategy.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/CachingHDFSSecretKeyEncryptionStrategy.java
@@ -18,6 +18,7 @@ package org.apache.accumulo.core.security.crypto;
 
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
+import java.io.EOFException;
 import java.io.IOException;
 import java.security.InvalidKeyException;
 import java.security.Key;
@@ -45,13 +46,13 @@ public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncrypti
   private SecretKeyCache secretKeyCache = new SecretKeyCache();
 
   @Override
-  public CryptoModuleParameters encryptSecretKey(CryptoModuleParameters context) {
+  public CryptoModuleParameters encryptSecretKey(CryptoModuleParameters context) throws IOException
{
     try {
       secretKeyCache.ensureSecretKeyCacheInitialized(context);
       doKeyEncryptionOperation(Cipher.WRAP_MODE, context);
     } catch (IOException e) {
       log.error("{}", e.getMessage(), e);
-      throw new RuntimeException(e);
+      throw new IOException(e);
     }
     return context;
   }
@@ -128,11 +129,14 @@ public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncrypti
         pathToKeyName = Property.CRYPTO_DEFAULT_KEY_STRATEGY_KEY_LOCATION.getDefaultValue();
       }
 
-      // TODO ACCUMULO-2530 Ensure volumes a properly supported
+      // TODO ACCUMULO-2530 Ensure volumes are properly supported
       Path pathToKey = new Path(pathToKeyName);
       FileSystem fs = FileSystem.get(CachedConfiguration.getInstance());
 
       DataInputStream in = null;
+      boolean invalidFile = false;
+      int keyEncryptionKeyLength = 0;
+
       try {
         if (!fs.exists(pathToKey)) {
           initializeKeyEncryptionKey(fs, pathToKey, context);
@@ -140,14 +144,29 @@ public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncrypti
 
         in = fs.open(pathToKey);
 
-        int keyEncryptionKeyLength = in.readInt();
+        keyEncryptionKeyLength = in.readInt();
+        // If the file length does not correctly relate to the expected key size, there is
an inconsistency and
+        // we have no way of knowing the correct key length.
+        // The keyEncryptionKeyLength+4 accounts for the integer read from the file.
+        if (fs.getFileStatus(pathToKey).getLen() != keyEncryptionKeyLength + 4) {
+          invalidFile = true;
+	// Passing this exception forward so we can provide the more useful error message
+          throw new IOException();
+        }
         keyEncryptionKey = new byte[keyEncryptionKeyLength];
         in.readFully(keyEncryptionKey);
 
         initialized = true;
 
+      } catch (EOFException e) {
+        throw new IOException("Could not initialize key encryption cache, malformed key encryption
key file", e);
       } catch (IOException e) {
-        log.error("Could not initialize key encryption cache", e);
+        if (invalidFile) {
+          throw new IOException("Could not initialize key encryption cache, malformed key
encryption key file. Expected key of lengh " + keyEncryptionKeyLength
+              + " but file contained " + (fs.getFileStatus(pathToKey).getLen() - 4) + "bytes
for key encryption key.");
+        } else {
+          throw new IOException("Could not initialize key encryption cache, unable to access
or find key encryption key file", e);
+        }
       } finally {
         IOUtils.closeQuietly(in);
       }
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/SecretKeyEncryptionStrategy.java
b/core/src/main/java/org/apache/accumulo/core/security/crypto/SecretKeyEncryptionStrategy.java
index 7d3c333..8dfdee1 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/SecretKeyEncryptionStrategy.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/SecretKeyEncryptionStrategy.java
@@ -16,12 +16,14 @@
  */
 package org.apache.accumulo.core.security.crypto;
 
+import java.io.IOException;
+
 /**
  *
  */
 public interface SecretKeyEncryptionStrategy {
 
-  CryptoModuleParameters encryptSecretKey(CryptoModuleParameters params);
+  CryptoModuleParameters encryptSecretKey(CryptoModuleParameters params) throws IOException;
 
   CryptoModuleParameters decryptSecretKey(CryptoModuleParameters params);
 
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 a403f69..d4bf4ae 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
@@ -28,8 +28,11 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Files;
 import java.security.InvalidKeyException;
 import java.security.Key;
 import java.security.NoSuchAlgorithmException;
@@ -37,6 +40,7 @@ import java.security.NoSuchProviderException;
 import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.Map.Entry;
+import java.util.Random;
 
 import javax.crypto.AEADBadTagException;
 import javax.crypto.BadPaddingException;
@@ -50,6 +54,7 @@ import org.apache.accumulo.core.conf.ConfigurationCopy;
 import org.apache.accumulo.core.conf.DefaultConfiguration;
 import org.apache.accumulo.core.conf.Property;
 import org.apache.hadoop.conf.Configuration;
+import org.junit.AfterClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -63,6 +68,11 @@ public class CryptoTest {
   public static final String CRYPTO_ON_CONF = "crypto-on-accumulo-site.xml";
   public static final String CRYPTO_OFF_CONF = "crypto-off-accumulo-site.xml";
   public static final String CRYPTO_ON_KEK_OFF_CONF = "crypto-on-no-key-encryption-accumulo-site.xml";
+  
+  //Used for kek file testing
+  private static File kekWorks;
+  private static File kekTooLong;
+  private static File kekTooShort;
 
   @Rule
   public ExpectedException exception = ExpectedException.none();
@@ -378,6 +388,68 @@ public class CryptoTest {
   }
 
   @Test
+  public void testKeyEncryptionKeyCatchCorrectlyUsesValidKEKFile() throws IOException {
+    kekWorks = createKekFile("kekWorks.kek", 16);
+    testKekFile(kekWorks);
+  }
+
+  @Test
+  public void testKeyEncryptionKeyCacheCorrectlyFailsWithInvalidLongKEKFile() throws IOException
{
+    kekTooLong = createKekFile("kekTooLong.kek", 8);
+    exception.expect(IOException.class);
+    testKekFile(kekTooLong);
+  }
+
+  @Test
+  public void testKeyEncryptionKeyCacheCorrectlyFailsWithInvalidShortKEKFile() throws IOException
{
+    kekTooShort = createKekFile("kekTooShort.kek", 32);
+    exception.expect(IOException.class);
+    testKekFile(kekTooShort);
+  }
+
+  @AfterClass
+  public static void removeAllTestKekFiles() throws IOException {
+	Files.deleteIfExists(kekWorks.toPath());
+    Files.deleteIfExists(kekTooShort.toPath());
+    Files.deleteIfExists(kekTooLong.toPath());
+  }
+
+  // Used to check reading of KEK files
+  @SuppressWarnings("deprecation")
+  private void testKekFile(File testFile) throws IOException {
+	assertTrue(testFile.exists());
+	assertFalse(testFile.isDirectory());
+		
+    AccumuloConfiguration conf = setAndGetAccumuloConfig(CRYPTO_ON_CONF);
+    CryptoModuleParameters params = CryptoModuleFactory.createParamsObjectFromAccumuloConfiguration(conf);
+    // TODO ACCUMULO-2530 this will need to be fixed when CachingHDFSSecretKeyEncryptionStrategy
is fixed
+    params.getAllOptions().put(Property.INSTANCE_DFS_DIR.getKey(), testFile.getParent());
+    byte[] ptk = new byte[16];
+    params.setPlaintextKey(ptk);
+    CachingHDFSSecretKeyEncryptionStrategy skc = new CachingHDFSSecretKeyEncryptionStrategy();
+    params.getAllOptions().put(Property.CRYPTO_DEFAULT_KEY_STRATEGY_KEY_LOCATION.getKey(),
testFile.getName());
+    skc.encryptSecretKey(params);
+  }
+
+  private File createKekFile(String filename, Integer size) throws IOException {
+    File dir = new File(System.getProperty("user.dir") + "/target/cryptoTest");
+    boolean unused = dir.mkdirs(); // if the directories don't already exist, it'll return
1. If they do, 0. Both cases can be fine.
+
+    File testFile = File.createTempFile(filename, ".kek", dir);
+    DataOutputStream os = new DataOutputStream(new FileOutputStream(testFile));
+    Integer kl = 16;
+    byte[] key = new byte[kl];
+    Random rand = new Random();
+    rand.nextBytes(key);
+    os.writeInt(size);
+    os.write(key);
+    os.flush();
+    os.close();
+    
+    return testFile;
+    
+  }
+
   public void AESGCM_Encryption_Test_Correct_Encryption_And_Decryption() throws IOException,
AEADBadTagException {
     AccumuloConfiguration conf = setAndGetAccumuloConfig(CRYPTO_ON_CONF);
     byte[] encryptedBytes = testEncryption(conf, new byte[] {0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10, 20});
@@ -567,5 +639,4 @@ public class CryptoTest {
       return 0;
     }
   }
-
 }

-- 
To stop receiving notification emails like this one, please contact
"commits@accumulo.apache.org" <commits@accumulo.apache.org>.

Mime
View raw message