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+pirc...@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>.
