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