From e14e71d5feff961b681d828b00e6f12cb197ebf5 Mon Sep 17 00:00:00 2001 From: Alejandro Abdelnur Date: Tue, 16 Sep 2014 14:32:49 -0700 Subject: [PATCH] HADOOP-11096. KMS: KeyAuthorizationKeyProvider should verify the keyversion belongs to the keyname on decrypt. (tucu) --- .../hadoop-common/CHANGES.txt | 3 ++ .../key/KeyProviderCryptoExtension.java | 8 +-- .../key/TestKeyProviderCryptoExtension.java | 2 +- .../server/KeyAuthorizationKeyProvider.java | 12 +++++ .../TestKeyAuthorizationKeyProvider.java | 53 +++++++++++++++++++ .../org/apache/hadoop/hdfs/DFSClient.java | 3 +- 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 3bf9d4baf5..9324acda2f 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 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 fed7e9e4d9..968e341338 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 @@ protected EncryptedKeyVersion(String keyName, * 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 @@ protected EncryptedKeyVersion(String keyName, * @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); } 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 70ec6feaf1..62e3310173 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 void testEncryptDecrypt() throws Exception { // Test the createForDecryption factory method EncryptedKeyVersion eek2 = - EncryptedKeyVersion.createForDecryption( + EncryptedKeyVersion.createForDecryption(eek.getEncryptionKeyName(), eek.getEncryptionKeyVersionName(), eek.getEncryptedKeyIv(), eek.getEncryptedKeyVersion().getMaterial()); 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 fe908e38c9..bccec4aeee 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 EncryptedKeyVersion generateEncryptedKey(String encryptionKeyName) 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); 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 a79926a9cd..1db3d70688 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 @@ private static KeyProvider.Options newOptions(Configuration conf) { 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; + } + } + ); + } + } 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 9da8efc748..456fac6342 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 @@ private KeyVersion decryptEncryptedDataEncryptionKey(FileEncryptionInfo " 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);