Repository: knox Updated Branches: refs/heads/master 7439e6968 -> 26182eb35
KNOX-1309 - Admin API resource names should be validated Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/26182eb3 Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/26182eb3 Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/26182eb3 Branch: refs/heads/master Commit: 26182eb355c4555773462534050af56240ea77aa Parents: 7439e69 Author: Phil Zampino <pzamp...@apache.org> Authored: Tue May 15 08:29:14 2018 -0400 Committer: Phil Zampino <pzamp...@apache.org> Committed: Tue May 15 08:44:05 2018 -0400 ---------------------------------------------------------------------- .../service/admin/TopologiesResource.java | 58 +++- .../knox/gateway/i18n/GatewaySpiMessages.java | 3 + .../gateway/GatewayAdminTopologyFuncTest.java | 267 +++++++++++++++++++ 3 files changed, 327 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/26182eb3/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java ---------------------------------------------------------------------- diff --git a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java index 3a39ce2..3b8f2fe 100644 --- a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java +++ b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java @@ -29,6 +29,7 @@ import org.apache.knox.gateway.config.GatewayConfig; import org.apache.knox.gateway.services.topology.TopologyService; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.BadRequestException; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -48,6 +49,7 @@ import java.io.File; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.net.URLDecoder; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -55,6 +57,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static javax.ws.rs.core.MediaType.APPLICATION_XML; @@ -80,6 +83,9 @@ public class TopologiesResource { private static final String DESCRIPTORS_API_PATH = "descriptors"; private static final String SINGLE_DESCRIPTOR_API_PATH = DESCRIPTORS_API_PATH + "/{name}"; + private static final int RESOURCE_NAME_LENGTH_MAX = 100; + private static final Pattern RESOURCE_NAME_PATTERN = Pattern.compile("^[\\w-/.]+$"); + private static GatewaySpiMessages log = MessagesFactory.get(GatewaySpiMessages.class); private static final Map<MediaType, String> mediaTypeFileExtensions = new HashMap<>(); @@ -149,6 +155,17 @@ public class TopologiesResource { public Topology uploadTopology(@PathParam("id") String id, Topology t) { Topology result = null; + try { + id = URLDecoder.decode(id, "UTF-8"); + } catch (Exception e) { + // Ignore + } + + if (!isValidResourceName(id)) { + log.invalidResourceName(id); + throw new BadRequestException("Invalid topology name: " + id); + } + GatewayServices gs = (GatewayServices) request.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE); @@ -305,6 +322,17 @@ public class TopologiesResource { public Response uploadProviderConfiguration(@PathParam("name") String name, @Context HttpHeaders headers, String content) { Response response = null; + try { + name = URLDecoder.decode(name, "UTF-8"); + } catch (Exception e) { + // Ignore + } + + if (!isValidResourceName(name)) { + log.invalidResourceName(name); + throw new BadRequestException("Invalid provider configuration name: " + name); + } + GatewayServices gs = (GatewayServices) request.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE); @@ -336,9 +364,22 @@ public class TopologiesResource { @PUT @Consumes({APPLICATION_JSON, TEXT_PLAIN}) @Path(SINGLE_DESCRIPTOR_API_PATH) - public Response uploadSimpleDescriptor(@PathParam("name") String name, @Context HttpHeaders headers, String content) { + public Response uploadSimpleDescriptor(@PathParam("name") String name, + @Context HttpHeaders headers, + String content) { Response response = null; + try { + name = URLDecoder.decode(name, "UTF-8"); + } catch (Exception e) { + // Ignore + } + + if (!isValidResourceName(name)) { + log.invalidResourceName(name); + throw new BadRequestException("Invalid descriptor name: " + name); + } + GatewayServices gs = (GatewayServices) request.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE); @@ -470,6 +511,21 @@ public class TopologiesResource { } + private static boolean isValidResourceName(final String name) { + boolean isValid = false; + + if (name != null) { + if (name.length() <= RESOURCE_NAME_LENGTH_MAX) { + if (RESOURCE_NAME_PATTERN.matcher(name).matches()) { + isValid = true; + } + } + } + + return isValid; + } + + private static class TopologyComparator implements Comparator<SimpleTopology> { @Override public int compare(SimpleTopology t1, SimpleTopology t2) { http://git-wip-us.apache.org/repos/asf/knox/blob/26182eb3/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java b/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java index 42d69d9..0f750c1 100644 --- a/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java +++ b/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java @@ -88,6 +88,9 @@ public interface GatewaySpiMessages { @Message(level = MessageLevel.ERROR, text = "Invalid resource URI {0} : {1}") void invalidResourceURI(final String uri, final String reason, @StackTrace(level = MessageLevel.DEBUG) Exception e ); + @Message(level = MessageLevel.ERROR, text = "Invalid resource name: {0}") + void invalidResourceName(final String resourceName); + @Message( level = MessageLevel.ERROR, text = "Topology {0} cannot be manually overwritten because it was generated from a simple descriptor." ) void disallowedOverwritingGeneratedTopology(final String topologyName); http://git-wip-us.apache.org/repos/asf/knox/blob/26182eb3/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java ---------------------------------------------------------------------- diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java index b7eefb1..0bb0b23 100644 --- a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java +++ b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.StringReader; import java.net.URI; import java.net.URISyntaxException; +import java.net.URLDecoder; import java.util.Arrays; import java.util.Enumeration; import java.util.HashMap; @@ -732,6 +733,32 @@ public class GatewayAdminTopologyFuncTest { LOG_EXIT(); } + + @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testPutTopologyWithInvalidName() throws Exception { + LOG_ENTER() ; + + String username = "admin"; + String password = "admin-password"; + String url = clusterUrl + "/api/v1/topologies/test-put-!nvalid"; + + String JsonPut = given().auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .get(clusterUrl + "/api/v1/topologies/test-cluster") + .getBody().asString(); + + given().auth().preemptive().basic(username, password) + .contentType(MediaType.APPLICATION_JSON) + .header("Accept", MediaType.APPLICATION_XML) + .body(JsonPut) + .then() + .statusCode(HttpStatus.SC_BAD_REQUEST) + .when().put(url).getBody().asString(); + + LOG_EXIT(); + } + + @Test( timeout = TestUtils.LONG_TIMEOUT ) public void testPutTopologyWithEntityInjection() throws Exception { LOG_ENTER() ; @@ -1219,6 +1246,36 @@ public class GatewayAdminTopologyFuncTest { @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testPutProviderConfigurationWithInvalidName() throws Exception { + LOG_ENTER(); + + final String username = "admin"; + final String password = "admin-password"; + final String serviceUrl = clusterUrl + "/api/v1/providerconfig"; + + final String newProviderConfigName = "new-provider&config"; + final String newProviderConfigFileName = newProviderConfigName + ".xml"; + + XMLTag newProviderConfigXML = createProviderConfiguration(); + + // Attempt to PUT a provider config + given().auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_XML) + .body(newProviderConfigXML.toBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_BAD_REQUEST) + .when().put(serviceUrl + "/" + newProviderConfigName); + + // Verify that the provider configuration was written to the expected location + File newProviderConfigFile = + new File(new File(config.getGatewayConfDir(), "shared-providers"), newProviderConfigFileName); + assertFalse(newProviderConfigFile.exists()); + + LOG_EXIT(); + } + + + @Test( timeout = TestUtils.LONG_TIMEOUT ) public void testDeleteProviderConfiguration() throws Exception { LOG_ENTER(); @@ -1598,6 +1655,216 @@ public class GatewayAdminTopologyFuncTest { LOG_EXIT(); } + @Test + public void testPutDescriptorWithValidEncodedName() throws Exception { + + final String encodedName = "new%2Ddescriptor"; + final String newDescriptorName = URLDecoder.decode(encodedName, "UTF-8"); + + final String username = "admin"; + final String password = "admin-password"; + final String serviceUrl = clusterUrl + "/api/v1/descriptors"; + + final String clusterName = "test-cluster"; + final String newDescriptorFileName = newDescriptorName + ".json"; + + String newDescriptorJSON = createDescriptor(clusterName); + + // Attempt to PUT the descriptor + given().auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(newDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_CREATED) + .when().put(serviceUrl + "/" + encodedName); + + // Verify that the descriptor was written to the expected location + File newDescriptorFile = new File(new File(config.getGatewayConfDir(), "descriptors"), newDescriptorFileName); + assertTrue(newDescriptorFile.exists()); + + // Request a listing of all the descriptors to verify that the PUT FAILED + ResponseBody responseBody = given().auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .then() + .statusCode(HttpStatus.SC_OK) + .contentType(MediaType.APPLICATION_JSON) + .when().get(serviceUrl).body(); + assertNotNull(responseBody); + boolean isCreated = false; + List<Map<String, String>> items = responseBody.path("items"); + for (Map<String, String> item : items) { + if(item.get("name").equals(newDescriptorFileName)) { + isCreated = true; + break; + } + } + assertTrue(isCreated); + + newDescriptorFile.delete(); + } + + + @Test + public void testPutDescriptorWithFileExtension() throws Exception { + + final String newDescriptorName = "newdescriptor"; + + final String username = "admin"; + final String password = "admin-password"; + final String serviceUrl = clusterUrl + "/api/v1/descriptors"; + + final String clusterName = "test-cluster"; + final String newDescriptorFileName = newDescriptorName + ".json"; + + String newDescriptorJSON = createDescriptor(clusterName); + + // Attempt to PUT the descriptor + given().auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(newDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_CREATED) + .when().put(serviceUrl + "/" + newDescriptorFileName); + + // Verify that the descriptor was written to the expected location + File newDescriptorFile = new File(new File(config.getGatewayConfDir(), "descriptors"), newDescriptorFileName); + assertTrue(newDescriptorFile.exists()); + + // Request a listing of all the descriptors to verify that the PUT FAILED + ResponseBody responseBody = given().auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .then() + .statusCode(HttpStatus.SC_OK) + .contentType(MediaType.APPLICATION_JSON) + .when().get(serviceUrl).body(); + assertNotNull(responseBody); + boolean isCreated = false; + List<Map<String, String>> items = responseBody.path("items"); + for (Map<String, String> item : items) { + if(item.get("name").equals(newDescriptorFileName)) { + isCreated = true; + break; + } + } + assertTrue(isCreated); + + newDescriptorFile.delete(); + } + + + @Test + public void testPutDescriptorWithInvalidEncodedName() throws Exception { + + final String encodedName = "'';!--%22%3CXSS%3E=&%7B()%7D"; + final String newDescriptorName = URLDecoder.decode(encodedName, "UTF-8"); + + final String username = "admin"; + final String password = "admin-password"; + final String serviceUrl = clusterUrl + "/api/v1/descriptors"; + + final String clusterName = "test-cluster"; + final String newDescriptorFileName = newDescriptorName + ".json"; + + String newDescriptorJSON = createDescriptor(clusterName); + + // Attempt to PUT the descriptor + given().auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(newDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_BAD_REQUEST) + .when().put(serviceUrl + "/" + encodedName); + + // Verify that the descriptor was NOT written to the expected location + File newDescriptorFile = new File(new File(config.getGatewayConfDir(), "descriptors"), newDescriptorFileName); + assertFalse(newDescriptorFile.exists()); + } + + + @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testPutDescriptorWithInvalidNameEncodedElement() throws Exception { + LOG_ENTER(); + + doTestPutDescriptorWithInvalidName("new<descriptor>"); + + LOG_EXIT(); + } + + + @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testPutDescriptorWithInvalidNamePercent() throws Exception { + LOG_ENTER(); + + doTestPutDescriptorWithInvalidName("newdes%criptor"); + + LOG_EXIT(); + } + + + @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testPutDescriptorWithInvalidNameXMLElement() throws Exception { + LOG_ENTER(); + + doTestPutDescriptorWithInvalidName("new<descriptor>"); + + LOG_EXIT(); + } + + + @Test( timeout = TestUtils.LONG_TIMEOUT ) + public void testPutDescriptorWithInvalidNameTooLong() throws Exception { + LOG_ENTER(); + + String descName = "newDescriptor"; + while (descName.length() < 101) { + descName += descName; + } + + doTestPutDescriptorWithInvalidName(descName); + + LOG_EXIT(); + } + + + private void doTestPutDescriptorWithInvalidName(final String newDescriptorName) throws Exception { + + assertNotNull(newDescriptorName); + + final String username = "admin"; + final String password = "admin-password"; + final String serviceUrl = clusterUrl + "/api/v1/descriptors"; + + final String clusterName = "test-cluster"; + final String newDescriptorFileName = newDescriptorName + ".json"; + + String newDescriptorJSON = createDescriptor(clusterName); + + // Attempt to PUT the descriptor + given().auth().preemptive().basic(username, password) + .header("Content-type", MediaType.APPLICATION_JSON) + .body(newDescriptorJSON.getBytes("utf-8")) + .then() + .statusCode(HttpStatus.SC_BAD_REQUEST) + .when().put(serviceUrl + "/" + newDescriptorName); + + // Verify that the descriptor was written to the expected location + File newDescriptorFile = new File(new File(config.getGatewayConfDir(), "descriptors"), newDescriptorFileName); + assertFalse(newDescriptorFile.exists()); + + // Request a listing of all the descriptors to verify that the PUT FAILED + ResponseBody responseBody = given().auth().preemptive().basic(username, password) + .header("Accept", MediaType.APPLICATION_JSON) + .then() + .statusCode(HttpStatus.SC_OK) + .contentType(MediaType.APPLICATION_JSON) + .when().get(serviceUrl).body(); + assertNotNull(responseBody); + List<Map<String, String>> items = responseBody.path("items"); + for (Map<String, String> item : items) { + assertFalse(item.get("name").equals(newDescriptorName)); + } + } + @Test( timeout = TestUtils.LONG_TIMEOUT ) public void testDeleteDescriptor() throws Exception {
