From c1106482ed10db5fbaca84e41fded52cc2788ff6 Mon Sep 17 00:00:00 2001
From: Lachlan Roberts
Date: Thu, 26 Aug 2021 16:01:35 +1000
Subject: [PATCH] Issue #6497 - Replace the Alias checkers with new
implementation.
Signed-off-by: Lachlan Roberts
---
.../java/org/eclipse/jetty/io/IOTest.java | 57 +++++
.../server/AllowedResourceAliasChecker.java | 228 +++++++++++++++++
.../jetty/server/SameFileAliasChecker.java | 2 +
.../SymlinkAllowedResourceAliasChecker.java | 66 +++++
.../handler/AllowSymLinkAliasChecker.java | 7 +
.../jetty/server/handler/ContextHandler.java | 23 +-
.../handler/AllowSymLinkAliasCheckerTest.java | 3 +-
.../ContextHandlerGetResourceTest.java | 7 +-
.../jetty/servlet/DefaultServletTest.java | 11 +-
.../jetty/test/AliasCheckerSymlinkTest.java | 232 ++++++++++++++++++
.../test-integration/src/test/resources/file | 1 +
.../src/test/resources/sibling/file | 1 +
.../test/resources/webroot/WEB-INF/web.xml | 1 +
.../src/test/resources/webroot/documents/file | 1 +
.../src/test/resources/webroot/file | 1 +
.../src/test/resources/webroot/index.html | 4 +
16 files changed, 630 insertions(+), 15 deletions(-)
create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java
create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java
create mode 100644 tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java
create mode 100644 tests/test-integration/src/test/resources/file
create mode 100644 tests/test-integration/src/test/resources/sibling/file
create mode 100644 tests/test-integration/src/test/resources/webroot/WEB-INF/web.xml
create mode 100644 tests/test-integration/src/test/resources/webroot/documents/file
create mode 100644 tests/test-integration/src/test/resources/webroot/file
create mode 100644 tests/test-integration/src/test/resources/webroot/index.html
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 972e491c6a57..2139fa1c7ad4 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
@@ -43,6 +43,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;
@@ -545,4 +547,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..07ddd95dac90
--- /dev/null
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java
@@ -0,0 +1,228 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+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.resource.PathResource;
+import org.eclipse.jetty.util.resource.Resource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * 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 = LoggerFactory.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.
+ */
+ public AllowedResourceAliasChecker(ContextHandler contextHandler)
+ {
+ this(contextHandler, false);
+ }
+
+ /**
+ * @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("Failed to check alias", 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.trace("getPath() failed", 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 1a50e5a2ffdd..406b6a4148ee 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
@@ -39,7 +39,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 = LoggerFactory.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..ae7560728cbf
--- /dev/null
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java
@@ -0,0 +1,66 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+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.resource.Resource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An extension of {@link AllowedResourceAliasChecker} which only allows aliased resources if they are symlinks.
+ */
+public class SymlinkAllowedResourceAliasChecker extends AllowedResourceAliasChecker
+{
+ private static final Logger LOG = LoggerFactory.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.trace("Failed to check alias", 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 4b65536b31ea..bf807bd14b73 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
@@ -28,11 +28,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 = LoggerFactory.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 414ff29d510a..5fe5f65f7c86 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
@@ -63,6 +63,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;
@@ -70,6 +71,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.Index;
@@ -81,6 +83,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.resource.Resource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -103,8 +106,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")
@@ -263,9 +266,8 @@ protected ContextHandler(Context context, HandlerContainer parent, String contex
_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);
@@ -2071,6 +2073,10 @@ private String normalizeHostname(String host)
public void addAliasCheck(AliasCheck check)
{
getAliasChecks().add(check);
+ if (check instanceof LifeCycle)
+ addManaged((LifeCycle)check);
+ else
+ addBean(check);
}
/**
@@ -2086,7 +2092,7 @@ public List getAliasChecks()
*/
public void setAliasChecks(List checks)
{
- getAliasChecks().clear();
+ clearAliasChecks();
getAliasChecks().addAll(checks);
}
@@ -2095,7 +2101,9 @@ public void setAliasChecks(List checks)
*/
public void clearAliasChecks()
{
- getAliasChecks().clear();
+ List aliasChecks = getAliasChecks();
+ aliasChecks.forEach(this::removeBean);
+ aliasChecks.clear();
}
/**
@@ -3028,7 +3036,9 @@ public interface AliasCheck
/**
* Approve all aliases.
+ * @deprecated use {@link org.eclipse.jetty.server.AllowedResourceAliasChecker} instead.
*/
+ @Deprecated
public static class ApproveAliases implements AliasCheck
{
@Override
@@ -3041,6 +3051,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 94aee97a9ffc..70ba419bc4cb 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
@@ -27,6 +27,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;
@@ -179,7 +180,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 d304c0bb2160..4d055bf4609d 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
@@ -20,6 +20,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;
@@ -104,11 +105,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 66957e4d79ef..61a2e34c62da 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
@@ -47,13 +47,13 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.logging.StacklessLogging;
+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;
@@ -1081,8 +1081,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));
@@ -2056,7 +2055,7 @@ public void testGetUtf8NfcFile() throws Exception
FS.ensureEmpty(docRoot);
context.addServlet(DefaultServlet.class, "/");
- context.addAliasCheck(new SameFileAliasChecker());
+ context.addAliasCheck(new AllowedResourceAliasChecker(context));
// Create file with UTF-8 NFC format
String filename = "swedish-" + new String(TypeUtil.fromHexString("C3A5"), UTF_8) + ".txt";
@@ -2096,7 +2095,7 @@ public void testGetUtf8NfdFile() throws Exception
FS.ensureEmpty(docRoot);
context.addServlet(DefaultServlet.class, "/");
- context.addAliasCheck(new SameFileAliasChecker());
+ context.addAliasCheck(new AllowedResourceAliasChecker(context));
// 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..64f7ab5cfc2f
--- /dev/null
+++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java
@@ -0,0 +1,232 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+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