Repository: knox Updated Branches: refs/heads/master 90b5a0073 -> 1afbbea2e
KNOX-1135 - Add configuration property for allowing remote configuration to be read by unauthenticated clients Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/1afbbea2 Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/1afbbea2 Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/1afbbea2 Branch: refs/heads/master Commit: 1afbbea2eaafbb3e789a4fcd86b8bc132587fc99 Parents: 90b5a00 Author: Phil Zampino <pzamp...@apache.org> Authored: Mon Apr 30 23:28:38 2018 -0400 Committer: Phil Zampino <pzamp...@apache.org> Committed: Tue May 1 12:24:41 2018 -0400 ---------------------------------------------------------------------- .../gateway/config/impl/GatewayConfigImpl.java | 7 ++ .../DefaultRemoteConfigurationMonitor.java | 39 +++++++++- ...emoteConfigurationRegistryClientService.java | 3 - .../knox/gateway/config/GatewayConfig.java | 8 ++ .../apache/knox/gateway/GatewayTestConfig.java | 6 ++ .../monitor/RemoteConfigurationMonitorTest.java | 78 ++++++++++++++++++++ 6 files changed, 134 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/1afbbea2/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java index 3684ed2..baa0507 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java @@ -231,6 +231,8 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig { /* @since 0.15 Remote configuration monitoring */ static final String CONFIG_REGISTRY_PREFIX = GATEWAY_CONFIG_FILE_PREFIX + ".remote.config.registry"; static final String REMOTE_CONFIG_MONITOR_CLIENT_NAME = GATEWAY_CONFIG_FILE_PREFIX + ".remote.config.monitor.client"; + static final String REMOTE_CONFIG_MONITOR_CLIENT_ALLOW_READ_ACCESS = + REMOTE_CONFIG_MONITOR_CLIENT_NAME + ".allowUnauthenticatedReadAccess"; private static List<String> DEFAULT_GLOBAL_RULES_SERVICES; @@ -990,6 +992,11 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig { return get(REMOTE_CONFIG_MONITOR_CLIENT_NAME); } + @Override + public boolean allowUnauthenticatedRemoteRegistryReadAccess() { + return Boolean.parseBoolean(get(REMOTE_CONFIG_MONITOR_CLIENT_ALLOW_READ_ACCESS, String.valueOf(false))); + } + /** * Returns whether the Remote Alias Service is enabled or not. * This value also depends on whether remote registry is enabled or not. http://git-wip-us.apache.org/repos/asf/knox/blob/1afbbea2/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultRemoteConfigurationMonitor.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultRemoteConfigurationMonitor.java b/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultRemoteConfigurationMonitor.java index 3c424d1..6501288 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultRemoteConfigurationMonitor.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/DefaultRemoteConfigurationMonitor.java @@ -28,8 +28,8 @@ import org.apache.zookeeper.ZooDefs; import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; @@ -68,11 +68,38 @@ class DefaultRemoteConfigurationMonitor implements RemoteConfigurationMonitor { }; } + private static final RemoteConfigurationRegistryClient.EntryACL WORLD_ANYONE_READ; + static { + WORLD_ANYONE_READ = new RemoteConfigurationRegistryClient.EntryACL() { + public String getId() { + return "anyone"; + } + + public String getType() { + return "world"; + } + + public Object getPermissions() { + return ZooDefs.Perms.READ; + } + + public boolean canRead() { + return true; + } + + public boolean canWrite() { + return false; + } + }; + } + private RemoteConfigurationRegistryClient client = null; private File providersDir; private File descriptorsDir; + private final List<RemoteConfigurationRegistryClient.EntryACL> replacementACL = new ArrayList<>(); + /** * @param config The gateway configuration * @param registryClientService The service from which the remote registry client should be acquired. @@ -88,11 +115,15 @@ class DefaultRemoteConfigurationMonitor implements RemoteConfigurationMonitor { this.client = registryClientService.get(clientName); if (this.client == null) { log.unresolvedClientConfigurationForRemoteMonitoring(clientName); + } else if (config.allowUnauthenticatedRemoteRegistryReadAccess()) { + replacementACL.add(WORLD_ANYONE_READ); } } else { log.missingClientConfigurationForRemoteMonitoring(); } } + + replacementACL.add(AUTHENTICATED_USERS_ALL); } @Override @@ -179,9 +210,9 @@ class DefaultRemoteConfigurationMonitor implements RemoteConfigurationMonitor { if (client.isAuthenticationConfigured()) { log.correctingSuspectWritableRemoteConfigurationEntry(name); - // Replace the existing ACL with one that permits only authenticated users - client.setACL(name, Collections.singletonList(AUTHENTICATED_USERS_ALL)); - } + // Replace the existing ACL with the replacement ACL for the authentication scenario + client.setACL(name, replacementACL); + } } } } http://git-wip-us.apache.org/repos/asf/knox/blob/1afbbea2/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java b/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java index 3bf7d2e..c17ef87 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/service/config/remote/LocalFileSystemRemoteConfigurationRegistryClientService.java @@ -74,15 +74,12 @@ public class LocalFileSystemRemoteConfigurationRegistryClientService implements @Override public void start() throws ServiceLifecycleException { - } @Override public void stop() throws ServiceLifecycleException { - } - private RemoteConfigurationRegistryClient createClient(RemoteConfigurationRegistryConfig config) { String rootDir = config.getConnectionString(); http://git-wip-us.apache.org/repos/asf/knox/blob/1afbbea2/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java index f4e8521..c727b90 100644 --- a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java +++ b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java @@ -350,6 +350,14 @@ public interface GatewayConfig { String getRemoteConfigurationMonitorClientName(); /** + * When new remote registry entries must be created, or new ACLs applied to existing entries, this method indicates + * whether unauthenticated connections should be given read access to those entries. + * + * @return true, if unauthenticated clients should be allowed to access remote registry entries. + */ + boolean allowUnauthenticatedRemoteRegistryReadAccess(); + + /** * Returns whether the Remote Alias Service is enabled or not. * This value also depends on whether remote registry is enabled or not. * if it is enabled then this option takes effect else this option has no http://git-wip-us.apache.org/repos/asf/knox/blob/1afbbea2/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java ---------------------------------------------------------------------- diff --git a/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java b/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java index 8ce7e9e..9659fe0 100644 --- a/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java +++ b/gateway-test-release-utils/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java @@ -663,4 +663,10 @@ public class GatewayTestConfig extends Configuration implements GatewayConfig { public boolean isClusterMonitorEnabled(String type) { return false; } + + @Override + public boolean allowUnauthenticatedRemoteRegistryReadAccess() { + return false; + } + } http://git-wip-us.apache.org/repos/asf/knox/blob/1afbbea2/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java ---------------------------------------------------------------------- diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java index 37668a8..cb2bcdf 100644 --- a/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java +++ b/gateway-test/src/test/java/org/apache/knox/gateway/topology/monitor/RemoteConfigurationMonitorTest.java @@ -79,6 +79,7 @@ public class RemoteConfigurationMonitorTest { private static final ACL ANY_AUTHENTICATED_USER_ALL = new ACL(ZooDefs.Perms.ALL, new Id("auth", "")); private static final ACL SASL_TESTUSER_ALL = new ACL(ZooDefs.Perms.ALL, new Id("sasl", ZK_USERNAME)); + private static final ACL WORLD_ANYONE_READ = new ACL(ZooDefs.Perms.READ, new Id("world", "anyone")); private static File testTmp; private static File providersDir; @@ -279,6 +280,83 @@ public class RemoteConfigurationMonitorTest { } + /** + * KNOX-1135 + */ + @Test + public void testZooKeeperConfigMonitorSASLNodesExistWithUnacceptableACLAllowUnauthenticatedReads() throws Exception { + final String configMonitorName = "zkConfigClient"; + final String alias = "zkPass"; + + // Setup the base GatewayConfig mock + GatewayConfig gc = EasyMock.createNiceMock(GatewayConfig.class); + EasyMock.expect(gc.getGatewayProvidersConfigDir()).andReturn(providersDir.getAbsolutePath()).anyTimes(); + EasyMock.expect(gc.getGatewayDescriptorsDir()).andReturn(descriptorsDir.getAbsolutePath()).anyTimes(); + EasyMock.expect(gc.getRemoteRegistryConfigurationNames()) + .andReturn(Collections.singletonList(configMonitorName)) + .anyTimes(); + final String registryConfig = + GatewayConfig.REMOTE_CONFIG_REGISTRY_TYPE + "=" + ZooKeeperClientService.TYPE + ";" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_ADDRESS + "=" + zkCluster.getConnectString() + ";" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_PRINCIPAL + "=" + ZK_USERNAME + ";" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_AUTH_TYPE + "=Digest;" + + GatewayConfig.REMOTE_CONFIG_REGISTRY_CREDENTIAL_ALIAS + "=" + alias; + EasyMock.expect(gc.allowUnauthenticatedRemoteRegistryReadAccess()).andReturn(true).anyTimes(); + EasyMock.expect(gc.getRemoteRegistryConfiguration(configMonitorName)) + .andReturn(registryConfig).anyTimes(); + EasyMock.expect(gc.getRemoteConfigurationMonitorClientName()).andReturn(configMonitorName).anyTimes(); + EasyMock.replay(gc); + + AliasService aliasService = EasyMock.createNiceMock(AliasService.class); + EasyMock.expect(aliasService.getPasswordFromAliasForGateway(alias)) + .andReturn(ZK_PASSWORD.toCharArray()) + .anyTimes(); + EasyMock.replay(aliasService); + + RemoteConfigurationRegistryClientService clientService = (new ZooKeeperClientServiceProvider()).newInstance(); + clientService.setAliasService(aliasService); + clientService.init(gc, Collections.emptyMap()); + clientService.start(); + + RemoteConfigurationMonitorFactory.setClientService(clientService); + + RemoteConfigurationMonitor cm = RemoteConfigurationMonitorFactory.get(gc); + assertNotNull("Failed to load RemoteConfigurationMonitor", cm); + + final ACL ANY_AUTHENTICATED_USER_ALL = new ACL(ZooDefs.Perms.ALL, new Id("auth", "")); + List<ACL> acls = Arrays.asList(ANY_AUTHENTICATED_USER_ALL, new ACL(ZooDefs.Perms.WRITE, ZooDefs.Ids.ANYONE_ID_UNSAFE)); + client.create().creatingParentsIfNeeded().withMode(CreateMode.PERSISTENT).withACL(acls).forPath(PATH_KNOX); + client.create().creatingParentsIfNeeded().withMode(CreateMode.PERSISTENT).withACL(acls).forPath(PATH_KNOX_CONFIG); + client.create().creatingParentsIfNeeded().withMode(CreateMode.PERSISTENT).withACL(acls).forPath(PATH_KNOX_PROVIDERS); + client.create().creatingParentsIfNeeded().withMode(CreateMode.PERSISTENT).withACL(acls).forPath(PATH_KNOX_DESCRIPTORS); + + // Make sure both ACLs were applied + List<ACL> preACLs = client.getACL().forPath(PATH_KNOX); + assertEquals(2, preACLs.size()); + + // Check that the config nodes really do exist (the monitor will NOT create them if they're present) + assertNotNull(client.checkExists().forPath(PATH_KNOX)); + assertNotNull(client.checkExists().forPath(PATH_KNOX_CONFIG)); + assertNotNull(client.checkExists().forPath(PATH_KNOX_PROVIDERS)); + assertNotNull(client.checkExists().forPath(PATH_KNOX_DESCRIPTORS)); + + try { + cm.start(); + } catch (Exception e) { + fail("Failed to start monitor: " + e.getMessage()); + } + + // Validate the expected ACLs on the Knox config znodes (make sure the monitor removed the world:anyone ACL) + List<ACL> expectedACLs = new ArrayList<>(); + expectedACLs.add(SASL_TESTUSER_ALL); + expectedACLs.add(WORLD_ANYONE_READ); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_CONFIG)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_PROVIDERS)); + validateKnoxConfigNodeACLs(expectedACLs, client.getACL().forPath(PATH_KNOX_DESCRIPTORS)); + } + + @Test public void testZooKeeperConfigMonitorSASLNodesExistWithAcceptableACL() throws Exception { final String configMonitorName = "zkConfigClient";
