Skip to content

Commit

Permalink
Issue #6497 - improve testing for AliasCheck with symlinks
Browse files Browse the repository at this point in the history 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Jul 23, 2021
1 parent be8c6ac commit 7c0c3c9
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 142 deletions.
22 changes: 14 additions & 8 deletions jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java
View file
Expand Up @@ -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...
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
}
}
208 changes: 111 additions & 97 deletions tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java
View file
Expand Up @@ -19,35 +19,40 @@
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;
import static org.junit.jupiter.api.Assertions.assertNotNull;

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
{
Expand All @@ -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<Arguments> 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));
}
}
1 change: 1 addition & 0 deletions tests/test-integration/src/test/resources/file
View file
@@ -0,0 +1 @@
This file is outside webroot.
13 changes: 1 addition & 12 deletions tests/test-integration/src/test/resources/sibling/file
View 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.
This file is inside a sibling dir to webroot.
2 changes: 1 addition & 1 deletion tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml
View file
@@ -1 +1 @@
should not be able to access this file.
This is the web.xml file.
13 changes: 1 addition & 12 deletions tests/test-integration/src/test/resources/webroot/documents/file
View 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.
This file is inside webroot/documents.

0 comments on commit 7c0c3c9

Please sign in to comment.