HADOOP-14524. Make CryptoCodec Closeable so it can be cleaned up proactively.

This commit is contained in:
Xiao Chen 2017-06-16 09:37:38 -07:00
parent 4e6348f346
commit 2afe9722af
9 changed files with 106 additions and 41 deletions

View File

@ -22,6 +22,8 @@ import org.apache.hadoop.classification.InterfaceStability;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import java.io.IOException;
@InterfaceAudience.Private @InterfaceAudience.Private
@InterfaceStability.Evolving @InterfaceStability.Evolving
public abstract class AesCtrCryptoCodec extends CryptoCodec { public abstract class AesCtrCryptoCodec extends CryptoCodec {
@ -61,4 +63,8 @@ public abstract class AesCtrCryptoCodec extends CryptoCodec {
IV[i] = (byte) sum; IV[i] = (byte) sum;
} }
} }
@Override
public void close() throws IOException {
}
} }

View File

@ -17,6 +17,7 @@
*/ */
package org.apache.hadoop.crypto; package org.apache.hadoop.crypto;
import java.io.Closeable;
import java.security.GeneralSecurityException; import java.security.GeneralSecurityException;
import java.util.List; import java.util.List;
@ -42,7 +43,7 @@ import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY
*/ */
@InterfaceAudience.Private @InterfaceAudience.Private
@InterfaceStability.Evolving @InterfaceStability.Evolving
public abstract class CryptoCodec implements Configurable { public abstract class CryptoCodec implements Configurable, Closeable {
public static Logger LOG = LoggerFactory.getLogger(CryptoCodec.class); public static Logger LOG = LoggerFactory.getLogger(CryptoCodec.class);
/** /**

View File

@ -315,6 +315,7 @@ public class CryptoInputStream extends FilterInputStream implements
super.close(); super.close();
freeBuffers(); freeBuffers();
codec.close();
closed = true; closed = true;
} }

View File

@ -239,6 +239,7 @@ public class CryptoOutputStream extends FilterOutputStream implements
flush(); flush();
if (closeOutputStream) { if (closeOutputStream) {
super.close(); super.close();
codec.close();
} }
freeBuffers(); freeBuffers();
} finally { } finally {

View File

@ -19,6 +19,7 @@ package org.apache.hadoop.crypto;
import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SECURE_RANDOM_IMPL_KEY; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SECURE_RANDOM_IMPL_KEY;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.security.GeneralSecurityException; import java.security.GeneralSecurityException;
@ -90,6 +91,16 @@ public class OpensslAesCtrCryptoCodec extends AesCtrCryptoCodec {
random.nextBytes(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 static class OpensslAesCtrCipher implements Encryptor, Decryptor {
private final OpensslCipher cipher; private final OpensslCipher cipher;
private final int mode; private final int mode;

View File

@ -254,6 +254,7 @@ public class KeyProviderCryptoExtension extends
// Generate random bytes for new key and IV // Generate random bytes for new key and IV
CryptoCodec cc = CryptoCodec.getInstance(keyProvider.getConf()); CryptoCodec cc = CryptoCodec.getInstance(keyProvider.getConf());
try {
final byte[] newKey = new byte[encryptionKey.getMaterial().length]; final byte[] newKey = new byte[encryptionKey.getMaterial().length];
cc.generateSecureRandom(newKey); cc.generateSecureRandom(newKey);
final byte[] iv = new byte[cc.getCipherSuite().getAlgorithmBlockSize()]; final byte[] iv = new byte[cc.getCipherSuite().getAlgorithmBlockSize()];
@ -274,6 +275,9 @@ public class KeyProviderCryptoExtension extends
return new EncryptedKeyVersion(encryptionKeyName, return new EncryptedKeyVersion(encryptionKeyName,
encryptionKey.getVersionName(), iv, encryptionKey.getVersionName(), iv,
new KeyVersion(encryptionKey.getName(), EEK, encryptedKey)); new KeyVersion(encryptionKey.getName(), EEK, encryptedKey));
} finally {
cc.close();
}
} }
@Override @Override
@ -300,6 +304,7 @@ public class KeyProviderCryptoExtension extends
EncryptedKeyVersion.deriveIV(encryptedKeyVersion.getEncryptedKeyIv()); EncryptedKeyVersion.deriveIV(encryptedKeyVersion.getEncryptedKeyIv());
CryptoCodec cc = CryptoCodec.getInstance(keyProvider.getConf()); CryptoCodec cc = CryptoCodec.getInstance(keyProvider.getConf());
try {
Decryptor decryptor = cc.createDecryptor(); Decryptor decryptor = cc.createDecryptor();
decryptor.init(encryptionKey.getMaterial(), encryptionIV); decryptor.init(encryptionKey.getMaterial(), encryptionIV);
final KeyVersion encryptedKV = final KeyVersion encryptedKV =
@ -314,6 +319,9 @@ public class KeyProviderCryptoExtension extends
byte[] decryptedKey = new byte[keyLen]; byte[] decryptedKey = new byte[keyLen];
bbOut.get(decryptedKey); bbOut.get(decryptedKey);
return new KeyVersion(encryptionKey.getName(), EK, decryptedKey); return new KeyVersion(encryptionKey.getName(), EK, decryptedKey);
} finally {
cc.close();
}
} }
@Override @Override

View File

@ -18,12 +18,17 @@
package org.apache.hadoop.crypto; package org.apache.hadoop.crypto;
import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.crypto.random.OsSecureRandom;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.test.GenericTestUtils;
import org.junit.BeforeClass; 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.assertEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
public class TestCryptoStreamsWithOpensslAesCtrCryptoCodec public class TestCryptoStreamsWithOpensslAesCtrCryptoCodec
extends TestCryptoStreams { extends TestCryptoStreams {
@ -32,8 +37,7 @@ public class TestCryptoStreamsWithOpensslAesCtrCryptoCodec
public static void init() throws Exception { public static void init() throws Exception {
GenericTestUtils.assumeInNativeProfile(); GenericTestUtils.assumeInNativeProfile();
Configuration conf = new Configuration(); Configuration conf = new Configuration();
conf.set( conf.set(HADOOP_SECURITY_CRYPTO_CODEC_CLASSES_AES_CTR_NOPADDING_KEY,
CommonConfigurationKeysPublic.HADOOP_SECURITY_CRYPTO_CODEC_CLASSES_AES_CTR_NOPADDING_KEY,
OpensslAesCtrCryptoCodec.class.getName()); OpensslAesCtrCryptoCodec.class.getName());
codec = CryptoCodec.getInstance(conf); codec = CryptoCodec.getInstance(conf);
assertNotNull("Unable to instantiate codec " + assertNotNull("Unable to instantiate codec " +
@ -42,4 +46,28 @@ public class TestCryptoStreamsWithOpensslAesCtrCryptoCodec
assertEquals(OpensslAesCtrCryptoCodec.class.getCanonicalName(), assertEquals(OpensslAesCtrCryptoCodec.class.getCanonicalName(),
codec.getClass().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"));
}
} }

View File

@ -286,6 +286,7 @@ public final class DataTransferSaslUtil {
codec.generateSecureRandom(inIv); codec.generateSecureRandom(inIv);
codec.generateSecureRandom(outKey); codec.generateSecureRandom(outKey);
codec.generateSecureRandom(outIv); codec.generateSecureRandom(outIv);
codec.close();
return new CipherOption(suite, inKey, inIv, outKey, outIv); return new CipherOption(suite, inKey, inIv, outKey, outIv);
} }
} }

View File

@ -66,16 +66,24 @@ public class CryptoUtils {
if (isEncryptedSpillEnabled(conf)) { if (isEncryptedSpillEnabled(conf)) {
byte[] iv = new byte[cryptoCodec.getCipherSuite().getAlgorithmBlockSize()]; byte[] iv = new byte[cryptoCodec.getCipherSuite().getAlgorithmBlockSize()];
cryptoCodec.generateSecureRandom(iv); cryptoCodec.generateSecureRandom(iv);
cryptoCodec.close();
return iv; return iv;
} else { } else {
return null; return null;
} }
} }
public static int cryptoPadding(Configuration conf) { public static int cryptoPadding(Configuration conf) throws IOException {
// Sizeof(IV) + long(start-offset) // Sizeof(IV) + long(start-offset)
return isEncryptedSpillEnabled(conf) ? CryptoCodec.getInstance(conf) if (!isEncryptedSpillEnabled(conf)) {
.getCipherSuite().getAlgorithmBlockSize() + 8 : 0; return 0;
}
final CryptoCodec cryptoCodec = CryptoCodec.getInstance(conf);
try {
return cryptoCodec.getCipherSuite().getAlgorithmBlockSize() + 8;
} finally {
cryptoCodec.close();
}
} }
private static byte[] getEncryptionKey() throws IOException { private static byte[] getEncryptionKey() throws IOException {