Repository: knox Updated Branches: refs/heads/master 5f6df55d4 -> 85ccaae92
KNOX-1619 - RemoteAliasService#entryChanged ArrayIndexOutOfBoundsException Signed-off-by: Kevin Risden <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/85ccaae9 Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/85ccaae9 Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/85ccaae9 Branch: refs/heads/master Commit: 85ccaae92945b1cb2b50e02f00eab8e7b0f457a6 Parents: 5f6df55 Author: Kevin Risden <[email protected]> Authored: Wed Nov 21 14:22:15 2018 -0500 Committer: Kevin Risden <[email protected]> Committed: Mon Nov 26 11:36:51 2018 -0500 ---------------------------------------------------------------------- .../security/impl/RemoteAliasService.java | 40 +++++--------------- .../security/impl/RemoteAliasServiceTest.java | 28 +++++--------- 2 files changed, 19 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/85ccaae9/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java index 91c6365..c7552db 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/RemoteAliasService.java @@ -64,8 +64,7 @@ public class RemoteAliasService implements AliasService { public static final String DEFAULT_CLUSTER_NAME = "__gateway"; public static final String GATEWAY_IDENTITY_PASSPHRASE = "gateway-identity-passphrase"; - private static final GatewayMessages LOG = MessagesFactory - .get(GatewayMessages.class); + private static final GatewayMessages LOG = MessagesFactory.get(GatewayMessages.class); // N.B. This is ZooKeeper-specific, and should be abstracted when another registry is supported private static final RemoteConfigurationRegistryClient.EntryACL AUTHENTICATED_USERS_ALL; @@ -259,9 +258,7 @@ public class RemoteAliasService implements AliasService { checkPathsExist(remoteClient); ensureEntry(buildClusterEntryName(clusterName), remoteClient); try { - remoteClient.createEntry(aliasEntryPath, encrypt(value)); - } catch (Exception e) { throw new AliasServiceException(e); } @@ -271,7 +268,6 @@ public class RemoteAliasService implements AliasService { "Failed to store alias %s for cluster %s in remote registry", alias, clusterName)); } - } } @@ -298,13 +294,10 @@ public class RemoteAliasService implements AliasService { "Failed to delete alias %s for cluster %s in remote registry", alias, clusterName)); } - } - } else { LOG.missingClientConfigurationForRemoteMonitoring(); } - } @Override @@ -349,7 +342,6 @@ public class RemoteAliasService implements AliasService { throw new AliasServiceException(e); } } - } /* @@ -434,7 +426,6 @@ public class RemoteAliasService implements AliasService { } else { LOG.missingClientConfigurationForRemoteMonitoring(); } - } @Override @@ -463,7 +454,6 @@ public class RemoteAliasService implements AliasService { "Unable to add listener for path " + PATH_KNOX_ALIAS_STORE_TOPOLOGY, e); } - } if(!config.isRemoteAliasServiceEnabled()) { @@ -540,7 +530,6 @@ public class RemoteAliasService implements AliasService { (Base64.encodeBase64String(result.salt) + "::" + Base64 .encodeBase64String(result.iv) + "::" + Base64 .encodeBase64String(result.cipher)).getBytes(StandardCharsets.UTF_8)); - } /** @@ -554,7 +543,9 @@ public class RemoteAliasService implements AliasService { final String line = new String(Base64.decodeBase64(encoded), StandardCharsets.UTF_8); final String[] parts = line.split("::"); - + if(parts.length != 3) { + throw new IllegalArgumentException("Data should have 3 parts split by ::"); + } return new String(encryptor .decrypt(Base64.decodeBase64(parts[0]), Base64.decodeBase64(parts[1]), Base64.decodeBase64(parts[2])), StandardCharsets.UTF_8); @@ -590,13 +581,12 @@ public class RemoteAliasService implements AliasService { /* remove the alias from local keystore */ final AliasService aliasService = GatewayServer.getGatewayServices() .getService(GatewayServices.ALIAS_SERVICE); - if (aliasService != null && paths.length > 1 - && aliasService instanceof RemoteAliasService) { + if (paths.length > 1 + && aliasService instanceof RemoteAliasService) { ((RemoteAliasService) aliasService) .removeAliasForClusterLocally(paths[0], paths[1]); } } - } catch (final Exception e) { LOG.errorRemovingAliasLocally(paths[0], paths[1], e.toString()); } @@ -612,7 +602,6 @@ public class RemoteAliasService implements AliasService { } catch (final Exception e) { LOG.errorRemovingAliasLocally(paths[0], paths[1], e.toString()); } - } else if (subPath != null) { /* Add a child listener for the cluster */ LOG.addRemoteListener(path); @@ -621,12 +610,10 @@ public class RemoteAliasService implements AliasService { } catch (Exception e) { LOG.errorAddingRemoteListener(path, e.toString()); } - } break; } - } } @@ -640,11 +627,7 @@ public class RemoteAliasService implements AliasService { final String alias; final RemoteAliasService remoteAliasService; - /** - * Create an instance - */ public RemoteAliasEntryListener(final String cluster, final String alias, final RemoteAliasService remoteAliasService) { - super(); this.cluster = cluster; this.alias = alias; this.remoteAliasService = remoteAliasService; @@ -658,21 +641,16 @@ public class RemoteAliasService implements AliasService { final AliasService aliasService = GatewayServer.getGatewayServices() .getService(GatewayServices.ALIAS_SERVICE); - if (aliasService != null - && aliasService instanceof RemoteAliasService) { + if (aliasService instanceof RemoteAliasService) { try { - ((RemoteAliasService) aliasService) - .addAliasForClusterLocally(cluster, alias, remoteAliasService.decrypt(new String(data, StandardCharsets.UTF_8))); + ((RemoteAliasService) aliasService).addAliasForClusterLocally(cluster, alias, + remoteAliasService.decrypt(new String(data, StandardCharsets.UTF_8))); } catch (final Exception e) { /* log and move on */ LOG.errorAddingAliasLocally(cluster, alias, e.toString()); } - } - } } - } - } http://git-wip-us.apache.org/repos/asf/knox/blob/85ccaae9/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasServiceTest.java ---------------------------------------------------------------------- diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasServiceTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasServiceTest.java index 855e5f6..e1429b5 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasServiceTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/security/impl/RemoteAliasServiceTest.java @@ -17,7 +17,6 @@ */ package org.apache.knox.gateway.security.impl; -import org.apache.curator.framework.CuratorFramework; import org.apache.curator.test.InstanceSpec; import org.apache.curator.test.TestingCluster; import org.apache.knox.gateway.config.GatewayConfig; @@ -35,6 +34,7 @@ import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -48,12 +48,8 @@ import static org.easymock.EasyMock.capture; */ public class RemoteAliasServiceTest { + private static final String configMonitorName = "remoteConfigMonitorClient"; private static TestingCluster zkNodes; - - private static CuratorFramework client; - - private static String configMonitorName = "remoteConfigMonitorClient"; - private static GatewayConfig gc; public RemoteAliasServiceTest() { @@ -88,7 +84,6 @@ public class RemoteAliasServiceTest { .andReturn(true).anyTimes(); EasyMock.replay(gc); - } private static void configureAndStartZKCluster() throws Exception { @@ -109,18 +104,10 @@ public class RemoteAliasServiceTest { // Start the cluster zkNodes.start(); - } @AfterClass public static void tearDownSuite() throws Exception { - // Clean up the ZK nodes, and close the client - if (client != null) { - client.delete().deletingChildrenIfNeeded() - .forPath(RemoteAliasService.PATH_KNOX_SECURITY); - client.close(); - } - // Shutdown the ZK cluster zkNodes.close(); } @@ -176,7 +163,6 @@ public class RemoteAliasServiceTest { zkAlias.init(gc, Collections.emptyMap()); zkAlias.start(); - /* Put */ zkAlias.addAliasForCluster(expectedClusterName, expectedAlias, expectedPassword); @@ -232,7 +218,6 @@ public class RemoteAliasServiceTest { Assert.assertEquals(aliases.size(), 2); Assert.assertTrue("Expected alias 'knox.test.alias' not found ", aliases.contains(testAutoGeneratedpasswordAlias)); - } @Test @@ -262,6 +247,13 @@ public class RemoteAliasServiceTest { final String clear = zkAlias.decrypt(encrypted); Assert.assertEquals(testPassword, clear); + try { + // Match default data that is put into a newly created znode + final byte[] badData = new byte[0]; + zkAlias.decrypt(new String(badData, StandardCharsets.UTF_8)); + Assert.fail("Should have failed to decrypt."); + } catch (IllegalArgumentException e) { + Assert.assertEquals("Data should have 3 parts split by ::", e.getMessage()); + } } - }
