diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java index a646d7d74496..343bec0138d0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java @@ -23,10 +23,11 @@ import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; -import java.util.Objects; +import java.util.HashSet; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.PathResource; @@ -40,7 +41,7 @@ *

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 implements ContextHandler.AliasCheck +public class AllowedResourceAliasChecker extends AbstractLifeCycle implements ContextHandler.AliasCheck { private static final Logger LOG = Log.getLogger(AllowedResourceAliasChecker.class); private static final LinkOption[] FOLLOW_LINKS = new LinkOption[0]; @@ -48,6 +49,8 @@ public class AllowedResourceAliasChecker implements ContextHandler.AliasCheck private final ContextHandler _contextHandler; private final boolean _checkSymlinkTargets; + private final HashSet _protectedPaths = new HashSet<>(); + private Path _basePath; /** * @param contextHandler the context handler to use. @@ -59,6 +62,40 @@ public AllowedResourceAliasChecker(ContextHandler contextHandler, boolean checkS _checkSymlinkTargets = checkSymlinkTargets; } + protected ContextHandler getContextHandler() + { + return _contextHandler; + } + + protected Path getBasePath() + { + return _basePath; + } + + @Override + protected void doStart() throws Exception + { + _basePath = getPath(_contextHandler.getBaseResource()); + if (_basePath == null) + throw new IllegalStateException("Could not obtain base resource path"); + + String[] protectedTargets = _contextHandler.getProtectedTargets(); + if (protectedTargets != null) + { + for (String s : protectedTargets) + { + _protectedPaths.add(_basePath.resolve(s)); + } + } + } + + @Override + protected void doStop() throws Exception + { + _basePath = null; + _protectedPaths.clear(); + } + @Override public boolean check(String uri, Resource resource) { @@ -104,31 +141,34 @@ public boolean check(String uri, Resource resource) */ protected boolean isProtectedPath(Path resourcePath, LinkOption[] linkOptions) throws IOException { - String basePath = Objects.requireNonNull(getPath(_contextHandler.getBaseResource())).toRealPath(linkOptions).toString(); - String targetPath = resourcePath.toRealPath(linkOptions).toString(); + // If the resource doesn't exist we cannot determine whether it is protected so we assume it is. + if (!Files.exists(resourcePath, linkOptions)) + return true; + + Path basePath = _basePath.toRealPath(linkOptions); + Path targetPath = resourcePath.toRealPath(linkOptions); + String target = targetPath.toString(); + // The target path must be under the base resource directory. if (!targetPath.startsWith(basePath)) return true; - String[] protectedTargets = _contextHandler.getProtectedTargets(); - if (protectedTargets != null) + for (Path protectedPath : _protectedPaths) { - for (String s : protectedTargets) + // We know the targetPath exists, so if protectedPath doesn't exist then targetPath cannot be a child of it. + if (!Files.exists(protectedPath, linkOptions)) + continue; + + // If the target path is protected then we will not allow it. + String protect = protectedPath.toRealPath(linkOptions).toString(); + if (StringUtil.startsWithIgnoreCase(target, protect)) { - // TODO: we are always following links for the base resource. - // We cannot use toRealPath(linkOptions) here as it throws if file does not exist, - // and the protected targets do not have to always exist. - String protectedTarget = new File(basePath, s).getCanonicalPath(); - if (StringUtil.startsWithIgnoreCase(targetPath, protectedTarget)) - { - if (targetPath.length() == protectedTarget.length()) - return true; - - // Check that the target prefix really is a path segment. - char c = targetPath.charAt(protectedTarget.length()); - if (c == File.separatorChar) - return true; - } + if (target.length() == protect.length()) + return true; + + // Check that the target prefix really is a path segment. + if (target.charAt(protect.length()) == File.separatorChar) + return true; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java index 1a874b92d118..554e4ba328a9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java @@ -57,7 +57,7 @@ public boolean check(String uri, Resource resource) return true; // Approve if path has symlink in under its resource base. - if (super.hasSymbolicLink(resourcePath)) + if (super.hasSymbolicLink(getBasePath(), resourcePath)) return true; } catch (IOException e) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java index 9453123bbce7..f4f77cd66e0a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java @@ -40,6 +40,11 @@ public class AllowSymLinkAliasChecker implements AliasCheck { private static final Logger LOG = Log.getLogger(AllowSymLinkAliasChecker.class); + public AllowSymLinkAliasChecker() + { + LOG.warn("Deprecated, use SymlinkAllowedResourceAliasChecker instead."); + } + @Override public boolean check(String uri, Resource resource) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 222bcf14db3b..2bfb251c5a62 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -88,6 +88,7 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.component.Graceful; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -2084,6 +2085,10 @@ private String normalizeHostname(String host) public void addAliasCheck(AliasCheck check) { getAliasChecks().add(check); + if (check instanceof LifeCycle) + addManaged((LifeCycle)check); + else + addBean(check); } /** @@ -2099,7 +2104,7 @@ public List getAliasChecks() */ public void setAliasChecks(List checks) { - getAliasChecks().clear(); + clearAliasChecks(); getAliasChecks().addAll(checks); } @@ -2108,7 +2113,9 @@ public void setAliasChecks(List checks) */ public void clearAliasChecks() { - getAliasChecks().clear(); + List aliasChecks = getAliasChecks(); + aliasChecks.forEach(this::removeBean); + aliasChecks.clear(); } /** 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 dc96c25f8a34..07f3f17f1d59 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 @@ -118,16 +118,16 @@ public static void beforeAll() throws Exception // Create and start Server and Client. _server = new Server(); _connector = new ServerConnector(_server); - _connector.setPort(8080); _server.addConnector(_connector); _context = new ServletContextHandler(); _context.setContextPath("/"); - _context.setBaseResource(new PathResource(webrootSymlink)); + _context.setBaseResource(new PathResource(webRootPath)); _context.setWelcomeFiles(new String[]{"index.html"}); _context.setProtectedTargets(new String[]{"/web-inf", "/meta-inf"}); _context.getMimeTypes().addMimeMapping("txt", "text/plain;charset=utf-8"); _server.setHandler(_context); _context.addServlet(DefaultServlet.class, "/"); + _context.clearAliasChecks(); _server.start(); _client = new HttpClient();