Skip to content

Commit

Permalink
Issue #6497 - Replace the Alias checkers with new implementation.
Browse files Browse the repository at this point in the history
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
lachlan-roberts committed Aug 26, 2021
1 parent 05c08e1 commit 9ef0a36
Show file tree
Hide file tree
Showing 17 changed files with 639 additions and 17 deletions.
Expand Up @@ -20,7 +20,7 @@

import org.eclipse.jetty.security.HashLoginService;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker;
import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker;
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 SymlinkAllowedResourceAliasChecker(webapp));
GCloudSessionDataStore ds = new GCloudSessionDataStore();

DefaultSessionCache ss = new DefaultSessionCache(webapp.getSessionHandler());
Expand Down
57 changes: 57 additions & 0 deletions jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java
Expand Up @@ -48,6 +48,8 @@
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.resource.PathResource;
import org.eclipse.jetty.util.resource.Resource;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
Expand Down Expand Up @@ -550,4 +552,59 @@ public void testSelectorWakeup() throws Exception
}
}
}

@Test
public void testSymbolicLink(TestInfo testInfo) throws Exception
{
File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
FS.ensureEmpty(dir);
File realFile = new File(dir, "real");
Path realPath = realFile.toPath();
FS.touch(realFile);

File linkFile = new File(dir, "link");
Path linkPath = linkFile.toPath();
Files.createSymbolicLink(linkPath, realPath);
Path targPath = linkPath.toRealPath();

System.err.printf("realPath = %s%n", realPath);
System.err.printf("linkPath = %s%n", linkPath);
System.err.printf("targPath = %s%n", targPath);

assertFalse(Files.isSymbolicLink(realPath));
assertTrue(Files.isSymbolicLink(linkPath));

Resource link = new PathResource(dir).addPath("link");
assertThat(link.isAlias(), is(true));
}

@Test
public void testSymbolicLinkDir(TestInfo testInfo) throws Exception
{
File dir = MavenTestingUtils.getTargetTestingDir(testInfo.getDisplayName());
FS.ensureEmpty(dir);

File realDirFile = new File(dir, "real");
Path realDirPath = realDirFile.toPath();
Files.createDirectories(realDirPath);

File linkDirFile = new File(dir, "link");
Path linkDirPath = linkDirFile.toPath();
Files.createSymbolicLink(linkDirPath, realDirPath);

File realFile = new File(realDirFile, "file");
Path realPath = realFile.toPath();
FS.touch(realFile);

File linkFile = new File(linkDirFile, "file");
Path linkPath = linkFile.toPath();
Path targPath = linkPath.toRealPath();

System.err.printf("realPath = %s%n", realPath);
System.err.printf("linkPath = %s%n", linkPath);
System.err.printf("targPath = %s%n", targPath);

assertFalse(Files.isSymbolicLink(realPath));
assertFalse(Files.isSymbolicLink(linkPath));
}
}
@@ -0,0 +1,225 @@
//
// ========================================================================
// 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.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.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;
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 {@link ContextHandler#isProtectedTarget(String)}.</p>
* <p>This will approve symlinks to outside of the resource base. This can be optionally configured to check that the
* target of the symlinks is also inside of the resource base and is not a protected target.</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);
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.
* @param checkSymlinkTargets true to check that the target of the symlink is an allowed resource.
*/
public AllowedResourceAliasChecker(ContextHandler contextHandler, boolean checkSymlinkTargets)
{
_contextHandler = contextHandler;
_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)
_basePath = Paths.get("/").toAbsolutePath();

String[] protectedTargets = _contextHandler.getProtectedTargets();
if (protectedTargets != null)
{
for (String s : protectedTargets)
{
Path path = _basePath.getFileSystem().getPath(_basePath.toString(), s);
_protectedPaths.add(path);
}
}
}

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

@Override
public boolean check(String uri, Resource resource)
{
// The existence check resolves the symlinks.
if (!resource.exists())
return false;

Path resourcePath = getPath(resource);
if (resourcePath == null)
return false;

try
{
if (isProtectedPath(resourcePath, NO_FOLLOW_LINKS))
return false;

if (_checkSymlinkTargets && hasSymbolicLink(resourcePath))
{
if (isProtectedPath(resourcePath, FOLLOW_LINKS))
return false;
}
}
catch (Throwable t)
{
LOG.warn(t);
return false;
}

return true;
}

/**
* <p>Determines whether the provided resource path is protected.</p>
*
* <p>The resource path is protected if it is under one of the protected targets defined by
* {@link ContextHandler#isProtectedTarget(String)} in which case the alias should not be allowed.
* The resource path may also attempt to traverse above the root path and should be denied.</p>
*
* @param resourcePath the resource {@link Path} to be tested.
* @param linkOptions an array of {@link LinkOption} to be provided to the {@link Path#toRealPath(LinkOption...)} method.
* @return true if the resource path is protected and the alias should not be allowed.
* @throws IOException if an I/O error occurs.
*/
protected boolean isProtectedPath(Path resourcePath, LinkOption[] linkOptions) throws IOException
{
// 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;

for (Path protectedPath : _protectedPaths)
{
String protect;
if (Files.exists(protectedPath, linkOptions))
protect = protectedPath.toRealPath(linkOptions).toString();
else if (linkOptions == NO_FOLLOW_LINKS)
protect = protectedPath.normalize().toAbsolutePath().toString();
else
protect = protectedPath.toFile().getCanonicalPath();

// If the target path is protected then we will not allow it.
if (StringUtil.startsWithIgnoreCase(target, protect))
{
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;
}
}

return false;
}

protected boolean hasSymbolicLink(Path path)
{
return hasSymbolicLink(path.getRoot(), path);
}

protected boolean hasSymbolicLink(Path base, Path path)
{
Path p = path;
while (!base.equals(p))
{
if (p == null)
throw new IllegalArgumentException("path was not child of base");

if (Files.isSymbolicLink(p))
return true;

p = p.getParent();
}

return false;
}

private 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{checkSymlinkTargets=%s}", AllowedResourceAliasChecker.class.getSimpleName(), hashCode(), _checkSymlinkTargets);
}
}
Expand Up @@ -44,7 +44,9 @@
* 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);
Expand Down
@@ -0,0 +1,71 @@
//
// ========================================================================
// 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.Path;

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

/**
* An extension of {@link AllowedResourceAliasChecker} which only allows aliased resources if they are symlinks.
*/
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, false);
}

@Override
public boolean check(String uri, Resource resource)
{
try
{
// Check the resource is allowed to be accessed.
if (!super.check(uri, resource))
return false;

// Approve if path is a symbolic link.
Path resourcePath = resource.getFile().toPath();
if (Files.isSymbolicLink(resourcePath))
return true;

// Approve if path has symlink in under its resource base.
if (super.hasSymbolicLink(getBasePath(), resourcePath))
return true;
}
catch (IOException e)
{
LOG.ignore(e);
return false;
}

return false;
}
}

0 comments on commit 9ef0a36

Please sign in to comment.