From 941527e27f75c0bf67620b67998691084daa3075 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 11 Aug 2022 11:52:27 +1000 Subject: [PATCH 1/7] make alias checking in ee10 use core alias checkers Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/server/AliasCheck.java | 18 ++ .../server/AllowedResourceAliasChecker.java | 9 +- .../eclipse/jetty/server/ResourceService.java | 18 +- .../handler/AllowSymLinkAliasChecker.java | 4 +- .../jetty/server/handler/ContextHandler.java | 158 ++++++++++-- .../jetty/server/handler/ContextRequest.java | 2 +- .../jetty/server/handler/ResourceHandler.java | 2 +- .../jetty/ee10/servlet/DefaultServlet.java | 16 +- .../ee10/servlet/ServletContextHandler.java | 125 ---------- .../AliasCheckerWebRootIsSymlinkTest.java | 159 ------------ tests/pom.xml | 2 +- tests/test-integration/pom.xml | 40 +++ ...AliasCheckerMultipleResourceBasesTest.java | 84 ++++--- .../jetty/test/AliasCheckerSymlinkTest.java | 230 ++++++++++++++++++ .../src/test/resources/altDir1/file1 | 1 + .../src/test/resources/altDir2/file2 | 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 + 20 files changed, 509 insertions(+), 367 deletions(-) create mode 100644 jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java delete mode 100644 jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerWebRootIsSymlinkTest.java create mode 100644 tests/test-integration/pom.xml rename {jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration => tests/test-integration}/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java (71%) create mode 100644 tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java create mode 100644 tests/test-integration/src/test/resources/altDir1/file1 create mode 100644 tests/test-integration/src/test/resources/altDir2/file2 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-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java new file mode 100644 index 00000000000..f3b36b60629 --- /dev/null +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java @@ -0,0 +1,18 @@ +package org.eclipse.jetty.server; + +import org.eclipse.jetty.util.resource.Resource; + +/** + * Interface to check aliases. + */ +public interface AliasCheck +{ + /** + * Check an alias + * + * @param pathInContext The path the aliased resource was created for + * @param resource The aliased resourced + * @return True if the resource is OK to be served. + */ + boolean checkAlias(String pathInContext, Resource resource); +} diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java index 402f03988ce..2ed617fc7de 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java @@ -26,7 +26,6 @@ import java.util.function.Supplier; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.LifeCycle; -import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.Resource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,7 +36,7 @@ import org.slf4j.LoggerFactory; *

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 +public class AllowedResourceAliasChecker extends AbstractLifeCycle implements AliasCheck { private static final Logger LOG = LoggerFactory.getLogger(AllowedResourceAliasChecker.class); protected static final LinkOption[] FOLLOW_LINKS = new LinkOption[0]; @@ -76,9 +75,7 @@ public class AllowedResourceAliasChecker extends AbstractLifeCycle implements Co private String[] getProtectedTargets() { - // TODO: Add protected targets to ContextHandler. - // return _contextHandler.getProtectedTargets(); - return null; + return _contextHandler.getProtectedTargets(); } private void extractBaseResourceFromContext() @@ -131,7 +128,7 @@ public class AllowedResourceAliasChecker extends AbstractLifeCycle implements Co } @Override - public boolean check(String pathInContext, Resource resource) + public boolean checkAlias(String pathInContext, Resource resource) { if (!_initialized) extractBaseResourceFromContext(); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 77d8f80396a..68d3306c454 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.http.QuotedQualityCSV; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; @@ -76,9 +77,22 @@ public class ResourceService { } - public HttpContent getContent(String path) throws IOException + public HttpContent getContent(String path, Request request) throws IOException { - return _contentFactory.getContent(path == null ? "" : path); + ContextHandler contextHandler = ContextHandler.getContextHandler(request); + return getContent(path, contextHandler); + } + + public HttpContent getContent(String path, AliasCheck aliasCheck) throws IOException + { + HttpContent content = _contentFactory.getContent(path == null ? "" : path); + if (content != null && aliasCheck != null) + { + if (!aliasCheck.checkAlias(path, content.getResource())) + return null; + } + + return content; } public HttpContent.ContentFactory getContentFactory() diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java index 15b816d6211..ad6017b5d3f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java @@ -16,7 +16,7 @@ package org.eclipse.jetty.server.handler; import java.nio.file.Files; import java.nio.file.Path; -import org.eclipse.jetty.server.handler.ContextHandler.AliasCheck; +import org.eclipse.jetty.server.AliasCheck; import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.Resource; import org.slf4j.Logger; @@ -41,7 +41,7 @@ public class AllowSymLinkAliasChecker implements AliasCheck } @Override - public boolean check(String pathInContext, Resource resource) + public boolean checkAlias(String pathInContext, Resource resource) { // Only support PathResource alias checking if (!(resource instanceof PathResource pathResource)) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 3e37a6cc468..8a3897bb11c 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -20,6 +20,7 @@ import java.net.URLClassLoader; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EventListener; import java.util.List; import java.util.Set; @@ -33,6 +34,7 @@ import java.util.stream.Collectors; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.server.AliasCheck; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; @@ -41,6 +43,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.DecoratedObjectFactory; +import org.eclipse.jetty.util.Index; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.URIUtil; @@ -48,13 +51,14 @@ import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.component.ClassLoaderDump; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.Graceful; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; import org.eclipse.jetty.util.thread.Invocable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ContextHandler extends Handler.Wrapper implements Attributes, Graceful +public class ContextHandler extends Handler.Wrapper implements Attributes, Graceful, AliasCheck { // TODO where should the alias checking go? // TODO add protected paths to ServletContextHandler? @@ -92,6 +96,8 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace private ClassLoader _classLoader; private Request.Processor _errorProcessor; private boolean _allowNullPathInContext; + private Index _protectedTargets = Index.empty(false); + private final List _aliasChecks = new CopyOnWriteArrayList<>(); public enum Availability { @@ -102,6 +108,22 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace SHUTDOWN, // graceful shutdown } + /** + * The type of protected target match + * @see #_protectedTargets + */ + private enum ProtectedTargetType + { + EXACT, + PREFIX + } + + public static ContextHandler getContextHandler(Request request) + { + ContextRequest contextRequest = Request.as(request, ContextRequest.class); + return (contextRequest == null) ? null : contextRequest.getContext().getContextHandler(); + } + private final AtomicReference _availability = new AtomicReference<>(Availability.STOPPED); public ContextHandler() @@ -729,6 +751,124 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace _persistentAttributes.clearAttributes(); } + /** + * Check the target when a target within a context is determined. If + * the target is protected, 404 is returned. + * + * @param target the target to test + * @return true if target is a protected target + */ + public boolean isProtectedTarget(String target) + { + if (target == null || _protectedTargets.isEmpty()) + return false; + + if (target.startsWith("//")) + target = URIUtil.compactPath(target); + + ProtectedTargetType type = _protectedTargets.getBest(target); + + return type == ProtectedTargetType.PREFIX || + type == ProtectedTargetType.EXACT && _protectedTargets.get(target) == ProtectedTargetType.EXACT; + } + + /** + * @param targets Array of URL prefix. Each prefix is in the form /path and will match either /path exactly or /path/anything + */ + public void setProtectedTargets(String[] targets) + { + Index.Builder builder = new Index.Builder<>(); + if (targets != null) + { + for (String t : targets) + { + if (!t.startsWith("/")) + throw new IllegalArgumentException("Bad protected target: " + t); + + builder.with(t, ProtectedTargetType.EXACT); + builder.with(t + "/", ProtectedTargetType.PREFIX); + builder.with(t + "?", ProtectedTargetType.PREFIX); + builder.with(t + "#", ProtectedTargetType.PREFIX); + builder.with(t + ";", ProtectedTargetType.PREFIX); + } + } + _protectedTargets = builder.caseSensitive(false).build(); + } + + public String[] getProtectedTargets() + { + if (_protectedTargets == null) + return null; + + return _protectedTargets.keySet().stream() + .filter(s -> _protectedTargets.get(s) == ProtectedTargetType.EXACT) + .toArray(String[]::new); + } + + /** + * Add an AliasCheck instance to possibly permit aliased resources + * + * @param check The alias checker + */ + public void addAliasCheck(AliasCheck check) + { + _aliasChecks.add(check); + if (check instanceof LifeCycle) + addManaged((LifeCycle)check); + else + addBean(check); + } + + /** + * @return Immutable list of Alias checks + */ + public List getAliasChecks() + { + return Collections.unmodifiableList(_aliasChecks); + } + + /** + * @param checks list of AliasCheck instances + */ + public void setAliasChecks(List checks) + { + clearAliasChecks(); + checks.forEach(this::addAliasCheck); + } + + /** + * clear the list of AliasChecks + */ + public void clearAliasChecks() + { + _aliasChecks.forEach(this::removeBean); + _aliasChecks.clear(); + } + + @Override + public boolean checkAlias(String pathInContext, Resource resource) + { + // Is the resource aliased? + if (resource.isAlias()) + { + if (LOG.isDebugEnabled()) + LOG.debug("Aliased resource: {}~={}", resource, resource.getAlias()); + + // alias checks + for (AliasCheck check : _aliasChecks) + { + if (check.checkAlias(pathInContext, resource)) + { + if (LOG.isDebugEnabled()) + LOG.debug("Aliased resource: {} approved by {}", resource, check); + return true; + } + } + return false; + } + return true; + } + @Override public String toString() { @@ -988,22 +1128,6 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace } } - /** - * Interface to check aliases - * TODO REVIEW!!! - */ - public interface AliasCheck - { - /** - * Check an alias - * - * @param pathInContext The path the aliased resource was created for - * @param resource The aliased resourced - * @return True if the resource is OK to be served. - */ - boolean check(String pathInContext, Resource resource); - } - /** * Listener for all threads entering context scope, including async IO callbacks */ diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java index 2aa2df98ead..6a4cd691ad0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextRequest.java @@ -110,7 +110,7 @@ public class ContextRequest extends Request.WrapperProcessor implements Invocabl } @Override - public org.eclipse.jetty.server.Context getContext() + public ContextHandler.Context getContext() { return _context; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java index 5e9e8660f98..5a149e0fa85 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java @@ -107,7 +107,7 @@ public class ResourceHandler extends Handler.Wrapper return super.handle(request); } - HttpContent content = _resourceService.getContent(request.getPathInContext()); + HttpContent content = _resourceService.getContent(request.getPathInContext(), request); if (content == null) return super.handle(request); // no content - try other handlers diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 7d1ddbc7a1d..846d0becdc5 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -319,7 +319,7 @@ public class DefaultServlet extends HttpServlet boolean included = req.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI) != null; try { - HttpContent content = _resourceService.getContent(pathInContext); + HttpContent content = _resourceService.getContent(pathInContext, ServletContextRequest.getBaseRequest(req)); if (content == null || !content.getResource().exists()) { if (included) @@ -899,20 +899,6 @@ public class DefaultServlet extends HttpServlet _servletContextHandler = servletContextHandler; } - @Override - public HttpContent getContent(String path) throws IOException - { - HttpContent httpContent = super.getContent(path); - - if (httpContent != null) - { - if (!_servletContextHandler.checkAlias(path, httpContent.getResource())) - return null; - } - - return httpContent; - } - @Override public String getWelcomeTarget(Request coreRequest) throws IOException { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index 261a0d854f0..b7779802182 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -85,7 +85,6 @@ import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.DeprecationWarning; import org.eclipse.jetty.util.ExceptionUtil; -import org.eclipse.jetty.util.Index; import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; @@ -220,8 +219,6 @@ public class ServletContextHandler extends ContextHandler implements Graceful private final List _servletRequestAttributeListeners = new CopyOnWriteArrayList<>(); private final List _contextListeners = new CopyOnWriteArrayList<>(); private final Set _durableListeners = new HashSet<>(); - private Index _protectedTargets = Index.empty(false); - private final List _aliasChecks = new CopyOnWriteArrayList<>(); protected final DecoratedObjectFactory _objFactory; // protected Class _defaultSecurityHandlerClass = org.eclipse.jetty.security.ConstraintSecurityHandler.class; @@ -602,60 +599,6 @@ public class ServletContextHandler extends ContextHandler implements Graceful } } - /** - * Check the target. Called by {@link #handle(Request)} when a target within a context is determined. If - * the target is protected, 404 is returned. - * - * @param target the target to test - * @return true if target is a protected target - */ - public boolean isProtectedTarget(String target) - { - if (target == null || _protectedTargets.isEmpty()) - return false; - - if (target.startsWith("//")) - target = URIUtil.compactPath(target); - - ProtectedTargetType type = _protectedTargets.getBest(target); - - return type == ProtectedTargetType.PREFIX || - type == ProtectedTargetType.EXACT && _protectedTargets.get(target) == ProtectedTargetType.EXACT; - } - - /** - * @param targets Array of URL prefix. Each prefix is in the form /path and will match either /path exactly or /path/anything - */ - public void setProtectedTargets(String[] targets) - { - Index.Builder builder = new Index.Builder<>(); - if (targets != null) - { - for (String t : targets) - { - if (!t.startsWith("/")) - throw new IllegalArgumentException("Bad protected target: " + t); - - builder.with(t, ProtectedTargetType.EXACT); - builder.with(t + "/", ProtectedTargetType.PREFIX); - builder.with(t + "?", ProtectedTargetType.PREFIX); - builder.with(t + "#", ProtectedTargetType.PREFIX); - builder.with(t + ";", ProtectedTargetType.PREFIX); - } - } - _protectedTargets = builder.caseSensitive(false).build(); - } - - public String[] getProtectedTargets() - { - if (_protectedTargets == null) - return null; - - return _protectedTargets.keySet().stream() - .filter(s -> _protectedTargets.get(s) == ProtectedTargetType.EXACT) - .toArray(String[]::new); - } - public void setDefaultRequestCharacterEncoding(String encoding) { _defaultRequestCharacterEncoding = encoding; @@ -875,34 +818,6 @@ public class ServletContextHandler extends ContextHandler implements Graceful return null; } - /** - * @param path the path to check the alias for - * @param resource the resource - * @return True if the alias is OK - */ - public boolean checkAlias(String path, Resource resource) - { - // Is the resource aliased? - if (resource.isAlias()) - { - if (LOG.isDebugEnabled()) - LOG.debug("Aliased resource: {}~={}", resource, resource.getAlias()); - - // alias checks - for (AliasCheck check : getAliasChecks()) - { - if (check.check(path, resource)) - { - if (LOG.isDebugEnabled()) - LOG.debug("Aliased resource: {} approved by {}", resource, check); - return true; - } - } - return false; - } - return true; - } - /** * Convert URL to Resource wrapper for {@link ResourceFactory#newResource(URL)} enables extensions to provide alternate resource implementations. * @@ -989,46 +904,6 @@ public class ServletContextHandler extends ContextHandler implements Graceful return host; } - /** - * Add an AliasCheck instance to possibly permit aliased resources - * - * @param check The alias checker - */ - public void addAliasCheck(AliasCheck check) - { - _aliasChecks.add(check); - if (check instanceof LifeCycle) - addManaged((LifeCycle)check); - else - addBean(check); - } - - /** - * @return Immutable list of Alias checks - */ - public List getAliasChecks() - { - return Collections.unmodifiableList(_aliasChecks); - } - - /** - * @param checks list of AliasCheck instances - */ - public void setAliasChecks(List checks) - { - clearAliasChecks(); - checks.forEach(this::addAliasCheck); - } - - /** - * clear the list of AliasChecks - */ - public void clearAliasChecks() - { - _aliasChecks.forEach(this::removeBean); - _aliasChecks.clear(); - } - /** * Listener for all threads entering context scope, including async IO callbacks */ diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerWebRootIsSymlinkTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerWebRootIsSymlinkTest.java deleted file mode 100644 index 59967c23e3f..00000000000 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerWebRootIsSymlinkTest.java +++ /dev/null @@ -1,159 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.io.InputStream; -import java.net.URI; -import java.net.URL; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; - -import jakarta.servlet.ServletContextEvent; -import jakarta.servlet.ServletContextListener; -import org.eclipse.jetty.client.HttpClient; -import org.eclipse.jetty.client.api.ContentResponse; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; -import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.servlet.DefaultServlet; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.resource.PathResource; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.junit.jupiter.api.Assertions.assertNotNull; - -public class AliasCheckerWebRootIsSymlinkTest -{ - private Server _server; - private ServerConnector _connector; - private HttpClient _client; - private ServletContextHandler _context; - private Path _webrootSymlink; - - private static Path getResource(String path) throws Exception - { - URL url = AliasCheckerWebRootIsSymlinkTest.class.getClassLoader().getResource(path); - assertNotNull(url); - return new File(url.toURI()).toPath(); - } - - private static void delete(Path path) - { - IO.delete(path.toFile()); - } - - private void setAliasChecker(ContextHandler.AliasCheck aliasChecker) - { - _context.clearAliasChecks(); - if (aliasChecker != null) - _context.addAliasCheck(aliasChecker); - } - - @BeforeEach - public void before() throws Exception - { - Path webRootPath = getResource("webroot"); - - // 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(_webrootSymlink)); - _context.setWelcomeFiles(new String[]{"index.html"}); - _context.getMimeTypes().addMimeMapping("txt", "text/plain;charset=utf-8"); - _server.setHandler(_context); - _context.addServlet(DefaultServlet.class, "/"); - _context.clearAliasChecks(); - - _client = new HttpClient(); - _client.start(); - } - - @AfterEach - public void after() throws Exception - { - // Try to delete all files now so that the symlinks do not confuse other tests. - Files.delete(_webrootSymlink); - - _client.stop(); - _server.stop(); - } - - @Test - public void test() throws Exception - { - // WEB-INF is a protected target, and enable symlink alias checker. - _context.setProtectedTargets(new String[]{"/WEB-INF"}); - setAliasChecker(new SymlinkAllowedResourceAliasChecker(_context)); - - CompletableFuture resource = new CompletableFuture<>(); - _context.addEventListener(new ServletContextListener() - { - @Override - public void contextInitialized(ServletContextEvent sce) - { - try - { - // Getting resource with API should allow you to bypass security constraints. - resource.complete(sce.getServletContext().getResourceAsStream("/WEB-INF/web.xml")); - } - catch (Throwable e) - { - throw new RuntimeException(e); - } - } - }); - _server.start(); - assertThat(_context.getBaseResource().isAlias(), equalTo(false)); - - // We can access web.xml with ServletContext.getResource(). - InputStream webXml = resource.get(5, TimeUnit.SECONDS); - assertNotNull(webXml); - String content = IO.toString(webXml); - assertThat(content, equalTo("This is the web.xml file.")); - - // Can access normal files in the webroot dir. - URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/file"); - ContentResponse response = _client.GET(uri); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(response.getContentAsString(), is("This file is inside webroot.")); - - // Cannot access web.xml with an external request. - uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/WEB-INF/web.xml"); - response = _client.GET(uri); - assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404)); - assertThat(response.getContentAsString(), not(containsString("This file is inside webroot."))); - } -} diff --git a/tests/pom.xml b/tests/pom.xml index 089bbed70e2..9cca6038521 100644 --- a/tests/pom.xml +++ b/tests/pom.xml @@ -5,7 +5,6 @@ org.eclipse.jetty jetty-project 12.0.0-SNAPSHOT - ../pom.xml org.eclipse.jetty.tests tests @@ -23,6 +22,7 @@ test-distribution + test-integration diff --git a/tests/test-integration/pom.xml b/tests/test-integration/pom.xml new file mode 100644 index 00000000000..76ec3ac0188 --- /dev/null +++ b/tests/test-integration/pom.xml @@ -0,0 +1,40 @@ + + + + org.eclipse.jetty.tests + tests + 12.0.0-SNAPSHOT + + 4.0.0 + test-integration + jar + Jetty Tests :: Integrations + + ${project.groupId}.integrations + + + + + org.eclipse.jetty + jetty-server + + + org.eclipse.jetty + jetty-client + + + org.slf4j + slf4j-api + + + org.eclipse.jetty + jetty-slf4j-impl + test + + + org.eclipse.jetty.toolchain + jetty-test-helper + test + + + diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java similarity index 71% rename from jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java rename to tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java index 05361a94a4b..53b180e4954 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerMultipleResourceBasesTest.java @@ -22,16 +22,16 @@ import java.nio.file.Path; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.server.AliasCheck; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.server.handler.ContextHandler; -import org.eclipse.jetty.servlet.DefaultServlet; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.server.handler.ResourceHandler; import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.resource.ResourceFactory; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -45,7 +45,7 @@ public class AliasCheckerMultipleResourceBasesTest private Server _server; private ServerConnector _connector; private HttpClient _client; - private ServletContextHandler _context; + private ContextHandler _context; private Path _webRootPath; private Path _altDir1Symlink; private Path _altDir2Symlink; @@ -62,12 +62,12 @@ public class AliasCheckerMultipleResourceBasesTest IO.delete(path.toFile()); } - private void setAliasCheckers(ContextHandler.AliasCheck... aliasChecks) + private void setAliasCheckers(AliasCheck... aliasChecks) { _context.clearAliasChecks(); if (aliasChecks != null) { - for (ContextHandler.AliasCheck aliasCheck : aliasChecks) + for (AliasCheck aliasCheck : aliasChecks) { _context.addAliasCheck(aliasCheck); } @@ -93,12 +93,10 @@ public class AliasCheckerMultipleResourceBasesTest _server = new Server(); _connector = new ServerConnector(_server); _server.addConnector(_connector); - _context = new ServletContextHandler(); + _context = new ContextHandler(); _context.setContextPath("/"); - _context.setBaseResource(new PathResource(_webRootPath)); - _context.setWelcomeFiles(new String[]{"index.html"}); - _context.getMimeTypes().addMimeMapping("txt", "text/plain;charset=utf-8"); + _context.setBaseResource(_webRootPath); _server.setHandler(_context); _context.clearAliasChecks(); @@ -116,44 +114,54 @@ public class AliasCheckerMultipleResourceBasesTest _server.stop(); } + private ResourceHandler newResourceHandler(Path resourceBase) + { + Resource resource = toResource(resourceBase); + ResourceHandler resourceHandler = new ResourceHandler(); + resourceHandler.setBaseResource(resource); + return resourceHandler; + } + + private Resource toResource(Path path) + { + return ResourceFactory.root().newResource(path); + } + @Test public void test() throws Exception { - ServletHolder servletHolder; - servletHolder = _context.addServlet(DefaultServlet.class, "/defaultServlet1/*"); - servletHolder.setInitParameter("resourceBase", _altDir1Symlink.toString()); - servletHolder.setInitParameter("pathInfoOnly", "true"); - servletHolder = _context.addServlet(DefaultServlet.class, "/defaultServlet2/*"); - servletHolder.setInitParameter("resourceBase", _altDir2Symlink.toString()); - servletHolder.setInitParameter("pathInfoOnly", "true"); - - setAliasCheckers( - new SymlinkAllowedResourceAliasChecker(_context, Resource.newResource(_altDir1Symlink)), - new SymlinkAllowedResourceAliasChecker(_context, Resource.newResource(_altDir2Symlink)) - ); - + Handler.Collection collection = new Handler.Collection(); + collection.addHandler(newResourceHandler(_altDir1Symlink)); + collection.addHandler(newResourceHandler(_altDir2Symlink)); + _context.setHandler(collection); _server.start(); - // Can access file 1 only through default servlet 1. - URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/defaultServlet1/file1"); + // With no alias checkers we cannot access file 1. + URI uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/file1"); ContentResponse response = _client.GET(uri); + assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404)); + + // With no alias checkers we cannot access file 2. + uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/file2"); + response = _client.GET(uri); + assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404)); + + // Set alias checkers to allow content under these alternative resource bases. + setAliasCheckers( + new SymlinkAllowedResourceAliasChecker(_context, toResource(_altDir1Symlink)), + new SymlinkAllowedResourceAliasChecker(_context, toResource(_altDir2Symlink)) + ); + + // Now we have set alias checkers we can access file 1. + uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/file1"); + response = _client.GET(uri); assertThat(response.getStatus(), is(HttpStatus.OK_200)); assertThat(response.getContentAsString(), is("file 1 contents")); - // File 2 cannot be found with default servlet 1. - uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/defaultServlet1/file2"); - response = _client.GET(uri); - assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404)); - - // Can access file 2 only through default servlet 2. - uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/defaultServlet2/file2"); + // Now we have set alias checkers we can access file 2. + uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/file2"); response = _client.GET(uri); assertThat(response.getStatus(), is(HttpStatus.OK_200)); assertThat(response.getContentAsString(), is("file 2 contents")); - - // File 1 cannot be found with default servlet 2. - uri = URI.create("http://localhost:" + _connector.getLocalPort() + "/defaultServlet2/file1"); - response = _client.GET(uri); - assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404)); } } 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 00000000000..dd6a10d21b5 --- /dev/null +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java @@ -0,0 +1,230 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 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.AliasCheck; +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.server.handler.ResourceHandler; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.resource.Resource; +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 ContextHandler _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(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 ContextHandler(); + _context.setContextPath("/"); + _context.setBaseResource(webRootPath); + _context.setProtectedTargets(new String[]{"/WEB-INF", "/META-INF"}); + _context.addHandler(new ResourceHandler()); + + _server.setHandler(_context); + _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(); + } + + private static class ApproveAliases implements AliasCheck + { + @Override + public boolean checkAlias(String pathInContext, Resource resource) + { + return true; + } + } + + public static Stream testCases() + { + AllowedResourceAliasChecker allowedResource = new AllowedResourceAliasChecker(_context); + SymlinkAllowedResourceAliasChecker symlinkAllowedResource = new SymlinkAllowedResourceAliasChecker(_context); + AllowSymLinkAliasChecker allowSymlinks = new AllowSymLinkAliasChecker(); + ApproveAliases approveAliases = new 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(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/altDir1/file1 b/tests/test-integration/src/test/resources/altDir1/file1 new file mode 100644 index 00000000000..a732ac0b9eb --- /dev/null +++ b/tests/test-integration/src/test/resources/altDir1/file1 @@ -0,0 +1 @@ +file 1 contents \ No newline at end of file diff --git a/tests/test-integration/src/test/resources/altDir2/file2 b/tests/test-integration/src/test/resources/altDir2/file2 new file mode 100644 index 00000000000..211e2282c76 --- /dev/null +++ b/tests/test-integration/src/test/resources/altDir2/file2 @@ -0,0 +1 @@ +file 2 contents \ 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 00000000000..47d79144910 --- /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 00000000000..b4508805362 --- /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 00000000000..7a16a16aa08 --- /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 00000000000..f7dc59cdc6b --- /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 From 8f27e9a46350960b1cf20d3078340641ce61a755 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 11 Aug 2022 13:59:54 +1000 Subject: [PATCH 2/7] ee9 should use jett-core protected targets and alias checkers Signed-off-by: Lachlan Roberts --- .../ee10/servlet/ServletContextHandler.java | 28 +-- .../ee9/nested/AllowSymLinkAliasChecker.java | 95 ------- .../nested/AllowedResourceAliasChecker.java | 237 ------------------ .../jetty/ee9/nested/ContextHandler.java | 128 +--------- .../ee9/nested/SameFileAliasChecker.java | 84 ------- .../SymlinkAllowedResourceAliasChecker.java | 88 ------- .../jetty/ee9/servlet/DefaultServletTest.java | 10 +- 7 files changed, 17 insertions(+), 653 deletions(-) delete mode 100644 jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowSymLinkAliasChecker.java delete mode 100644 jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowedResourceAliasChecker.java delete mode 100644 jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SameFileAliasChecker.java delete mode 100644 jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SymlinkAllowedResourceAliasChecker.java diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index b7779802182..b04f57da783 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -149,16 +149,6 @@ public class ServletContextHandler extends ContextHandler implements Graceful DESTROYED } - /** - * The type of protected target match - * @see #_protectedTargets - */ - protected enum ProtectedTargetType - { - EXACT, - PREFIX - } - public static ServletContextHandler getServletContextHandler(ServletContext servletContext, String purpose) { if (servletContext instanceof ServletContextApi servletContextApi) @@ -799,23 +789,7 @@ public class ServletContextHandler extends ContextHandler implements Graceful if (baseResource == null) return null; - try - { - // addPath with accept non-canonical paths that don't go above the root, - // but will treat them as aliases. So unless allowed by an AliasChecker - // they will be rejected below. - Resource resource = baseResource.resolve(pathInContext); - - if (checkAlias(pathInContext, resource)) - return resource; - return null; - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } - - return null; + return baseResource.resolve(pathInContext); } /** diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowSymLinkAliasChecker.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowSymLinkAliasChecker.java deleted file mode 100644 index 1f3da903fa1..00000000000 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowSymLinkAliasChecker.java +++ /dev/null @@ -1,95 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.ee9.nested; - -import java.nio.file.Files; -import java.nio.file.Path; - -import org.eclipse.jetty.util.resource.PathResource; -import org.eclipse.jetty.util.resource.Resource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Symbolic Link AliasChecker. - *

An instance of this class can be registered with {@link ContextHandler#addAliasCheck(ContextHandler.AliasCheck)} - * 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 SymlinkAllowedResourceAliasChecker} instead. - */ -@Deprecated -public class AllowSymLinkAliasChecker implements ContextHandler.AliasCheck -{ - private static final Logger LOG = LoggerFactory.getLogger(AllowSymLinkAliasChecker.class); - - public AllowSymLinkAliasChecker() - { - LOG.warn("Deprecated, use SymlinkAllowedResourceAliasChecker instead."); - } - - @Override - public boolean check(String pathInContext, Resource resource) - { - // Only support PathResource alias checking - if (!(resource instanceof PathResource)) - return false; - - PathResource pathResource = (PathResource)resource; - - try - { - Path path = pathResource.getPath(); - Path alias = pathResource.getAliasPath(); - - if (PathResource.isSameName(alias, path)) - return false; // Unknown why this is an alias - - if (hasSymbolicLink(path) && Files.isSameFile(path, alias)) - { - if (LOG.isDebugEnabled()) - LOG.debug("Allow symlink {} --> {}", resource, pathResource.getAliasPath()); - return true; - } - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } - - return false; - } - - private boolean hasSymbolicLink(Path path) - { - // Is file itself a symlink? - if (Files.isSymbolicLink(path)) - { - return true; - } - - // Lets try each path segment - Path base = path.getRoot(); - for (Path segment : path) - { - base = base.resolve(segment); - if (Files.isSymbolicLink(base)) - { - return true; - } - } - - return false; - } -} diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowedResourceAliasChecker.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowedResourceAliasChecker.java deleted file mode 100644 index 1b4554d25a1..00000000000 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AllowedResourceAliasChecker.java +++ /dev/null @@ -1,237 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.ee9.nested; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.LinkOption; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Objects; - -import org.eclipse.jetty.util.component.AbstractLifeCycle; -import org.eclipse.jetty.util.component.LifeCycle; -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 a protected target as defined by 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 = LoggerFactory.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<>(); - private final AllowedResourceAliasCheckListener _listener = new AllowedResourceAliasCheckListener(); - protected Path _base; - - /** - * @param contextHandler the context handler to use. - */ - public AllowedResourceAliasChecker(ContextHandler contextHandler) - { - _contextHandler = Objects.requireNonNull(contextHandler); - } - - protected ContextHandler getContextHandler() - { - return _contextHandler; - } - - protected void initialize() - { - _base = _contextHandler.getCoreContextHandler().getResourceBase().getPath(); - if (_base == null) - return; - - try - { - 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)); - } - } - catch (IOException e) - { - LOG.warn("Base resource failure ({} is disabled): {}", this.getClass().getName(), _base, e); - _base = null; - } - } - - @Override - protected void doStart() throws Exception - { - // We can only initialize if ContextHandler in started state, the baseResource can be changed even in starting state. - // If the ContextHandler is not started add a listener to delay initialization until fully started. - if (_contextHandler.isStarted()) - initialize(); - else - _contextHandler.addEventListener(_listener); - } - - @Override - protected void doStop() throws Exception - { - _contextHandler.removeEventListener(_listener); - _base = null; - _protected.clear(); - } - - @Override - public boolean check(String pathInContext, Resource resource) - { - if (_base == null) - return false; - - 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 resource.getPath(); - return (resource == null) ? null : resource.getPath(); - } - catch (Throwable t) - { - LOG.trace("getPath() failed", t); - return null; - } - } - - private class AllowedResourceAliasCheckListener implements LifeCycle.Listener - { - @Override - public void lifeCycleStarted(LifeCycle event) - { - initialize(); - } - } - - @Override - public String toString() - { - String[] protectedTargets = _contextHandler.getProtectedTargets(); - return String.format("%s@%x{base=%s,protected=%s}", - this.getClass().getSimpleName(), - hashCode(), - _base, - (protectedTargets == null) ? null : Arrays.asList(protectedTargets)); - } -} diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index fbaef65425f..fcc853ee516 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -66,6 +66,8 @@ import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.server.AliasCheck; +import org.eclipse.jetty.server.AllowedResourceAliasChecker; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Response; @@ -75,7 +77,6 @@ import org.eclipse.jetty.server.handler.ContextRequest; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.ExceptionUtil; -import org.eclipse.jetty.util.Index; import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; @@ -86,7 +87,6 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.component.Environment; import org.eclipse.jetty.util.component.Graceful; -import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; import org.slf4j.Logger; @@ -186,16 +186,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu DESTROYED } - /** - * The type of protected target match - * @see #_protectedTargets - */ - private enum ProtectedTargetType - { - EXACT, - PREFIX - } - private final CoreContextHandler _coreContextHandler; protected ContextStatus _contextStatus = ContextStatus.NOTSET; protected APIContext _apiContext; @@ -221,8 +211,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu private final List _servletRequestAttributeListeners = new CopyOnWriteArrayList<>(); private final List _contextListeners = new CopyOnWriteArrayList<>(); private final Set _durableListeners = new HashSet<>(); - private Index _protectedTargets = Index.empty(false); - private final List _aliasChecks = new CopyOnWriteArrayList<>(); public ContextHandler() { @@ -882,7 +870,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu if (new_context) requestInitialized(baseRequest, request); - if (new_context && isProtectedTarget(target)) + if (new_context && _coreContextHandler.isProtectedTarget(target)) { baseRequest.setHandled(true); response.sendError(HttpServletResponse.SC_NOT_FOUND); @@ -989,16 +977,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public boolean isProtectedTarget(String target) { - if (target == null || _protectedTargets.isEmpty()) - return false; - - if (target.startsWith("//")) - target = URIUtil.compactPath(target); - - ProtectedTargetType type = _protectedTargets.getBest(target); - - return type == ProtectedTargetType.PREFIX || - type == ProtectedTargetType.EXACT && _protectedTargets.get(target) == ProtectedTargetType.EXACT; + return _coreContextHandler.isProtectedTarget(target); } /** @@ -1006,32 +985,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setProtectedTargets(String[] targets) { - Index.Builder builder = new Index.Builder<>(); - if (targets != null) - { - for (String t : targets) - { - if (!t.startsWith("/")) - throw new IllegalArgumentException("Bad protected target: " + t); - - builder.with(t, ProtectedTargetType.EXACT); - builder.with(t + "/", ProtectedTargetType.PREFIX); - builder.with(t + "?", ProtectedTargetType.PREFIX); - builder.with(t + "#", ProtectedTargetType.PREFIX); - builder.with(t + ";", ProtectedTargetType.PREFIX); - } - } - _protectedTargets = builder.caseSensitive(false).build(); + _coreContextHandler.setProtectedTargets(targets); } public String[] getProtectedTargets() { - if (_protectedTargets == null) - return null; - - return _protectedTargets.keySet().stream() - .filter(s -> _protectedTargets.get(s) == ProtectedTargetType.EXACT) - .toArray(String[]::new); + return _coreContextHandler.getProtectedTargets(); } @Override @@ -1437,7 +1396,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu // alias checks for (AliasCheck check : getAliasChecks()) { - if (check.check(path, resource)) + if (check.checkAlias(path, resource)) { if (LOG.isDebugEnabled()) LOG.debug("Aliased resource: {} approved by {}", resource, check); @@ -1542,11 +1501,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void addAliasCheck(AliasCheck check) { - _aliasChecks.add(check); - if (check instanceof LifeCycle) - addManaged((LifeCycle)check); - else - addBean(check); + _coreContextHandler.addAliasCheck(check); } /** @@ -1554,7 +1509,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public List getAliasChecks() { - return Collections.unmodifiableList(_aliasChecks); + return _coreContextHandler.getAliasChecks(); } /** @@ -1562,8 +1517,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setAliasChecks(List checks) { - clearAliasChecks(); - checks.forEach(this::addAliasCheck); + _coreContextHandler.setAliasChecks(checks); } /** @@ -1571,8 +1525,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void clearAliasChecks() { - _aliasChecks.forEach(this::removeBean); - _aliasChecks.clear(); + _coreContextHandler.clearAliasChecks(); } /* Handle a request from a connection. @@ -2350,65 +2303,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } } - /** - * Interface to check aliases - */ - public interface AliasCheck - { - - /** - * Check an alias - * - * @param pathInContext The path the aliased resource was created for - * @param resource The aliased resourced - * @return True if the resource is OK to be served. - */ - boolean check(String pathInContext, Resource resource); - } - - /** - * Approve all aliases. - * @deprecated use {@link org.eclipse.jetty.server.AllowedResourceAliasChecker} instead. - */ - @Deprecated - public static class ApproveAliases implements AliasCheck - { - public ApproveAliases() - { - LOG.warn("ApproveAliases is deprecated"); - } - - @Override - public boolean check(String pathInContext, Resource resource) - { - return true; - } - } - - /** - * 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 - public boolean check(String pathInContext, Resource resource) - { - if (resource.exists()) - return false; - - String a = resource.getAlias().toString(); - String r = resource.getURI().toString(); - - if (a.length() > r.length()) - return a.startsWith(r) && a.length() == r.length() + 1 && a.endsWith("/"); - if (a.length() < r.length()) - return r.startsWith(a) && r.length() == a.length() + 1 && r.endsWith("/"); - - return a.equals(r); - } - } - /** * Listener for all threads entering context scope, including async IO callbacks */ diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SameFileAliasChecker.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SameFileAliasChecker.java deleted file mode 100644 index e68c34cc84e..00000000000 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SameFileAliasChecker.java +++ /dev/null @@ -1,84 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.ee9.nested; - -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; - -import org.eclipse.jetty.util.resource.PathResource; -import org.eclipse.jetty.util.resource.Resource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Alias checking for working with FileSystems that normalize access to the - * File System. - *

- * The Java {@link Files#isSameFile(Path, Path)} method is used to determine - * if the requested file is the same as the alias file. - *

- *

- * For File Systems that are case insensitive (eg: Microsoft Windows FAT32 and NTFS), - * the access to the file can be in any combination or style of upper and lowercase. - *

- *

- * For File Systems that normalize UTF-8 access (eg: Mac OSX on HFS+ or APFS, - * 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 AllowedResourceAliasChecker} instead. - */ -@Deprecated -public class SameFileAliasChecker implements ContextHandler.AliasCheck -{ - private static final Logger LOG = LoggerFactory.getLogger(SameFileAliasChecker.class); - - public SameFileAliasChecker() - { - LOG.warn("SameFileAliasChecker is deprecated"); - } - - @Override - public boolean check(String pathInContext, Resource resource) - { - // Do not allow any file separation characters in the URI. - if (File.separatorChar != '/' && pathInContext.indexOf(File.separatorChar) >= 0) - return false; - - // Only support PathResource alias checking - if (!(resource instanceof PathResource)) - return false; - - try - { - PathResource pathResource = (PathResource)resource; - Path path = pathResource.getPath(); - Path alias = pathResource.getAliasPath(); - - if (Files.isSameFile(path, alias)) - { - if (LOG.isDebugEnabled()) - LOG.debug("Allow alias to same file {} --> {}", path, alias); - return true; - } - } - catch (IOException e) - { - LOG.trace("IGNORED", e); - } - return false; - } -} diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SymlinkAllowedResourceAliasChecker.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SymlinkAllowedResourceAliasChecker.java deleted file mode 100644 index 84cadc9e5f8..00000000000 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SymlinkAllowedResourceAliasChecker.java +++ /dev/null @@ -1,88 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.ee9.nested; - -import java.io.File; -import java.nio.file.Files; -import java.nio.file.Path; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * 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 = LoggerFactory.getLogger(SymlinkAllowedResourceAliasChecker.class); - - /** - * @param contextHandler the context handler to use. - */ - public SymlinkAllowedResourceAliasChecker(ContextHandler contextHandler) - { - super(contextHandler); - } - - @Override - protected boolean check(String pathInContext, Path path) - { - if (_base == null) - return false; - - // 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-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java index fd3889cdd4a..0e80ef41771 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java @@ -40,10 +40,8 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.ee9.nested.AllowedResourceAliasChecker; import org.eclipse.jetty.ee9.nested.ResourceContentFactory; import org.eclipse.jetty.ee9.nested.ResourceService; -import org.eclipse.jetty.ee9.nested.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.http.DateGenerator; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpField; @@ -51,9 +49,11 @@ import org.eclipse.jetty.http.HttpHeader; 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.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.toolchain.test.jupiter.WorkDir; @@ -1132,7 +1132,7 @@ public class DefaultServletTest response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.NOT_FOUND_404)); - context.addAliasCheck(new SymlinkAllowedResourceAliasChecker(context)); + context.addAliasCheck(new SymlinkAllowedResourceAliasChecker(context.getCoreContextHandler())); 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)); @@ -2108,7 +2108,7 @@ public class DefaultServletTest FS.ensureEmpty(docRoot); context.addServlet(DefaultServlet.class, "/"); - context.addAliasCheck(new AllowedResourceAliasChecker(context)); + context.addAliasCheck(new AllowedResourceAliasChecker(context.getCoreContextHandler())); // Create file with UTF-8 NFC format String filename = "swedish-" + new String(TypeUtil.fromHexString("C3A5"), UTF_8) + ".txt"; @@ -2148,7 +2148,7 @@ public class DefaultServletTest FS.ensureEmpty(docRoot); context.addServlet(DefaultServlet.class, "/"); - context.addAliasCheck(new AllowedResourceAliasChecker(context)); + context.addAliasCheck(new AllowedResourceAliasChecker(context.getCoreContextHandler())); // Create file with UTF-8 NFD format String filename = "swedish-a" + new String(TypeUtil.fromHexString("CC8A"), UTF_8) + ".txt"; From 4c599c94cda7ee57f812df0a6222c06bbd06234b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 11 Aug 2022 14:03:18 +1000 Subject: [PATCH 3/7] fix licence header in AliasCheck Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/server/AliasCheck.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java index f3b36b60629..a7335b7f5ed 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java @@ -1,3 +1,16 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 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 org.eclipse.jetty.util.resource.Resource; From d738f4b99f9ac21679770c6febaabca6cb12a588 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 15 Aug 2022 10:46:48 +1000 Subject: [PATCH 4/7] Update javadoc for the AliasCheck interface Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/server/AliasCheck.java | 10 +- .../handler/AllowSymLinkAliasChecker.java | 95 ------------------- 2 files changed, 7 insertions(+), 98 deletions(-) delete mode 100644 jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java index a7335b7f5ed..da51380713b 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/AliasCheck.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.server; +import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.resource.Resource; /** @@ -21,10 +22,13 @@ import org.eclipse.jetty.util.resource.Resource; public interface AliasCheck { /** - * Check an alias + * Check if an alias is allowed to be served. If any {@link AliasCheck} returns + * true then the alias will be allowed to be served, therefore any alias checker + * should take things like the {@link ContextHandler#getProtectedTargets()} and + * Security Constraints into consideration before allowing a return a value of true. * - * @param pathInContext The path the aliased resource was created for - * @param resource The aliased resourced + * @param pathInContext The path the aliased resource was created for. + * @param resource The aliased resourced. * @return True if the resource is OK to be served. */ boolean checkAlias(String pathInContext, Resource resource); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java deleted file mode 100644 index 0eb73fe9696..00000000000 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java +++ /dev/null @@ -1,95 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2022 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.handler; - -import java.nio.file.Files; -import java.nio.file.Path; - -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.util.resource.PathResource; -import org.eclipse.jetty.util.resource.Resource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Symbolic Link AliasChecker. - *

An instance of this class can be registered with {@link Handler} subclasses - * 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 checkAlias(String pathInContext, Resource resource) - { - // Only support PathResource alias checking - if (!(resource instanceof PathResource pathResource)) - return false; - - try - { - Path path = pathResource.getPath(); - Path alias = pathResource.getAliasPath(); - - if (PathResource.isSameName(alias, path)) - return false; // Unknown why this is an alias - - if (hasSymbolicLink(path) && Files.isSameFile(path, alias)) - { - if (LOG.isDebugEnabled()) - LOG.debug("Allow symlink {} --> {}", resource, pathResource.getAliasPath()); - return true; - } - } - catch (Exception e) - { - LOG.trace("IGNORED", e); - } - - return false; - } - - private boolean hasSymbolicLink(Path path) - { - // Is file itself a symlink? - if (Files.isSymbolicLink(path)) - { - return true; - } - - // Lets try each path segment - Path base = path.getRoot(); - for (Path segment : path) - { - base = base.resolve(segment); - if (Files.isSymbolicLink(base)) - { - return true; - } - } - - return false; - } -} From a6b88cfeadf9fefee2fb6adde5c61e29a28e9d91 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 15 Aug 2022 11:00:25 +1000 Subject: [PATCH 5/7] fix failures in ResourceHandlerTest Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/server/handler/ContextHandler.java | 4 ++++ .../eclipse/jetty/server/handler/ResourceHandlerTest.java | 2 +- .../eclipse/jetty/ee10/servlet/ServletContextHandler.java | 5 ----- .../java/org/eclipse/jetty/ee9/nested/ContextHandler.java | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 8a3897bb11c..7cb8343038e 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -40,6 +40,7 @@ import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.DecoratedObjectFactory; @@ -144,6 +145,9 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Grace setContextPath(contextPath); if (parent != null) parent.addHandler(this); + + if (File.separatorChar == '/') + addAliasCheck(new SymlinkAllowedResourceAliasChecker(this)); } protected Context newContext() diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java index e96c7b21eab..94d22612da4 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java @@ -674,12 +674,12 @@ public class ResourceHandlerTest _server.addConnector(_local); _rootResourceHandler = new ResourceHandler(); - _rootResourceHandler.setBaseResource(ResourceFactory.root().newResource(docRoot)); _rootResourceHandler.setWelcomeFiles("welcome.txt"); _rootResourceHandler.setRedirectWelcome(false); ContextHandler contextHandler = new ContextHandler("/context"); contextHandler.setHandler(_rootResourceHandler); + contextHandler.setBaseResource(ResourceFactory.root().newResource(docRoot)); _contextHandlerCollection = new ContextHandlerCollection(); _contextHandlerCollection.addHandler(contextHandler); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index 77cad50b457..56d8c579438 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.ee10.servlet; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -78,7 +77,6 @@ import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.ContextRequest; @@ -264,9 +262,6 @@ public class ServletContextHandler extends ContextHandler implements Graceful public ServletContextHandler(Container parent, String contextPath, SessionHandler sessionHandler, SecurityHandler securityHandler, ServletHandler servletHandler, ErrorHandler errorHandler, int options) { _servletContext = newServletContextApi(); - - if (File.separatorChar == '/') - addAliasCheck(new SymlinkAllowedResourceAliasChecker(this)); if (contextPath != null) setContextPath(contextPath); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java index 24661c3a4a7..fcc853ee516 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java @@ -1394,7 +1394,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu LOG.debug("Aliased resource: {}~={}", resource, resource.getAlias()); // alias checks - for (AliasCheck check : _aliasChecks) + for (AliasCheck check : getAliasChecks()) { if (check.checkAlias(path, resource)) { From 51d61d4c828f1f5ee4e6b681646ba9f5b91b2ecb Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 15 Aug 2022 11:14:11 +1000 Subject: [PATCH 6/7] fix compile issues after removal of AllowSymLinkAliasChecker Signed-off-by: Lachlan Roberts --- .../ee10/servlet/security/AliasedConstraintTest.java | 3 --- .../eclipse/jetty/test/AliasCheckerSymlinkTest.java | 11 ----------- 2 files changed, 14 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/AliasedConstraintTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/AliasedConstraintTest.java index 8d7b87eadbd..2471962f26f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/AliasedConstraintTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/AliasedConstraintTest.java @@ -25,7 +25,6 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerList; import org.eclipse.jetty.server.handler.ResourceHandler; @@ -78,8 +77,6 @@ public class AliasedConstraintTest server.setHandler(new HandlerList(context, new DefaultHandler())); - context.addAliasCheck(new AllowSymLinkAliasChecker()); - server.addBean(loginService); security = new ConstraintSecurityHandler(); diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java index dd6a10d21b5..92867049f23 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java @@ -28,7 +28,6 @@ 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.server.handler.ResourceHandler; import org.eclipse.jetty.util.IO; @@ -165,7 +164,6 @@ public class AliasCheckerSymlinkTest { AllowedResourceAliasChecker allowedResource = new AllowedResourceAliasChecker(_context); SymlinkAllowedResourceAliasChecker symlinkAllowedResource = new SymlinkAllowedResourceAliasChecker(_context); - AllowSymLinkAliasChecker allowSymlinks = new AllowSymLinkAliasChecker(); ApproveAliases approveAliases = new ApproveAliases(); return Stream.of( @@ -187,15 +185,6 @@ public class AliasCheckerSymlinkTest 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."), From 5ce20a69d5ef120af89a8bd1213f0ceb5fcf92ee Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 15 Aug 2022 13:12:57 +1000 Subject: [PATCH 7/7] fix AliasCheckerSymlinkTest by adding the required resources Signed-off-by: Lachlan Roberts --- tests/test-integration/src/test/resources/file | 1 + tests/test-integration/src/test/resources/sibling/file | 1 + 2 files changed, 2 insertions(+) create mode 100644 tests/test-integration/src/test/resources/file create mode 100644 tests/test-integration/src/test/resources/sibling/file diff --git a/tests/test-integration/src/test/resources/file b/tests/test-integration/src/test/resources/file new file mode 100644 index 00000000000..4b745d8b20e --- /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 00000000000..f57b0b89d7a --- /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