Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id F1DD81127A for ; Wed, 17 Sep 2014 06:21:04 +0000 (UTC) Received: (qmail 17198 invoked by uid 500); 17 Sep 2014 06:21:04 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 17125 invoked by uid 500); 17 Sep 2014 06:21:04 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 17115 invoked by uid 99); 17 Sep 2014 06:21:04 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 17 Sep 2014 06:21:04 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 650B4A17D40; Wed, 17 Sep 2014 06:21:04 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: tucu@apache.org To: common-commits@hadoop.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: git commit: HADOOP-11096. KMS: KeyAuthorizationKeyProvider should verify the keyversion belongs to the keyname on decrypt. (tucu) Date: Wed, 17 Sep 2014 06:21:04 +0000 (UTC) Repository: hadoop Updated Branches: refs/heads/trunk 0e7d1dbf9 -> e14e71d5f HADOOP-11096. KMS: KeyAuthorizationKeyProvider should verify the keyversion belongs to the keyname on decrypt. (tucu) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e14e71d5 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e14e71d5 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e14e71d5 Branch: refs/heads/trunk Commit: e14e71d5feff961b681d828b00e6f12cb197ebf5 Parents: 0e7d1db Author: Alejandro Abdelnur Authored: Tue Sep 16 14:32:49 2014 -0700 Committer: Alejandro Abdelnur Committed: Tue Sep 16 23:20:35 2014 -0700 ---------------------------------------------------------------------- hadoop-common-project/hadoop-common/CHANGES.txt | 3 ++ .../crypto/key/KeyProviderCryptoExtension.java | 8 +-- .../key/TestKeyProviderCryptoExtension.java | 2 +- .../kms/server/KeyAuthorizationKeyProvider.java | 12 +++++ .../server/TestKeyAuthorizationKeyProvider.java | 53 ++++++++++++++++++++ .../java/org/apache/hadoop/hdfs/DFSClient.java | 3 +- 6 files changed, 76 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/e14e71d5/hadoop-common-project/hadoop-common/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 3bf9d4b..9324acd 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -815,6 +815,9 @@ Release 2.6.0 - UNRELEASED HADOOP-11088. Unittest TestKeyShell, TestCredShell and TestKMS assume UNIX path separator for JECKS key store path. (Xiaoyu Yao via cnauroth) + HADOOP-11096. KMS: KeyAuthorizationKeyProvider should verify the keyversion + belongs to the keyname on decrypt. (tucu) + Release 2.5.1 - 2014-09-05 INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/e14e71d5/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderCryptoExtension.java ---------------------------------------------------------------------- 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 fed7e9e..968e341 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 @@ -91,6 +91,8 @@ public class KeyProviderCryptoExtension extends * returned EncryptedKeyVersion will only partially be populated; it is not * necessarily suitable for operations besides decryption. * + * @param keyName Key name of the encryption key use to encrypt the + * encrypted key. * @param encryptionKeyVersionName Version name of the encryption key used * to encrypt the encrypted key. * @param encryptedKeyIv Initialization vector of the encrypted @@ -100,12 +102,12 @@ public class KeyProviderCryptoExtension extends * @param encryptedKeyMaterial Key material of the encrypted key. * @return EncryptedKeyVersion suitable for decryption. */ - public static EncryptedKeyVersion createForDecryption(String - encryptionKeyVersionName, byte[] encryptedKeyIv, + public static EncryptedKeyVersion createForDecryption(String keyName, + String encryptionKeyVersionName, byte[] encryptedKeyIv, byte[] encryptedKeyMaterial) { KeyVersion encryptedKeyVersion = new KeyVersion(null, EEK, encryptedKeyMaterial); - return new EncryptedKeyVersion(null, encryptionKeyVersionName, + return new EncryptedKeyVersion(keyName, encryptionKeyVersionName, encryptedKeyIv, encryptedKeyVersion); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/e14e71d5/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderCryptoExtension.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderCryptoExtension.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderCryptoExtension.java index 70ec6fe..62e3310 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderCryptoExtension.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyProviderCryptoExtension.java @@ -121,7 +121,7 @@ public class TestKeyProviderCryptoExtension { // Test the createForDecryption factory method EncryptedKeyVersion eek2 = - EncryptedKeyVersion.createForDecryption( + EncryptedKeyVersion.createForDecryption(eek.getEncryptionKeyName(), eek.getEncryptionKeyVersionName(), eek.getEncryptedKeyIv(), eek.getEncryptedKeyVersion().getMaterial()); http://git-wip-us.apache.org/repos/asf/hadoop/blob/e14e71d5/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java index fe908e3..bccec4a 100644 --- a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java +++ b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KeyAuthorizationKeyProvider.java @@ -192,9 +192,21 @@ public class KeyAuthorizationKeyProvider extends KeyProviderCryptoExtension { return provider.generateEncryptedKey(encryptionKeyName); } + private void verifyKeyVersionBelongsToKey(EncryptedKeyVersion ekv) + throws IOException { + String kn = ekv.getEncryptionKeyName(); + String kvn = ekv.getEncryptionKeyVersionName(); + KeyVersion kv = provider.getKeyVersion(kvn); + if (!kv.getName().equals(kn)) { + throw new IllegalArgumentException(String.format( + "KeyVersion '%s' does not belong to the key '%s'", kvn, kn)); + } + } + @Override public KeyVersion decryptEncryptedKey(EncryptedKeyVersion encryptedKeyVersion) throws IOException, GeneralSecurityException { + verifyKeyVersionBelongsToKey(encryptedKeyVersion); doAccessCheck( encryptedKeyVersion.getEncryptionKeyName(), KeyOpType.DECRYPT_EEK); return provider.decryptEncryptedKey(encryptedKeyVersion); http://git-wip-us.apache.org/repos/asf/hadoop/blob/e14e71d5/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKeyAuthorizationKeyProvider.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKeyAuthorizationKeyProvider.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKeyAuthorizationKeyProvider.java index a79926a..1db3d70 100644 --- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKeyAuthorizationKeyProvider.java +++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKeyAuthorizationKeyProvider.java @@ -215,4 +215,57 @@ public class TestKeyAuthorizationKeyProvider { return options; } + + @Test(expected = IllegalArgumentException.class) + public void testDecryptWithKeyVersionNameKeyMismatch() throws Exception { + final Configuration conf = new Configuration(); + KeyProvider kp = + new UserProvider.Factory().createProvider(new URI("user:///"), conf); + KeyACLs mock = mock(KeyACLs.class); + when(mock.isACLPresent("testKey", KeyOpType.MANAGEMENT)).thenReturn(true); + when(mock.isACLPresent("testKey", KeyOpType.GENERATE_EEK)).thenReturn(true); + when(mock.isACLPresent("testKey", KeyOpType.DECRYPT_EEK)).thenReturn(true); + when(mock.isACLPresent("testKey", KeyOpType.ALL)).thenReturn(true); + UserGroupInformation u1 = UserGroupInformation.createRemoteUser("u1"); + UserGroupInformation u2 = UserGroupInformation.createRemoteUser("u2"); + UserGroupInformation u3 = UserGroupInformation.createRemoteUser("u3"); + UserGroupInformation sudo = UserGroupInformation.createRemoteUser("sudo"); + when(mock.hasAccessToKey("testKey", u1, + KeyOpType.MANAGEMENT)).thenReturn(true); + when(mock.hasAccessToKey("testKey", u2, + KeyOpType.GENERATE_EEK)).thenReturn(true); + when(mock.hasAccessToKey("testKey", u3, + KeyOpType.DECRYPT_EEK)).thenReturn(true); + when(mock.hasAccessToKey("testKey", sudo, + KeyOpType.ALL)).thenReturn(true); + final KeyProviderCryptoExtension kpExt = + new KeyAuthorizationKeyProvider( + KeyProviderCryptoExtension.createKeyProviderCryptoExtension(kp), + mock); + + sudo.doAs( + new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + Options opt = newOptions(conf); + Map m = new HashMap(); + m.put("key.acl.name", "testKey"); + opt.setAttributes(m); + KeyVersion kv = + kpExt.createKey("foo", SecureRandom.getSeed(16), opt); + kpExt.rollNewVersion(kv.getName()); + kpExt.rollNewVersion(kv.getName(), SecureRandom.getSeed(16)); + EncryptedKeyVersion ekv = kpExt.generateEncryptedKey(kv.getName()); + ekv = EncryptedKeyVersion.createForDecryption( + ekv.getEncryptionKeyName() + "x", + ekv.getEncryptionKeyVersionName(), + ekv.getEncryptedKeyIv(), + ekv.getEncryptedKeyVersion().getMaterial()); + kpExt.decryptEncryptedKey(ekv); + return null; + } + } + ); + } + } http://git-wip-us.apache.org/repos/asf/hadoop/blob/e14e71d5/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java index 9da8efc..456fac6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java @@ -1321,7 +1321,8 @@ public class DFSClient implements java.io.Closeable, RemotePeerFactory, " an encrypted file"); } EncryptedKeyVersion ekv = EncryptedKeyVersion.createForDecryption( - feInfo.getEzKeyVersionName(), feInfo.getIV(), + //TODO: here we have to put the keyName to be provided by HDFS-6987 + null, feInfo.getEzKeyVersionName(), feInfo.getIV(), feInfo.getEncryptedDataEncryptionKey()); try { return provider.decryptEncryptedKey(ekv);