From 4ebc23ba7b16c7b9acf38b5a864682a6c8890690 Mon Sep 17 00:00:00 2001 From: Xiao Chen Date: Thu, 15 Jun 2017 14:25:52 -0700 Subject: [PATCH] HADOOP-14524. Make CryptoCodec Closeable so it can be cleaned up proactively. --- .../hadoop/crypto/AesCtrCryptoCodec.java | 6 ++++ .../org/apache/hadoop/crypto/CryptoCodec.java | 3 +- .../hadoop/crypto/CryptoInputStream.java | 1 + .../hadoop/crypto/CryptoOutputStream.java | 1 + .../crypto/OpensslAesCtrCryptoCodec.java | 13 +++++++- .../key/KeyProviderCryptoExtension.java | 27 ++++++++++------ ...toStreamsWithOpensslAesCtrCryptoCodec.java | 32 +++++++++++++++++-- .../sasl/DataTransferSaslUtil.java | 1 + .../apache/hadoop/mapreduce/CryptoUtils.java | 14 ++++++-- 9 files changed, 82 insertions(+), 16 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/AesCtrCryptoCodec.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/AesCtrCryptoCodec.java index 5e286b9a60..3e52560259 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/AesCtrCryptoCodec.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/AesCtrCryptoCodec.java @@ -22,6 +22,8 @@ import com.google.common.base.Preconditions; +import java.io.IOException; + @InterfaceAudience.Private @InterfaceStability.Evolving public abstract class AesCtrCryptoCodec extends CryptoCodec { @@ -61,4 +63,8 @@ public void calculateIV(byte[] initIV, long counter, byte[] IV) { IV[i] = (byte) sum; } } + + @Override + public void close() throws IOException { + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoCodec.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoCodec.java index 493e23de74..d9c16bbc7a 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoCodec.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoCodec.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.crypto; +import java.io.Closeable; import java.security.GeneralSecurityException; import java.util.List; @@ -42,7 +43,7 @@ */ @InterfaceAudience.Private @InterfaceStability.Evolving -public abstract class CryptoCodec implements Configurable { +public abstract class CryptoCodec implements Configurable, Closeable { public static Logger LOG = LoggerFactory.getLogger(CryptoCodec.class); /** diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoInputStream.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoInputStream.java index b7ded9201c..0be6e349e0 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoInputStream.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoInputStream.java @@ -315,6 +315,7 @@ public void close() throws IOException { super.close(); freeBuffers(); + codec.close(); closed = true; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java index d2f146aa3e..9fb0ff69fd 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java @@ -239,6 +239,7 @@ public synchronized void close() throws IOException { flush(); if (closeOutputStream) { super.close(); + codec.close(); } freeBuffers(); } finally { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/OpensslAesCtrCryptoCodec.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/OpensslAesCtrCryptoCodec.java index d0a12e9fdb..d08e58882c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/OpensslAesCtrCryptoCodec.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/OpensslAesCtrCryptoCodec.java @@ -19,6 +19,7 @@ import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SECURE_RANDOM_IMPL_KEY; +import java.io.Closeable; import java.io.IOException; import java.nio.ByteBuffer; import java.security.GeneralSecurityException; @@ -89,7 +90,17 @@ public Decryptor createDecryptor() throws GeneralSecurityException { public void generateSecureRandom(byte[] bytes) { random.nextBytes(bytes); } - + + @Override + public void close() throws IOException { + try { + Closeable r = (Closeable) this.random; + r.close(); + } catch (ClassCastException e) { + } + super.close(); + } + private static class OpensslAesCtrCipher implements Encryptor, Decryptor { private final OpensslCipher cipher; private final int mode; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderCryptoExtension.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderCryptoExtension.java index 9ae98b4f14..ea5ff28c09 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderCryptoExtension.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderCryptoExtension.java @@ -274,12 +274,16 @@ public EncryptedKeyVersion generateEncryptedKey(String encryptionKeyName) // Generate random bytes for new key and IV CryptoCodec cc = CryptoCodec.getInstance(keyProvider.getConf()); - final byte[] newKey = new byte[encryptionKey.getMaterial().length]; - cc.generateSecureRandom(newKey); - final byte[] iv = new byte[cc.getCipherSuite().getAlgorithmBlockSize()]; - cc.generateSecureRandom(iv); - Encryptor encryptor = cc.createEncryptor(); - return generateEncryptedKey(encryptor, encryptionKey, newKey, iv); + try { + final byte[] newKey = new byte[encryptionKey.getMaterial().length]; + cc.generateSecureRandom(newKey); + final byte[] iv = new byte[cc.getCipherSuite().getAlgorithmBlockSize()]; + cc.generateSecureRandom(iv); + Encryptor encryptor = cc.createEncryptor(); + return generateEncryptedKey(encryptor, encryptionKey, newKey, iv); + } finally { + cc.close(); + } } private EncryptedKeyVersion generateEncryptedKey(final Encryptor encryptor, @@ -322,9 +326,13 @@ public EncryptedKeyVersion reencryptEncryptedKey(EncryptedKeyVersion ekv) final KeyVersion dek = decryptEncryptedKey(ekv); final CryptoCodec cc = CryptoCodec.getInstance(keyProvider.getConf()); - final Encryptor encryptor = cc.createEncryptor(); - return generateEncryptedKey(encryptor, ekNow, dek.getMaterial(), - ekv.getEncryptedKeyIv()); + try { + final Encryptor encryptor = cc.createEncryptor(); + return generateEncryptedKey(encryptor, ekNow, dek.getMaterial(), + ekv.getEncryptedKeyIv()); + } finally { + cc.close(); + } } @Override @@ -364,6 +372,7 @@ public KeyVersion decryptEncryptedKey( bbOut.flip(); byte[] decryptedKey = new byte[keyLen]; bbOut.get(decryptedKey); + cc.close(); return new KeyVersion(encryptionKey.getName(), EK, decryptedKey); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoStreamsWithOpensslAesCtrCryptoCodec.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoStreamsWithOpensslAesCtrCryptoCodec.java index cc02f48939..241e876e6b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoStreamsWithOpensslAesCtrCryptoCodec.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoStreamsWithOpensslAesCtrCryptoCodec.java @@ -18,12 +18,17 @@ package org.apache.hadoop.crypto; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.crypto.random.OsSecureRandom; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.test.GenericTestUtils; import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.internal.util.reflection.Whitebox; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_CRYPTO_CODEC_CLASSES_AES_CTR_NOPADDING_KEY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; public class TestCryptoStreamsWithOpensslAesCtrCryptoCodec extends TestCryptoStreams { @@ -32,8 +37,7 @@ public class TestCryptoStreamsWithOpensslAesCtrCryptoCodec public static void init() throws Exception { GenericTestUtils.assumeInNativeProfile(); Configuration conf = new Configuration(); - conf.set( - CommonConfigurationKeysPublic.HADOOP_SECURITY_CRYPTO_CODEC_CLASSES_AES_CTR_NOPADDING_KEY, + conf.set(HADOOP_SECURITY_CRYPTO_CODEC_CLASSES_AES_CTR_NOPADDING_KEY, OpensslAesCtrCryptoCodec.class.getName()); codec = CryptoCodec.getInstance(conf); assertNotNull("Unable to instantiate codec " + @@ -42,4 +46,28 @@ public static void init() throws Exception { assertEquals(OpensslAesCtrCryptoCodec.class.getCanonicalName(), codec.getClass().getCanonicalName()); } + + @Test + public void testCodecClosesRandom() throws Exception { + GenericTestUtils.assumeInNativeProfile(); + Configuration conf = new Configuration(); + conf.set(HADOOP_SECURITY_CRYPTO_CODEC_CLASSES_AES_CTR_NOPADDING_KEY, + OpensslAesCtrCryptoCodec.class.getName()); + conf.set( + CommonConfigurationKeysPublic.HADOOP_SECURITY_SECURE_RANDOM_IMPL_KEY, + OsSecureRandom.class.getName()); + CryptoCodec codecWithRandom = CryptoCodec.getInstance(conf); + assertNotNull( + "Unable to instantiate codec " + OpensslAesCtrCryptoCodec.class + .getName() + ", is the required " + "version of OpenSSL installed?", + codecWithRandom); + OsSecureRandom random = + (OsSecureRandom) Whitebox.getInternalState(codecWithRandom, "random"); + // trigger the OsSecureRandom to create an internal FileInputStream + random.nextBytes(new byte[10]); + assertNotNull(Whitebox.getInternalState(random, "stream")); + // verify closing the codec closes the codec's random's stream. + codecWithRandom.close(); + assertNull(Whitebox.getInternalState(random, "stream")); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/sasl/DataTransferSaslUtil.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/sasl/DataTransferSaslUtil.java index 5e07550a86..f4651eb581 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/sasl/DataTransferSaslUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/sasl/DataTransferSaslUtil.java @@ -286,6 +286,7 @@ public static CipherOption negotiateCipherOption(Configuration conf, codec.generateSecureRandom(inIv); codec.generateSecureRandom(outKey); codec.generateSecureRandom(outIv); + codec.close(); return new CipherOption(suite, inKey, inIv, outKey, outIv); } } diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/CryptoUtils.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/CryptoUtils.java index c05b6b0815..00119cd1b5 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/CryptoUtils.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/CryptoUtils.java @@ -66,16 +66,24 @@ public static byte[] createIV(Configuration conf) throws IOException { if (isEncryptedSpillEnabled(conf)) { byte[] iv = new byte[cryptoCodec.getCipherSuite().getAlgorithmBlockSize()]; cryptoCodec.generateSecureRandom(iv); + cryptoCodec.close(); return iv; } else { return null; } } - public static int cryptoPadding(Configuration conf) { + public static int cryptoPadding(Configuration conf) throws IOException { // Sizeof(IV) + long(start-offset) - return isEncryptedSpillEnabled(conf) ? CryptoCodec.getInstance(conf) - .getCipherSuite().getAlgorithmBlockSize() + 8 : 0; + if (!isEncryptedSpillEnabled(conf)) { + return 0; + } + final CryptoCodec cryptoCodec = CryptoCodec.getInstance(conf); + try { + return cryptoCodec.getCipherSuite().getAlgorithmBlockSize() + 8; + } finally { + cryptoCodec.close(); + } } private static byte[] getEncryptionKey() throws IOException {