From 48c9064b33dd07fd158b0415b0cd6d1a0a6ef06d Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Sun, 23 Oct 2016 10:58:36 -0700 Subject: [PATCH] HADOOP-13749. KMSClientProvider combined with KeyProviderCache can result in wrong UGI being used. Contributed by Xiaoyu Yao. (cherry picked from commit d0a347984da175948b553a675dc357491df2fd0f) --- .../crypto/key/kms/KMSClientProvider.java | 52 +++++++++---------- .../hadoop/security/UserGroupInformation.java | 14 +++++ .../hadoop/crypto/key/kms/server/TestKMS.java | 2 + 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java index 3d8194169bf..65f24873a58 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java @@ -373,7 +373,6 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, private ConnectionConfigurator configurator; private DelegationTokenAuthenticatedURL.Token authToken; private final int authRetry; - private final UserGroupInformation actualUgi; @Override public String toString() { @@ -455,15 +454,6 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, KMS_CLIENT_ENC_KEY_CACHE_NUM_REFILL_THREADS_DEFAULT), new EncryptedQueueRefiller()); authToken = new DelegationTokenAuthenticatedURL.Token(); - UserGroupInformation.AuthenticationMethod authMethod = - UserGroupInformation.getCurrentUser().getAuthenticationMethod(); - if (authMethod == UserGroupInformation.AuthenticationMethod.PROXY) { - actualUgi = UserGroupInformation.getCurrentUser().getRealUser(); - } else if (authMethod == UserGroupInformation.AuthenticationMethod.TOKEN) { - actualUgi = UserGroupInformation.getLoginUser(); - } else { - actualUgi =UserGroupInformation.getCurrentUser(); - } } private static Path extractKMSPath(URI uri) throws MalformedURLException, IOException { @@ -530,19 +520,9 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, throws IOException { HttpURLConnection conn; try { - // if current UGI is different from UGI at constructor time, behave as - // proxyuser - UserGroupInformation currentUgi = UserGroupInformation.getCurrentUser(); - final String doAsUser = (currentUgi.getAuthenticationMethod() == - UserGroupInformation.AuthenticationMethod.PROXY) - ? currentUgi.getShortUserName() : null; - - // If current UGI contains kms-dt && is not proxy, doAs it to use its dt. - // Otherwise, create the HTTP connection using the UGI at constructor time - UserGroupInformation ugiToUse = - (currentUgiContainsKmsDt() && doAsUser == null) ? - currentUgi : actualUgi; - conn = ugiToUse.doAs(new PrivilegedExceptionAction() { + final String doAsUser = getDoAsUser(); + conn = getActualUgi().doAs(new PrivilegedExceptionAction + () { @Override public HttpURLConnection run() throws Exception { DelegationTokenAuthenticatedURL authUrl = @@ -919,7 +899,7 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, token, url, doAsUser); final DelegationTokenAuthenticatedURL authUrl = new DelegationTokenAuthenticatedURL(configurator); - return actualUgi.doAs( + return getActualUgi().doAs( new PrivilegedExceptionAction() { @Override public Long run() throws Exception { @@ -942,7 +922,7 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, final String doAsUser = getDoAsUser(); final DelegationTokenAuthenticatedURL.Token token = generateDelegationToken(dToken); - return actualUgi.doAs( + return getActualUgi().doAs( new PrivilegedExceptionAction() { @Override public Void run() throws Exception { @@ -1014,7 +994,7 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, new DelegationTokenAuthenticatedURL(configurator); try { final String doAsUser = getDoAsUser(); - token = actualUgi.doAs(new PrivilegedExceptionAction>() { + token = getActualUgi().doAs(new PrivilegedExceptionAction>() { @Override public Token run() throws Exception { // Not using the cached token here.. Creating a new token here @@ -1060,6 +1040,26 @@ public class KMSClientProvider extends KeyProvider implements CryptoExtension, return false; } + private UserGroupInformation getActualUgi() throws IOException { + final UserGroupInformation currentUgi = UserGroupInformation + .getCurrentUser(); + if (LOG.isDebugEnabled()) { + UserGroupInformation.logAllUserInfo(currentUgi); + } + // Use current user by default + UserGroupInformation actualUgi = currentUgi; + if (currentUgi.getRealUser() != null) { + // Use real user for proxy user + actualUgi = currentUgi.getRealUser(); + } else if (!currentUgiContainsKmsDt() && + !currentUgi.hasKerberosCredentials()) { + // Use login user for user that does not have either + // Kerberos credential or KMS delegation token for KMS operations + actualUgi = currentUgi.getLoginUser(); + } + return actualUgi; + } + /** * Shutdown valueQueue executor threads */ diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java index 526ee4143dc..45247b0b136 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java @@ -1811,6 +1811,20 @@ public class UserGroupInformation { } } + public static void logAllUserInfo(UserGroupInformation ugi) throws + IOException { + if (LOG.isDebugEnabled()) { + LOG.debug("UGI: " + ugi); + if (ugi.getRealUser() != null) { + LOG.debug("+RealUGI: " + ugi.getRealUser()); + } + LOG.debug("+LoginUGI: " + ugi.getLoginUser()); + for (Token token : ugi.getTokens()) { + LOG.debug("+UGI token:" + token); + } + } + } + private void print() throws IOException { System.out.println("User: " + getUserName()); System.out.print("Group Ids: "); diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java index 58c1b812f6f..44f546c1eaa 100644 --- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java +++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java @@ -1832,6 +1832,7 @@ public class TestKMS { } else { otherUgi = UserGroupInformation.createUserForTesting("client1", new String[] {"other group"}); + UserGroupInformation.setLoginUser(otherUgi); } try { // test delegation token renewal via renewer @@ -2152,6 +2153,7 @@ public class TestKMS { loginUserFromKeytabAndReturnUGI("client", keytab.getAbsolutePath()); } else { proxyUgi = UserGroupInformation.createRemoteUser("client"); + UserGroupInformation.setLoginUser(proxyUgi); } final UserGroupInformation clientUgi = proxyUgi;