Repository: hive Updated Branches: refs/heads/master a25be60e9 -> e2d07e277
HIVE-13787 : LLAP: bug in recent security patches (wrong argument order; using full user name in id) (Sergey Shelukhin, reviewed by Siddharth Seth) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/e2d07e27 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/e2d07e27 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/e2d07e27 Branch: refs/heads/master Commit: e2d07e2775b2befea56d70fc3908e7670c79d08b Parents: a25be60 Author: Sergey Shelukhin <[email protected]> Authored: Mon May 23 13:52:10 2016 -0700 Committer: Sergey Shelukhin <[email protected]> Committed: Mon May 23 13:52:10 2016 -0700 ---------------------------------------------------------------------- .../hadoop/hive/llap/security/LlapTokenIdentifier.java | 13 +++++++++---- .../hadoop/hive/llap/daemon/impl/LlapDaemon.java | 2 +- .../hadoop/hive/llap/daemon/impl/LlapTokenChecker.java | 8 +++++--- .../hadoop/hive/llap/daemon/impl/QueryTracker.java | 13 +++++++++---- .../hive/llap/daemon/impl/TaskExecutorTestHelpers.java | 2 +- 5 files changed, 25 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/e2d07e27/llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java ---------------------------------------------------------------------- diff --git a/llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java b/llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java index e28eddd..7c47f0b 100644 --- a/llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java +++ b/llap-common/src/java/org/apache/hadoop/hive/llap/security/LlapTokenIdentifier.java @@ -28,6 +28,8 @@ import org.apache.hadoop.io.Text; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.delegation.AbstractDelegationTokenIdentifier; +import com.google.common.base.Preconditions; + /** For now, a LLAP token gives access to any LLAP server. */ public class LlapTokenIdentifier extends AbstractDelegationTokenIdentifier { private static final String KIND = "LLAP_TOKEN"; @@ -42,6 +44,7 @@ public class LlapTokenIdentifier extends AbstractDelegationTokenIdentifier { public LlapTokenIdentifier(Text owner, Text renewer, Text realUser, String clusterId, String appId) { super(owner, renewer, realUser); + Preconditions.checkNotNull(clusterId); this.clusterId = clusterId; this.appId = appId == null ? "" : appId; } @@ -57,7 +60,9 @@ public class LlapTokenIdentifier extends AbstractDelegationTokenIdentifier { public void readFields(DataInput in) throws IOException { super.readFields(in); clusterId = in.readUTF(); + Preconditions.checkNotNull(clusterId); appId = in.readUTF(); + appId = appId == null ? "" : appId; } @Override @@ -76,7 +81,7 @@ public class LlapTokenIdentifier extends AbstractDelegationTokenIdentifier { @Override public int hashCode() { final int prime = 31; - int result = prime * super.hashCode() + ((appId == null) ? 0 : appId.hashCode()); + int result = prime * super.hashCode() + (StringUtils.isBlank(appId) ? 0 : appId.hashCode()); return prime * result + ((clusterId == null) ? 0 : clusterId.hashCode()); } @@ -85,14 +90,14 @@ public class LlapTokenIdentifier extends AbstractDelegationTokenIdentifier { if (this == obj) return true; if (!(obj instanceof LlapTokenIdentifier) || !super.equals(obj)) return false; LlapTokenIdentifier other = (LlapTokenIdentifier) obj; - return (appId == null ? other.appId == null : appId.equals(other.appId)) + return (StringUtils.isBlank(appId) + ? StringUtils.isBlank(other.appId) : appId.equals(other.appId)) && (clusterId == null ? other.clusterId == null : clusterId.equals(other.clusterId)); } @Override public String toString() { - return KIND + "; " + super.toString() + ", cluster " + clusterId + ", app secret hash " - + (StringUtils.isBlank(appId) ? 0 : appId.hashCode()); + return KIND + "; " + super.toString() + ", cluster " + clusterId + ", app ID " + appId; } @InterfaceAudience.Private http://git-wip-us.apache.org/repos/asf/hive/blob/e2d07e27/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java ---------------------------------------------------------------------- diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java index de817e3..5ab7b3c 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java @@ -143,7 +143,7 @@ public class LlapDaemon extends CompositeService implements ContainerRunner, Lla } String hostName = MetricsUtils.getHostName(); try { - daemonId = new DaemonId(UserGroupInformation.getCurrentUser().getUserName(), + daemonId = new DaemonId(UserGroupInformation.getCurrentUser().getShortUserName(), LlapUtil.generateClusterName(daemonConf), hostName, appName, System.currentTimeMillis()); } catch (IOException ex) { throw new RuntimeException(ex); http://git-wip-us.apache.org/repos/asf/hive/blob/e2d07e27/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java ---------------------------------------------------------------------- diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java index 03ee055..04df929 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTokenChecker.java @@ -14,6 +14,7 @@ package org.apache.hadoop.hive.llap.daemon.impl; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import java.util.ArrayList; @@ -95,6 +96,7 @@ public final class LlapTokenChecker { public static void checkPermissions( String clusterId, String userName, String appId, Object hint) throws IOException { if (!UserGroupInformation.isSecurityEnabled()) return; + Preconditions.checkNotNull(userName); UserGroupInformation current = UserGroupInformation.getCurrentUser(); String kerberosName = current.hasKerberosCredentials() ? current.getShortUserName() : null; List<LlapTokenIdentifier> tokens = getLlapTokens(current, clusterId); @@ -104,7 +106,7 @@ public final class LlapTokenChecker { @VisibleForTesting static void checkPermissionsInternal(String kerberosName, List<LlapTokenIdentifier> tokens, String userName, String appId, Object hint) { - if (kerberosName != null && StringUtils.isEmpty(appId) && kerberosName.equals(userName)) { + if (kerberosName != null && StringUtils.isBlank(appId) && kerberosName.equals(userName)) { return; } if (tokens != null) { @@ -132,6 +134,6 @@ public final class LlapTokenChecker { private static boolean checkTokenPermissions( String userName, String appId, String tokenUser, String tokenAppId) { return userName.equals(tokenUser) - && (StringUtils.isEmpty(appId) || appId.equals(tokenAppId)); + && (StringUtils.isBlank(appId) || appId.equals(tokenAppId)); } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/hive/blob/e2d07e27/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java ---------------------------------------------------------------------- diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java index 8abd198..c55436b 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java @@ -14,6 +14,7 @@ package org.apache.hadoop.hive.llap.daemon.impl; +import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -21,6 +22,7 @@ import java.util.concurrent.TimeUnit; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.token.Token; import org.apache.tez.common.CallableWithNdc; @@ -134,9 +136,12 @@ public class QueryTracker extends AbstractService { boolean isExistingQueryInfo = true; QueryInfo queryInfo = queryInfoMap.get(queryIdentifier); if (queryInfo == null) { + String tokenUser = tokenInfo.getLeft(), tokenAppId = tokenInfo.getRight(); + if (UserGroupInformation.isSecurityEnabled()) { + Preconditions.checkNotNull(tokenUser); + } queryInfo = new QueryInfo(queryIdentifier, appIdString, dagName, dagIdentifier, user, - getSourceCompletionMap(queryIdentifier), localDirsBase, localFs, - tokenInfo.getLeft(), tokenInfo.getRight()); + getSourceCompletionMap(queryIdentifier), localDirsBase, localFs, tokenUser, tokenAppId); QueryInfo old = queryInfoMap.putIfAbsent(queryIdentifier, queryInfo); if (old != null) { queryInfo = old; @@ -341,8 +346,8 @@ public class QueryTracker extends AbstractService { private QueryInfo checkPermissionsAndGetQuery(QueryIdentifier queryId) throws IOException { QueryInfo queryInfo = queryInfoMap.get(queryId); if (queryInfo == null) return null; - LlapTokenChecker.checkPermissions(clusterId, queryInfo.getTokenAppId(), - queryInfo.getTokenUserName(), queryInfo.getQueryIdentifier()); + LlapTokenChecker.checkPermissions(clusterId, queryInfo.getTokenUserName(), + queryInfo.getTokenAppId(), queryInfo.getQueryIdentifier()); return queryInfo; } http://git-wip-us.apache.org/repos/asf/hive/blob/e2d07e27/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java ---------------------------------------------------------------------- diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java b/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java index 279baf1..e0f0676 100644 --- a/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java +++ b/llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java @@ -79,7 +79,7 @@ public class TaskExecutorTestHelpers { QueryInfo queryInfo = new QueryInfo(queryIdentifier, "fake_app_id_string", "fake_dag_name", 1, "fakeUser", new ConcurrentHashMap<String, LlapDaemonProtocolProtos.SourceStateProto>(), - new String[0], null, null, null); + new String[0], null, "fakeUser", null); return queryInfo; }
