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..0d9908cc3101 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 @@ -20,7 +20,7 @@ import org.eclipse.jetty.security.HashLoginService; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker; +import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; 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 SymlinkAllowedResourceAliasChecker(webapp)); GCloudSessionDataStore ds = new GCloudSessionDataStore(); DefaultSessionCache ss = new DefaultSessionCache(webapp.getSessionHandler()); diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java index 2e96a2209412..909ed0d09995 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java @@ -48,6 +48,8 @@ import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.resource.PathResource; +import org.eclipse.jetty.util.resource.Resource; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; @@ -550,4 +552,59 @@ public void testSelectorWakeup() throws Exception } } } + + @Test + public void testSymbolicLink(TestInfo testInfo) throws Exception + { + File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName()); + FS.ensureEmpty(dir); + File realFile = new File(dir, "real"); + Path realPath = realFile.toPath(); + FS.touch(realFile); + + File linkFile = new File(dir, "link"); + Path linkPath = linkFile.toPath(); + Files.createSymbolicLink(linkPath, realPath); + Path targPath = linkPath.toRealPath(); + + System.err.printf("realPath = %s%n", realPath); + System.err.printf("linkPath = %s%n", linkPath); + System.err.printf("targPath = %s%n", targPath); + + assertFalse(Files.isSymbolicLink(realPath)); + assertTrue(Files.isSymbolicLink(linkPath)); + + Resource link = new PathResource(dir).addPath("link"); + assertThat(link.isAlias(), is(true)); + } + + @Test + public void testSymbolicLinkDir(TestInfo testInfo) throws Exception + { + File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName()); + FS.ensureEmpty(dir); + + File realDirFile = new File(dir, "real"); + Path realDirPath = realDirFile.toPath(); + Files.createDirectories(realDirPath); + + File linkDirFile = new File(dir, "link"); + Path linkDirPath = linkDirFile.toPath(); + Files.createSymbolicLink(linkDirPath, realDirPath); + + File realFile = new File(realDirFile, "file"); + Path realPath = realFile.toPath(); + FS.touch(realFile); + + File linkFile = new File(linkDirFile, "file"); + Path linkPath = linkFile.toPath(); + Path targPath = linkPath.toRealPath(); + + System.err.printf("realPath = %s%n", realPath); + System.err.printf("linkPath = %s%n", linkPath); + System.err.printf("targPath = %s%n", targPath); + + assertFalse(Files.isSymbolicLink(realPath)); + assertFalse(Files.isSymbolicLink(linkPath)); + } } 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..20af655ad8f4 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java @@ -0,0 +1,225 @@ +// +// ======================================================================== +// 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.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.HashSet; + +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.StringUtil; +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 {@link ContextHandler#isProtectedTarget(String)}.

+ *

This will approve symlinks to outside of the resource base. This can be optionally configured to check that the + * target of the symlinks is also inside of the resource base and is not a protected target.

+ *

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); + private static final LinkOption[] FOLLOW_LINKS = new LinkOption[0]; + private static final LinkOption[] NO_FOLLOW_LINKS = new LinkOption[]{LinkOption.NOFOLLOW_LINKS}; + + private final ContextHandler _contextHandler; + private final boolean _checkSymlinkTargets; + private final HashSet _protectedPaths = new HashSet<>(); + private Path _basePath; + + /** + * @param contextHandler the context handler to use. + * @param checkSymlinkTargets true to check that the target of the symlink is an allowed resource. + */ + public AllowedResourceAliasChecker(ContextHandler contextHandler, boolean checkSymlinkTargets) + { + _contextHandler = contextHandler; + _checkSymlinkTargets = checkSymlinkTargets; + } + + protected ContextHandler getContextHandler() + { + return _contextHandler; + } + + protected Path getBasePath() + { + return _basePath; + } + + @Override + protected void doStart() throws Exception + { + _basePath = getPath(_contextHandler.getBaseResource()); + if (_basePath == null) + _basePath = Paths.get("/").toAbsolutePath(); + + String[] protectedTargets = _contextHandler.getProtectedTargets(); + if (protectedTargets != null) + { + for (String s : protectedTargets) + { + Path path = _basePath.getFileSystem().getPath(_basePath.toString(), s); + _protectedPaths.add(path); + } + } + } + + @Override + protected void doStop() throws Exception + { + _basePath = null; + _protectedPaths.clear(); + } + + @Override + public boolean check(String uri, Resource resource) + { + // The existence check resolves the symlinks. + if (!resource.exists()) + return false; + + Path resourcePath = getPath(resource); + if (resourcePath == null) + return false; + + try + { + if (isProtectedPath(resourcePath, NO_FOLLOW_LINKS)) + return false; + + if (_checkSymlinkTargets && hasSymbolicLink(resourcePath)) + { + if (isProtectedPath(resourcePath, FOLLOW_LINKS)) + return false; + } + } + catch (Throwable t) + { + LOG.warn(t); + return false; + } + + return true; + } + + /** + *

Determines whether the provided resource path is protected.

+ * + *

The resource path is protected if it is under one of the protected targets defined by + * {@link ContextHandler#isProtectedTarget(String)} in which case the alias should not be allowed. + * The resource path may also attempt to traverse above the root path and should be denied.

+ * + * @param resourcePath the resource {@link Path} to be tested. + * @param linkOptions an array of {@link LinkOption} to be provided to the {@link Path#toRealPath(LinkOption...)} method. + * @return true if the resource path is protected and the alias should not be allowed. + * @throws IOException if an I/O error occurs. + */ + protected boolean isProtectedPath(Path resourcePath, LinkOption[] linkOptions) throws IOException + { + // If the resource doesn't exist we cannot determine whether it is protected so we assume it is. + if (!Files.exists(resourcePath, linkOptions)) + return true; + + Path basePath = _basePath.toRealPath(linkOptions); + Path targetPath = resourcePath.toRealPath(linkOptions); + String target = targetPath.toString(); + + // The target path must be under the base resource directory. + if (!targetPath.startsWith(basePath)) + return true; + + for (Path protectedPath : _protectedPaths) + { + String protect; + if (Files.exists(protectedPath, linkOptions)) + protect = protectedPath.toRealPath(linkOptions).toString(); + else if (linkOptions == NO_FOLLOW_LINKS) + protect = protectedPath.normalize().toAbsolutePath().toString(); + else + protect = protectedPath.toFile().getCanonicalPath(); + + // If the target path is protected then we will not allow it. + if (StringUtil.startsWithIgnoreCase(target, protect)) + { + if (target.length() == protect.length()) + return true; + + // Check that the target prefix really is a path segment. + if (target.charAt(protect.length()) == File.separatorChar) + return true; + } + } + + return false; + } + + protected boolean hasSymbolicLink(Path path) + { + return hasSymbolicLink(path.getRoot(), path); + } + + protected boolean hasSymbolicLink(Path base, Path path) + { + Path p = path; + while (!base.equals(p)) + { + if (p == null) + throw new IllegalArgumentException("path was not child of base"); + + if (Files.isSymbolicLink(p)) + return true; + + p = p.getParent(); + } + + return false; + } + + private 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{checkSymlinkTargets=%s}", AllowedResourceAliasChecker.class.getSimpleName(), hashCode(), _checkSymlinkTargets); + } +} \ No newline at end of file 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..554e4ba328a9 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java @@ -0,0 +1,71 @@ +// +// ======================================================================== +// 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.Path; + +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.resource.Resource; + +/** + * An extension of {@link AllowedResourceAliasChecker} which only allows aliased resources if they are symlinks. + */ +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, false); + } + + @Override + public boolean check(String uri, Resource resource) + { + try + { + // Check the resource is allowed to be accessed. + if (!super.check(uri, resource)) + return false; + + // Approve if path is a symbolic link. + Path resourcePath = resource.getFile().toPath(); + if (Files.isSymbolicLink(resourcePath)) + return true; + + // Approve if path has symlink in under its resource base. + if (super.hasSymbolicLink(getBasePath(), resourcePath)) + return true; + } + catch (IOException e) + { + LOG.ignore(e); + return false; + } + + return false; + } +} 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..f4f77cd66e0a 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,11 +33,18 @@ * 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); + public AllowSymLinkAliasChecker() + { + LOG.warn("Deprecated, use SymlinkAllowedResourceAliasChecker instead."); + } + @Override public boolean check(String uri, Resource resource) { 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 97ef7e7442cd..2bfb251c5a62 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(); } /** @@ -2970,7 +2978,9 @@ public interface AliasCheck /** * Approve all aliases. + * @deprecated use {@link org.eclipse.jetty.server.AllowedResourceAliasChecker} instead. */ + @Deprecated public static class ApproveAliases implements AliasCheck { @Override @@ -2983,6 +2993,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..8dc13acb416c 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 @@ -32,6 +32,7 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.LocalConnector; 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.BufferUtil; @@ -184,7 +185,7 @@ private void setupServer() throws Exception fileResourceContext.setBaseResource(new PathResource(rootPath)); fileResourceContext.clearAliasChecks(); - fileResourceContext.addAliasCheck(new AllowSymLinkAliasChecker()); + fileResourceContext.addAliasCheck(new SymlinkAllowedResourceAliasChecker(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..2b1f1bf167d6 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; @@ -109,11 +110,13 @@ public static void beforeClass() throws Exception server = new Server(); context = new ContextHandler("/"); context.clearAliasChecks(); - context.addAliasCheck(new ContextHandler.ApproveNonExistentDirectoryAliases()); context.setBaseResource(Resource.newResource(docroot)); context.addAliasCheck(new ContextHandler.AliasCheck() { - final AllowSymLinkAliasChecker symlinkcheck = new AllowSymLinkAliasChecker(); + final SymlinkAllowedResourceAliasChecker symlinkcheck = new SymlinkAllowedResourceAliasChecker(context); + { + context.addBean(symlinkcheck); + } @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 85ac1d39fe91..b0a67320a1c0 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.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; @@ -1097,8 +1097,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 SymlinkAllowedResourceAliasChecker(context)); rawResponse = connector.getResponse("GET /context/dir/link.txt HTTP/1.0\r\n\r\n"); response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); @@ -2070,7 +2069,7 @@ public void testGetUtf8NfcFile() throws Exception FS.ensureEmpty(docRoot); context.addServlet(DefaultServlet.class, "/"); - context.addAliasCheck(new SameFileAliasChecker()); + context.addAliasCheck(new AllowedResourceAliasChecker(context, true)); // Create file with UTF-8 NFC format String filename = "swedish-" + new String(TypeUtil.fromHexString("C3A5"), UTF_8) + ".txt"; @@ -2110,7 +2109,7 @@ public void testGetUtf8NfdFile() throws Exception FS.ensureEmpty(docRoot); context.addServlet(DefaultServlet.class, "/"); - context.addAliasCheck(new SameFileAliasChecker()); + context.addAliasCheck(new AllowedResourceAliasChecker(context, true)); // Create file with UTF-8 NFD format String filename = "swedish-a" + new String(TypeUtil.fromHexString("CC8A"), UTF_8) + ".txt"; 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..5749fe86678c --- /dev/null +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java @@ -0,0 +1,237 @@ +// +// ======================================================================== +// 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, false); + AllowedResourceAliasChecker allowedResourceTarget = new AllowedResourceAliasChecker(_context, true); + SymlinkAllowedResourceAliasChecker symlinkAllowedResource = new SymlinkAllowedResourceAliasChecker(_context); + 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(allowedResourceTarget, "/symlinkFile", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResourceTarget, "/symlinkExternalFile", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResourceTarget, "/symlinkDir/file", HttpStatus.OK_200, "This file is inside webroot/documents."), + Arguments.of(allowedResourceTarget, "/symlinkParentDir/webroot/file", HttpStatus.OK_200, "This file is inside webroot."), + Arguments.of(allowedResourceTarget, "/symlinkParentDir/webroot/WEB-INF/web.xml", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResourceTarget, "/symlinkSiblingDir/file", HttpStatus.NOT_FOUND_404, null), + Arguments.of(allowedResourceTarget, "/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