diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java index b1b297c..01124ca 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java @@ -464,6 +464,70 @@ public class RecoverableZooKeeper { } /** + * getAcl is an idempotent operation. Retry before throwing exception + * @return list of ACLs + */ + public List getAcl(String path, Stat stat) + throws KeeperException, InterruptedException { + TraceScope traceScope = null; + try { + traceScope = Trace.startSpan("RecoverableZookeeper.getAcl"); + RetryCounter retryCounter = retryCounterFactory.create(); + while (true) { + try { + return checkZk().getACL(path, stat); + } catch (KeeperException e) { + switch (e.code()) { + case CONNECTIONLOSS: + case SESSIONEXPIRED: + case OPERATIONTIMEOUT: + retryOrThrow(retryCounter, e, "getAcl"); + break; + + default: + throw e; + } + } + retryCounter.sleepUntilNextRetry(); + } + } finally { + if (traceScope != null) traceScope.close(); + } + } + + /** + * setAcl is an idempotent operation. Retry before throwing exception + * @return list of ACLs + */ + public Stat setAcl(String path, List acls, int version) + throws KeeperException, InterruptedException { + TraceScope traceScope = null; + try { + traceScope = Trace.startSpan("RecoverableZookeeper.setAcl"); + RetryCounter retryCounter = retryCounterFactory.create(); + while (true) { + try { + return checkZk().setACL(path, acls, version); + } catch (KeeperException e) { + switch (e.code()) { + case CONNECTIONLOSS: + case SESSIONEXPIRED: + case OPERATIONTIMEOUT: + retryOrThrow(retryCounter, e, "setAcl"); + break; + + default: + throw e; + } + } + retryCounter.sleepUntilNextRetry(); + } + } finally { + if (traceScope != null) traceScope.close(); + } + } + + /** *

* NONSEQUENTIAL create is idempotent operation. * Retry before throwing exceptions. diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java index 413bc98..1575d55 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java @@ -999,7 +999,11 @@ public class ZKUtil { try { javax.security.auth.login.Configuration testConfig = javax.security.auth.login.Configuration.getConfiguration(); - if(testConfig.getAppConfigurationEntry("Client") == null) { + if (testConfig.getAppConfigurationEntry("Client") == null + && testConfig.getAppConfigurationEntry( + JaasConfiguration.CLIENT_KEYTAB_KERBEROS_CONFIG_NAME) == null + && testConfig.getAppConfigurationEntry( + JaasConfiguration.SERVER_KEYTAB_KERBEROS_CONFIG_NAME) == null) { return false; } } catch(Exception e) { @@ -1008,15 +1012,19 @@ public class ZKUtil { } // Master & RSs uses hbase.zookeeper.client.* - return("kerberos".equalsIgnoreCase(conf.get("hbase.security.authentication")) && - conf.get("hbase.zookeeper.client.keytab.file") != null); + return "kerberos".equalsIgnoreCase(conf.get("hbase.security.authentication")); } private static ArrayList createACL(ZooKeeperWatcher zkw, String node) { + return createACL(zkw, node, isSecureZooKeeper(zkw.getConfiguration())); + } + + public static ArrayList createACL(ZooKeeperWatcher zkw, String node, + boolean isSecureZooKeeper) { if (!node.startsWith(zkw.baseZNode)) { return Ids.OPEN_ACL_UNSAFE; } - if (isSecureZooKeeper(zkw.getConfiguration())) { + if (isSecureZooKeeper) { String superUser = zkw.getConfiguration().get("hbase.superuser"); ArrayList acls = new ArrayList(); // add permission to hbase supper user @@ -1025,13 +1033,7 @@ public class ZKUtil { } // Certain znodes are accessed directly by the client, // so they must be readable by non-authenticated clients - if ((node.equals(zkw.baseZNode) == true) || - (zkw.isAnyMetaReplicaZnode(node)) || - (node.equals(zkw.getMasterAddressZNode()) == true) || - (node.equals(zkw.clusterIdZNode) == true) || - (node.equals(zkw.rsZNode) == true) || - (node.equals(zkw.backupMasterAddressesZNode) == true) || - (node.startsWith(zkw.tableZNode) == true)) { + if (zkw.isClientReadable(node)) { acls.addAll(Ids.CREATOR_ALL_ACL); acls.addAll(Ids.READ_ACL_UNSAFE); } else { diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java index b428d98..6256c02 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java @@ -21,7 +21,6 @@ package org.apache.hadoop.hbase.zookeeper; import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -36,11 +35,15 @@ import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.ZooKeeperConnectionException; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.WatchedEvent; import org.apache.zookeeper.Watcher; -import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.ZooDefs.Ids; +import org.apache.zookeeper.ZooDefs.Perms; import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Id; +import org.apache.zookeeper.data.Stat; /** * Acts as the single ZooKeeper Watcher. One instance of this is instantiated @@ -183,6 +186,107 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable { } } + /** Returns whether the znode is supposed to be readable by the client + * and DOES NOT contain sensitive information (world readable).*/ + public boolean isClientReadable(String node) { + return + node.equals(baseZNode) || + isAnyMetaReplicaZnode(node) || + node.equals(getMasterAddressZNode()) || + node.equals(clusterIdZNode)|| + node.equals(rsZNode) || + node.equals(backupMasterAddressesZNode) || + // /hbase/table and /hbase/table/foo is allowed, /hbase/table-lock is not + node.equals(tableZNode) || + node.startsWith(tableZNode + "/"); + } + + /** + * On master start, we check the znode ACLs under the root directory and set the ACLs properly + * if needed. If the cluster goes from an unsecure setup to a secure setup, this step is needed + * so that the existing znodes created with open permissions are now changed with restrictive + * perms. + */ + public void checkAndSetZNodeAcls() { + if (!ZKUtil.isSecureZooKeeper(getConfiguration())) { + return; + } + + // Check the base znodes permission first. Only do the recursion if base znode's perms are not + // correct. + try { + List actualAcls = recoverableZooKeeper.getAcl(baseZNode, new Stat()); + + if (!isBaseZnodeAclSetup(actualAcls)) { + LOG.info("setting znode ACLs"); + setZnodeAclsRecursive(baseZNode); + } + } catch(KeeperException.NoNodeException nne) { + return; + } catch(InterruptedException ie) { + interruptedException(ie); + } catch (IOException|KeeperException e) { + LOG.warn("Received exception while checking and setting zookeeper ACLs", e); + } + } + + /** + * Set the znode perms recursively. This will do post-order recursion, so that baseZnode ACLs + * will be set last in case the master fails in between. + * @param znode + */ + private void setZnodeAclsRecursive(String znode) throws KeeperException, InterruptedException { + List children = recoverableZooKeeper.getChildren(znode, false); + + for (String child : children) { + setZnodeAclsRecursive(ZKUtil.joinZNode(znode, child)); + } + List acls = ZKUtil.createACL(this, znode, true); + LOG.info("Setting ACLs for znode:" + znode + " , acl:" + acls); + recoverableZooKeeper.setAcl(znode, acls, -1); + } + + /** + * Checks whether the ACLs returned from the base znode (/hbase) is set for secure setup. + * @param acls acls from zookeeper + * @return + * @throws IOException + */ + private boolean isBaseZnodeAclSetup(List acls) throws IOException { + String superUser = conf.get("hbase.superuser"); + + // this assumes that current authenticated user is the same as zookeeper client user + // configured via JAAS + String hbaseUser = UserGroupInformation.getCurrentUser().getShortUserName(); + + if (acls.isEmpty()) { + return false; + } + + for (ACL acl : acls) { + int perms = acl.getPerms(); + Id id = acl.getId(); + // We should only set at most 3 possible ACLs for 3 Ids. One for everyone, one for superuser + // and one for the hbase user + if (Ids.ANYONE_ID_UNSAFE.equals(id)) { + if (perms != Perms.READ) { + return false; + } + } else if (superUser != null && new Id("sasl", superUser).equals(id)) { + if (perms != Perms.ALL) { + return false; + } + } else if (new Id("sasl", hbaseUser).equals(id)) { + if (perms != Perms.ALL) { + return false; + } + } else { + return false; + } + } + return true; + } + @Override public String toString() { return this.identifier + ", quorum=" + quorum + ", baseZNode=" + baseZNode; @@ -304,7 +408,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable { String pattern = conf.get("zookeeper.znode.metaserver","meta-region-server"); if (znode.equals(pattern)) return HRegionInfo.DEFAULT_REPLICA_ID; // the non-default replicas are of the pattern meta-region-server- - String nonDefaultPattern = pattern + "-"; + String nonDefaultPattern = pattern + "-"; return Integer.parseInt(znode.substring(nonDefaultPattern.length())); } @@ -552,6 +656,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable, Closeable { * * @throws InterruptedException */ + @Override public void close() { try { if (recoverableZooKeeper != null) { diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index ced6699..be11aca 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -781,6 +781,11 @@ public class HMaster extends HRegionServer implements MasterServices, Server { // master initialization. See HBASE-5916. this.serverManager.clearDeadServersWithSameHostNameAndPortOfOnlineServer(); + // Check and set the znode ACLs if needed in case we are overtaking a non-secure configuration + status.setStatus("Checking ZNode ACLs"); + zooKeeper.checkAndSetZNodeAcls(); + + status.setStatus("Calling postStartMaster coprocessors"); if (this.cpHost != null) { // don't let cp initialization errors kill the master try {