You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2016/05/23 20:56:01 UTC

hive git commit: HIVE-13787 : LLAP: bug in recent security patches (wrong argument order; using full user name in id) (Sergey Shelukhin, reviewed by Siddharth Seth)

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 <se...@apache.org>
Authored: Mon May 23 13:52:10 2016 -0700
Committer: Sergey Shelukhin <se...@apache.org>
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;
   }