Skip to content

Commit

Permalink
Issue #6497 - Make AllowedResourceAliasChecker a LifeCycle
Browse files Browse the repository at this point in the history 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Jul 30, 2021
1 parent 68d93fb commit a7639bc
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 26 deletions.
82 changes: 61 additions & 21 deletions jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java
View file
Expand Up @@ -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;
Expand All @@ -40,14 +41,16 @@
* <p>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.</p>
*/
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];
private static final LinkOption[] NO_FOLLOW_LINKS = new LinkOption[]{LinkOption.NOFOLLOW_LINKS};

private final ContextHandler _contextHandler;
private final boolean _checkSymlinkTargets;
private final HashSet<Path> _protectedPaths = new HashSet<>();
private Path _basePath;

/**
* @param contextHandler the context handler to use.
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java
View file
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/AllowSymLinkAliasChecker.java
View file
Expand Up @@ -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)
{
Expand Down
11 changes: 9 additions & 2 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -2099,7 +2104,7 @@ public List<AliasCheck> getAliasChecks()
*/
public void setAliasChecks(List<AliasCheck> checks)
{
getAliasChecks().clear();
clearAliasChecks();
getAliasChecks().addAll(checks);
}

Expand All @@ -2108,7 +2113,9 @@ public void setAliasChecks(List<AliasCheck> checks)
*/
public void clearAliasChecks()
{
getAliasChecks().clear();
List<AliasCheck> aliasChecks = getAliasChecks();
aliasChecks.forEach(this::removeBean);
aliasChecks.clear();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/test-integration/src/test/java/org/eclipse/jetty/test/AliasCheckerSymlinkTest.java
View file
Expand Up @@ -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();
Expand Down

0 comments on commit a7639bc

Please sign in to comment.