diff --git a/jetty-gcloud/jetty-gcloud-session-manager/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionTester.java b/jetty-gcloud/jetty-gcloud-session-manager/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionTester.java index 5f4460aa7490..8f2d6e028948 100644 --- a/jetty-gcloud/jetty-gcloud-session-manager/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionTester.java +++ b/jetty-gcloud/jetty-gcloud-session-manager/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionTester.java @@ -19,8 +19,8 @@ package org.eclipse.jetty.gcloud.session; import org.eclipse.jetty.security.HashLoginService; +import org.eclipse.jetty.server.AllowedResourceAliasChecker; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker; import org.eclipse.jetty.server.session.DefaultSessionCache; import org.eclipse.jetty.server.session.DefaultSessionIdManager; import org.eclipse.jetty.webapp.WebAppContext; @@ -47,7 +47,7 @@ public static void main(String[] args) throws Exception WebAppContext webapp = new WebAppContext(); webapp.setContextPath("/"); webapp.setWar("../../jetty-distribution/target/distribution/demo-base/webapps/test.war"); - webapp.addAliasCheck(new AllowSymLinkAliasChecker()); + webapp.addAliasCheck(new AllowedResourceAliasChecker(webapp)); GCloudSessionDataStore ds = new GCloudSessionDataStore(); DefaultSessionCache ss = new DefaultSessionCache(webapp.getSessionHandler()); diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebappConstants.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebappConstants.java index 697e30034702..e93172a2352d 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebappConstants.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebappConstants.java @@ -181,5 +181,5 @@ public class OSGiWebappConstants /** * Set of extra dirs that must not be served by osgi webapps */ - public static final String[] DEFAULT_PROTECTED_OSGI_TARGETS = {"/osgi-inf", "/osgi-opts"}; + public static final String[] DEFAULT_PROTECTED_OSGI_TARGETS = {"/OSGI-INF", "/OSGI-OPTS"}; } 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 new file mode 100644 index 000000000000..f900944990ff --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java @@ -0,0 +1,210 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.resource.PathResource; +import org.eclipse.jetty.util.resource.Resource; + +/** + *

This will approve any alias to anything inside of the {@link ContextHandler}s resource base which + * is not protected by a protected target as defined by {@link ContextHandler#getProtectedTargets()} at start.

+ *

Aliases approved by this may still be able to bypass SecurityConstraints, so this class would need to be extended + * to enforce any additional security constraints that are required.

+ */ +public class AllowedResourceAliasChecker extends AbstractLifeCycle implements ContextHandler.AliasCheck +{ + private static final Logger LOG = Log.getLogger(AllowedResourceAliasChecker.class); + protected static final LinkOption[] FOLLOW_LINKS = new LinkOption[0]; + protected static final LinkOption[] NO_FOLLOW_LINKS = new LinkOption[]{LinkOption.NOFOLLOW_LINKS}; + + private final ContextHandler _contextHandler; + private final List _protected = new ArrayList<>(); + protected Path _base; + + /** + * @param contextHandler the context handler to use. + */ + public AllowedResourceAliasChecker(ContextHandler contextHandler) + { + _contextHandler = contextHandler; + } + + protected ContextHandler getContextHandler() + { + return _contextHandler; + } + + @Override + protected void doStart() throws Exception + { + _base = getPath(_contextHandler.getBaseResource()); + if (_base == null) + _base = Paths.get("/").toAbsolutePath(); + if (Files.exists(_base, NO_FOLLOW_LINKS)) + _base = _base.toRealPath(FOLLOW_LINKS); + + String[] protectedTargets = _contextHandler.getProtectedTargets(); + if (protectedTargets != null) + { + for (String s : protectedTargets) + _protected.add(_base.getFileSystem().getPath(_base.toString(), s)); + } + } + + @Override + protected void doStop() throws Exception + { + _base = null; + _protected.clear(); + } + + @Override + public boolean check(String pathInContext, Resource resource) + { + try + { + // The existence check resolves the symlinks. + if (!resource.exists()) + return false; + + Path path = getPath(resource); + if (path == null) + return false; + + return check(pathInContext, path); + } + catch (Throwable t) + { + if (LOG.isDebugEnabled()) + LOG.debug("Failed to check alias", t); + return false; + } + } + + protected boolean check(String pathInContext, Path path) + { + // Allow any aliases (symlinks, 8.3, casing, etc.) so long as + // the resulting real file is allowed. + return isAllowed(getRealPath(path)); + } + + protected boolean isAllowed(Path path) + { + // If the resource doesn't exist we cannot determine whether it is protected so we assume it is. + if (path != null && Files.exists(path)) + { + // Walk the path parent links looking for the base resource, but failing if any steps are protected + while (path != null) + { + // If the path is the same file as the base, then it is contained in the base and + // is not protected. + if (isSameFile(path, _base)) + return true; + + // If the path is the same file as any protected resources, then it is protected. + for (Path p : _protected) + { + if (isSameFile(path, p)) + return false; + } + + // Walks up the aliased path name, not the real path name. + // If WEB-INF is a link to /var/lib/webmeta then after checking + // a URI of /WEB-INF/file.xml the parent is /WEB-INF and not /var/lib/webmeta + path = path.getParent(); + } + } + + return false; + } + + protected boolean isSameFile(Path path1, Path path2) + { + if (Objects.equals(path1, path2)) + return true; + try + { + if (Files.isSameFile(path1, path2)) + return true; + } + catch (Throwable t) + { + if (LOG.isDebugEnabled()) + LOG.debug("ignored", t); + } + return false; + } + + private static Path getRealPath(Path path) + { + if (path == null || !Files.exists(path)) + return null; + try + { + path = path.toRealPath(FOLLOW_LINKS); + if (Files.exists(path)) + return path; + } + catch (IOException e) + { + if (LOG.isDebugEnabled()) + LOG.debug("No real path for {}", path, e); + } + return null; + } + + protected Path getPath(Resource resource) + { + try + { + if (resource instanceof PathResource) + return ((PathResource)resource).getPath(); + return resource.getFile().toPath(); + } + catch (Throwable t) + { + LOG.ignore(t); + return null; + } + } + + @Override + public String toString() + { + return String.format("%s@%x{base=%s,protected=%s}", + this.getClass().getSimpleName(), + hashCode(), + _base, + Arrays.asList(_contextHandler.getProtectedTargets())); + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SameFileAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SameFileAliasChecker.java index 5f7009e96a3b..50eb3e84faa7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/SameFileAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SameFileAliasChecker.java @@ -44,7 +44,9 @@ * or Linux on XFS) the the actual file could be stored using UTF-16, * but be accessed using NFD UTF-8 or NFC UTF-8 for the same file. *

+ * @deprecated use {@link org.eclipse.jetty.server.AllowedResourceAliasChecker} instead. */ +@Deprecated public class SameFileAliasChecker implements AliasCheck { private static final Logger LOG = Log.getLogger(SameFileAliasChecker.class); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java new file mode 100644 index 000000000000..331c03fcc178 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java @@ -0,0 +1,91 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + * An extension of {@link AllowedResourceAliasChecker} which will allow symlinks alias to arbitrary + * targets, so long as the symlink file itself is an allowed resource. + */ +public class SymlinkAllowedResourceAliasChecker extends AllowedResourceAliasChecker +{ + private static final Logger LOG = Log.getLogger(SymlinkAllowedResourceAliasChecker.class); + + /** + * @param contextHandler the context handler to use. + */ + public SymlinkAllowedResourceAliasChecker(ContextHandler contextHandler) + { + super(contextHandler); + } + + @Override + protected boolean check(String pathInContext, Path path) + { + // do not allow any file separation characters in the URI, as we need to know exactly what are the segments + if (File.separatorChar != '/' && pathInContext.indexOf(File.separatorChar) >= 0) + return false; + + // Split the URI path into segments, to walk down the resource tree and build the realURI of any symlink found + // We rebuild the realURI, segment by segment, getting the real name at each step, so that we can distinguish between + // alias types. Specifically, so we can allow a symbolic link so long as it's realpath is not protected. + String[] segments = pathInContext.substring(1).split("/"); + Path fromBase = _base; + StringBuilder realURI = new StringBuilder(); + + try + { + for (String segment : segments) + { + // Add the segment to the path and realURI. + fromBase = fromBase.resolve(segment); + realURI.append("/").append(fromBase.toRealPath(NO_FOLLOW_LINKS).getFileName()); + + // If the ancestor of the alias is a symlink, then check if the real URI is protected, otherwise allow. + // This allows symlinks like /other->/WEB-INF and /external->/var/lib/docroot + // This does not allow symlinks like /WeB-InF->/var/lib/other + if (Files.isSymbolicLink(fromBase)) + return !getContextHandler().isProtectedTarget(realURI.toString()); + + // If the ancestor is not allowed then do not allow. + if (!isAllowed(fromBase)) + return false; + + // TODO as we are building the realURI of the resource, it would be possible to + // re-check that against security constraints. + } + } + catch (Throwable t) + { + if (LOG.isDebugEnabled()) + LOG.debug("Failed to check alias", t); + return false; + } + + // No symlink found, so must be allowed. Double check it is the right path we checked. + return isSameFile(fromBase, path); + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java index 9133532440f7..9453123bbce7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java @@ -33,7 +33,9 @@ * to check resources that are aliased to other locations. The checker uses the * Java {@link Files#readSymbolicLink(Path)} and {@link Path#toRealPath(java.nio.file.LinkOption...)} * APIs to check if a file is aliased with symbolic links.

+ * @deprecated use {@link org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker} instead. */ +@Deprecated public class AllowSymLinkAliasChecker implements AliasCheck { private static final Logger LOG = Log.getLogger(AllowSymLinkAliasChecker.class); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 45cef634bccc..282e9747ecac 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -68,6 +68,7 @@ import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.server.AllowedResourceAliasChecker; import org.eclipse.jetty.server.ClassLoaderDump; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Dispatcher; @@ -75,6 +76,7 @@ import org.eclipse.jetty.server.HandlerContainer; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.AttributesMap; import org.eclipse.jetty.util.FutureCallback; @@ -86,6 +88,7 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.component.Graceful; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -108,8 +111,8 @@ * The executor is made available via a context attributed {@code org.eclipse.jetty.server.Executor}. *

*

- * By default, the context is created with alias checkers for {@link AllowSymLinkAliasChecker} (unix only) and {@link ApproveNonExistentDirectoryAliases}. If - * these alias checkers are not required, then {@link #clearAliasChecks()} or {@link #setAliasChecks(List)} should be called. + * By default, the context is created with the {@link AllowedResourceAliasChecker} which is configured to allow symlinks. If + * this alias checker is not required, then {@link #clearAliasChecks()} or {@link #setAliasChecks(List)} should be called. *

*/ @ManagedObject("URI Context") @@ -264,9 +267,8 @@ private ContextHandler(Context context, HandlerContainer parent, String contextP _scontext = context == null ? new Context() : context; _attributes = new AttributesMap(); _initParams = new HashMap<>(); - addAliasCheck(new ApproveNonExistentDirectoryAliases()); if (File.separatorChar == '/') - addAliasCheck(new AllowSymLinkAliasChecker()); + addAliasCheck(new SymlinkAllowedResourceAliasChecker(this)); if (contextPath != null) setContextPath(contextPath); @@ -2083,6 +2085,10 @@ private String normalizeHostname(String host) public void addAliasCheck(AliasCheck check) { getAliasChecks().add(check); + if (check instanceof LifeCycle) + addManaged((LifeCycle)check); + else + addBean(check); } /** @@ -2098,7 +2104,7 @@ public List getAliasChecks() */ public void setAliasChecks(List checks) { - getAliasChecks().clear(); + clearAliasChecks(); getAliasChecks().addAll(checks); } @@ -2107,7 +2113,9 @@ public void setAliasChecks(List checks) */ public void clearAliasChecks() { - getAliasChecks().clear(); + List aliasChecks = getAliasChecks(); + aliasChecks.forEach(this::removeBean); + aliasChecks.clear(); } /** @@ -2971,6 +2979,7 @@ public interface AliasCheck /** * Approve all aliases. */ + @Deprecated public static class ApproveAliases implements AliasCheck { @Override @@ -2983,6 +2992,7 @@ public boolean check(String path, Resource resource) /** * Approve Aliases of a non existent directory. If a directory "/foobar/" does not exist, then the resource is aliased to "/foobar". Accept such aliases. */ + @Deprecated public static class ApproveNonExistentDirectoryAliases implements AliasCheck { @Override diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasCheckerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasCheckerTest.java index 91c201b36735..b504cc8c9938 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasCheckerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasCheckerTest.java @@ -30,6 +30,7 @@ import java.util.stream.Stream; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.server.AllowedResourceAliasChecker; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.toolchain.test.FS; @@ -184,7 +185,7 @@ private void setupServer() throws Exception fileResourceContext.setBaseResource(new PathResource(rootPath)); fileResourceContext.clearAliasChecks(); - fileResourceContext.addAliasCheck(new AllowSymLinkAliasChecker()); + fileResourceContext.addAliasCheck(new AllowedResourceAliasChecker(fileResourceContext)); server.setHandler(fileResourceContext); server.start(); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java index a82c14a07e8d..7070ae3dcd53 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerGetResourceTest.java @@ -25,6 +25,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.resource.Resource; @@ -113,7 +114,7 @@ public static void beforeClass() throws Exception context.setBaseResource(Resource.newResource(docroot)); context.addAliasCheck(new ContextHandler.AliasCheck() { - final AllowSymLinkAliasChecker symlinkcheck = new AllowSymLinkAliasChecker(); + final SymlinkAllowedResourceAliasChecker symlinkcheck = new SymlinkAllowedResourceAliasChecker(context); @Override public boolean check(String path, Resource resource) diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java index 45d5ae95c167..60f73cc8e0f5 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java @@ -52,13 +52,13 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.server.AllowedResourceAliasChecker; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.ResourceContentFactory; import org.eclipse.jetty.server.ResourceService; import org.eclipse.jetty.server.SameFileAliasChecker; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; @@ -1143,7 +1143,7 @@ public void testSymLinks() throws Exception response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.NOT_FOUND_404)); - context.addAliasCheck(new AllowSymLinkAliasChecker()); + context.addAliasCheck(new AllowedResourceAliasChecker(context)); rawResponse = connector.getResponse("GET /context/dir/link.txt HTTP/1.0\r\n\r\n"); response = HttpTester.parseResponse(rawResponse); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index 8bf6fe0aad1f..97394422ff6e 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -92,7 +92,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL public static final String SERVER_SYS_CLASSES = "org.eclipse.jetty.webapp.systemClasses"; public static final String SERVER_SRV_CLASSES = "org.eclipse.jetty.webapp.serverClasses"; - private static final String[] DEFAULT_PROTECTED_TARGETS = {"/web-inf", "/meta-inf"}; + private static final String[] DEFAULT_PROTECTED_TARGETS = {"/WEB-INF", "/META-INF"}; public static final String[] DEFAULT_CONFIGURATION_CLASSES = { 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 new file mode 100644 index 000000000000..daee7c2db9e1 --- /dev/null +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java @@ -0,0 +1,227 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.test; + +import java.io.File; +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.SymlinkAllowedResourceAliasChecker; +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.AfterAll; +import org.junit.jupiter.api.BeforeAll; +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 Server _server; + private static ServerConnector _connector; + private static HttpClient _client; + private static ServletContextHandler _context; + + private static Path _symlinkFile; + private static Path _symlinkExternalFile; + private static Path _symlinkDir; + private static Path _symlinkParentDir; + private static Path _symlinkSiblingDir; + private static Path _webInfSymlink; + private static Path _webrootSymlink; + + private static Path getResource(String path) throws Exception + { + URL url = AliasCheckerSymlinkTest.class.getClassLoader().getResource(path); + assertNotNull(url); + return new File(url.toURI()).toPath(); + } + + 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 beforeAll() throws Exception + { + Path webRootPath = getResource("webroot"); + Path fileInWebroot = webRootPath.resolve("file"); + + // Create symlink file that targets inside the webroot directory. + _symlinkFile = webRootPath.resolve("symlinkFile"); + delete(_symlinkFile); + Files.createSymbolicLink(_symlinkFile, fileInWebroot).toFile().deleteOnExit(); + + // Create symlink file that targets outside the webroot directory. + _symlinkExternalFile = webRootPath.resolve("symlinkExternalFile"); + delete(_symlinkExternalFile); + Files.createSymbolicLink(_symlinkExternalFile, getResource("file")).toFile().deleteOnExit(); + + // Symlink to a directory inside of the webroot. + _symlinkDir = webRootPath.resolve("symlinkDir"); + delete(_symlinkDir); + Files.createSymbolicLink(_symlinkDir, webRootPath.resolve("documents")).toFile().deleteOnExit(); + + // Symlink to a directory parent of the webroot. + _symlinkParentDir = webRootPath.resolve("symlinkParentDir"); + delete(_symlinkParentDir); + Files.createSymbolicLink(_symlinkParentDir, webRootPath.resolve("..")).toFile().deleteOnExit(); + + // Symlink to a directory outside of the webroot. + _symlinkSiblingDir = webRootPath.resolve("symlinkSiblingDir"); + delete(_symlinkSiblingDir); + Files.createSymbolicLink(_symlinkSiblingDir, webRootPath.resolve("../sibling")).toFile().deleteOnExit(); + + // Symlink to the WEB-INF directory. + _webInfSymlink = webRootPath.resolve("webInfSymlink"); + delete(_webInfSymlink); + Files.createSymbolicLink(_webInfSymlink, webRootPath.resolve("WEB-INF")).toFile().deleteOnExit(); + + // External symlink to webroot. + _webrootSymlink = webRootPath.resolve("../webrootSymlink"); + delete(_webrootSymlink); + Files.createSymbolicLink(_webrootSymlink, webRootPath).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, "/"); + _context.clearAliasChecks(); + _server.start(); + + _client = new HttpClient(); + _client.start(); + } + + @AfterAll + public static void afterAll() throws Exception + { + // Try to delete all files now so that the symlinks do not confuse other tests. + Files.delete(_symlinkFile); + Files.delete(_symlinkExternalFile); + Files.delete(_symlinkDir); + Files.delete(_symlinkParentDir); + Files.delete(_symlinkSiblingDir); + Files.delete(_webInfSymlink); + Files.delete(_webrootSymlink); + + _client.stop(); + _server.stop(); + } + + public static Stream testCases() + { + AllowedResourceAliasChecker allowedResource = new AllowedResourceAliasChecker(_context); + SymlinkAllowedResourceAliasChecker symlinkAllowedResource = new SymlinkAllowedResourceAliasChecker(_context); + AllowSymLinkAliasChecker allowSymlinks = new AllowSymLinkAliasChecker(); + ContextHandler.ApproveAliases approveAliases = new ContextHandler.ApproveAliases(); + + return Stream.of( + // AllowedResourceAliasChecker that checks the target of symlinks. + Arguments.of(allowedResource, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResource, "/symlinkExternalFile", HttpStatus.NOT_FOUND_404, null), + 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.NOT_FOUND_404, null), + Arguments.of(allowedResource, "/symlinkSiblingDir/file", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResource, "/webInfSymlink/web.xml", HttpStatus.NOT_FOUND_404, null), + + // SymlinkAllowedResourceAliasChecker that does not check the target of symlinks, but only approves files obtained through a symlink. + Arguments.of(symlinkAllowedResource, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(symlinkAllowedResource, "/symlinkExternalFile", HttpStatus.OK_200, "This file is outside webroot."), + Arguments.of(symlinkAllowedResource, "/symlinkDir/file", HttpStatus.OK_200, "This file is inside webroot/documents."), + Arguments.of(symlinkAllowedResource, "/symlinkParentDir/webroot/file", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(symlinkAllowedResource, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + Arguments.of(symlinkAllowedResource, "/symlinkSiblingDir/file", HttpStatus.OK_200, "This file is inside a sibling dir to webroot."), + Arguments.of(symlinkAllowedResource, "/webInfSymlink/web.xml", HttpStatus.OK_200, "This is the web.xml file."), + + // 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) + ); + } + + @ParameterizedTest + @MethodSource("testCases") + public void test(ContextHandler.AliasCheck aliasChecker, String path, int httpStatus, String responseContent) throws Exception + { + 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 new file mode 100644 index 000000000000..f57b0b89d7ae --- /dev/null +++ b/tests/test-integration/src/test/resources/sibling/file @@ -0,0 +1 @@ +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 new file mode 100644 index 000000000000..47d791449100 --- /dev/null +++ b/tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml @@ -0,0 +1 @@ +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 new file mode 100644 index 000000000000..b45088053628 --- /dev/null +++ b/tests/test-integration/src/test/resources/webroot/documents/file @@ -0,0 +1 @@ +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 new file mode 100644 index 000000000000..7a16a16aa081 --- /dev/null +++ b/tests/test-integration/src/test/resources/webroot/file @@ -0,0 +1 @@ +This file is inside webroot. \ No newline at end of file diff --git a/tests/test-integration/src/test/resources/webroot/index.html b/tests/test-integration/src/test/resources/webroot/index.html new file mode 100644 index 000000000000..f7dc59cdc6b6 --- /dev/null +++ b/tests/test-integration/src/test/resources/webroot/index.html @@ -0,0 +1,4 @@ + +

hello world

+

body of index.html

+ \ No newline at end of file