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

jetty-gcloud/jetty-gcloud-session-manager/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionTester.java

@@ -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;
@@ -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());

jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java

@@ -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;
@@ -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));
+    }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/AllowedResourceAliasChecker.java

@@ -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);
+    }
+}

jetty-server/src/main/java/org/eclipse/jetty/server/SameFileAliasChecker.java

@@ -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);

jetty-server/src/main/java/org/eclipse/jetty/server/SymlinkAllowedResourceAliasChecker.java

@@ -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;
+    }
+}