From 8f27e9a46350960b1cf20d3078340641ce61a755 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 11 Aug 2022 13:59:54 +1000 Subject: [PATCH] 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";