HADOOP-10881. Clarify usage of encryption and encrypted encryption key in KeyProviderCryptoExtension. (wang)

Conflicts:
	hadoop-common-project/hadoop-common/CHANGES.txt

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1619539 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -122,6 +122,9 @@ Release 2.6.0 - UNRELEASED
     HADOOP-10817. ProxyUsers configuration should support configurable
     prefixes. (tucu)
 
+    HADOOP-10881. Clarify usage of encryption and encrypted encryption
+    key in KeyProviderCryptoExtension. (wang)
+
   OPTIMIZATIONS
 
     HADOOP-10838. Byte array native checksumming. (James Thomas via todd)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderCryptoExtension.java

@@ -21,7 +21,6 @@ package org.apache.hadoop.crypto.key;
 import java.io.IOException;
 import java.security.GeneralSecurityException;
 import java.security.SecureRandom;
-
 import javax.crypto.Cipher;
 import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
@@ -30,51 +29,109 @@ import com.google.common.base.Preconditions;
 import org.apache.hadoop.classification.InterfaceAudience;
 
 /**
- * A KeyProvider with Cytographic Extensions specifically for generating
+ * A KeyProvider with Cryptographic Extensions specifically for generating
- * Encrypted Keys as well as decrypting them
+ * and decrypting encrypted encryption keys.
  *
  */
 @InterfaceAudience.Private
 public class KeyProviderCryptoExtension extends
     KeyProviderExtension<KeyProviderCryptoExtension.CryptoExtension> {
 
+  /**
+   * Designates an encrypted encryption key, or EEK.
+   */
   public static final String EEK = "EEK";
+  /**
+   * Designates a decrypted encrypted encryption key, that is, an encryption key
+   * (EK).
+   */
   public static final String EK = "EK";
 
   /**
-   * This is a holder class whose instance contains the keyVersionName, iv
+   * An encrypted encryption key (EEK) and related information. An EEK must be
-   * used to generate the encrypted Key and the encrypted KeyVersion
+   * decrypted using the key's encryption key before it can be used.
    */
   public static class EncryptedKeyVersion {
-    private String keyName;
+    private String encryptionKeyName;
-    private String keyVersionName;
+    private String encryptionKeyVersionName;
-    private byte[] iv;
+    private byte[] encryptedKeyIv;
-    private KeyVersion encryptedKey;
+    private KeyVersion encryptedKeyVersion;
 
-    protected EncryptedKeyVersion(String keyName, String keyVersionName,
+    /**
-        byte[] iv, KeyVersion encryptedKey) {
+     * Create a new EncryptedKeyVersion.
-      this.keyName = keyName;
+     *
-      this.keyVersionName = keyVersionName;
+     * @param keyName                  Name of the encryption key used to
-      this.iv = iv;
+     *                                 encrypt the encrypted key.
-      this.encryptedKey = encryptedKey;
+     * @param encryptionKeyVersionName Version name of the encryption key used
+     *                                 to encrypt the encrypted key.
+     * @param encryptedKeyIv           Initialization vector of the encrypted
+     *                                 key. The IV of the encryption key used to
+     *                                 encrypt the encrypted key is derived from
+     *                                 this IV.
+     * @param encryptedKeyVersion      The encrypted encryption key version.
+     */
+    protected EncryptedKeyVersion(String keyName,
+        String encryptionKeyVersionName, byte[] encryptedKeyIv,
+        KeyVersion encryptedKeyVersion) {
+      this.encryptionKeyName = keyName;
+      this.encryptionKeyVersionName = encryptionKeyVersionName;
+      this.encryptedKeyIv = encryptedKeyIv;
+      this.encryptedKeyVersion = encryptedKeyVersion;
     }
 
-    public String getKeyName() {
+    /**
-      return keyName;
+     * @return Name of the encryption key used to encrypt the encrypted key.
+     */
+    public String getEncryptionKeyName() {
+      return encryptionKeyName;
     }
 
-    public String getKeyVersionName() {
+    /**
-      return keyVersionName;
+     * @return Version name of the encryption key used to encrypt the encrypted
+     * key.
+     */
+    public String getEncryptionKeyVersionName() {
+      return encryptionKeyVersionName;
     }
 
-    public byte[] getIv() {
+    /**
-      return iv;
+     * @return Initialization vector of the encrypted key. The IV of the
+     * encryption key used to encrypt the encrypted key is derived from this
+     * IV.
+     */
+    public byte[] getEncryptedKeyIv() {
+      return encryptedKeyIv;
     }
 
-    public KeyVersion getEncryptedKey() {
+    /**
-      return encryptedKey;
+     * @return The encrypted encryption key version.
+     */
+    public KeyVersion getEncryptedKeyVersion() {
+      return encryptedKeyVersion;
     }
 
+    /**
+     * Derive the initialization vector (IV) for the encryption key from the IV
+     * of the encrypted key. This derived IV is used with the encryption key to
+     * decrypt the encrypted key.
+     * <p/>
+     * The alternative to this is using the same IV for both the encryption key
+     * and the encrypted key. Even a simple symmetric transformation like this
+     * improves security by avoiding IV re-use. IVs will also be fairly unique
+     * among different EEKs.
+     *
+     * @param encryptedKeyIV of the encrypted key (i.e. {@link
+     * #getEncryptedKeyIv()})
+     * @return IV for the encryption key
+     */
+    protected static byte[] deriveIV(byte[] encryptedKeyIV) {
+      byte[] rIv = new byte[encryptedKeyIV.length];
+      // Do a simple XOR transformation to flip all the bits
+      for (int i = 0; i < encryptedKeyIV.length; i++) {
+        rIv[i] = (byte) (encryptedKeyIV[i] ^ 0xff);
+      }
+      return rIv;
+    }
   }
 
   /**
@@ -141,53 +198,56 @@ public class KeyProviderCryptoExtension extends
       this.keyProvider = keyProvider;
     }
 
-    // the IV used to encrypt a EK typically will be the same IV used to
-    // encrypt data with the EK. To avoid any chance of weakening the
-    // encryption because the same IV is used, we simply XOR the IV thus we
-    // are not using the same IV for 2 different encryptions (even if they
-    // are done using different keys)
-    private byte[] flipIV(byte[] iv) {
-      byte[] rIv = new byte[iv.length];
-      for (int i = 0; i < iv.length; i++) {
-        rIv[i] = (byte) (iv[i] ^ 0xff);
-      }
-      return rIv;
-    }
-
     @Override
     public EncryptedKeyVersion generateEncryptedKey(String encryptionKeyName)
         throws IOException, GeneralSecurityException {
-      KeyVersion keyVer = keyProvider.getCurrentKey(encryptionKeyName);
+      // Fetch the encryption key
-      Preconditions.checkNotNull(keyVer, "No KeyVersion exists for key '%s' ",
+      KeyVersion encryptionKey = keyProvider.getCurrentKey(encryptionKeyName);
-          encryptionKeyName);
+      Preconditions.checkNotNull(encryptionKey,
-      byte[] newKey = new byte[keyVer.getMaterial().length];
+          "No KeyVersion exists for key '%s' ", encryptionKeyName);
-      SecureRandom.getInstance("SHA1PRNG").nextBytes(newKey);
+      // Generate random bytes for new key and IV
       Cipher cipher = Cipher.getInstance("AES/CTR/NoPadding");
-      byte[] iv = SecureRandom.getSeed(cipher.getBlockSize());
+      SecureRandom random = SecureRandom.getInstance("SHA1PRNG");
-      cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(keyVer.getMaterial(),
+      final byte[] newKey = new byte[encryptionKey.getMaterial().length];
-          "AES"), new IvParameterSpec(flipIV(iv)));
+      random.nextBytes(newKey);
-      byte[] ek = cipher.doFinal(newKey);
+      final byte[] iv = random.generateSeed(cipher.getBlockSize());
+      // Encryption key IV is derived from new key's IV
+      final byte[] encryptionIV = EncryptedKeyVersion.deriveIV(iv);
+      // Encrypt the new key
+      cipher.init(Cipher.ENCRYPT_MODE,
+          new SecretKeySpec(encryptionKey.getMaterial(), "AES"),
+          new IvParameterSpec(encryptionIV));
+      final byte[] encryptedKey = cipher.doFinal(newKey);
       return new EncryptedKeyVersion(encryptionKeyName,
-          keyVer.getVersionName(), iv,
+          encryptionKey.getVersionName(), iv,
-          new KeyVersion(keyVer.getName(), EEK, ek));
+          new KeyVersion(encryptionKey.getName(), EEK, encryptedKey));
     }
 
     @Override
     public KeyVersion decryptEncryptedKey(
         EncryptedKeyVersion encryptedKeyVersion) throws IOException,
         GeneralSecurityException {
-      KeyVersion keyVer =
+      // Fetch the encryption key material
-          keyProvider.getKeyVersion(encryptedKeyVersion.getKeyVersionName());
+      final String encryptionKeyVersionName =
-      Preconditions.checkNotNull(keyVer, "KeyVersion name '%s' does not exist",
+          encryptedKeyVersion.getEncryptionKeyVersionName();
-          encryptedKeyVersion.getKeyVersionName());
+      final KeyVersion encryptionKey =
-      KeyVersion keyVersion = encryptedKeyVersion.getEncryptedKey();
+          keyProvider.getKeyVersion(encryptionKeyVersionName);
+      Preconditions.checkNotNull(encryptionKey,
+          "KeyVersion name '%s' does not exist", encryptionKeyVersionName);
+      final byte[] encryptionKeyMaterial = encryptionKey.getMaterial();
+      // Encryption key IV is determined from encrypted key's IV
+      final byte[] encryptionIV =
+          EncryptedKeyVersion.deriveIV(encryptedKeyVersion.getEncryptedKeyIv());
+      // Init the cipher with encryption key parameters
       Cipher cipher = Cipher.getInstance("AES/CTR/NoPadding");
       cipher.init(Cipher.DECRYPT_MODE,
-          new SecretKeySpec(keyVersion.getMaterial(), "AES"),
+          new SecretKeySpec(encryptionKeyMaterial, "AES"),
-          new IvParameterSpec(flipIV(encryptedKeyVersion.getIv())));
+          new IvParameterSpec(encryptionIV));
-      byte[] ek =
+      // Decrypt the encrypted key
-          cipher.doFinal(encryptedKeyVersion.getEncryptedKey().getMaterial());
+      final KeyVersion encryptedKV =
-      return new KeyVersion(keyVer.getName(), EK, ek);
+          encryptedKeyVersion.getEncryptedKeyVersion();
+      final byte[] decryptedKey = cipher.doFinal(encryptedKV.getMaterial());
+      return new KeyVersion(encryptionKey.getName(), EK, decryptedKey);
     }
 
     @Override

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java

@@ -646,25 +646,28 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension {
   public KeyVersion decryptEncryptedKey(
       EncryptedKeyVersion encryptedKeyVersion) throws IOException,
                                                       GeneralSecurityException {
-    checkNotNull(encryptedKeyVersion.getKeyVersionName(), "versionName");
+    checkNotNull(encryptedKeyVersion.getEncryptionKeyVersionName(),
-    checkNotNull(encryptedKeyVersion.getIv(), "iv");
+        "versionName");
-    Preconditions.checkArgument(encryptedKeyVersion.getEncryptedKey()
+    checkNotNull(encryptedKeyVersion.getEncryptedKeyIv(), "iv");
-        .getVersionName().equals(KeyProviderCryptoExtension.EEK),
+    Preconditions.checkArgument(
+        encryptedKeyVersion.getEncryptedKeyVersion().getVersionName()
+            .equals(KeyProviderCryptoExtension.EEK),
         "encryptedKey version name must be '%s', is '%s'",
-        KeyProviderCryptoExtension.EK, encryptedKeyVersion.getEncryptedKey()
+        KeyProviderCryptoExtension.EK,
-            .getVersionName());
+        encryptedKeyVersion.getEncryptedKeyVersion().getVersionName()
-    checkNotNull(encryptedKeyVersion.getEncryptedKey(), "encryptedKey");
+    );
+    checkNotNull(encryptedKeyVersion.getEncryptedKeyVersion(), "encryptedKey");
     Map<String, String> params = new HashMap<String, String>();
     params.put(KMSRESTConstants.EEK_OP, KMSRESTConstants.EEK_DECRYPT);
     Map<String, Object> jsonPayload = new HashMap<String, Object>();
     jsonPayload.put(KMSRESTConstants.NAME_FIELD,
-        encryptedKeyVersion.getKeyName());
+        encryptedKeyVersion.getEncryptionKeyName());
     jsonPayload.put(KMSRESTConstants.IV_FIELD, Base64.encodeBase64String(
-        encryptedKeyVersion.getIv()));
+        encryptedKeyVersion.getEncryptedKeyIv()));
     jsonPayload.put(KMSRESTConstants.MATERIAL_FIELD, Base64.encodeBase64String(
-            encryptedKeyVersion.getEncryptedKey().getMaterial()));
+            encryptedKeyVersion.getEncryptedKeyVersion().getMaterial()));
     URL url = createURL(KMSRESTConstants.KEY_VERSION_RESOURCE,
-        encryptedKeyVersion.getKeyVersionName(),
+        encryptedKeyVersion.getEncryptionKeyVersionName(),
         KMSRESTConstants.EEK_SUB_RESOURCE, params);
     HttpURLConnection conn = createConnection(url, HTTP_POST);
     conn.setRequestProperty(CONTENT_TYPE, APPLICATION_JSON_MIME);

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderCryptoExtension.java

@@ -17,51 +17,112 @@
  */
 package org.apache.hadoop.crypto.key;
 
-import org.apache.hadoop.conf.Configuration;
-import org.junit.Assert;
-import org.junit.Test;
-
 import java.net.URI;
 import java.security.SecureRandom;
+import java.util.Arrays;
+
+import javax.crypto.Cipher;
+import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.apache.hadoop.conf.Configuration;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+
+import static org.apache.hadoop.crypto.key.KeyProvider.KeyVersion;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 public class TestKeyProviderCryptoExtension {
 
   private static final String CIPHER = "AES";
+  private static final String ENCRYPTION_KEY_NAME = "fooKey";
+
+  private static Configuration conf;
+  private static KeyProvider kp;
+  private static KeyProviderCryptoExtension kpExt;
+  private static KeyProvider.Options options;
+  private static KeyVersion encryptionKey;
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    conf = new Configuration();
+    kp = new UserProvider.Factory().createProvider(new URI("user:///"), conf);
+    kpExt = KeyProviderCryptoExtension.createKeyProviderCryptoExtension(kp);
+    options = new KeyProvider.Options(conf);
+    options.setCipher(CIPHER);
+    options.setBitLength(128);
+    encryptionKey =
+        kp.createKey(ENCRYPTION_KEY_NAME, SecureRandom.getSeed(16), options);
+  }
 
   @Test
   public void testGenerateEncryptedKey() throws Exception {
-    Configuration conf = new Configuration();
+    // Generate a new EEK and check it
-    KeyProvider kp =
-        new UserProvider.Factory().createProvider(new URI("user:///"), conf);
-    KeyProvider.Options options = new KeyProvider.Options(conf);
-    options.setCipher(CIPHER);
-    options.setBitLength(128);
-    KeyProvider.KeyVersion kv = kp.createKey("foo", SecureRandom.getSeed(16),
-        options);
-    KeyProviderCryptoExtension kpExt =
-        KeyProviderCryptoExtension.createKeyProviderCryptoExtension(kp);
-
     KeyProviderCryptoExtension.EncryptedKeyVersion ek1 =
-        kpExt.generateEncryptedKey(kv.getName());
+        kpExt.generateEncryptedKey(encryptionKey.getName());
-    Assert.assertEquals(KeyProviderCryptoExtension.EEK,
+    assertEquals("Version name of EEK should be EEK",
-        ek1.getEncryptedKey().getVersionName());
+        KeyProviderCryptoExtension.EEK,
-    Assert.assertEquals("foo", ek1.getKeyName());
+        ek1.getEncryptedKeyVersion().getVersionName());
-    Assert.assertNotNull(ek1.getEncryptedKey().getMaterial());
+    assertEquals("Name of EEK should be encryption key name",
-    Assert.assertEquals(kv.getMaterial().length,
+        ENCRYPTION_KEY_NAME, ek1.getEncryptionKeyName());
-        ek1.getEncryptedKey().getMaterial().length);
+    assertNotNull("Expected encrypted key material",
-    KeyProvider.KeyVersion k1 = kpExt.decryptEncryptedKey(ek1);
+        ek1.getEncryptedKeyVersion().getMaterial());
-    Assert.assertEquals(KeyProviderCryptoExtension.EK, k1.getVersionName());
+    assertEquals("Length of encryption key material and EEK material should "
-    KeyProvider.KeyVersion k1a = kpExt.decryptEncryptedKey(ek1);
+            + "be the same", encryptionKey.getMaterial().length,
-    Assert.assertArrayEquals(k1.getMaterial(), k1a.getMaterial());
+        ek1.getEncryptedKeyVersion().getMaterial().length
-    Assert.assertEquals(kv.getMaterial().length, k1.getMaterial().length);
+    );
 
-    KeyProviderCryptoExtension.EncryptedKeyVersion ek2 =
+    // Decrypt EEK into an EK and check it
-        kpExt.generateEncryptedKey(kv.getName());
+    KeyVersion k1 = kpExt.decryptEncryptedKey(ek1);
-    KeyProvider.KeyVersion k2 = kpExt.decryptEncryptedKey(ek2);
+    assertEquals(KeyProviderCryptoExtension.EK, k1.getVersionName());
-    boolean eq = true;
+    assertEquals(encryptionKey.getMaterial().length, k1.getMaterial().length);
-    for (int i = 0; eq && i < ek2.getEncryptedKey().getMaterial().length; i++) {
+    if (Arrays.equals(k1.getMaterial(), encryptionKey.getMaterial())) {
-      eq = k2.getMaterial()[i] == k1.getMaterial()[i];
+      fail("Encrypted key material should not equal encryption key material");
     }
-    Assert.assertFalse(eq);
+    if (Arrays.equals(ek1.getEncryptedKeyVersion().getMaterial(),
+        encryptionKey.getMaterial())) {
+      fail("Encrypted key material should not equal decrypted key material");
+    }
+    // Decrypt it again and it should be the same
+    KeyVersion k1a = kpExt.decryptEncryptedKey(ek1);
+    assertArrayEquals(k1.getMaterial(), k1a.getMaterial());
+
+    // Generate another EEK and make sure it's different from the first
+    KeyProviderCryptoExtension.EncryptedKeyVersion ek2 =
+        kpExt.generateEncryptedKey(encryptionKey.getName());
+    KeyVersion k2 = kpExt.decryptEncryptedKey(ek2);
+    if (Arrays.equals(k1.getMaterial(), k2.getMaterial())) {
+      fail("Generated EEKs should have different material!");
+    }
+    if (Arrays.equals(ek1.getEncryptedKeyIv(), ek2.getEncryptedKeyIv())) {
+      fail("Generated EEKs should have different IVs!");
+    }
+  }
+
+  @Test
+  public void testEncryptDecrypt() throws Exception {
+    // Get an EEK
+    KeyProviderCryptoExtension.EncryptedKeyVersion eek =
+        kpExt.generateEncryptedKey(encryptionKey.getName());
+    final byte[] encryptedKeyIv = eek.getEncryptedKeyIv();
+    final byte[] encryptedKeyMaterial = eek.getEncryptedKeyVersion()
+        .getMaterial();
+    // Decrypt it manually
+    Cipher cipher = Cipher.getInstance("AES/CTR/NoPadding");
+    cipher.init(Cipher.DECRYPT_MODE,
+        new SecretKeySpec(encryptionKey.getMaterial(), "AES"),
+        new IvParameterSpec(KeyProviderCryptoExtension.EncryptedKeyVersion
+            .deriveIV(encryptedKeyIv)));
+    final byte[] manualMaterial = cipher.doFinal(encryptedKeyMaterial);
+    // Decrypt it with the API
+    KeyVersion decryptedKey = kpExt.decryptEncryptedKey(eek);
+    final byte[] apiMaterial = decryptedKey.getMaterial();
+
+    assertArrayEquals("Wrong key material from decryptEncryptedKey",
+        manualMaterial, apiMaterial);
   }
 }

hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSServerJSONUtils.java

@@ -64,12 +64,12 @@ public class KMSServerJSONUtils {
     Map json = new LinkedHashMap();
     if (encryptedKeyVersion != null) {
       json.put(KMSRESTConstants.VERSION_NAME_FIELD,
-          encryptedKeyVersion.getKeyVersionName());
+          encryptedKeyVersion.getEncryptionKeyVersionName());
       json.put(KMSRESTConstants.IV_FIELD,
           Base64.encodeBase64URLSafeString(
-              encryptedKeyVersion.getIv()));
+              encryptedKeyVersion.getEncryptedKeyIv()));
       json.put(KMSRESTConstants.ENCRYPTED_KEY_VERSION_FIELD,
-          toJSON(encryptedKeyVersion.getEncryptedKey()));
+          toJSON(encryptedKeyVersion.getEncryptedKeyVersion()));
     }
     return json;
   }

hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java

@@ -485,10 +485,10 @@ public class TestKMS {
 
         EncryptedKeyVersion ek1 = kpExt.generateEncryptedKey(kv.getName());
         Assert.assertEquals(KeyProviderCryptoExtension.EEK,
-            ek1.getEncryptedKey().getVersionName());
+            ek1.getEncryptedKeyVersion().getVersionName());
-        Assert.assertNotNull(ek1.getEncryptedKey().getMaterial());
+        Assert.assertNotNull(ek1.getEncryptedKeyVersion().getMaterial());
         Assert.assertEquals(kv.getMaterial().length,
-            ek1.getEncryptedKey().getMaterial().length);
+            ek1.getEncryptedKeyVersion().getMaterial().length);
         KeyProvider.KeyVersion k1 = kpExt.decryptEncryptedKey(ek1);
         Assert.assertEquals(KeyProviderCryptoExtension.EK, k1.getVersionName());
         KeyProvider.KeyVersion k1a = kpExt.decryptEncryptedKey(ek1);
@@ -498,8 +498,8 @@ public class TestKMS {
         EncryptedKeyVersion ek2 = kpExt.generateEncryptedKey(kv.getName());
         KeyProvider.KeyVersion k2 = kpExt.decryptEncryptedKey(ek2);
         boolean isEq = true;
-        for (int i = 0; isEq && i < ek2.getEncryptedKey().getMaterial().length;
+        for (int i = 0; isEq && i < ek2.getEncryptedKeyVersion()
-            i++) {
+            .getMaterial().length; i++) {
           isEq = k2.getMaterial()[i] == k1.getMaterial()[i];
         }
         Assert.assertFalse(isEq);