Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #6681 alias checker changes to Jetty 9.4 #6979

Merged
merged 4 commits into from Oct 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -19,8 +19,8 @@
package org.eclipse.jetty.gcloud.session;

import org.eclipse.jetty.security.HashLoginService;
import org.eclipse.jetty.server.AllowedResourceAliasChecker;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker;
import org.eclipse.jetty.server.session.DefaultSessionCache;
import org.eclipse.jetty.server.session.DefaultSessionIdManager;
import org.eclipse.jetty.webapp.WebAppContext;
Expand All @@ -47,7 +47,7 @@ public static void main(String[] args) throws Exception
WebAppContext webapp = new WebAppContext();
webapp.setContextPath("/");
webapp.setWar("../../jetty-distribution/target/distribution/demo-base/webapps/test.war");
webapp.addAliasCheck(new AllowSymLinkAliasChecker());
webapp.addAliasCheck(new AllowedResourceAliasChecker(webapp));
GCloudSessionDataStore ds = new GCloudSessionDataStore();

DefaultSessionCache ss = new DefaultSessionCache(webapp.getSessionHandler());
Expand Down
Expand Up @@ -181,5 +181,5 @@ public class OSGiWebappConstants
/**
* Set of extra dirs that must not be served by osgi webapps
*/
public static final String[] DEFAULT_PROTECTED_OSGI_TARGETS = {"/osgi-inf", "/osgi-opts"};
public static final String[] DEFAULT_PROTECTED_OSGI_TARGETS = {"/OSGI-INF", "/OSGI-OPTS"};
}
@@ -0,0 +1,210 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.server;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;

import org.eclipse.jetty.server.handler.ContextHandler;
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;
import org.eclipse.jetty.util.resource.Resource;

/**
* <p>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 {@link ContextHandler#getProtectedTargets()} at start.</p>
* <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 extends AbstractLifeCycle implements ContextHandler.AliasCheck
{
private static final Logger LOG = Log.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<Path> _protected = new ArrayList<>();
protected Path _base;

/**
* @param contextHandler the context handler to use.
*/
public AllowedResourceAliasChecker(ContextHandler contextHandler)
{
_contextHandler = contextHandler;
}

protected ContextHandler getContextHandler()
{
return _contextHandler;
}

@Override
protected void doStart() throws Exception
{
_base = getPath(_contextHandler.getBaseResource());
if (_base == null)
_base = Paths.get("/").toAbsolutePath();
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));
}
}

@Override
protected void doStop() throws Exception
{
_base = null;
_protected.clear();
}

@Override
public boolean check(String pathInContext, Resource resource)
{
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 ((PathResource)resource).getPath();
return resource.getFile().toPath();
}
catch (Throwable t)
{
LOG.ignore(t);
return null;
}
}

@Override
public String toString()
{
return String.format("%s@%x{base=%s,protected=%s}",
this.getClass().getSimpleName(),
hashCode(),
_base,
Arrays.asList(_contextHandler.getProtectedTargets()));
}
}
Expand Up @@ -18,6 +18,7 @@

package org.eclipse.jetty.server;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -44,14 +45,20 @@
* 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.
* </p>
* @deprecated use {@link org.eclipse.jetty.server.AllowedResourceAliasChecker} instead.
*/
@Deprecated
public class SameFileAliasChecker implements AliasCheck
{
private static final Logger LOG = Log.getLogger(SameFileAliasChecker.class);

@Override
public boolean check(String uri, Resource resource)
{
// Do not allow any file separation characters in the URI.
if (File.separatorChar != '/' && uri.indexOf(File.separatorChar) >= 0)
return false;

// Only support PathResource alias checking
if (!(resource instanceof PathResource))
return false;
Expand Down
@@ -0,0 +1,91 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.server;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;

import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;

/**
* 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 = Log.getLogger(SymlinkAllowedResourceAliasChecker.class);

/**
* @param contextHandler the context handler to use.
*/
public SymlinkAllowedResourceAliasChecker(ContextHandler contextHandler)
{
super(contextHandler);
}

@Override
protected boolean check(String pathInContext, Path path)
{
// 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);
}
}
Expand Up @@ -33,11 +33,18 @@
* 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.</p>
* @deprecated use {@link org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker} instead.
*/
@Deprecated
public class AllowSymLinkAliasChecker implements AliasCheck
{
private static final Logger LOG = Log.getLogger(AllowSymLinkAliasChecker.class);

public AllowSymLinkAliasChecker()
{
LOG.warn(getClass().getSimpleName() + " is deprecated");
}

@Override
public boolean check(String uri, Resource resource)
{
Expand Down