You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by jd...@apache.org on 2016/05/01 03:31:00 UTC
[06/11] hive git commit: HIVE-13447 : LLAP: check ZK acls for
registry and fail if they are too permissive (Sergey Shelukhin,
reviewed by Prasanth Jayachandran)
HIVE-13447 : LLAP: check ZK acls for registry and fail if they are too permissive (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/1289aff1
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/1289aff1
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/1289aff1
Branch: refs/heads/llap
Commit: 1289aff1adbff9f35032d4a6ed074207fe1eb4de
Parents: 134b6cc
Author: Sergey Shelukhin <se...@apache.org>
Authored: Fri Apr 29 10:55:48 2016 -0700
Committer: Sergey Shelukhin <se...@apache.org>
Committed: Fri Apr 29 10:55:48 2016 -0700
----------------------------------------------------------------------
.../org/apache/hadoop/hive/conf/HiveConf.java | 3 +
.../impl/LlapZookeeperRegistryImpl.java | 74 +++++++++++++++-----
.../hadoop/hive/ql/exec/tez/DagUtils.java | 1 -
3 files changed, 60 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/1289aff1/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
index fd725cb..db4b9e8 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -2803,6 +2803,9 @@ public class HiveConf extends Configuration {
false,
"Whether to setup split locations to match nodes on which llap daemons are running," +
" instead of using the locations provided by the split itself"),
+ LLAP_VALIDATE_ACLS("hive.llap.validate.acls", true,
+ "Whether LLAP should reject permissive ACLs in some cases (e.g. its own management\n" +
+ "protocol or ZK paths), similar to how ssh refuses a key with bad access permissions."),
SPARK_CLIENT_FUTURE_TIMEOUT("hive.spark.client.future.timeout",
"60s", new TimeValidator(TimeUnit.SECONDS),
http://git-wip-us.apache.org/repos/asf/hive/blob/1289aff1/llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java
----------------------------------------------------------------------
diff --git a/llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java b/llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java
index d51249a..6981061 100644
--- a/llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java
+++ b/llap-client/src/java/org/apache/hadoop/hive/llap/registry/impl/LlapZookeeperRegistryImpl.java
@@ -70,10 +70,12 @@ import org.apache.hadoop.yarn.conf.YarnConfiguration;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.client.ZooKeeperSaslClient;
import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
public class LlapZookeeperRegistryImpl implements ServiceRegistry {
@@ -88,10 +90,13 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
private static final String IPC_SHUFFLE = "shuffle";
private static final String IPC_LLAP = "llap";
private final static String ROOT_NAMESPACE = "llap";
+ private final static String USER_SCOPE_PATH_PREFIX = "user-";
private final Configuration conf;
private final CuratorFramework zooKeeperClient;
- private final String pathPrefix;
+ private final String pathPrefix, userPathPrefix;
+ private String userNameFromPrincipal; // Only set when setting up the secure config for ZK.
+
private PersistentEphemeralNode znode;
private String znodePath; // unique identity for this instance
private final ServiceRecordMarshal encoder; // to marshal/unmarshal znode data
@@ -125,23 +130,23 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
@Override
public List<ACL> getDefaultAcl() {
- List<ACL> nodeAcls = new ArrayList<ACL>();
- if (UserGroupInformation.isSecurityEnabled()) {
- // Read all to the world
- nodeAcls.addAll(ZooDefs.Ids.READ_ACL_UNSAFE);
- // Create/Delete/Write/Admin to the authenticated user
- nodeAcls.add(new ACL(ZooDefs.Perms.ALL, ZooDefs.Ids.AUTH_IDS));
- } else {
- // ACLs for znodes on a non-kerberized cluster
- // Create/Read/Delete/Write/Admin to the world
- nodeAcls.addAll(ZooDefs.Ids.OPEN_ACL_UNSAFE);
- }
- return nodeAcls;
+ // We always return something from getAclForPath so this should not happen.
+ LOG.warn("getDefaultAcl was called");
+ return Lists.newArrayList(ZooDefs.Ids.OPEN_ACL_UNSAFE);
}
@Override
public List<ACL> getAclForPath(String path) {
- return getDefaultAcl();
+ if (!UserGroupInformation.isSecurityEnabled() || path == null
+ || !path.contains(userPathPrefix)) {
+ // No security or the path is below the user path - full access.
+ return Lists.newArrayList(ZooDefs.Ids.OPEN_ACL_UNSAFE);
+ }
+ // Read all to the world
+ List<ACL> nodeAcls = new ArrayList<ACL>(ZooDefs.Ids.READ_ACL_UNSAFE);
+ // Create/Delete/Write/Admin to creator
+ nodeAcls.addAll(ZooDefs.Ids.CREATOR_ALL_ACL);
+ return nodeAcls;
}
};
@@ -172,7 +177,8 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
// worker does not respond due to communication interruptions it will retain the same sequence
// number when it returns back. If session timeout expires, the node will be deleted and new
// addition of the same node (restart) will get next sequence number
- this.pathPrefix = "/" + RegistryUtils.currentUser() + "/" + instanceName + "/workers/worker-";
+ this.userPathPrefix = USER_SCOPE_PATH_PREFIX + RegistryUtils.currentUser();
+ this.pathPrefix = "/" + userPathPrefix + "/" + instanceName + "/workers/worker-";
this.instancesCache = null;
this.instances = null;
this.stateChangeListeners = new HashSet<>();
@@ -276,9 +282,11 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
}
znodePath = znode.getActualPath();
+ if (HiveConf.getBoolVar(conf, ConfVars.LLAP_VALIDATE_ACLS)) {
+ checkAcls();
+ }
// Set a watch on the znode
- if (zooKeeperClient.checkExists()
- .forPath(znodePath) == null) {
+ if (zooKeeperClient.checkExists().forPath(znodePath) == null) {
// No node exists, throw exception
throw new Exception("Unable to create znode for this LLAP instance on ZooKeeper.");
}
@@ -297,6 +305,31 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
return uniq.toString();
}
+ private void checkAcls() throws Exception {
+ if (!UserGroupInformation.isSecurityEnabled()) return;
+ String pathToCheck = znodePath;
+ // We are trying to check ACLs on the "workers" directory, which noone except us should be
+ // able to write to. Higher-level directories shouldn't matter - we don't read them.
+ int ix = pathToCheck.lastIndexOf('/');
+ if (ix > 0) {
+ pathToCheck = pathToCheck.substring(0, ix);
+ }
+ List<ACL> acls = zooKeeperClient.usingNamespace(null).getACL().forPath(pathToCheck);
+ if (acls == null || acls.isEmpty()) {
+ // Can there be no ACLs? There's some access (to get ACLs), so assume it means free for all.
+ throw new SecurityException("No ACLs on " + pathToCheck);
+ }
+ // This could be brittle.
+ assert userNameFromPrincipal != null;
+ Id currentUser = new Id("sasl", userNameFromPrincipal);
+ for (ACL acl : acls) {
+ if ((acl.getPerms() & ~ZooDefs.Perms.READ) == 0 || currentUser.equals(acl.getId())) {
+ continue; // Read permission/no permissions, or the expected user.
+ }
+ throw new SecurityException("The ACL " + acl + " is unnacceptable for " + pathToCheck);
+ }
+ }
+
@Override
public void unregister() throws IOException {
// Nothing for the zkCreate models
@@ -643,6 +676,7 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
System.setProperty(ZooKeeperSaslClient.LOGIN_CONTEXT_NAME_KEY, SASL_LOGIN_CONTEXT_NAME);
principal = SecurityUtil.getServerPrincipal(principal, "0.0.0.0");
+ userNameFromPrincipal = getUserNameFromPrincipal(principal);
JaasConfiguration jaasConf = new JaasConfiguration(SASL_LOGIN_CONTEXT_NAME, principal,
keyTabFile);
@@ -650,6 +684,12 @@ public class LlapZookeeperRegistryImpl implements ServiceRegistry {
javax.security.auth.login.Configuration.setConfiguration(jaasConf);
}
+ private String getUserNameFromPrincipal(String principal) {
+ // Based on SecurityUtil.
+ String[] components = principal.split("[/@]");
+ return (components == null || components.length != 3) ? principal : components[0];
+ }
+
/**
* A JAAS configuration for ZooKeeper clients intended to use for SASL
* Kerberos.
http://git-wip-us.apache.org/repos/asf/hive/blob/1289aff1/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java
index 8aca779..79da860 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java
@@ -577,7 +577,6 @@ public class DagUtils {
}
}
- // TODO# HERE?
if (mapWork instanceof MergeFileWork) {
Path outputPath = ((MergeFileWork) mapWork).getOutputDir();
// prepare the tmp output directory. The output tmp directory should