KNOX-3418: Path traversal → arbitrary file write/overwrite in the Apa…

gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/DefaultTopologyService.java

@@ -306,13 +306,17 @@ public void setAliasService(AliasService as) {
   public void deployTopology(Topology t){
 
     try {
+      File topology = new File(topologiesDirectory.getAbsolutePath() + "/" + t.getName() + ".xml");
+      if (!topology.getCanonicalFile().toPath().startsWith(topologiesDirectory.getCanonicalFile().toPath())) {
+        throw new IOException("Resolved topology path escapes managed directory: " + t.getName());
+      }
+
       File temp = new File(topologiesDirectory.getAbsolutePath() + "/" + t.getName() + ".xml.temp");
       Marshaller mr = jaxbContext.createMarshaller();
 
       mr.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true);
       mr.marshal(t, temp);
 
-      File topology = new File(topologiesDirectory.getAbsolutePath() + "/" + t.getName() + ".xml");
       if(!temp.renameTo(topology)) {
         FileUtils.forceDelete(temp);
         throw new IOException("Could not rename temp file");
@@ -716,8 +720,15 @@ private static boolean writeConfig(File dest, String name, String content) {
 
     File destFile = new File(dest, name);
     try {
-      FileUtils.writeStringToFile(destFile, content, StandardCharsets.UTF_8);
-      log.wroteConfigurationFile(destFile.getAbsolutePath());
+      final File canonicalDest = dest.getCanonicalFile();
+      final File canonicalFile = destFile.getCanonicalFile();
+      if (!canonicalFile.toPath().startsWith(canonicalDest.toPath())) {
+        log.failedToWriteConfigurationFile(destFile.getAbsolutePath(),
+            new IOException("Resolved path escapes managed directory: " + name));
+        return false;
+      }
+      FileUtils.writeStringToFile(canonicalFile, content, StandardCharsets.UTF_8);
+      log.wroteConfigurationFile(canonicalFile.getAbsolutePath());
       result = true;
     } catch (IOException e) {
       log.failedToWriteConfigurationFile(destFile.getAbsolutePath(), e);

gateway-server/src/main/java/org/apache/knox/gateway/topology/monitor/ZkRemoteConfigurationMonitorService.java

@@ -212,14 +212,14 @@ public void stop() throws ServiceLifecycleException {
 
     @Override
     public boolean createProvider(String name, String content) {
-        String entryPath = "/knox/config/shared-providers/" + name;
+        String entryPath = "/knox/config/shared-providers/" + FilenameUtils.getName(name);
         client.createEntry(entryPath, content);
         return (client.getEntryData(entryPath) != null);
     }
 
     @Override
     public boolean createDescriptor(String name, String content) {
-        String entryPath = "/knox/config/descriptors/" + name;
+        String entryPath = "/knox/config/descriptors/" + FilenameUtils.getName(name);
         client.createEntry(entryPath, content);
         return (client.getEntryData(entryPath) != null);
     }

gateway-server/src/test/java/org/apache/knox/gateway/services/topology/DefaultTopologyServiceTest.java

@@ -595,6 +595,45 @@ public void testConfigurationCRUDAPI() throws Exception {
     }
   }
 
+  @Test
+  public void testDeployRejectsPathTraversal() throws Exception {
+    File dir = createDir();
+    File topologyDir = new File(dir, "topologies");
+    topologyDir.mkdirs();
+
+    File descriptorsDir = new File(dir, "descriptors");
+    descriptorsDir.mkdirs();
+
+    File sharedProvidersDir = new File(dir, "shared-providers");
+    sharedProvidersDir.mkdirs();
+
+    try {
+      TopologyService ts = new DefaultTopologyService();
+      Map<String, String> c = new HashMap<>();
+
+      GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+      EasyMock.expect(config.getReadOnlyOverrideTopologyNames()).andReturn(Collections.emptyList()).anyTimes();
+      EasyMock.expect(config.getGatewayTopologyDir()).andReturn(topologyDir.getAbsolutePath()).anyTimes();
+      EasyMock.expect(config.getGatewayConfDir()).andReturn(descriptorsDir.getParentFile().getAbsolutePath()).anyTimes();
+      EasyMock.replay(config);
+
+      ts.init(config, c);
+
+      final String traversalName = "../../evil.json";
+      final File escapeTarget = new File(dir.getParentFile(), "evil.json");
+      assertFalse("precondition: escape target must not pre-exist", escapeTarget.exists());
+
+      assertFalse("deployProviderConfiguration must reject a path-traversal name",
+                  ts.deployProviderConfiguration(traversalName, "malicious"));
+      assertFalse("deployDescriptor must reject a path-traversal name",
+                  ts.deployDescriptor(traversalName, "malicious"));
+
+      assertFalse("no file may be written outside the managed directory", escapeTarget.exists());
+    } finally {
+      FileUtils.deleteQuietly(dir);
+    }
+  }
+
   @Test
   public void testProviderParamsOrderIsPreserved() {
 

gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java

@@ -43,6 +43,7 @@
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
 import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
@@ -55,8 +56,6 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
@@ -90,7 +89,7 @@ public class TopologiesResource {
   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 final Pattern RESOURCE_NAME_PATTERN    = Pattern.compile("^[\\w.-]+$");
 
   private static GatewaySpiMessages log = MessagesFactory.get(GatewaySpiMessages.class);
 
@@ -171,12 +170,6 @@ public SimpleTopologyWrapper getTopologies() {
   public Topology uploadTopology(@PathParam("id") String id, Topology t) {
     Topology result = null;
 
-    try {
-      id = URLDecoder.decode(id, StandardCharsets.UTF_8.name());
-    } catch (Exception e) {
-      // Ignore
-    }
-
     if (!isValidResourceName(id)) {
       log.invalidResourceName(id);
       throw new BadRequestException("Invalid topology name: " + id);
@@ -188,6 +181,17 @@ public Topology uploadTopology(@PathParam("id") String id, Topology t) {
     t.setName(id);
     TopologyService ts = gs.getService(ServiceType.TOPOLOGY_SERVICE);
 
+    GatewayConfig config =
+            (GatewayConfig) request.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+    if (config != null &&
+            config.getReadOnlyOverrideTopologyNames().contains(FilenameUtils.getBaseName(id))) {
+      log.disallowedOverwritingReadOnlyTopology(id);
+      throw new WebApplicationException(
+          status(Response.Status.FORBIDDEN)
+              .entity("{ \"error\" : \"Cannot overwrite read-only topology: " + id + "\" }")
+              .build());
+    }
+
     // Check for existing topology with the same name, to see if it had been generated
     boolean existingGenerated = false;
     for (org.apache.knox.gateway.topology.Topology existingTopology : ts.getTopologies()) {
@@ -338,12 +342,6 @@ public Response deleteSimpleDescriptor(@PathParam("name") String name) {
   public Response uploadProviderConfiguration(@PathParam("name") String name, @Context HttpHeaders headers, String content) {
     Response response = null;
 
-    try {
-      name = URLDecoder.decode(name, StandardCharsets.UTF_8.name());
-    } catch (Exception e) {
-      // Ignore
-    }
-
     if (!isValidResourceName(name)) {
       log.invalidResourceName(name);
       throw new BadRequestException("Invalid provider configuration name: " + name);
@@ -393,12 +391,6 @@ public Response uploadSimpleDescriptor(@PathParam("name") String name,
                                          String content) {
     Response response = null;
 
-    try {
-      name = URLDecoder.decode(name, StandardCharsets.UTF_8.name());
-    } catch (Exception e) {
-      // Ignore
-    }
-
     if (!isValidResourceName(name)) {
       log.invalidResourceName(name);
       throw new BadRequestException("Invalid descriptor name: " + name);
@@ -554,8 +546,9 @@ private File getExistingConfigFile(Collection<File> existing, String candidateNa
     return result;
   }
 
-  private static boolean isValidResourceName(final String name) {
+  static boolean isValidResourceName(final String name) {
     return name != null && name.length() <= RESOURCE_NAME_LENGTH_MAX &&
+        !name.contains("..") &&
         RESOURCE_NAME_PATTERN.matcher(name).matches();
   }
 

gateway-service-admin/src/test/java/org/apache/knox/gateway/service/admin/TopologyResourceTest.java

@@ -19,14 +19,28 @@
 
 import org.apache.knox.gateway.topology.Topology;
 import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.topology.TopologyService;
+
+import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Response;
+
+import java.lang.reflect.Field;
+import java.util.Collections;
+import java.util.List;
 
 import org.easymock.EasyMock;
 import org.junit.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class TopologyResourceTest {
 
@@ -174,6 +188,28 @@ public void testTopologyURLMethods(){
 
   }
 
+  @Test
+  public void testResourceNameValidation() {
+    assertTrue(TopologiesResource.isValidResourceName("foo"));
+    assertTrue(TopologiesResource.isValidResourceName("foo.json"));
+    assertTrue(TopologiesResource.isValidResourceName("my-provider_1"));
+    assertTrue(TopologiesResource.isValidResourceName("a.b.c"));
+
+    assertFalse(TopologiesResource.isValidResourceName("../../etc/passwd"));
+    assertFalse(TopologiesResource.isValidResourceName(".."));
+    assertFalse(TopologiesResource.isValidResourceName("foo/bar"));
+    assertFalse(TopologiesResource.isValidResourceName("a/../../b"));
+    assertFalse(TopologiesResource.isValidResourceName("foo..bar"));
+
+    assertFalse(TopologiesResource.isValidResourceName("%2f"));
+    assertFalse(TopologiesResource.isValidResourceName("%252f"));
+
+    assertFalse(TopologiesResource.isValidResourceName(null));
+    assertFalse(TopologiesResource.isValidResourceName(""));
+    assertFalse(TopologiesResource.isValidResourceName("a".repeat(101)));
+    assertTrue(TopologiesResource.isValidResourceName("a".repeat(100)));
+  }
+
   private void setDefaultExpectations(HttpServletRequest request){
     EasyMock.expect( request.getPathInfo() ).andReturn( pathInfo ).anyTimes();
     EasyMock.expect( request.getContextPath() ).andReturn( reqContext ).anyTimes();
@@ -186,4 +222,76 @@ private void setMockRequestHeader(HttpServletRequest request, String header, Str
     EasyMock.expect( request.getHeader( header ) ).andReturn( expected ).anyTimes();
   }
 
+  @Test
+  public void testUploadTopologyRefusesReadOnlyOverride() throws Exception {
+    TopologyService ts = EasyMock.createMock(TopologyService.class);
+
+    GatewayServices gs = EasyMock.createNiceMock(GatewayServices.class);
+    EasyMock.expect(gs.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(ts).anyTimes();
+
+    GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+    EasyMock.expect(config.getReadOnlyOverrideTopologyNames())
+        .andReturn(List.of("manager")).anyTimes();
+
+    HttpServletRequest request = mockRequest(gs, config);
+
+    EasyMock.replay(ts, gs, config, request);
+
+    TopologiesResource res = new TopologiesResource();
+    setRequestField(res, request);
+
+    try {
+      res.uploadTopology("manager", new org.apache.knox.gateway.service.admin.beans.Topology());
+      fail("Expected WebApplicationException for read-only override topology");
+    } catch (WebApplicationException e) {
+      assertEquals(Response.Status.FORBIDDEN.getStatusCode(), e.getResponse().getStatus());
+    }
+
+    EasyMock.verify(ts);
+  }
+
+  @Test
+  public void testUploadTopologyAllowedWhenNotReadOnly() throws Exception {
+    TopologyService ts = EasyMock.createMock(TopologyService.class);
+    EasyMock.expect(ts.getTopologies()).andReturn(Collections.emptyList()).anyTimes();
+    ts.deployTopology(EasyMock.anyObject(Topology.class));
+    EasyMock.expectLastCall().once();
+
+    GatewayServices gs = EasyMock.createNiceMock(GatewayServices.class);
+    EasyMock.expect(gs.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(ts).anyTimes();
+
+    GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+    EasyMock.expect(config.getReadOnlyOverrideTopologyNames())
+        .andReturn(Collections.emptyList()).anyTimes();
+
+    HttpServletRequest request = mockRequest(gs, config);
+
+    EasyMock.replay(ts, gs, config, request);
+
+    TopologiesResource res = new TopologiesResource();
+    setRequestField(res, request);
+
+    res.uploadTopology("sandbox", new org.apache.knox.gateway.service.admin.beans.Topology());
+
+    EasyMock.verify(ts);
+  }
+
+  private HttpServletRequest mockRequest(GatewayServices gs, GatewayConfig config) {
+    ServletContext context = EasyMock.createNiceMock(ServletContext.class);
+    EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE))
+        .andReturn(gs).anyTimes();
+    EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE))
+        .andReturn(config).anyTimes();
+    HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class);
+    EasyMock.expect(request.getServletContext()).andReturn(context).anyTimes();
+    EasyMock.replay(context);
+    return request;
+  }
+
+  private void setRequestField(TopologiesResource res, HttpServletRequest request) throws Exception {
+    Field f = TopologiesResource.class.getDeclaredField("request");
+    f.setAccessible(true);
+    f.set(res, request);
+  }
+
 }

gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java

@@ -76,6 +76,9 @@ public interface GatewaySpiMessages {
   @Message( level = MessageLevel.ERROR, text = "Topology {0} cannot be manually overwritten because it was generated from a simple descriptor." )
   void disallowedOverwritingGeneratedTopology(String topologyName);
 
+  @Message( level = MessageLevel.INFO, text = "Read-only topology {0} cannot be overwritten." )
+  void disallowedOverwritingReadOnlyTopology(String topologyName);
+
   @Message( level = MessageLevel.INFO, text = "Read-only descriptor {0} cannot be overwritten." )
   void disallowedOverwritingGeneratedDescriptor(String name);
 

gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java

@@ -1806,7 +1806,8 @@ public void testPutDescriptorWithValidEncodedName() throws Exception {
     String newDescriptorJSON = createDescriptor(clusterName);
 
     // Attempt to PUT the descriptor
-    given().auth().preemptive().basic(username, password)
+    given().urlEncodingEnabled(false)
+           .auth().preemptive().basic(username, password)
            .header("Content-type", MediaType.APPLICATION_JSON)
            .body(newDescriptorJSON.getBytes(StandardCharsets.UTF_8.name()))
            .then()