diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java index acfcd6c38448..12221e50deac 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java @@ -18,18 +18,18 @@ package org.eclipse.jetty.server; -import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.util.StringUtil; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.resource.Resource; - import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.resource.Resource; + /** * This will approve an alias to any resource which is not a protected target. * Except symlinks... @@ -82,8 +82,8 @@ public boolean check(String uri, Resource resource) private boolean isProtectedPath(Path path, boolean followLinks) throws IOException { - String basePath = followLinks ? _contextHandler.getBaseResource().getFile().toPath().toRealPath().toString() : - _contextHandler.getBaseResource().getFile().toPath().toRealPath(LinkOption.NOFOLLOW_LINKS).toString(); + String basePath = followLinks ? _contextHandler.getBaseResource().getFile().toPath().toRealPath().toString() + : _contextHandler.getBaseResource().getFile().toPath().toRealPath(LinkOption.NOFOLLOW_LINKS).toString(); String targetPath = path.toString(); if (!targetPath.startsWith(basePath)) @@ -124,4 +124,10 @@ private boolean hasSymbolicLink(Path path) return false; } + + @Override + public String toString() + { + return String.format("%s@%x{checkSymlinkTargets=%s}", AllowedResourceAliasChecker.class.getSimpleName(), hashCode(), _checkSymlinkTargets); + } } \ No newline at end of file diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java index aa08c95d84f0..a19c98d90b31 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java @@ -19,24 +19,29 @@ package org.eclipse.jetty.test; import java.io.File; -import java.io.FileInputStream; +import java.net.URI; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.util.stream.Stream; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.AllowedResourceAliasChecker; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker; +import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.servlet.DefaultServlet; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.resource.PathResource; -import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -44,10 +49,10 @@ public class AliasCheckerSymlinkTest { - private static String _fileContents; - private static Path _webRootPath; - private Server _server; - private HttpClient _client; + private static Server _server; + private static ServerConnector _connector; + private static HttpClient _client; + private static ServletContextHandler _context; private static Path getResource(String path) throws Exception { @@ -61,129 +66,138 @@ private static void delete(Path path) IO.delete(path.toFile()); } + private static void setAliasChecker(ContextHandler.AliasCheck aliasChecker) + { + _context.clearAliasChecks(); + if (aliasChecker != null) + _context.addAliasCheck(aliasChecker); + } + @BeforeAll - public static void setup() throws Exception + public static void beforeAll() throws Exception { - _webRootPath = getResource("webroot"); - Path fileInWebroot = _webRootPath.resolve("file"); - _fileContents = IO.toString(new FileInputStream(fileInWebroot.toFile())); + Path webRootPath = getResource("webroot"); + Path fileInWebroot = webRootPath.resolve("file"); // Create symlink file that targets inside the webroot directory. - Path symlinkFile = _webRootPath.resolve("symlinkFile"); + Path symlinkFile = webRootPath.resolve("symlinkFile"); delete(symlinkFile); Files.createSymbolicLink(symlinkFile, fileInWebroot).toFile().deleteOnExit(); // Create symlink file that targets outside the webroot directory. - Path symlinkExternalFile = _webRootPath.resolve("symlinkExternalFile"); + Path symlinkExternalFile = webRootPath.resolve("symlinkExternalFile"); delete(symlinkExternalFile); - Files.createSymbolicLink(symlinkExternalFile, getResource("message.txt")).toFile().deleteOnExit(); + Files.createSymbolicLink(symlinkExternalFile, getResource("file")).toFile().deleteOnExit(); // Symlink to a directory inside of the webroot. - Path simlinkDir = _webRootPath.resolve("simlinkDir"); - delete(simlinkDir); - Files.createSymbolicLink(simlinkDir, _webRootPath.resolve("documents")).toFile().deleteOnExit(); + Path symlinkDir = webRootPath.resolve("symlinkDir"); + delete(symlinkDir); + Files.createSymbolicLink(symlinkDir, webRootPath.resolve("documents")).toFile().deleteOnExit(); // Symlink to a directory parent of the webroot. - Path symlinkParentDir = _webRootPath.resolve("symlinkParentDir"); + Path symlinkParentDir = webRootPath.resolve("symlinkParentDir"); delete(symlinkParentDir); - Files.createSymbolicLink(symlinkParentDir, _webRootPath.resolve("..")).toFile().deleteOnExit(); + Files.createSymbolicLink(symlinkParentDir, webRootPath.resolve("..")).toFile().deleteOnExit(); // Symlink to a directory outside of the webroot. - Path symlinkSiblingDir = _webRootPath.resolve("symlinkSiblingDir"); + Path symlinkSiblingDir = webRootPath.resolve("symlinkSiblingDir"); delete(symlinkSiblingDir); - Files.createSymbolicLink(symlinkSiblingDir, _webRootPath.resolve("../sibling")).toFile().deleteOnExit(); + Files.createSymbolicLink(symlinkSiblingDir, webRootPath.resolve("../sibling")).toFile().deleteOnExit(); // Symlink to the WEB-INF directory. - Path webInfSymlink = _webRootPath.resolve("webInfSymlink"); + Path webInfSymlink = webRootPath.resolve("webInfSymlink"); delete(webInfSymlink); - Files.createSymbolicLink(webInfSymlink, _webRootPath.resolve("WEB-INF")).toFile().deleteOnExit(); - } - - @BeforeEach - public void before() throws Exception - { - // TODO: don't use 8080 explicitly - _server = new Server(8080); - - ServletContextHandler context = new ServletContextHandler(); - context.setContextPath("/"); - context.setBaseResource(new PathResource(_webRootPath)); - context.setWelcomeFiles(new String[]{"index.html"}); - context.setProtectedTargets(new String[]{"/web-inf", "/meta-inf"}); - context.getMimeTypes().addMimeMapping("txt", "text/plain;charset=utf-8"); - - _server.setHandler(context); - context.addServlet(DefaultServlet.class, "/"); + Files.createSymbolicLink(webInfSymlink, webRootPath.resolve("WEB-INF")).toFile().deleteOnExit(); + + // Create and start Server and Client. + _server = new Server(); + _connector = new ServerConnector(_server); + _server.addConnector(_connector); + _context = new ServletContextHandler(); + _context.setContextPath("/"); + _context.setBaseResource(new PathResource(webRootPath)); + _context.setWelcomeFiles(new String[]{"index.html"}); + _context.setProtectedTargets(new String[]{"/web-inf", "/meta-inf"}); + _context.getMimeTypes().addMimeMapping("txt", "text/plain;charset=utf-8"); + _server.setHandler(_context); + _context.addServlet(DefaultServlet.class, "/"); _server.start(); _client = new HttpClient(); _client.start(); - - context.addAliasCheck(new AllowedResourceAliasChecker(context)); } - @AfterEach - public void after() throws Exception + @AfterAll + public static void afterAll() throws Exception { _client.stop(); _server.stop(); } - // todo : no alias checker, symlink alias checker, AllowedResourceAliasChecker (not following symlinks), AllowedResourceAliasChecker (following symlinks) - @Test - public void symlinkToInsideWebroot() throws Exception - { - ContentResponse response = _client.GET("http://localhost:8080/symlinkFile"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is(_fileContents)); - } - - @Test - public void symlinkToOutsideWebroot() throws Exception - { - ContentResponse response = _client.GET("http://localhost:8080/symlinkExternalFile"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is(_fileContents)); - } - - @Test - public void symlinkToDirectoryInsideWebroot() throws Exception - { - ContentResponse response = _client.GET("http://localhost:8080/simlinkDir/file"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is(_fileContents)); - } - - @Test - public void symlinkToParentDirectory() throws Exception - { - ContentResponse response = _client.GET("http://localhost:8080/symlinkParentDir/webroot/file"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is(_fileContents)); - - response = _client.GET("http://localhost:8080/symlinkParentDir/webroot/WEB-INF/web.xml"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is("should not be able to access this file.")); - } - - @Test - public void symlinkToSiblingDirectory() throws Exception + public static Stream testCases() { - ContentResponse response = _client.GET("http://localhost:8080/symlinkSiblingDir/file"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is(_fileContents)); - - // TODO Test .. or %2e%2e up from symlinked directory "http://localhost:8080/symlinkExternalDir/%2e%2e/webroot/file" -// ContentResponse response = _client.GET("http://localhost:8080/symlinkSiblingDir/%2e%2e/webroot/WEB-INF/web.xml"); -// assertThat(response.getStatus(), is(HttpStatus.OK_200)); -// assertThat(response.getContentAsString(), is("should not be able to access this file.")); + AllowedResourceAliasChecker allowedResource = new AllowedResourceAliasChecker(_context); + AllowedResourceAliasChecker allowedResourceSymlinks = new AllowedResourceAliasChecker(_context, true); + AllowSymLinkAliasChecker allowSymlinks = new AllowSymLinkAliasChecker(); + ContextHandler.ApproveAliases approveAliases = new ContextHandler.ApproveAliases(); + + return Stream.of( + // AllowedResourceAliasChecker that does not check the target of symlinks. + Arguments.of(allowedResource, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResource, "/symlinkExternalFile", HttpStatus.OK_200, "This file is outside webroot."), + Arguments.of(allowedResource, "/symlinkDir/file", HttpStatus.OK_200, "This file is inside webroot/documents."), + Arguments.of(allowedResource, "/symlinkParentDir/webroot/file", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResource, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + Arguments.of(allowedResource, "/symlinkSiblingDir/file", HttpStatus.OK_200, "This file is inside a sibling dir to webroot."), + Arguments.of(allowedResource, "/webInfSymlink/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + + // AllowedResourceAliasChecker that checks the target of symlinks. + Arguments.of(allowedResourceSymlinks, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResourceSymlinks, "/symlinkExternalFile", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResourceSymlinks, "/symlinkDir/file", HttpStatus.OK_200, "This file is inside webroot/documents."), + Arguments.of(allowedResourceSymlinks, "/symlinkParentDir/webroot/file", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResourceSymlinks, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResourceSymlinks, "/symlinkSiblingDir/file", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResourceSymlinks, "/webInfSymlink/web.xml", HttpStatus.NOT_FOUND_404, null), + + // The AllowSymLinkAliasChecker. + Arguments.of(allowSymlinks, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowSymlinks, "/symlinkExternalFile", HttpStatus.OK_200, "This file is outside webroot."), + Arguments.of(allowSymlinks, "/symlinkDir/file", HttpStatus.OK_200, "This file is inside webroot/documents."), + Arguments.of(allowSymlinks, "/symlinkParentDir/webroot/file", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowSymlinks, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + Arguments.of(allowSymlinks, "/symlinkSiblingDir/file", HttpStatus.OK_200, "This file is inside a sibling dir to webroot."), + Arguments.of(allowSymlinks, "/webInfSymlink/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + + // The ApproveAliases (approves everything regardless). + Arguments.of(approveAliases, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(approveAliases, "/symlinkExternalFile", HttpStatus.OK_200, "This file is outside webroot."), + Arguments.of(approveAliases, "/symlinkDir/file", HttpStatus.OK_200, "This file is inside webroot/documents."), + Arguments.of(approveAliases, "/symlinkParentDir/webroot/file", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(approveAliases, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + Arguments.of(approveAliases, "/symlinkSiblingDir/file", HttpStatus.OK_200, "This file is inside a sibling dir to webroot."), + Arguments.of(approveAliases, "/webInfSymlink/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + + // No alias checker (any symlink should be an alias). + Arguments.of(null, "/symlinkFile", HttpStatus.NOT_FOUND_404, null), + Arguments.of(null, "/symlinkExternalFile", HttpStatus.NOT_FOUND_404, null), + Arguments.of(null, "/symlinkDir/file", HttpStatus.NOT_FOUND_404, null), + Arguments.of(null, "/symlinkParentDir/webroot/file", HttpStatus.NOT_FOUND_404, null), + Arguments.of(null, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.NOT_FOUND_404, null), + Arguments.of(null, "/symlinkSiblingDir/file", HttpStatus.NOT_FOUND_404, null), + Arguments.of(null, "/webInfSymlink/web.xml", HttpStatus.NOT_FOUND_404, null) + ); } - @Test - public void symlinkToProtectedDirectoryInsideWebroot() throws Exception + @ParameterizedTest + @MethodSource("testCases") + public void test(ContextHandler.AliasCheck aliasChecker, String path, int httpStatus, String responseContent) throws Exception { - ContentResponse response = _client.GET("http://localhost:8080/webInfSymlink/web.xml"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is("should not be able to access this file.")); + setAliasChecker(aliasChecker); + URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + path); + ContentResponse response = _client.GET(uri); + assertThat(response.getStatus(), is(httpStatus)); + if (responseContent != null) + assertThat(response.getContentAsString(), is(responseContent)); } } diff --git a/tests/test-integration/src/test/resources/file b/tests/test-integration/src/test/resources/file new file mode 100644 index 000000000000..4b745d8b20e7 --- /dev/null +++ b/tests/test-integration/src/test/resources/file @@ -0,0 +1 @@ +This file is outside webroot. \ No newline at end of file diff --git a/tests/test-integration/src/test/resources/sibling/file b/tests/test-integration/src/test/resources/sibling/file index 516ae7511543..f57b0b89d7ae 100644 --- a/tests/test-integration/src/test/resources/sibling/file +++ b/tests/test-integration/src/test/resources/sibling/file @@ -1,12 +1 @@ -Lorem ipsum dolor sit amet, consectetur adipiscing elit. In quis felis nunc. -Quisque suscipit mauris et ante auctor ornare rhoncus lacus aliquet. Pellentesque -habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. -Vestibulum sit amet felis augue, vel convallis dolor. Cras accumsan vehicula diam -at faucibus. Etiam in urna turpis, sed congue mi. Morbi et lorem eros. Donec vulputate -velit in risus suscipit lobortis. Aliquam id urna orci, nec sollicitudin ipsum. -Cras a orci turpis. Donec suscipit vulputate cursus. Mauris nunc tellus, fermentum -eu auctor ut, mollis at diam. Quisque porttitor ultrices metus, vitae tincidunt massa -sollicitudin a. Vivamus porttitor libero eget purus hendrerit cursus. Integer aliquam -consequat mauris quis luctus. Cras enim nibh, dignissim eu faucibus ac, mollis nec neque. -Aliquam purus mauris, consectetur nec convallis lacinia, porta sed ante. Suspendisse -et cursus magna. Donec orci enim, molestie a lobortis eu, imperdiet vitae neque. \ No newline at end of file +This file is inside a sibling dir to webroot. \ No newline at end of file diff --git a/tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml b/tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml index 55130579d92e..47d791449100 100644 --- a/tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml +++ b/tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml @@ -1 +1 @@ -should not be able to access this file. \ No newline at end of file +This is the web.xml file. \ No newline at end of file diff --git a/tests/test-integration/src/test/resources/webroot/documents/file b/tests/test-integration/src/test/resources/webroot/documents/file index 516ae7511543..b45088053628 100644 --- a/tests/test-integration/src/test/resources/webroot/documents/file +++ b/tests/test-integration/src/test/resources/webroot/documents/file @@ -1,12 +1 @@ -Lorem ipsum dolor sit amet, consectetur adipiscing elit. In quis felis nunc. -Quisque suscipit mauris et ante auctor ornare rhoncus lacus aliquet. Pellentesque -habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. -Vestibulum sit amet felis augue, vel convallis dolor. Cras accumsan vehicula diam -at faucibus. Etiam in urna turpis, sed congue mi. Morbi et lorem eros. Donec vulputate -velit in risus suscipit lobortis. Aliquam id urna orci, nec sollicitudin ipsum. -Cras a orci turpis. Donec suscipit vulputate cursus. Mauris nunc tellus, fermentum -eu auctor ut, mollis at diam. Quisque porttitor ultrices metus, vitae tincidunt massa -sollicitudin a. Vivamus porttitor libero eget purus hendrerit cursus. Integer aliquam -consequat mauris quis luctus. Cras enim nibh, dignissim eu faucibus ac, mollis nec neque. -Aliquam purus mauris, consectetur nec convallis lacinia, porta sed ante. Suspendisse -et cursus magna. Donec orci enim, molestie a lobortis eu, imperdiet vitae neque. \ No newline at end of file +This file is inside webroot/documents. \ No newline at end of file diff --git a/tests/test-integration/src/test/resources/webroot/file b/tests/test-integration/src/test/resources/webroot/file index 516ae7511543..7a16a16aa081 100644 --- a/tests/test-integration/src/test/resources/webroot/file +++ b/tests/test-integration/src/test/resources/webroot/file @@ -1,12 +1 @@ -Lorem ipsum dolor sit amet, consectetur adipiscing elit. In quis felis nunc. -Quisque suscipit mauris et ante auctor ornare rhoncus lacus aliquet. Pellentesque -habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. -Vestibulum sit amet felis augue, vel convallis dolor. Cras accumsan vehicula diam -at faucibus. Etiam in urna turpis, sed congue mi. Morbi et lorem eros. Donec vulputate -velit in risus suscipit lobortis. Aliquam id urna orci, nec sollicitudin ipsum. -Cras a orci turpis. Donec suscipit vulputate cursus. Mauris nunc tellus, fermentum -eu auctor ut, mollis at diam. Quisque porttitor ultrices metus, vitae tincidunt massa -sollicitudin a. Vivamus porttitor libero eget purus hendrerit cursus. Integer aliquam -consequat mauris quis luctus. Cras enim nibh, dignissim eu faucibus ac, mollis nec neque. -Aliquam purus mauris, consectetur nec convallis lacinia, porta sed ante. Suspendisse -et cursus magna. Donec orci enim, molestie a lobortis eu, imperdiet vitae neque. \ No newline at end of file +This file is inside webroot. \ No newline at end of file