You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2018/11/15 23:51:50 UTC
[GitHub] asfgit closed pull request #1467: DRILL-5671: Set secure ACLs
(Access Control List) for Drill ZK nodes in a secure cluster
asfgit closed pull request #1467: DRILL-5671: Set secure ACLs (Access Control List) for Drill ZK nodes in a secure cluster
URL: https://github.com/apache/drill/pull/1467
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/distribution/src/resources/drill-override-example.conf b/distribution/src/resources/drill-override-example.conf
index 296cd8b60e3..5aa45a9dd5d 100644
--- a/distribution/src/resources/drill-override-example.conf
+++ b/distribution/src/resources/drill-override-example.conf
@@ -72,6 +72,22 @@ drill.exec: {
count: 7200,
delay: 500
}
+ # This option controls whether Drill specifies ACLs when it creates znodes.
+ # If this is 'false', then anyone has all privileges for all Drill znodes.
+ # This corresponds to ZOO_OPEN_ACL_UNSAFE.
+ # Setting this flag to 'true' enables the provider specified in "acl_provider"
+ apply_secure_acl: false,
+
+ # This option specified the ACL provider to be used by Drill.
+ # Custom ACL providers can be provided in the Drillbit classpath and Drill can be made to pick them
+ # by changing this option.
+ # Note: This option has no effect if "apply_secure_acl" is 'false'
+ #
+ # The default "creator-all" will setup ACLs such that
+ # - Only the Drillbit user will have all privileges(create, delete, read, write, admin). Same as ZOO_CREATOR_ALL_ACL
+ # - Other users will only be able to read the cluster-discovery(list of Drillbits in the cluster) znodes.
+ #
+ acl_provider: "creator-all"
},
http: {
enabled: true,
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
index eed4ff830b6..e7f0cd28fa4 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
@@ -50,6 +50,8 @@ private ExecConstants() {
public static final String ZK_TIMEOUT = "drill.exec.zk.timeout";
public static final String ZK_ROOT = "drill.exec.zk.root";
public static final String ZK_REFRESH = "drill.exec.zk.refresh";
+ public static final String ZK_ACL_PROVIDER = "drill.exec.zk.acl_provider";
+ public static final String ZK_APPLY_SECURE_ACL = "drill.exec.zk.apply_secure_acl";
public static final String BIT_RETRY_TIMES = "drill.exec.rpc.bit.server.retry.count";
public static final String BIT_RETRY_DELAY = "drill.exec.rpc.bit.server.retry.delay";
public static final String BIT_TIMEOUT = "drill.exec.bit.timeout";
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLContextProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLContextProvider.java
new file mode 100644
index 00000000000..dafd5f8d5d3
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLContextProvider.java
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+/**
+ * Defines the functions required by {@link ZKACLProviderDelegate} to access ZK-ACL related information
+ */
+
+public interface ZKACLContextProvider {
+
+ String getClusterPath();
+
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLContextProviderImpl.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLContextProviderImpl.java
new file mode 100644
index 00000000000..fc29208f180
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLContextProviderImpl.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+public class ZKACLContextProviderImpl implements ZKACLContextProvider {
+
+ private final String clusterPath;
+
+ public ZKACLContextProviderImpl(String clusterPath) {
+ this.clusterPath = clusterPath;
+ }
+
+ @Override
+ public String getClusterPath() { return clusterPath; }
+
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProvider.java
new file mode 100644
index 00000000000..54bee586409
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProvider.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import org.apache.zookeeper.data.ACL;
+
+import java.util.List;
+
+/**
+ * This class defines the methods that are required to specify
+ * ACLs on Drill ZK nodes
+ */
+
+public interface ZKACLProvider {
+
+ /**
+ * Returns the list of ZK ACLs {@link ACL} to apply by default
+ * on Drill znodes
+ * @return List of ZK ACLs {@link ACL}
+ */
+ List<ACL> getDrillDefaultAcl();
+
+ /**
+ * Returns the list of ZK ACLs {@link ACL} to apply for a specific
+ * Drill znode
+ * @param path The path for which the ACL is being looked up
+ * @return List of ZK ACLs {@link ACL} for the provided path
+ */
+ List<ACL> getDrillAclForPath(String path);
+
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderDelegate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderDelegate.java
new file mode 100644
index 00000000000..a3f7ee3bc8e
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderDelegate.java
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import org.apache.curator.framework.api.ACLProvider;
+import org.apache.drill.shaded.guava.com.google.common.annotations.VisibleForTesting;
+import org.apache.zookeeper.data.ACL;
+
+import java.util.List;
+
+/**
+ * This class hides the {@link ZKACLProvider} from Curator-specific functions
+ *
+ * This is done so that ACL Providers have to be aware only about ZK ACLs and the Drill {@link ZKACLProvider} interface
+ * ACL Providers should not be concerned with the framework (Curator) used by Drill to access ZK.
+ * If Drill stops using Curator, then existing {@link ZKACLProvider} implementations will still work.
+ */
+
+public class ZKACLProviderDelegate implements ACLProvider {
+ @VisibleForTesting
+ ZKACLProvider aclProvider;
+
+ public ZKACLProviderDelegate(ZKACLProvider aclProvider) {
+ this.aclProvider = aclProvider;
+ }
+
+ @Override
+ final public List<ACL> getDefaultAcl() {
+ return aclProvider.getDrillDefaultAcl();
+ }
+
+ @Override
+ final public List<ACL> getAclForPath(String path) {
+ return aclProvider.getDrillAclForPath(path);
+ }
+
+}
\ No newline at end of file
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderFactory.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderFactory.java
new file mode 100644
index 00000000000..f43aec0093d
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderFactory.java
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import org.apache.drill.common.config.DrillConfig;
+import static org.apache.drill.exec.ExecConstants.ZK_ACL_PROVIDER;
+import static org.apache.drill.exec.ExecConstants.ZK_APPLY_SECURE_ACL;
+
+import org.apache.drill.common.scanner.persistence.ScanResult;
+import org.apache.drill.exec.exception.DrillbitStartupException;
+import org.apache.drill.exec.server.BootStrapContext;
+import org.apache.drill.shaded.guava.com.google.common.base.Strings;
+
+import java.lang.reflect.Constructor;
+import java.util.Collection;
+
+/**
+ * This factory returns a {@link ZKACLProviderDelegate} which will be used to set ACLs on Drill ZK nodes
+ * If secure ACLs are required, the {@link ZKACLProviderFactory} looks up and instantiates a {@link ZKACLProviderDelegate}
+ * specified in the config file. Else it returns the {@link ZKDefaultACLProvider}
+ */
+public class ZKACLProviderFactory {
+
+ private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ZKACLProviderFactory.class);
+
+ public static ZKACLProviderDelegate getACLProvider(DrillConfig config, String drillClusterPath, BootStrapContext context)
+ throws DrillbitStartupException {
+ ZKACLContextProvider stateProvider = new ZKACLContextProviderImpl(drillClusterPath);
+
+ if (config.getBoolean(ZK_APPLY_SECURE_ACL)) {
+ logger.trace("Using secure ZK ACL. Drill cluster path " + drillClusterPath);
+ ZKACLProviderDelegate aclProvider = findACLProvider(config, stateProvider, context);
+ return aclProvider;
+ }
+ logger.trace("Using un-secure default ZK ACL");
+ final ZKDefaultACLProvider defaultAclProvider = new ZKDefaultACLProvider(stateProvider);
+ return new ZKACLProviderDelegate(defaultAclProvider);
+ }
+
+ public static ZKACLProviderDelegate findACLProvider(DrillConfig config, ZKACLContextProvider contextProvider,
+ BootStrapContext context) throws DrillbitStartupException {
+ if (!config.hasPath(ZK_ACL_PROVIDER)) {
+ throw new DrillbitStartupException(String.format("BOOT option '%s' is missing in config.", ZK_ACL_PROVIDER));
+ }
+
+ final String aclProviderName = config.getString(ZK_ACL_PROVIDER);
+
+ if (Strings.isNullOrEmpty(aclProviderName)) {
+ throw new DrillbitStartupException(String.format("Invalid value '%s' for BOOT option '%s'", aclProviderName,
+ ZK_ACL_PROVIDER));
+ }
+
+ ScanResult scan = context.getClasspathScan();
+ final Collection<Class<? extends ZKACLProvider>> aclProviderImpls =
+ scan.getImplementations(ZKACLProvider.class);
+ logger.debug("Found ZkACLProvider implementations: {}", aclProviderImpls);
+
+ for (Class<? extends ZKACLProvider> clazz : aclProviderImpls) {
+ final ZKACLProviderTemplate template = clazz.getAnnotation(ZKACLProviderTemplate.class);
+ if (template == null) {
+ logger.warn("{} doesn't have {} annotation. Skipping.", clazz.getCanonicalName(),
+ ZKACLProviderTemplate.class);
+ continue;
+ }
+
+ if (template.type().equalsIgnoreCase(aclProviderName)) {
+ Constructor<?> validConstructor = null;
+ Class constructorArgumentClass = ZKACLContextProvider.class;
+ for (Constructor<?> c : clazz.getConstructors()) {
+ Class<?>[] params = c.getParameterTypes();
+ if (params.length == 1 && params[0] == constructorArgumentClass) {
+ validConstructor = c;
+ break;
+ }
+ }
+ if (validConstructor == null) {
+ logger.warn("Skipping ZKACLProvider implementation class '{}' since it doesn't " +
+ "implement a constructor [{}({})]", clazz.getCanonicalName(), clazz.getName(),
+ constructorArgumentClass.getName());
+ continue;
+ }
+ try {
+ final ZKACLProvider aclProvider = (ZKACLProvider) validConstructor.newInstance(contextProvider);
+ return new ZKACLProviderDelegate(aclProvider);
+ } catch (ReflectiveOperationException e ) {
+ throw new DrillbitStartupException(
+ String.format("Failed to create and initialize the ZKACLProvider class '%s'",
+ clazz.getCanonicalName()), e);
+ }
+ }
+ }
+ String errMsg = String.format("Failed to find the implementation of '%s' for type '%s'",
+ ZKACLProvider.class.getCanonicalName(), aclProviderName);
+ logger.error(errMsg);
+ throw new DrillbitStartupException(errMsg);
+ }
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderTemplate.java
new file mode 100644
index 00000000000..2fed3c1f9fb
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKACLProviderTemplate.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation for {@link ZKACLProviderDelegate} implementation to identify the
+ * implementation type. Implementation type is set in BOOT option <i>drill.exec.zk.acl_provider</i>.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.TYPE})
+public @interface ZKACLProviderTemplate {
+ /**
+ * {@link ZKACLProviderDelegate} implementation type.
+ * @return Returns a name describing the type of the ACL Provider
+ */
+ String type();
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
index 82e45b2f9a7..853c41082b8 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
@@ -31,11 +31,14 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.drill.exec.exception.DrillbitStartupException;
+import org.apache.drill.exec.server.BootStrapContext;
import org.apache.drill.shaded.guava.com.google.common.base.Throwables;
import org.apache.commons.collections.keyvalue.MultiKey;
import org.apache.curator.RetryPolicy;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.api.ACLProvider;
import org.apache.curator.framework.state.ConnectionState;
import org.apache.curator.framework.state.ConnectionStateListener;
import org.apache.curator.retry.RetryNTimes;
@@ -78,11 +81,18 @@
private ConcurrentHashMap<MultiKey, DrillbitEndpoint> endpointsMap = new ConcurrentHashMap<MultiKey,DrillbitEndpoint>();
private static final Pattern ZK_COMPLEX_STRING = Pattern.compile("(^.*?)/(.*)/([^/]*)$");
- public ZKClusterCoordinator(DrillConfig config) throws IOException{
- this(config, null);
+ public ZKClusterCoordinator(DrillConfig config, String connect)
+ throws IOException, DrillbitStartupException {
+ this(config, connect, null);
}
- public ZKClusterCoordinator(DrillConfig config, String connect) throws IOException {
+ public ZKClusterCoordinator(DrillConfig config, BootStrapContext context)
+ throws IOException, DrillbitStartupException {
+ this(config, null, context);
+ }
+
+ public ZKClusterCoordinator(DrillConfig config, String connect, BootStrapContext context)
+ throws IOException, DrillbitStartupException {
connect = connect == null || connect.isEmpty() ? config.getString(ExecConstants.ZK_CONNECTION) : connect;
String clusterId = config.getString(ExecConstants.SERVICE_NAME);
String zkRoot = config.getString(ExecConstants.ZK_ROOT);
@@ -99,6 +109,10 @@ public ZKClusterCoordinator(DrillConfig config, String connect) throws IOExcepti
logger.debug("Connect {}, zkRoot {}, clusterId: " + clusterId, connect, zkRoot);
this.serviceName = clusterId;
+ String drillClusterPath = "/" + zkRoot + "/" + clusterId;
+
+ ACLProvider aclProvider = ZKACLProviderFactory.getACLProvider(config, drillClusterPath, context);
+
RetryPolicy rp = new RetryNTimes(config.getInt(ExecConstants.ZK_RETRY_TIMES),
config.getInt(ExecConstants.ZK_RETRY_DELAY));
curator = CuratorFrameworkFactory.builder()
@@ -106,6 +120,7 @@ public ZKClusterCoordinator(DrillConfig config, String connect) throws IOExcepti
.connectionTimeoutMs(config.getInt(ExecConstants.ZK_TIMEOUT))
.retryPolicy(rp)
.connectString(connect)
+ .aclProvider(aclProvider)
.build();
curator.getConnectionStateListenable().addListener(new InitialConnectionListener());
curator.start();
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKDefaultACLProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKDefaultACLProvider.java
new file mode 100644
index 00000000000..c3151f42721
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKDefaultACLProvider.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.data.ACL;
+
+import java.util.List;
+
+/**
+ * ZKDefaultACLProvider provides the ACLs for znodes created in a un-secure installation.
+ *
+ * If this ACLProvider is used, everyone gets unrestricted access to the Drill znodes
+ *
+ */
+@ZKACLProviderTemplate(type = "open")
+public class ZKDefaultACLProvider implements ZKACLProvider {
+
+ public ZKDefaultACLProvider(ZKACLContextProvider context) { }
+
+ public List<ACL> getDrillDefaultAcl() {
+ return ZooDefs.Ids.OPEN_ACL_UNSAFE;
+ }
+
+ public List<ACL> getDrillAclForPath(String path) {
+ return ZooDefs.Ids.OPEN_ACL_UNSAFE;
+ }
+
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKSecureACLProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKSecureACLProvider.java
new file mode 100644
index 00000000000..c36e65cb107
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKSecureACLProvider.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+
+import java.util.List;
+
+/**
+ * ZKSecureACLProvider restricts access to znodes created by Drill in a secure installation.
+ *
+ * For all other znodes, only the creator of the znode, i.e the Drillbit user, has full access.
+ * In addition to the above, all znodes under the cluster path are readable by anyone.
+ *
+ */
+@ZKACLProviderTemplate(type = "creator-all")
+public class ZKSecureACLProvider implements ZKACLProvider {
+
+ static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(ZKSecureACLProvider.class);
+
+ /**
+ * DEFAULT_ACL gives the creator of a znode full-access to it
+ */
+ private static final ImmutableList<ACL> DEFAULT_ACL = new ImmutableList.Builder<ACL>()
+ .addAll(Ids.CREATOR_ALL_ACL.iterator())
+ .build();
+ /**
+ * DRILL_CLUSTER_ACL gives the creator full access and everyone else only read access.
+ * Used on the Drillbit discovery znode (znode path /<drill.exec.zk.root>/<drill.exec.cluster-id>)
+ * i.e. the node that contains the list of Drillbits in the cluster.
+ */
+ private static final ImmutableList<ACL> DRILL_CLUSTER_ACL = new ImmutableList.Builder<ACL>()
+ .addAll(Ids.READ_ACL_UNSAFE.iterator())
+ .addAll(Ids.CREATOR_ALL_ACL.iterator())
+ .build();
+ final String drillClusterPath;
+
+ public ZKSecureACLProvider(ZKACLContextProvider contextProvider) {
+ this.drillClusterPath = contextProvider.getClusterPath();
+ }
+
+ @Override
+ public List<ACL> getDrillDefaultAcl() {
+ return DEFAULT_ACL;
+ }
+
+ @Override
+ public List<ACL> getDrillAclForPath(String path) {
+ logger.trace("getAclForPath " + path);
+ if (path.startsWith(drillClusterPath)) {
+ logger.trace("getAclForPath drillClusterPath " + drillClusterPath);
+ return DRILL_CLUSTER_ACL;
+ }
+ return DEFAULT_ACL;
+ }
+
+}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
index dd1c5f19faf..5b26ff796b9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java
@@ -157,7 +157,7 @@ public Drillbit(
coord = serviceSet.getCoordinator();
storeProvider = new CachingPersistentStoreProvider(new LocalPersistentStoreProvider(config));
} else {
- coord = new ZKClusterCoordinator(config);
+ coord = new ZKClusterCoordinator(config, context);
storeProvider = new PersistentStoreRegistry<ClusterCoordinator>(this.coord, config).newPStoreProvider();
}
diff --git a/exec/java-exec/src/main/resources/drill-module.conf b/exec/java-exec/src/main/resources/drill-module.conf
index f083c6603c8..42f744e3659 100644
--- a/exec/java-exec/src/main/resources/drill-module.conf
+++ b/exec/java-exec/src/main/resources/drill-module.conf
@@ -29,7 +29,8 @@ drill {
org.apache.drill.exec.rpc.security.AuthenticatorFactory,
org.apache.drill.exec.server.rest.auth.DrillHttpConstraintSecurityHandler,
org.apache.drill.exec.store.dfs.FormatPlugin,
- org.apache.drill.exec.store.StoragePlugin
+ org.apache.drill.exec.store.StoragePlugin,
+ org.apache.drill.exec.coord.zk.ZKACLProvider
],
annotations: ${?drill.classpath.scanning.annotations} [
@@ -43,7 +44,8 @@ drill {
org.apache.drill.exec.store,
org.apache.drill.exec.rpc.user.security,
org.apache.drill.exec.rpc.security,
- org.apache.drill.exec.server.rest.auth
+ org.apache.drill.exec.server.rest.auth,
+ org.apache.drill.exec.coord.zk
],
// caches scanned result during build time
@@ -123,7 +125,9 @@ drill.exec: {
retry: {
count: 7200,
delay: 500
- }
+ },
+ apply_secure_acl: false,
+ acl_provider: "creator-all"
},
http: {
enabled: true,
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestZKACL.java b/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestZKACL.java
new file mode 100644
index 00000000000..07e0465d76e
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/coord/zk/TestZKACL.java
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.coord.zk;
+
+import com.typesafe.config.ConfigValueFactory;
+import org.apache.curator.RetryPolicy;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.api.ACLProvider;
+import org.apache.curator.retry.RetryNTimes;
+import org.apache.curator.test.TestingServer;
+import org.apache.drill.categories.SecurityTest;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.common.scanner.persistence.ScanResult;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.server.BootStrapContext;
+import org.apache.drill.exec.server.options.SystemOptionManager;
+import org.apache.zookeeper.data.ACL;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.List;
+
+@Ignore("See DRILL-6823")
+@Category(SecurityTest.class)
+public class TestZKACL {
+
+ private TestingServer server;
+ private final static String cluster_config_znode = "test-cluster_config_znode";
+ private final static byte[] cluster_config_data = "drill-node-1".getBytes();
+ private final static String drill_zk_root = "drill-test-drill_zk_root";
+ private final static String drill_cluster_name = "test-drillbits";
+ private static final String drillClusterPath = "/" + drill_zk_root + "/" + drill_cluster_name;
+ private static final RetryPolicy retryPolicy = new RetryNTimes(1, 1000);
+
+ private static final String drillUDFName = "test-udfs";
+ private final static byte[] udf_data = "test-udf-1".getBytes();
+ private static final String drillUDFPath = "/" + drill_zk_root + "/" + drillUDFName;
+ private static ACLProvider aclProviderDelegate;
+
+ private static CuratorFramework client;
+
+ @Before
+ public void setUp() throws Exception {
+ System.setProperty("zookeeper.authProvider.1", "org.apache.zookeeper.server.auth.SASLAuthenticationProvider");
+ String configPath = (ClassLoader.getSystemResource("zkacltest.conf")).getPath();
+ System.setProperty("java.security.auth.login.config", configPath);
+ server = new TestingServer();
+
+
+ final DrillConfig config = new DrillConfig(DrillConfig.create().withValue(ExecConstants.ZK_ACL_PROVIDER,
+ ConfigValueFactory.fromAnyRef("creator-all")
+ ).withValue(ExecConstants.ZK_APPLY_SECURE_ACL, ConfigValueFactory.fromAnyRef(true)));
+
+ final ScanResult result = ClassPathScanner.fromPrescan(config);
+ final BootStrapContext bootStrapContext =
+ new BootStrapContext(config, SystemOptionManager.createDefaultOptionDefinitions(), result);
+ aclProviderDelegate = ZKACLProviderFactory.getACLProvider(config, drillClusterPath, bootStrapContext);
+
+ server.start();
+
+ client = CuratorFrameworkFactory.builder().
+ retryPolicy(retryPolicy).
+ connectString(server.getConnectString()).
+ aclProvider(aclProviderDelegate).
+ build();
+ client.start();
+ }
+
+ /**
+ * Test ACLs on znodes required to discover the cluster
+ *
+ * ZK libraries only supports one client instance per-machine per-server and it is cached.
+ * This test will fail when run after other ZK tests that setup the client in a way that will cause this test to fail
+ */
+
+ @Test
+ public void testClusterDiscoveryPaths() {
+ try {
+ String path = PathUtils.join(drillClusterPath, cluster_config_znode);
+ client.create().creatingParentsIfNeeded().forPath(path, cluster_config_data);
+ List<ACL> remoteACLs = client.getACL().forPath(path);
+ List<ACL> desiredACLs = ((ZKACLProviderDelegate) aclProviderDelegate).aclProvider.getDrillAclForPath(drillClusterPath);
+
+ // Check the ACLs
+ for (ACL remote : remoteACLs) {
+ boolean found = false;
+ for (ACL desired : desiredACLs) {
+ // desiredACL list is READ_ACL_UNSAFE (READ, WORLD_ANYONE) + CREATOR_ALL_ACL(ALL, AUTH)
+ // AUTH in CREATOR_ALL would translate to SASL, username. Hence the replacement
+ // Note: The username("testuser1") should match the username in java.security.auth.login.config
+ found = desired.toString().equals(remote.toString().replace("sasl", "auth").replace("testuser1", ""));
+
+ if (found) { break; }
+ }
+ Assert.assertTrue(found);
+ }
+ // check if the data can be read
+ byte[] actual = client.getData().forPath(path);
+ Assert.assertArrayEquals("testClusterDiscoveryPaths data mismatch", cluster_config_data, actual);
+
+ } catch (Exception e) {
+ throw new IllegalStateException("testClusterDiscoveryPaths failed");
+ }
+ }
+
+ /**
+ * Test ACLs on znodes other than ones required to discover the cluster
+ *
+ * ZK libraries only supports one client instance per-machine per-server and it is cached.
+ * This test will fail when run after other ZK tests that setup the client in a way that will cause this test to fail
+ */
+ @Test
+ public void testNonClusterDiscoveryPaths() {
+ try {
+ client.create().creatingParentsIfNeeded().forPath(drillUDFPath, udf_data);
+ List<ACL> remoteACLs = client.getACL().forPath(drillUDFPath);
+ List<ACL> desiredACLs = ((ZKACLProviderDelegate) aclProviderDelegate).aclProvider.getDrillAclForPath(drillUDFPath);
+ Assert.assertEquals(remoteACLs.size(), desiredACLs.size());
+ for (ACL remote : remoteACLs) {
+ boolean found = false;
+ for (ACL desired : desiredACLs) {
+ // desiredACL list is READ_ACL_UNSAFE (READ, WORLD_ANYONE) + CREATOR_ALL_ACL(ALL, AUTH)
+ // AUTH in CREATOR_ALL would translate to SASL, username. Hence the replacement
+ // Note: The username("testuser1") should match the username in java.security.auth.login.config
+ found = desired.toString().equals(remote.toString().replace("sasl", "auth").replace("testuser1", ""));
+ if (found) { break; }
+ }
+ Assert.assertTrue(found);
+ }
+ // check if the data can be read
+ byte[] actual = client.getData().forPath(drillUDFPath);
+ Assert.assertArrayEquals("testNonClusterDiscoveryPaths data mismatch", udf_data, actual);
+
+ } catch (Exception e) {
+ throw new IllegalStateException("testNonClusterDiscoveryPaths failed");
+ }
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ client.close();
+ server.close();
+ }
+}
diff --git a/exec/java-exec/src/test/resources/zkacltest.conf b/exec/java-exec/src/test/resources/zkacltest.conf
new file mode 100644
index 00000000000..e1b545531ff
--- /dev/null
+++ b/exec/java-exec/src/test/resources/zkacltest.conf
@@ -0,0 +1,28 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// This file contains ZK client and server config for testing ZK ACLs
+
+ Server {
+ org.apache.zookeeper.server.auth.DigestLoginModule required
+ user_testuser1="testpassword"
+ user_testuser2="testpassword";
+ };
+
+ Client {
+ org.apache.zookeeper.server.auth.DigestLoginModule required
+ username="testuser1"
+ password="testpassword";
+ };
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services