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

Issue #6497 - Replace the Alias checkers with new implementation. (Jetty-10) #6668

Closed
Closed
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
57 changes: 57 additions & 0 deletions jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java
Expand Up @@ -43,6 +43,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 @@ -545,4 +547,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,228 @@
//
// ========================================================================
// Copyright (c) 1995-2021 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.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.resource.PathResource;
import org.eclipse.jetty.util.resource.Resource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* <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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* is not protected by {@link ContextHandler#isProtectedTarget(String)}.</p>
* is not protected by {@link #isProtectedTarget(Path, LinkOption[])}.</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 = LoggerFactory.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.
*/
public AllowedResourceAliasChecker(ContextHandler contextHandler)
{
this(contextHandler, false);
}

/**
* @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("Failed to check alias", 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.
Comment on lines +138 to +139
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* <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.
* <p>The resource path is protected if it, or one of it's parents is in
* {@link ContextHandler#getProtectedTargets()} when fetched from {@link #doStart()}.</p>
* {@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;
}
}
Comment on lines +161 to +181
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me of why we can't walk the parent tree again?


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.trace("getPath() failed", t);
return null;
}
}

@Override
public String toString()
{
return String.format("%s@%x{checkSymlinkTargets=%s}", AllowedResourceAliasChecker.class.getSimpleName(), hashCode(), _checkSymlinkTargets);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EOL

Expand Up @@ -39,7 +39,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 = LoggerFactory.getLogger(SameFileAliasChecker.class);
Expand Down
@@ -0,0 +1,66 @@
//
// ========================================================================
// Copyright (c) 1995-2021 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.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.resource.Resource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* An extension of {@link AllowedResourceAliasChecker} which only allows aliased resources if they are symlinks.
*/
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, 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.trace("Failed to check alias", e);
return false;
}

return false;
}
}
Expand Up @@ -28,11 +28,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 = LoggerFactory.getLogger(AllowSymLinkAliasChecker.class);

public AllowSymLinkAliasChecker()
{
LOG.warn("Deprecated, use SymlinkAllowedResourceAliasChecker instead.");
}

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