You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by vi...@apache.org on 2014/01/16 23:44:09 UTC

[1/2] git commit: ACCUMULO-2209 Removing DefaultKeyEncryptionStrategy and having explicitly named strategies

Updated Branches:
  refs/heads/1.6.0-SNAPSHOT 08a980497 -> f4409d9d8


ACCUMULO-2209 Removing DefaultKeyEncryptionStrategy and having explicitly named strategies


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/f4409d9d
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/f4409d9d
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/f4409d9d

Branch: refs/heads/1.6.0-SNAPSHOT
Commit: f4409d9d8f6b843893cf6b6f5ba39306f0c6975f
Parents: 9ba06ff
Author: John Vines <vi...@apache.org>
Authored: Mon Jan 13 14:41:29 2014 -0500
Committer: John Vines <vi...@apache.org>
Committed: Thu Jan 16 17:43:51 2014 -0500

----------------------------------------------------------------------
 conf/examples/crypto/accumulo-site.xml          |   2 +-
 .../CachingHDFSSecretKeyEncryptionStrategy.java |   3 +
 .../security/crypto/CryptoModuleParameters.java |   2 +-
 .../DefaultSecretKeyEncryptionStrategy.java     | 188 -------------------
 .../NonCachingSecretKeyEncryptionStrategy.java  | 188 +++++++++++++++++++
 .../core/security/crypto/CryptoTest.java        |   8 +-
 .../test/resources/crypto-on-accumulo-site.xml  |   2 +-
 7 files changed, 198 insertions(+), 195 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/conf/examples/crypto/accumulo-site.xml
----------------------------------------------------------------------
diff --git a/conf/examples/crypto/accumulo-site.xml b/conf/examples/crypto/accumulo-site.xml
index ca47f9a..a7e5c54 100644
--- a/conf/examples/crypto/accumulo-site.xml
+++ b/conf/examples/crypto/accumulo-site.xml
@@ -146,7 +146,7 @@
 
     <property>
       <name>crypto.secret.key.encryption.strategy.class</name>
-      <value>org.apache.accumulo.core.security.crypto.DefaultSecretKeyEncryptionStrategy</value>
+      <value>org.apache.accumulo.core.security.crypto.CachingHDFSSecretKeyEncryptionStrategy</value>
     </property>
 
     <property>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/core/src/main/java/org/apache/accumulo/core/security/crypto/CachingHDFSSecretKeyEncryptionStrategy.java
----------------------------------------------------------------------
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 4a8e0ed..5b0b361 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
@@ -35,6 +35,9 @@ import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.log4j.Logger;
 
+/**
+ * A {@link SecretKeyEncryptionStrategy} that gets its key from HDFS and caches it for IO.
+ */
 public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncryptionStrategy {
   
   private static final Logger log = Logger.getLogger(CachingHDFSSecretKeyEncryptionStrategy.class);

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleParameters.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleParameters.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleParameters.java
index 64e88d3..5ae072a 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleParameters.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleParameters.java
@@ -247,7 +247,7 @@ public class CryptoModuleParameters {
    * Sets the class name of the key encryption strategy class. The class obeys the {@link SecretKeyEncryptionStrategy} interface. It instructs the
    * {@link DefaultCryptoModule} on how to encrypt the keys it uses to secure the streams.
    * <p>
-   * The default implementation of this interface, {@link DefaultSecretKeyEncryptionStrategy}, creates a random key encryption key (KEK) as another symmetric
+   * The default implementation of this interface, {@link CachingHDFSSecretKeyEncryptionStrategy}, creates a random key encryption key (KEK) as another symmetric
    * key and places the KEK into HDFS. <i>This is not really very secure.</i> Users of the crypto modules are encouraged to either safeguard that KEK carefully
    * or to obtain and use another {@link SecretKeyEncryptionStrategy} class.
    * <p>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultSecretKeyEncryptionStrategy.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultSecretKeyEncryptionStrategy.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultSecretKeyEncryptionStrategy.java
deleted file mode 100644
index f0ece50..0000000
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultSecretKeyEncryptionStrategy.java
+++ /dev/null
@@ -1,188 +0,0 @@
-/*
- * 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.accumulo.core.security.crypto;
-
-import java.io.DataInputStream;
-import java.io.DataOutputStream;
-import java.io.IOException;
-import java.security.InvalidKeyException;
-import java.security.Key;
-import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
-
-import javax.crypto.Cipher;
-import javax.crypto.IllegalBlockSizeException;
-import javax.crypto.spec.SecretKeySpec;
-
-import org.apache.accumulo.core.conf.Property;
-import org.apache.accumulo.core.util.CachedConfiguration;
-import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.Path;
-import org.apache.log4j.Logger;
-
-public class DefaultSecretKeyEncryptionStrategy implements SecretKeyEncryptionStrategy {
-  
-  private static final Logger log = Logger.getLogger(DefaultSecretKeyEncryptionStrategy.class);
-
-  private void doKeyEncryptionOperation(int encryptionMode, CryptoModuleParameters params, String pathToKeyName, Path pathToKey, FileSystem fs)
-      throws IOException {
-    DataInputStream in = null;
-    try {
-      if (!fs.exists(pathToKey)) {
-        
-        if (encryptionMode == Cipher.UNWRAP_MODE) {
-          log.error("There was a call to decrypt the session key but no key encryption key exists.  Either restore it, reconfigure the conf file to point to it in HDFS, or throw the affected data away and begin again.");
-          throw new RuntimeException("Could not find key encryption key file in configured location in HDFS ("+pathToKeyName+")");
-        } else {
-          DataOutputStream out = null;
-          try {
-            out = fs.create(pathToKey);
-            // Very important, lets hedge our bets
-            fs.setReplication(pathToKey, (short) 5);
-            SecureRandom random = DefaultCryptoModuleUtils.getSecureRandom(params.getRandomNumberGenerator(), params.getRandomNumberGeneratorProvider());
-            int keyLength = params.getKeyLength();
-            byte[] newRandomKeyEncryptionKey = new byte[keyLength / 8];
-            random.nextBytes(newRandomKeyEncryptionKey);
-            out.writeInt(newRandomKeyEncryptionKey.length);
-            out.write(newRandomKeyEncryptionKey);
-            out.flush();
-          } finally {
-            if (out != null) {
-              out.close();        
-            }
-          }
-
-        }
-      }
-      in = fs.open(pathToKey);
-            
-      int keyEncryptionKeyLength = in.readInt();
-      byte[] keyEncryptionKey = new byte[keyEncryptionKeyLength];
-      in.read(keyEncryptionKey);
-      
-      Cipher cipher = DefaultCryptoModuleUtils.getCipher(params.getAllOptions().get(Property.CRYPTO_DEFAULT_KEY_STRATEGY_CIPHER_SUITE.getKey()));
-
-      try {
-        cipher.init(encryptionMode, new SecretKeySpec(keyEncryptionKey, params.getAlgorithmName()));
-      } catch (InvalidKeyException e) {
-        log.error(e);
-        throw new RuntimeException(e);
-      }      
-      
-      if (Cipher.UNWRAP_MODE == encryptionMode) {
-        try {
-          Key plaintextKey = cipher.unwrap(params.getEncryptedKey(), params.getAlgorithmName(), Cipher.SECRET_KEY);
-          params.setPlaintextKey(plaintextKey.getEncoded());
-        } catch (InvalidKeyException e) {
-          log.error(e);
-          throw new RuntimeException(e);
-        } catch (NoSuchAlgorithmException e) {
-          log.error(e);
-          throw new RuntimeException(e);
-        }
-      } else {
-        Key plaintextKey = new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName());
-        try {
-          byte[] encryptedSecretKey = cipher.wrap(plaintextKey);
-          params.setEncryptedKey(encryptedSecretKey);
-          params.setOpaqueKeyEncryptionKeyID(pathToKeyName);
-        } catch (InvalidKeyException e) {
-          log.error(e);
-          throw new RuntimeException(e);
-        } catch (IllegalBlockSizeException e) {
-          log.error(e);
-          throw new RuntimeException(e);
-        }
-        
-      }
-      
-    } finally {
-      if (in != null) {
-        in.close();
-      }
-    }
-  }
-
-
-  private String getFullPathToKey(CryptoModuleParameters params) {
-    String pathToKeyName = params.getAllOptions().get(Property.CRYPTO_DEFAULT_KEY_STRATEGY_KEY_LOCATION.getKey());
-    String instanceDirectory = params.getAllOptions().get(Property.INSTANCE_DFS_DIR.getKey());
-    
-    
-    if (pathToKeyName == null) {
-      pathToKeyName = Property.CRYPTO_DEFAULT_KEY_STRATEGY_KEY_LOCATION.getDefaultValue();
-    }
-    
-    if (instanceDirectory == null) {
-      instanceDirectory = Property.INSTANCE_DFS_DIR.getDefaultValue();
-    }
-    
-    if (!pathToKeyName.startsWith("/")) {
-      pathToKeyName = "/" + pathToKeyName;
-    }
-    
-    String fullPath = instanceDirectory + pathToKeyName;
-    return fullPath;
-  }
-  
-  @Override
-  public CryptoModuleParameters encryptSecretKey(CryptoModuleParameters params) {
-    String hdfsURI = params.getAllOptions().get(Property.INSTANCE_DFS_URI.getKey());
-    if (hdfsURI == null) {
-      hdfsURI = Property.INSTANCE_DFS_URI.getDefaultValue();
-    }
-    
-    String fullPath = getFullPathToKey(params);
-    Path pathToKey = new Path(fullPath);
-    
-    try {
-      FileSystem fs = FileSystem.get(CachedConfiguration.getInstance());   
-      doKeyEncryptionOperation(Cipher.WRAP_MODE, params, fullPath, pathToKey, fs);
-      
-    } catch (IOException e) {
-      log.error(e);
-      throw new RuntimeException(e);
-    }
-    
-    return params;
-  }
-  
-  @Override
-  public CryptoModuleParameters decryptSecretKey(CryptoModuleParameters params) {
-    String hdfsURI = params.getAllOptions().get(Property.INSTANCE_DFS_URI.getKey());
-    if (hdfsURI == null) {
-      hdfsURI = Property.INSTANCE_DFS_URI.getDefaultValue(); 
-    }
-    
-    String pathToKeyName = getFullPathToKey(params);
-    Path pathToKey = new Path(pathToKeyName);
-    
-    try {
-      FileSystem fs = FileSystem.get(CachedConfiguration.getInstance());   
-      doKeyEncryptionOperation(Cipher.UNWRAP_MODE, params, pathToKeyName, pathToKey, fs);
-      
-      
-    } catch (IOException e) {
-      log.error(e);
-      throw new RuntimeException(e);
-    }
-        
-    return params;
-  }
-  
-}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/core/src/main/java/org/apache/accumulo/core/security/crypto/NonCachingSecretKeyEncryptionStrategy.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/NonCachingSecretKeyEncryptionStrategy.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/NonCachingSecretKeyEncryptionStrategy.java
new file mode 100644
index 0000000..e72fd06
--- /dev/null
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/NonCachingSecretKeyEncryptionStrategy.java
@@ -0,0 +1,188 @@
+/*
+ * 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.accumulo.core.security.crypto;
+
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.security.InvalidKeyException;
+import java.security.Key;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+
+import javax.crypto.Cipher;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.util.CachedConfiguration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.log4j.Logger;
+
+public class NonCachingSecretKeyEncryptionStrategy implements SecretKeyEncryptionStrategy {
+  
+  private static final Logger log = Logger.getLogger(NonCachingSecretKeyEncryptionStrategy.class);
+
+  private void doKeyEncryptionOperation(int encryptionMode, CryptoModuleParameters params, String pathToKeyName, Path pathToKey, FileSystem fs)
+      throws IOException {
+    DataInputStream in = null;
+    try {
+      if (!fs.exists(pathToKey)) {
+        
+        if (encryptionMode == Cipher.UNWRAP_MODE) {
+          log.error("There was a call to decrypt the session key but no key encryption key exists.  Either restore it, reconfigure the conf file to point to it in HDFS, or throw the affected data away and begin again.");
+          throw new RuntimeException("Could not find key encryption key file in configured location in HDFS ("+pathToKeyName+")");
+        } else {
+          DataOutputStream out = null;
+          try {
+            out = fs.create(pathToKey);
+            // Very important, lets hedge our bets
+            fs.setReplication(pathToKey, (short) 5);
+            SecureRandom random = DefaultCryptoModuleUtils.getSecureRandom(params.getRandomNumberGenerator(), params.getRandomNumberGeneratorProvider());
+            int keyLength = params.getKeyLength();
+            byte[] newRandomKeyEncryptionKey = new byte[keyLength / 8];
+            random.nextBytes(newRandomKeyEncryptionKey);
+            out.writeInt(newRandomKeyEncryptionKey.length);
+            out.write(newRandomKeyEncryptionKey);
+            out.flush();
+          } finally {
+            if (out != null) {
+              out.close();        
+            }
+          }
+
+        }
+      }
+      in = fs.open(pathToKey);
+            
+      int keyEncryptionKeyLength = in.readInt();
+      byte[] keyEncryptionKey = new byte[keyEncryptionKeyLength];
+      in.read(keyEncryptionKey);
+      
+      Cipher cipher = DefaultCryptoModuleUtils.getCipher(params.getAllOptions().get(Property.CRYPTO_DEFAULT_KEY_STRATEGY_CIPHER_SUITE.getKey()));
+
+      try {
+        cipher.init(encryptionMode, new SecretKeySpec(keyEncryptionKey, params.getAlgorithmName()));
+      } catch (InvalidKeyException e) {
+        log.error(e);
+        throw new RuntimeException(e);
+      }      
+      
+      if (Cipher.UNWRAP_MODE == encryptionMode) {
+        try {
+          Key plaintextKey = cipher.unwrap(params.getEncryptedKey(), params.getAlgorithmName(), Cipher.SECRET_KEY);
+          params.setPlaintextKey(plaintextKey.getEncoded());
+        } catch (InvalidKeyException e) {
+          log.error(e);
+          throw new RuntimeException(e);
+        } catch (NoSuchAlgorithmException e) {
+          log.error(e);
+          throw new RuntimeException(e);
+        }
+      } else {
+        Key plaintextKey = new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName());
+        try {
+          byte[] encryptedSecretKey = cipher.wrap(plaintextKey);
+          params.setEncryptedKey(encryptedSecretKey);
+          params.setOpaqueKeyEncryptionKeyID(pathToKeyName);
+        } catch (InvalidKeyException e) {
+          log.error(e);
+          throw new RuntimeException(e);
+        } catch (IllegalBlockSizeException e) {
+          log.error(e);
+          throw new RuntimeException(e);
+        }
+        
+      }
+      
+    } finally {
+      if (in != null) {
+        in.close();
+      }
+    }
+  }
+
+
+  private String getFullPathToKey(CryptoModuleParameters params) {
+    String pathToKeyName = params.getAllOptions().get(Property.CRYPTO_DEFAULT_KEY_STRATEGY_KEY_LOCATION.getKey());
+    String instanceDirectory = params.getAllOptions().get(Property.INSTANCE_DFS_DIR.getKey());
+    
+    
+    if (pathToKeyName == null) {
+      pathToKeyName = Property.CRYPTO_DEFAULT_KEY_STRATEGY_KEY_LOCATION.getDefaultValue();
+    }
+    
+    if (instanceDirectory == null) {
+      instanceDirectory = Property.INSTANCE_DFS_DIR.getDefaultValue();
+    }
+    
+    if (!pathToKeyName.startsWith("/")) {
+      pathToKeyName = "/" + pathToKeyName;
+    }
+    
+    String fullPath = instanceDirectory + pathToKeyName;
+    return fullPath;
+  }
+  
+  @Override
+  public CryptoModuleParameters encryptSecretKey(CryptoModuleParameters params) {
+    String hdfsURI = params.getAllOptions().get(Property.INSTANCE_DFS_URI.getKey());
+    if (hdfsURI == null) {
+      hdfsURI = Property.INSTANCE_DFS_URI.getDefaultValue();
+    }
+    
+    String fullPath = getFullPathToKey(params);
+    Path pathToKey = new Path(fullPath);
+    
+    try {
+      FileSystem fs = FileSystem.get(CachedConfiguration.getInstance());   
+      doKeyEncryptionOperation(Cipher.WRAP_MODE, params, fullPath, pathToKey, fs);
+      
+    } catch (IOException e) {
+      log.error(e);
+      throw new RuntimeException(e);
+    }
+    
+    return params;
+  }
+  
+  @Override
+  public CryptoModuleParameters decryptSecretKey(CryptoModuleParameters params) {
+    String hdfsURI = params.getAllOptions().get(Property.INSTANCE_DFS_URI.getKey());
+    if (hdfsURI == null) {
+      hdfsURI = Property.INSTANCE_DFS_URI.getDefaultValue(); 
+    }
+    
+    String pathToKeyName = getFullPathToKey(params);
+    Path pathToKey = new Path(pathToKeyName);
+    
+    try {
+      FileSystem fs = FileSystem.get(CachedConfiguration.getInstance());   
+      doKeyEncryptionOperation(Cipher.UNWRAP_MODE, params, pathToKeyName, pathToKey, fs);
+      
+      
+    } catch (IOException e) {
+      log.error(e);
+      throw new RuntimeException(e);
+    }
+        
+    return params;
+  }
+  
+}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/core/src/test/java/org/apache/accumulo/core/security/crypto/CryptoTest.java
----------------------------------------------------------------------
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 14c39b6..963f41c 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
@@ -101,7 +101,7 @@ public class CryptoTest {
     assertEquals(128, params.getKeyLength());
     assertEquals("SHA1PRNG", params.getRandomNumberGenerator());
     assertEquals("SUN", params.getRandomNumberGeneratorProvider());
-    assertEquals("org.apache.accumulo.core.security.crypto.DefaultSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
+    assertEquals("org.apache.accumulo.core.security.crypto.CachingHDFSSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
     
     restoreOldConfiguration(oldSiteConfigProperty, conf);    
   }
@@ -241,7 +241,7 @@ public class CryptoTest {
 
     assertTrue(cryptoModule instanceof DefaultCryptoModule);
     assertNotNull(params.getKeyEncryptionStrategyClass());
-    assertEquals("org.apache.accumulo.core.security.crypto.DefaultSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
+    assertEquals("org.apache.accumulo.core.security.crypto.CachingHDFSSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
     
     byte[] resultingBytes = setUpSampleEncryptedBytes(cryptoModule, params);
 
@@ -279,7 +279,7 @@ public class CryptoTest {
 
     assertTrue(cryptoModule instanceof DefaultCryptoModule);
     assertNotNull(params.getKeyEncryptionStrategyClass());
-    assertEquals("org.apache.accumulo.core.security.crypto.DefaultSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
+    assertEquals("org.apache.accumulo.core.security.crypto.CachingHDFSSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
     
     byte[] resultingBytes = setUpSampleEncryptedBytes(cryptoModule, params);
 
@@ -313,7 +313,7 @@ public class CryptoTest {
 
     assertTrue(cryptoModule instanceof DefaultCryptoModule);
     assertNotNull(params.getKeyEncryptionStrategyClass());
-    assertEquals("org.apache.accumulo.core.security.crypto.DefaultSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
+    assertEquals("org.apache.accumulo.core.security.crypto.CachingHDFSSecretKeyEncryptionStrategy", params.getKeyEncryptionStrategyClass());
     
     byte[] resultingBytes = setUpSampleEncryptedBytes(cryptoModule, params);
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f4409d9d/core/src/test/resources/crypto-on-accumulo-site.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/crypto-on-accumulo-site.xml b/core/src/test/resources/crypto-on-accumulo-site.xml
index 6200c48..3da7437 100644
--- a/core/src/test/resources/crypto-on-accumulo-site.xml
+++ b/core/src/test/resources/crypto-on-accumulo-site.xml
@@ -134,7 +134,7 @@
     </property>
     <property>
       <name>crypto.secret.key.encryption.strategy.class</name>
-      <value>org.apache.accumulo.core.security.crypto.DefaultSecretKeyEncryptionStrategy</value>
+      <value>org.apache.accumulo.core.security.crypto.CachingHDFSSecretKeyEncryptionStrategy</value>
     </property>
     <property>
       <name>instance.dfs.dir</name>


[2/2] git commit: ACCUMULO-1998 Resolving corner cases around blocked output stream behavior and migration

Posted by vi...@apache.org.
ACCUMULO-1998 Resolving corner cases around blocked output stream behavior and migration


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/9ba06ff2
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/9ba06ff2
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/9ba06ff2

Branch: refs/heads/1.6.0-SNAPSHOT
Commit: 9ba06ff2d515d982b47b4f121a14bb5a98a024f0
Parents: 08a9804
Author: John Vines <vi...@apache.org>
Authored: Thu Jan 9 17:35:02 2014 -0500
Committer: John Vines <vi...@apache.org>
Committed: Thu Jan 16 17:43:51 2014 -0500

----------------------------------------------------------------------
 .../security/crypto/BlockedOutputStream.java    |  6 +--
 .../security/crypto/CryptoModuleFactory.java    |  4 +-
 .../security/crypto/DefaultCryptoModule.java    |  4 +-
 .../security/crypto/BlockedIOStreamTest.java    | 44 ++++++++++++++++++++
 .../apache/accumulo/tserver/log/DfsLogger.java  | 12 +++---
 5 files changed, 58 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/9ba06ff2/core/src/main/java/org/apache/accumulo/core/security/crypto/BlockedOutputStream.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/BlockedOutputStream.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/BlockedOutputStream.java
index 9ca00b7..1f3cf3b 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/BlockedOutputStream.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/BlockedOutputStream.java
@@ -44,7 +44,7 @@ public class BlockedOutputStream extends OutputStream {
   }
 
   @Override
-  public void flush() throws IOException {
+  public synchronized void flush() throws IOException {
     int size = bb.position();
     if (size == 0)
       return;
@@ -64,9 +64,10 @@ public class BlockedOutputStream extends OutputStream {
 
   @Override
   public void write(int b) throws IOException {
-    bb.put((byte) b);
+    // Checking before provides same functionality but causes the case of previous flush() failing to now throw a buffer out of bounds error
     if (bb.remaining() == 0)
       flush();
+    bb.put((byte) b);
   }
 
   @Override
@@ -90,7 +91,6 @@ public class BlockedOutputStream extends OutputStream {
   @Override
   public void close() throws IOException {
     flush();
-    bb = null;
     out.close();
   }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9ba06ff2/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
index 65acc6b..3fd43a0 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
@@ -287,7 +287,9 @@ public class CryptoModuleFactory {
     params.setPadding(cipherTransformParts[2]);
     params.setRandomNumberGenerator(cryptoOpts.get(Property.CRYPTO_SECURE_RNG.getKey()));
     params.setRandomNumberGeneratorProvider(cryptoOpts.get(Property.CRYPTO_SECURE_RNG_PROVIDER.getKey()));
-    params.setBlockStreamSize(Integer.parseInt(cryptoOpts.get(Property.CRYPTO_BLOCK_STREAM_SIZE.getKey())));
+    String blockStreamSize = cryptoOpts.get(Property.CRYPTO_BLOCK_STREAM_SIZE.getKey());
+    if (blockStreamSize != null)
+      params.setBlockStreamSize(Integer.parseInt(blockStreamSize));
 
     return params;
   }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9ba06ff2/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultCryptoModule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultCryptoModule.java b/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultCryptoModule.java
index 347887c..dfad05e 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultCryptoModule.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/DefaultCryptoModule.java
@@ -355,7 +355,7 @@ public class DefaultCryptoModule implements CryptoModule {
         if (marker.equals(ENCRYPTION_HEADER_MARKER_V2))
           params.setBlockStreamSize(dataIn.readInt());
         else
-          params.setBlockStreamSize(-1);
+          params.setBlockStreamSize(0);
       } else {
         
         log.trace("Read something off of the encrypted input stream that was not the encryption header marker, so pushing back bytes and returning the given stream");
@@ -398,7 +398,7 @@ public class DefaultCryptoModule implements CryptoModule {
     
     InputStream blockedDecryptingInputStream = new CipherInputStream(params.getEncryptedInputStream(), cipher);
     
-    if (params.getBlockStreamSize() != -1)
+    if (params.getBlockStreamSize() > 0)
       blockedDecryptingInputStream = new BlockedInputStream(blockedDecryptingInputStream, cipher.getBlockSize(), params.getBlockStreamSize());
 
     log.trace("Initialized cipher input stream with transformation ["+getCipherTransformation(params)+"]");

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9ba06ff2/core/src/test/java/org/apache/accumulo/core/security/crypto/BlockedIOStreamTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/security/crypto/BlockedIOStreamTest.java b/core/src/test/java/org/apache/accumulo/core/security/crypto/BlockedIOStreamTest.java
index faba913..6fb52dd 100644
--- a/core/src/test/java/org/apache/accumulo/core/security/crypto/BlockedIOStreamTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/security/crypto/BlockedIOStreamTest.java
@@ -22,6 +22,7 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.IOException;
+import java.util.Random;
 
 import org.apache.accumulo.core.Constants;
 import org.junit.Test;
@@ -71,4 +72,47 @@ public class BlockedIOStreamTest {
   public void testSmallBufferBlockedIO() throws IOException {
     writeRead(16, (12 + 4) * (int) (Math.ceil(25.0/12) + Math.ceil(31.0/12)));
   }
+  
+  @Test
+  public void testSpillingOverOutputStream() throws IOException {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    // buffer will be size 12
+    BlockedOutputStream blockOut = new BlockedOutputStream(baos, 16, 16);
+    Random r = new Random(22);
+    
+    byte[] undersized = new byte[11];
+    byte[] perfectSized = new byte[12];
+    byte[] overSized = new byte[13];
+    byte[] perfectlyOversized = new byte[13];
+    byte filler = (byte) r.nextInt();
+    
+    r.nextBytes(undersized);
+    r.nextBytes(perfectSized);
+    r.nextBytes(overSized);
+    r.nextBytes(perfectlyOversized);
+    
+    // 1 block
+    blockOut.write(undersized);
+    blockOut.write(filler);
+    blockOut.flush();
+    
+    // 2 blocks
+    blockOut.write(perfectSized);
+    blockOut.write(filler);
+    blockOut.flush();
+    
+    // 2 blocks
+    blockOut.write(overSized);
+    blockOut.write(filler);
+    blockOut.flush();
+    
+    // 3 blocks
+    blockOut.write(undersized);
+    blockOut.write(perfectlyOversized);
+    blockOut.write(filler);
+    blockOut.flush();
+    
+    baos.close();
+    assertEquals(16*8, baos.toByteArray().length);
+  }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9ba06ff2/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
----------------------------------------------------------------------
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
index cc28ac2..3074614 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
@@ -366,14 +366,14 @@ public class DfsLogger {
       logFile.write(LOG_FILE_HEADER_V3.getBytes());
 
       CryptoModuleParameters params = CryptoModuleFactory.createParamsObjectFromAccumuloConfiguration(conf.getConfiguration());
-      
+
       params.setPlaintextOutputStream(new NoFlushOutputStream(logFile));
-      
+
       // In order to bootstrap the reading of this file later, we have to record the CryptoModule that was used to encipher it here,
       // so that that crypto module can re-read its own parameters.
-      
+
       logFile.writeUTF(conf.getConfiguration().get(Property.CRYPTO_MODULE_CLASS));
-      
+
       
       params = cryptoModule.getEncryptingOutputStream(params);
       OutputStream encipheringOutputStream = params.getEncryptedOutputStream();
@@ -437,7 +437,7 @@ public class DfsLogger {
           log.info("Interrupted");
         }
     }
-    
+
     if (encryptingLogFile != null)
       try {
         encryptingLogFile.close();
@@ -492,7 +492,7 @@ public class DfsLogger {
         work.exception = e;
       }
     }
-    
+
     synchronized (closeLock) {
       // use a different lock for close check so that adding to work queue does not need
       // to wait on walog I/O operations