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

Jetty 10.0.x 6497 alias checkers alt #6681

Merged
merged 7 commits into from Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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));
}
}
Expand Up @@ -351,18 +351,18 @@ public void doStop() throws Exception
}

@Override
public Resource getResource(String uriInContext) throws MalformedURLException
public Resource getResource(String pathInContext) throws MalformedURLException
{
Resource resource = null;
// Try to get regular resource
resource = super.getResource(uriInContext);
resource = super.getResource(pathInContext);

// If no regular resource exists check for access to /WEB-INF/lib or
// /WEB-INF/classes
if ((resource == null || !resource.exists()) && uriInContext != null && _classes != null)
if ((resource == null || !resource.exists()) && pathInContext != null && _classes != null)
{
// Canonicalize again to look for the resource inside /WEB-INF subdirectories.
String uri = URIUtil.canonicalPath(uriInContext);
String uri = URIUtil.canonicalPath(pathInContext);
if (uri == null)
return null;

Expand Down
Expand Up @@ -134,5 +134,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,163 @@
//
// ========================================================================
// 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.LinkOption;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import org.eclipse.jetty.server.handler.ContextHandler;
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>
* <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);
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)
{
// The existence check resolves the symlinks.
if (!resource.exists())
return false;

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

try
{
Path link = path.toRealPath(NO_FOLLOW_LINKS);
Path real = path.toRealPath(FOLLOW_LINKS);
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved

return isAllowed(real) && (link.equals(real) || isAllowed(link));
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
}
catch (Throwable t)
{
LOG.warn("Failed to check alias", t);
return false;
}
}

protected boolean isAllowed(Path path) throws IOException
{
// If the resource doesn't exist we cannot determine whether it is protected so we assume it is.
if (Files.exists(path))
{
while (path != null)
{
if (Files.isSameFile(path, _base))
return true;

for (Path protectedPath : _protected)
{
if (Files.exists(protectedPath))
{
protectedPath = protectedPath.toRealPath(FOLLOW_LINKS);
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
if (Files.exists(protectedPath) && Files.isSameFile(path, protectedPath))
return false;
}
}

path = path.getParent();
}
}

return false;
}

protected 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{base=%s,protected=%s}",
this.getClass().getSimpleName(),
hashCode(),
_base,
Arrays.asList(_contextHandler.getProtectedTargets()));
}
}
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -39,13 +39,15 @@
* 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);

@Override
public boolean check(String uri, Resource resource)
public boolean check(String pathInContext, Resource resource)
{
// Only support PathResource alias checking
if (!(resource instanceof PathResource))
Expand Down
@@ -0,0 +1,74 @@
//
// ========================================================================
// 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.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);
}

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

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

String[] segments = pathInContext.split("/");
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
Path fromBase = _base;

try
{
for (String segment : segments)
{
fromBase = fromBase.resolve(segment);
if (!Files.exists(fromBase))
return false;
if (Files.isSymbolicLink(fromBase))
return true;
if (!isAllowed(fromBase))
return false;
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
}
}
catch (Throwable t)
{
LOG.warn("Failed to check alias", t);
return false;
}

return true;
}
}
Expand Up @@ -28,13 +28,20 @@
* 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)
public boolean check(String pathInContext, Resource resource)
{
// Only support PathResource alias checking
if (!(resource instanceof PathResource))
Expand Down