accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] 02/03: ACCUMULO-4737 Clean up cipher algorithm configuration
Date Thu, 09 Nov 2017 21:54:52 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 daeffd8d9cf980814fb7d131a0d89cbdfd856298
Author: Nick <31989480+PircDef@users.noreply.github.com>
AuthorDate: Mon Nov 6 11:56:59 2017 -0500

    ACCUMULO-4737 Clean up cipher algorithm configuration
    
    Renamed crypto.cipher.algorithm.name to crypto.cipher.key.algorithm.name
    Removed the unused mode/padding code in favor of passing around the crypto suite (instead
of splitting it up and rebuilding it)
    Accumulo will now use crypto.cipher.key.algorithm.name to build the encryption key
    Accumulo will now use crypto.cipher.suite to build the Java Cipher object
    Unit test was updated to reflect the change
    
    Additionally, this was stumbled upon when implementing a separate cipher algorithm option
for WAL files, so that change has been included.
    
    Added sanity check for the config file along with tests
---
 .../accumulo/core/conf/ConfigSanityCheck.java      | 19 ++++++
 .../org/apache/accumulo/core/conf/Property.java    | 14 ++++-
 .../accumulo/core/file/rfile/bcfile/BCFile.java    |  8 +--
 .../CachingHDFSSecretKeyEncryptionStrategy.java    |  6 +-
 .../core/security/crypto/CryptoModuleFactory.java  | 21 ++-----
 .../security/crypto/CryptoModuleParameters.java    | 73 +++++++---------------
 .../core/security/crypto/DefaultCryptoModule.java  | 46 +++++---------
 .../NonCachingSecretKeyEncryptionStrategy.java     |  6 +-
 .../security/crypto/RFileCipherOutputStream.java   | 15 +++--
 .../accumulo/core/conf/ConfigSanityCheckTest.java  | 16 +++++
 .../accumulo/core/security/crypto/CryptoTest.java  | 13 ++--
 .../src/test/resources/crypto-on-accumulo-site.xml |  6 +-
 .../crypto-on-no-key-encryption-accumulo-site.xml  |  2 +-
 .../org/apache/accumulo/tserver/log/DfsLogger.java |  4 ++
 .../org/apache/accumulo/test/ShellConfigIT.java    |  4 +-
 15 files changed, 121 insertions(+), 132 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
index 9c2ed6c..baf1818 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
@@ -17,6 +17,7 @@
 package org.apache.accumulo.core.conf;
 
 import java.util.Map.Entry;
+import java.util.Objects;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -48,6 +49,8 @@ public class ConfigSanityCheck {
   public static void validate(Iterable<Entry<String,String>> entries) {
     String instanceZkTimeoutValue = null;
     boolean usingVolumes = false;
+    String cipherSuite = null;
+    String keyAlgorithm = null;
     for (Entry<String,String> entry : entries) {
       String key = entry.getKey();
       String value = entry.getValue();
@@ -75,6 +78,14 @@ public class ConfigSanityCheck {
         Preconditions.checkArgument(bsize > 0 && bsize < Integer.MAX_VALUE,
key + " must be greater than 0 and less than " + Integer.MAX_VALUE + " but was: "
             + bsize);
       }
+
+      if (key.equals(Property.CRYPTO_CIPHER_SUITE.getKey())) {
+        cipherSuite = Objects.requireNonNull(value);
+      }
+
+      if (key.equals(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey())) {
+        keyAlgorithm = Objects.requireNonNull(value);
+      }
     }
 
     if (instanceZkTimeoutValue != null) {
@@ -84,6 +95,14 @@ public class ConfigSanityCheck {
     if (!usingVolumes) {
       log.warn("Use of {} and {} are deprecated. Consider using {} instead.", INSTANCE_DFS_URI,
INSTANCE_DFS_DIR, Property.INSTANCE_VOLUMES);
     }
+
+    if (cipherSuite.equals("NullCipher") && !keyAlgorithm.equals("NullCipher")) {
+      fatal(Property.CRYPTO_CIPHER_SUITE.getKey() + " should be configured when " + Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey()
+ " is set.");
+    }
+
+    if (!cipherSuite.equals("NullCipher") && keyAlgorithm.equals("NullCipher")) {
+      fatal(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey() + " should be configured when
" + Property.CRYPTO_CIPHER_SUITE.getKey() + " is set.");
+    }
   }
 
   private interface CheckTimeDuration {
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index e08df9e..f0e2338 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -48,10 +48,18 @@ public enum Property {
       "Fully qualified class name of the class that implements the CryptoModule interface,
to be used in setting up encryption at rest for the WAL and "
           + "(future) other parts of the code."),
   @Experimental
-  CRYPTO_CIPHER_SUITE("crypto.cipher.suite", "NullCipher", PropertyType.STRING, "Describes
the cipher suite to use for the write-ahead log"),
+  CRYPTO_CIPHER_SUITE("crypto.cipher.suite", "NullCipher", PropertyType.STRING,
+      "Describes the cipher suite to use for rfile encryption. If a WAL cipher suite is not
set, it will default to this value. The suite should be in the "
+          + "form of algorithm/mode/padding, e.g. AES/CBC/NoPadding"),
   @Experimental
-  CRYPTO_CIPHER_ALGORITHM_NAME("crypto.cipher.algorithm.name", "NullCipher", PropertyType.STRING,
-      "States the name of the algorithm used in the corresponding cipher suite. Do not make
these different, unless you enjoy mysterious exceptions and bugs."),
+  CRYPTO_WAL_CIPHER_SUITE(
+      "crypto.wal.cipher.suite",
+      "NullCipher",
+      PropertyType.STRING,
+      "Describes the cipher suite to use for the write-ahead log. Defaults to 'cyrpto.cipher.suite'
and will use that value for WAL encryption unless otherwise specified."),
+  @Experimental
+  CRYPTO_CIPHER_KEY_ALGORITHM_NAME("crypto.cipher.key.algorithm.name", "NullCipher", PropertyType.STRING,
+      "States the name of the algorithm used for the key for the corresponding cipher suite.
The key type must be compatible with the cipher suite."),
   @Experimental
   CRYPTO_BLOCK_STREAM_SIZE("crypto.block.stream.size", "1K", PropertyType.BYTES,
       "The size of the buffer above the cipher stream. Used for reading files and padding
walog entries."),
diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/BCFile.java b/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/BCFile.java
index a169619..f9a61a7 100644
--- a/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/BCFile.java
+++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/BCFile.java
@@ -283,9 +283,9 @@ public final class BCFile {
       /**
        * Get the raw size of the block.
        *
-       * Caution: size() comes from DataOutputStream which returns Integer.MAX_VALUE on an
overflow. This results in a value of 2GiB meaning that
-       * an unknown amount of data, at least 2GiB large, has been written. RFiles handle
this issue by keeping track of the position of blocks
-       * instead of relying on blocks to provide this information.
+       * Caution: size() comes from DataOutputStream which returns Integer.MAX_VALUE on an
overflow. This results in a value of 2GiB meaning that an unknown
+       * amount of data, at least 2GiB large, has been written. RFiles handle this issue
by keeping track of the position of blocks instead of relying on blocks
+       * to provide this information.
        *
        * @return the number of uncompressed bytes written through the BlockAppender so far.
        */
@@ -395,7 +395,7 @@ public final class BCFile {
           long offsetIndexMeta = out.position();
           metaIndex.write(out);
 
-          if (cryptoParams.getAlgorithmName() == null || cryptoParams.getAlgorithmName().equals(Property.CRYPTO_CIPHER_SUITE.getDefaultValue()))
{
+          if (cryptoParams.getCipherSuite() == null || cryptoParams.getCipherSuite().equals(Property.CRYPTO_CIPHER_SUITE.getDefaultValue()))
{
             out.writeLong(offsetIndexMeta);
             API_VERSION_1.write(out);
           } else {
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 7b79d99..ed6d6a5 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
@@ -72,7 +72,7 @@ public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncrypti
     Cipher cipher = DefaultCryptoModuleUtils.getCipher(params.getAllOptions().get(Property.CRYPTO_DEFAULT_KEY_STRATEGY_CIPHER_SUITE.getKey()));
 
     try {
-      cipher.init(encryptionMode, new SecretKeySpec(secretKeyCache.getKeyEncryptionKey(),
params.getAlgorithmName()));
+      cipher.init(encryptionMode, new SecretKeySpec(secretKeyCache.getKeyEncryptionKey(),
params.getKeyAlgorithmName()));
     } catch (InvalidKeyException e) {
       log.error("{}", e.getMessage(), e);
       throw new RuntimeException(e);
@@ -80,7 +80,7 @@ public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncrypti
 
     if (Cipher.UNWRAP_MODE == encryptionMode) {
       try {
-        Key plaintextKey = cipher.unwrap(params.getEncryptedKey(), params.getAlgorithmName(),
Cipher.SECRET_KEY);
+        Key plaintextKey = cipher.unwrap(params.getEncryptedKey(), params.getKeyAlgorithmName(),
Cipher.SECRET_KEY);
         params.setPlaintextKey(plaintextKey.getEncoded());
       } catch (InvalidKeyException e) {
         log.error("{}", e.getMessage(), e);
@@ -90,7 +90,7 @@ public class CachingHDFSSecretKeyEncryptionStrategy implements SecretKeyEncrypti
         throw new RuntimeException(e);
       }
     } else {
-      Key plaintextKey = new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName());
+      Key plaintextKey = new SecretKeySpec(params.getPlaintextKey(), params.getKeyAlgorithmName());
       try {
         byte[] encryptedSecretKey = cipher.wrap(plaintextKey);
         params.setEncryptedKey(encryptedSecretKey);
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 6a295ad..e56ee92 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
@@ -239,14 +239,6 @@ public class CryptoModuleFactory {
 
   }
 
-  public static String[] parseCipherTransform(String cipherTransform) {
-    if (cipherTransform == null) {
-      return new String[3];
-    }
-
-    return cipherTransform.split("/");
-  }
-
   public static CryptoModuleParameters createParamsObjectFromAccumuloConfiguration(AccumuloConfiguration
conf) {
     CryptoModuleParameters params = new CryptoModuleParameters();
 
@@ -264,26 +256,21 @@ public class CryptoModuleFactory {
   }
 
   public static CryptoModuleParameters fillParamsObjectFromStringMap(CryptoModuleParameters
params, Map<String,String> cryptoOpts) {
-
-    // Parse the cipher suite for the mode and padding options
-    String[] cipherTransformParts = parseCipherTransform(cryptoOpts.get(Property.CRYPTO_CIPHER_SUITE.getKey()));
-
+    params.setCipherSuite(cryptoOpts.get(Property.CRYPTO_CIPHER_SUITE.getKey()));
     // If no encryption has been specified, then we abort here.
-    if (cipherTransformParts[0] == null || cipherTransformParts[0].equals("NullCipher"))
{
+    if (params.getCipherSuite() == null || params.getCipherSuite().equals("NullCipher"))
{
       params.setAllOptions(cryptoOpts);
-      params.setAlgorithmName("NullCipher");
+
       return params;
     }
 
     params.setAllOptions(cryptoOpts);
 
-    params.setAlgorithmName(cryptoOpts.get(Property.CRYPTO_CIPHER_ALGORITHM_NAME.getKey()));
-    params.setEncryptionMode(cipherTransformParts[1]);
+    params.setKeyAlgorithmName(cryptoOpts.get(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey()));
     params.setKeyEncryptionStrategyClass(cryptoOpts.get(Property.CRYPTO_SECRET_KEY_ENCRYPTION_STRATEGY_CLASS.getKey()));
     params.setKeyLength(Integer.parseInt(cryptoOpts.get(Property.CRYPTO_CIPHER_KEY_LENGTH.getKey())));
     params.setOverrideStreamsSecretKeyEncryptionStrategy(Boolean.parseBoolean(cryptoOpts.get(Property.CRYPTO_OVERRIDE_KEY_STRATEGY_WITH_CONFIGURED_STRATEGY
         .getKey())));
-    params.setPadding(cipherTransformParts[2]);
     params.setRandomNumberGenerator(cryptoOpts.get(Property.CRYPTO_SECURE_RNG.getKey()));
     params.setRandomNumberGeneratorProvider(cryptoOpts.get(Property.CRYPTO_SECURE_RNG_PROVIDER.getKey()));
     String blockStreamSize = cryptoOpts.get(Property.CRYPTO_BLOCK_STREAM_SIZE.getKey());
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 a7bb93d..b5a1ae4 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
@@ -37,17 +37,17 @@ import javax.crypto.CipherOutputStream;
 public class CryptoModuleParameters {
 
   /**
-   * Gets the name of the symmetric algorithm to use for encryption.
+   * Gets the name of the symmetric algorithm to use for the creation of encryption keys.
    *
-   * @see CryptoModuleParameters#setAlgorithmName(String)
+   * @see CryptoModuleParameters#setKeyAlgorithmName(String)
    */
 
-  public String getAlgorithmName() {
-    return algorithmName;
+  public String getKeyAlgorithmName() {
+    return keyAlgorithmName;
   }
 
   /**
-   * Sets the name of the symmetric algorithm to use for an encryption stream.
+   * Sets the name of the symmetric algorithm to use for the creation of encryption keys.
    * <p>
    * Valid names are names recognized by your cryptographic engine provider. For the default
Java provider, valid names would include things like "AES", "RC4",
    * "DESede", etc.
@@ -56,73 +56,45 @@ public class CryptoModuleParameters {
    * decryption. <br>
    * For <b>decryption</b>, this value is often disregarded in favor of the value
encoded with the ciphertext.
    *
-   * @param algorithmName
+   * @param keyAlgorithmName
    *          the name of the cryptographic algorithm to use.
    * @see <a href="http://docs.oracle.com/javase/1.5.0/docs/guide/security/jce/JCERefGuide.html#AppA">Standard
Algorithm Names in JCE</a>
    *
    */
 
-  public void setAlgorithmName(String algorithmName) {
-    this.algorithmName = algorithmName;
+  public void setKeyAlgorithmName(String keyAlgorithmName) {
+    this.keyAlgorithmName = keyAlgorithmName;
   }
 
   /**
-   * Gets the name of the encryption mode to use for encryption.
+   * Gets the name of the cipher suite used for encryption
    *
-   * @see CryptoModuleParameters#setEncryptionMode(String)
+   * @see CryptoModuleParameters#setCipherSuite(String)
    */
 
-  public String getEncryptionMode() {
-    return encryptionMode;
+  public String getCipherSuite() {
+    return cipherSuite;
   }
 
   /**
-   * Sets the name of the encryption mode to use for an encryption stream.
+   * Sets the name of the crypto suite to use for an encryption stream.
    * <p>
-   * Valid names are names recognized by your cryptographic engine provider. For the default
Java provider, valid names would include things like "EBC", "CBC",
-   * "CFB", etc.
-   * <p>
-   * For <b>encryption</b>, this value is <b>required</b> and is
always used. Its value should be prepended or otherwise included with the ciphertext for future
-   * decryption. <br>
-   * For <b>decryption</b>, this value is often disregarded in favor of the value
encoded with the ciphertext.
+   * Valid names are names recognized by your cryptographic engine provider.
    *
-   * @param encryptionMode
-   *          the name of the encryption mode to use.
-   * @see <a href="http://docs.oracle.com/javase/1.5.0/docs/guide/security/jce/JCERefGuide.html#AppA">Standard
Mode Names in JCE</a>
-   *
-   */
-
-  public void setEncryptionMode(String encryptionMode) {
-    this.encryptionMode = encryptionMode;
-  }
-
-  /**
-   * Gets the name of the padding type to use for encryption.
+   * The format for input should be: algorithm/mode/padding
    *
-   * @see CryptoModuleParameters#setPadding(String)
-   */
-
-  public String getPadding() {
-    return padding;
-  }
-
-  /**
-   * Sets the name of the padding type to use for an encryption stream.
-   * <p>
-   * Valid names are names recognized by your cryptographic engine provider. For the default
Java provider, valid names would include things like "NoPadding",
-   * "None", etc.
+   * For the default Java provider, valid names would include things like "AES/CBC/NoPadding".
    * <p>
    * For <b>encryption</b>, this value is <b>required</b> and is
always used. Its value should be prepended or otherwise included with the ciphertext for future
    * decryption. <br>
    * For <b>decryption</b>, this value is often disregarded in favor of the value
encoded with the ciphertext.
    *
-   * @param padding
-   *          the name of the padding type to use.
-   * @see <a href="http://docs.oracle.com/javase/1.5.0/docs/guide/security/jce/JCERefGuide.html#AppA">Standard
Padding Names in JCE</a>
+   * @param cipherSuite
+   *          the cipher suite to use.
    *
    */
-  public void setPadding(String padding) {
-    this.padding = padding;
+  public void setCipherSuite(String cipherSuite) {
+    this.cipherSuite = cipherSuite;
   }
 
   /**
@@ -602,9 +574,8 @@ public class CryptoModuleParameters {
     this.allOptions = allOptions;
   }
 
-  private String algorithmName = null;
-  private String encryptionMode = null;
-  private String padding = null;
+  private String cipherSuite = null;
+  private String keyAlgorithmName = null;
   private byte[] plaintextKey;
   private int keyLength = 0;
   private String randomNumberGenerator = null;
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 c5c41cd..1798e89 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
@@ -56,10 +56,9 @@ public class DefaultCryptoModule implements CryptoModule {
 
   @Override
   public CryptoModuleParameters initializeCipher(CryptoModuleParameters params) {
-    String cipherTransformation = getCipherTransformation(params);
 
     log.trace(String.format("Using cipher suite \"%s\" with key length %d with RNG \"%s\"
and RNG provider \"%s\" and key encryption strategy \"%s\"",
-        cipherTransformation, params.getKeyLength(), params.getRandomNumberGenerator(), params.getRandomNumberGeneratorProvider(),
+        params.getCipherSuite(), params.getKeyLength(), params.getRandomNumberGenerator(),
params.getRandomNumberGeneratorProvider(),
         params.getKeyEncryptionStrategyClass()));
 
     if (params.getSecureRandom() == null) {
@@ -67,11 +66,11 @@ public class DefaultCryptoModule implements CryptoModule {
       params.setSecureRandom(secureRandom);
     }
 
-    Cipher cipher = DefaultCryptoModuleUtils.getCipher(cipherTransformation);
+    Cipher cipher = DefaultCryptoModuleUtils.getCipher(params.getCipherSuite());
 
     if (params.getInitializationVector() == null) {
       try {
-        cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName()),
params.getSecureRandom());
+        cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(params.getPlaintextKey(), params.getKeyAlgorithmName()),
params.getSecureRandom());
       } catch (InvalidKeyException e) {
         log.error("Accumulo encountered an unknown error in generating the secret key object
(SecretKeySpec) for an encrypted stream");
         throw new RuntimeException(e);
@@ -81,7 +80,7 @@ public class DefaultCryptoModule implements CryptoModule {
 
     } else {
       try {
-        cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName()),
+        cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(params.getPlaintextKey(), params.getKeyAlgorithmName()),
             new IvParameterSpec(params.getInitializationVector()));
       } catch (InvalidKeyException e) {
         log.error("Accumulo encountered an unknown error in generating the secret key object
(SecretKeySpec) for an encrypted stream");
@@ -98,15 +97,6 @@ public class DefaultCryptoModule implements CryptoModule {
 
   }
 
-  private String getCipherTransformation(CryptoModuleParameters params) {
-    String cipherSuite = params.getAlgorithmName() + "/" + params.getEncryptionMode() + "/"
+ params.getPadding();
-    return cipherSuite;
-  }
-
-  private String[] parseCipherSuite(String cipherSuite) {
-    return cipherSuite.split("/");
-  }
-
   private boolean validateNotEmpty(String givenValue, boolean allIsWell, StringBuilder buf,
String errorMessage) {
     if (givenValue == null || givenValue.equals("")) {
       buf.append(errorMessage);
@@ -145,15 +135,14 @@ public class DefaultCryptoModule implements CryptoModule {
           "The following problems were found with the CryptoModuleParameters object you provided
for an encrypt operation:\n");
       boolean allIsWell = true;
 
-      allIsWell = validateNotEmpty(params.getAlgorithmName(), allIsWell, errorBuf, "No algorithm
name was specified.");
+      allIsWell = validateNotEmpty(params.getCipherSuite(), allIsWell, errorBuf, "No cipher
suite was specified.");
 
-      if (allIsWell && params.getAlgorithmName().equals("NullCipher")) {
+      if (allIsWell && params.getCipherSuite().equals("NullCipher")) {
         return true;
       }
 
-      allIsWell = validateNotEmpty(params.getPadding(), allIsWell, errorBuf, "No padding
was specified.");
       allIsWell = validateNotZero(params.getKeyLength(), allIsWell, errorBuf, "No key length
was specified.");
-      allIsWell = validateNotEmpty(params.getEncryptionMode(), allIsWell, errorBuf, "No encryption
mode was specified.");
+      allIsWell = validateNotEmpty(params.getKeyAlgorithmName(), allIsWell, errorBuf, "No
key algorithm name was specified.");
       allIsWell = validateNotEmpty(params.getRandomNumberGenerator(), allIsWell, errorBuf,
"No random number generator was specified.");
       allIsWell = validateNotEmpty(params.getRandomNumberGeneratorProvider(), allIsWell,
errorBuf, "No random number generate provider was specified.");
       allIsWell = validateNotNull(params.getPlaintextOutputStream(), allIsWell, errorBuf,
"No plaintext output stream was specified.");
@@ -171,9 +160,7 @@ public class DefaultCryptoModule implements CryptoModule {
           "The following problems were found with the CryptoModuleParameters object you provided
for a decrypt operation:\n");
       boolean allIsWell = true;
 
-      allIsWell = validateNotEmpty(params.getPadding(), allIsWell, errorBuf, "No padding
was specified.");
       allIsWell = validateNotZero(params.getKeyLength(), allIsWell, errorBuf, "No key length
was specified.");
-      allIsWell = validateNotEmpty(params.getEncryptionMode(), allIsWell, errorBuf, "No encryption
mode was specified.");
       allIsWell = validateNotEmpty(params.getRandomNumberGenerator(), allIsWell, errorBuf,
"No random number generator was specified.");
       allIsWell = validateNotEmpty(params.getRandomNumberGeneratorProvider(), allIsWell,
errorBuf, "No random number generate provider was specified.");
       allIsWell = validateNotNull(params.getEncryptedInputStream(), allIsWell, errorBuf,
"No encrypted input stream was specified.");
@@ -211,7 +198,7 @@ public class DefaultCryptoModule implements CryptoModule {
     }
 
     // If they want a null output stream, just return their plaintext stream as the encrypted
stream
-    if (params.getAlgorithmName().equals("NullCipher")) {
+    if (params.getCipherSuite().equals("NullCipher")) {
       params.setEncryptedOutputStream(params.getPlaintextOutputStream());
       return params;
     }
@@ -271,8 +258,8 @@ public class DefaultCryptoModule implements CryptoModule {
 
       // Write out the cipher suite and algorithm used to encrypt this file. In case the
admin changes, we want to still
       // decode the old format.
-      dataOut.writeUTF(getCipherTransformation(params));
-      dataOut.writeUTF(params.getAlgorithmName());
+      dataOut.writeUTF(params.getCipherSuite());
+      dataOut.writeUTF(params.getKeyAlgorithmName());
 
       // Write the init vector to the log file
       dataOut.writeInt(params.getInitializationVector().length);
@@ -313,10 +300,8 @@ public class DefaultCryptoModule implements CryptoModule {
         // Set the cipher parameters
         String cipherSuiteFromFile = dataIn.readUTF();
         String algorithmNameFromFile = dataIn.readUTF();
-        String[] cipherSuiteParts = parseCipherSuite(cipherSuiteFromFile);
-        params.setAlgorithmName(algorithmNameFromFile);
-        params.setEncryptionMode(cipherSuiteParts[1]);
-        params.setPadding(cipherSuiteParts[2]);
+        params.setCipherSuite(cipherSuiteFromFile);
+        params.setKeyAlgorithmName(algorithmNameFromFile);
 
         // Read the secret key and initialization vector from the file
         int initVectorLength = dataIn.readInt();
@@ -382,10 +367,10 @@ public class DefaultCryptoModule implements CryptoModule {
       throw new RuntimeException("CryptoModuleParameters object failed validation for decrypt");
     }
 
-    Cipher cipher = DefaultCryptoModuleUtils.getCipher(getCipherTransformation(params));
+    Cipher cipher = DefaultCryptoModuleUtils.getCipher(params.getCipherSuite());
 
     try {
-      cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName()),
+      cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(params.getPlaintextKey(), params.getKeyAlgorithmName()),
           new IvParameterSpec(params.getInitializationVector()));
     } catch (InvalidKeyException e) {
       log.error("Error when trying to initialize cipher with secret key");
@@ -400,7 +385,7 @@ public class DefaultCryptoModule implements CryptoModule {
     if (params.getBlockStreamSize() > 0)
       blockedDecryptingInputStream = new BlockedInputStream(blockedDecryptingInputStream,
cipher.getBlockSize(), params.getBlockStreamSize());
 
-    log.trace("Initialized cipher input stream with transformation [{}]", getCipherTransformation(params));
+    log.trace("Initialized cipher input stream with transformation [{}]", params.getCipherSuite());
 
     params.setPlaintextInputStream(blockedDecryptingInputStream);
 
@@ -420,5 +405,4 @@ public class DefaultCryptoModule implements CryptoModule {
 
     return params;
   }
-
 }
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
index 5c9ca8c..2d2d2f7 100644
--- 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
@@ -83,7 +83,7 @@ public class NonCachingSecretKeyEncryptionStrategy implements SecretKeyEncryptio
       // check if the number of bytes read into the array is the same as the value of the
length field,
       if (bytesRead == keyEncryptionKeyLength) {
         try {
-          cipher.init(encryptionMode, new SecretKeySpec(keyEncryptionKey, params.getAlgorithmName()));
+          cipher.init(encryptionMode, new SecretKeySpec(keyEncryptionKey, params.getKeyAlgorithmName()));
         } catch (InvalidKeyException e) {
           log.error("{}", e.getMessage(), e);
           throw new RuntimeException(e);
@@ -91,7 +91,7 @@ public class NonCachingSecretKeyEncryptionStrategy implements SecretKeyEncryptio
 
         if (Cipher.UNWRAP_MODE == encryptionMode) {
           try {
-            Key plaintextKey = cipher.unwrap(params.getEncryptedKey(), params.getAlgorithmName(),
Cipher.SECRET_KEY);
+            Key plaintextKey = cipher.unwrap(params.getEncryptedKey(), params.getKeyAlgorithmName(),
Cipher.SECRET_KEY);
             params.setPlaintextKey(plaintextKey.getEncoded());
           } catch (InvalidKeyException e) {
             log.error("{}", e.getMessage(), e);
@@ -101,7 +101,7 @@ public class NonCachingSecretKeyEncryptionStrategy implements SecretKeyEncryptio
             throw new RuntimeException(e);
           }
         } else {
-          Key plaintextKey = new SecretKeySpec(params.getPlaintextKey(), params.getAlgorithmName());
+          Key plaintextKey = new SecretKeySpec(params.getPlaintextKey(), params.getKeyAlgorithmName());
           try {
             byte[] encryptedSecretKey = cipher.wrap(plaintextKey);
             params.setEncryptedKey(encryptedSecretKey);
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/RFileCipherOutputStream.java
b/core/src/main/java/org/apache/accumulo/core/security/crypto/RFileCipherOutputStream.java
index 7dad802..cb85d45 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/RFileCipherOutputStream.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/RFileCipherOutputStream.java
@@ -24,9 +24,8 @@ import javax.crypto.CipherOutputStream;
 
 /**
  *
- * This class extends {@link CipherOutputStream} to include a way to track the number of
bytes that have
- * been encrypted by the stream. The write method also includes a mechanism to stop writing
and
- * throw an exception if exceeding a maximum number of bytes is attempted.
+ * This class extends {@link CipherOutputStream} to include a way to track the number of
bytes that have been encrypted by the stream. The write method also
+ * includes a mechanism to stop writing and throw an exception if exceeding a maximum number
of bytes is attempted.
  *
  */
 public class RFileCipherOutputStream extends CipherOutputStream {
@@ -35,7 +34,7 @@ public class RFileCipherOutputStream extends CipherOutputStream {
   // will cause an exception. Given that each block in an rfile is encrypted separately,
and blocks
   // should be written such that a block cannot ever reach 16GiB, this is believed to be
a safe number.
   // If this does cause an exception, it is an issue best addressed elsewhere.
-  private final long maxOutputSize = 1L << 34; //16GiB
+  private final long maxOutputSize = 1L << 34; // 16GiB
 
   // The total number of bytes that have been written out
   private long count = 0;
@@ -54,8 +53,8 @@ public class RFileCipherOutputStream extends CipherOutputStream {
   }
 
   /**
-   * Override of CipherOutputStream's write to count the number of bytes that have been encrypted.
-   * This method now throws an exception if an attempt to write bytes beyond a maximum is
made.
+   * Override of CipherOutputStream's write to count the number of bytes that have been encrypted.
This method now throws an exception if an attempt to write
+   * bytes beyond a maximum is made.
    */
   @Override
   public void write(byte b[], int off, int len) throws IOException {
@@ -72,8 +71,8 @@ public class RFileCipherOutputStream extends CipherOutputStream {
   }
 
   /**
-   * Override of CipherOutputStream's write for a single byte to count it. This method now
throws
-   * an exception if an attempt to write bytes beyond a maximum is made.
+   * Override of CipherOutputStream's write for a single byte to count it. This method now
throws an exception if an attempt to write bytes beyond a maximum is
+   * made.
    */
   @Override
   public void write(int b) throws IOException {
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java b/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java
index 7ac2113..9f2ff8e 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java
@@ -28,6 +28,8 @@ public class ConfigSanityCheckTest {
   @Before
   public void setUp() {
     m = new java.util.HashMap<>();
+    m.put(Property.CRYPTO_CIPHER_SUITE.getKey(), "NullCipher");
+    m.put(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey(), "NullCipher");
   }
 
   @Test
@@ -77,4 +79,18 @@ public class ConfigSanityCheckTest {
     m.put(Property.INSTANCE_ZK_TIMEOUT.getKey(), "10ms");
     ConfigSanityCheck.validate(m.entrySet());
   }
+
+  @Test(expected = SanityCheckException.class)
+  public void testFail_cipherSuiteSetKeyAlgorithmNotSet() {
+    m.put(Property.CRYPTO_CIPHER_SUITE.getKey(), "AES/CBC/NoPadding");
+    m.put(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey(), "NullCipher");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test(expected = SanityCheckException.class)
+  public void testFail_cipherSuiteNotSetKeyAlgorithmSet() {
+    m.put(Property.CRYPTO_CIPHER_SUITE.getKey(), "NullCipher");
+    m.put(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey(), "AES");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
 }
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 a32a465..4467828 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
@@ -19,7 +19,6 @@ package org.apache.accumulo.core.security.crypto;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayInputStream;
@@ -71,9 +70,7 @@ public class CryptoTest {
     CryptoModuleParameters params = CryptoModuleFactory.createParamsObjectFromAccumuloConfiguration(conf);
 
     assertNotNull(params);
-    assertEquals("NullCipher", params.getAlgorithmName());
-    assertNull(params.getEncryptionMode());
-    assertNull(params.getPadding());
+    assertEquals("NullCipher", params.getCipherSuite());
 
     CryptoModule cryptoModule = CryptoModuleFactory.getCryptoModule(conf);
     assertNotNull(cryptoModule);
@@ -95,9 +92,9 @@ public class CryptoTest {
     CryptoModuleParameters params = CryptoModuleFactory.createParamsObjectFromAccumuloConfiguration(conf);
 
     assertNotNull(params);
-    assertEquals("AES", params.getAlgorithmName());
-    assertEquals("CFB", params.getEncryptionMode());
-    assertEquals("NoPadding", params.getPadding());
+    assertEquals("AES/CFB/NoPadding", params.getCipherSuite());
+    assertEquals("AES/CBC/NoPadding", params.getAllOptions().get(Property.CRYPTO_WAL_CIPHER_SUITE.getKey()));
+    assertEquals("AES", params.getKeyAlgorithmName());
     assertEquals(128, params.getKeyLength());
     assertEquals("SHA1PRNG", params.getRandomNumberGenerator());
     assertEquals("SUN", params.getRandomNumberGeneratorProvider());
@@ -314,7 +311,7 @@ public class CryptoTest {
     // from those configured within the site configuration. After doing this, we should
     // still be able to read the file that was created with a different set of parameters.
     params = CryptoModuleFactory.createParamsObjectFromAccumuloConfiguration(conf);
-    params.setAlgorithmName("DESede");
+    params.setKeyAlgorithmName("DESede");
     params.setKeyLength(24 * 8);
 
     ByteArrayInputStream in = new ByteArrayInputStream(resultingBytes);
diff --git a/core/src/test/resources/crypto-on-accumulo-site.xml b/core/src/test/resources/crypto-on-accumulo-site.xml
index ebfc9ae..f5824c4 100644
--- a/core/src/test/resources/crypto-on-accumulo-site.xml
+++ b/core/src/test/resources/crypto-on-accumulo-site.xml
@@ -80,7 +80,11 @@
       <value>AES/CFB/NoPadding</value>
     </property>
     <property>
-      <name>crypto.cipher.algorithm.name</name>
+      <name>crypto.wal.cipher.suite</name>
+      <value>AES/CBC/NoPadding</value>
+    </property>
+    <property>
+      <name>crypto.cipher.key.algorithm.name</name>
       <value>AES</value>
     </property>
     <property>
diff --git a/core/src/test/resources/crypto-on-no-key-encryption-accumulo-site.xml b/core/src/test/resources/crypto-on-no-key-encryption-accumulo-site.xml
index d980783..a82f9f9 100644
--- a/core/src/test/resources/crypto-on-no-key-encryption-accumulo-site.xml
+++ b/core/src/test/resources/crypto-on-no-key-encryption-accumulo-site.xml
@@ -80,7 +80,7 @@
       <value>AES/CFB/NoPadding</value>
     </property>
     <property>
-      <name>crypto.cipher.algorithm.name</name>
+      <name>crypto.cipher.key.algorithm.name</name>
       <value>AES</value>
     </property>
     <property>
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 d336c3c..d48907f 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
@@ -467,6 +467,10 @@ public class DfsLogger implements Comparable<DfsLogger> {
       logFile.write(LOG_FILE_HEADER_V3.getBytes(UTF_8));
 
       CryptoModuleParameters params = CryptoModuleFactory.createParamsObjectFromAccumuloConfiguration(conf.getConfiguration());
+      // Immediately update to the correct cipher. Doing this here keeps the CryptoModule
independent of the writers using it
+      if (params.getAllOptions().get(Property.CRYPTO_WAL_CIPHER_SUITE.getKey()) != null)
{
+        params.setCipherSuite(params.getAllOptions().get(Property.CRYPTO_WAL_CIPHER_SUITE.getKey()));
+      }
 
       NoFlushOutputStream nfos = new NoFlushOutputStream(logFile);
       params.setPlaintextOutputStream(nfos);
diff --git a/test/src/main/java/org/apache/accumulo/test/ShellConfigIT.java b/test/src/main/java/org/apache/accumulo/test/ShellConfigIT.java
index 977cc36..f90435d 100644
--- a/test/src/main/java/org/apache/accumulo/test/ShellConfigIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/ShellConfigIT.java
@@ -93,11 +93,11 @@ public class ShellConfigIT extends AccumuloClusterHarness {
       Assert.fail("Unknown token type");
     }
 
-    assertTrue(Property.CRYPTO_CIPHER_ALGORITHM_NAME.isExperimental());
+    assertTrue(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.isExperimental());
 
     String configOutput = ts.exec("config");
 
     assertTrue(configOutput.contains(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER));
-    assertFalse(configOutput.contains(Property.CRYPTO_CIPHER_ALGORITHM_NAME.getKey()));
+    assertFalse(configOutput.contains(Property.CRYPTO_CIPHER_KEY_ALGORITHM_NAME.getKey()));
   }
 }

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

Mime
View raw message