it = namesList.iterator();
+ String rootName = it.next();
+ for (int i = 0; it.hasNext(); i++) {
+ names[i] = it.next();
+ }
+ Path cleanedPathToResolve = fs.getPath(rootName, names);
+ return cleanedPathToResolve;
+ }
}
diff --git a/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java b/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java
index 9ec46b830..2520753ab 100644
--- a/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/common/file/root/RootedFileSystemProviderTest.java
@@ -19,28 +19,29 @@
package org.apache.sshd.common.file.root;
+import java.io.File;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.channels.Channel;
import java.nio.channels.FileChannel;
-import java.nio.file.DirectoryStream;
-import java.nio.file.FileSystem;
-import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
-import java.nio.file.OpenOption;
-import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
-import java.nio.file.StandardOpenOption;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.*;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.Objects;
import java.util.Random;
import java.util.TreeSet;
import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.util.test.CommonTestSupportUtils;
import org.apache.sshd.util.test.NoIoTestCase;
-import org.junit.BeforeClass;
+import org.junit.Assert;
+import org.junit.Assume;
import org.junit.FixMethodOrder;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -49,7 +50,7 @@
/**
* Tests the RootedFileSystemProvider implementation of {@link java.nio.file.spi.FileSystemProvider} checking that
* permissions for generic FS commands are not permitted outside of the root directory.
- *
+ *
* Individual tests are form pairs (e.g. testX, testXInvalid) where testXInvalid is expected to test a parent path of
* {@link RootedFileSystem#getRoot()}
*
@@ -58,19 +59,21 @@
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
@Category({ NoIoTestCase.class })
public class RootedFileSystemProviderTest extends AssertableFile {
- private static RootedFileSystem fileSystem;
- private static Path rootSandbox;
+ private static final String SKIP_ON_WINDOWS = "Test fails due to windows normalizing paths before opening them, " +
+ "allowing one to open a file like \"C:\\directory_doesnt_exist\\..\\myfile.txt\" whereas this is blocked in unix";
+ private static final String DOESNT_EXIST = "../doesnt_exist/../";
- public RootedFileSystemProviderTest() {
- super();
- }
+ private final RootedFileSystem fileSystem;
+ private final Path rootSandbox;
+ private final FileHelper fileHelper;
- @BeforeClass
- public static void initializeFileSystem() throws IOException {
+ public RootedFileSystemProviderTest() throws Exception {
+ super();
+ fileHelper = new FileHelper();
Path targetFolder = Objects.requireNonNull(
CommonTestSupportUtils.detectTargetFolder(RootedFileSystemProviderTest.class),
"Failed to detect target folder");
- rootSandbox = FileHelper.createTestSandbox(targetFolder.resolve(TEMP_SUBFOLDER_NAME));
+ rootSandbox = fileHelper.createTestSandbox(targetFolder.resolve(TEMP_SUBFOLDER_NAME));
fileSystem = (RootedFileSystem) new RootedFileSystemProvider().newFileSystem(rootSandbox, Collections.emptyMap());
}
@@ -86,144 +89,216 @@ public void testRoot() {
/* mkdir */
@Test
public void testMkdir() throws IOException {
- Path created = FileHelper.createDirectory(fileSystem.getPath(getCurrentTestName()));
- assertTrue(exists(created) && isDir(created) && isReadable(created));
+ Path created = fileHelper.createDirectory(fileSystem.getPath(getCurrentTestName()));
+ try {
+ assertTrue(exists(created) && isDir(created) && isReadable(created));
+ } finally {
+ Files.delete(created);
+ }
}
- @Test(expected = InvalidPathException.class)
- public void testMkdirInvalid() throws IOException {
- Path parent = FileHelper.createDirectory(fileSystem.getPath("../" + getCurrentTestName()));
- fail(String.format("Unexpected success in creating directory %s", parent.toString()));
+ @Test
+ public void testMkdirInvalid() {
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+
+ String parent = DOESNT_EXIST + getCurrentTestName();
+ assertThrows(String.format("Unexpected success in creating directory %s", parent), NoSuchFileException.class,
+ () -> fileHelper.createDirectory(fileSystem.getPath(parent)));
}
/* rmdir */
@Test
public void testRmdir() throws IOException {
- Path created = FileHelper.createDirectory(fileSystem.getPath(getCurrentTestName()));
- Path deleted = FileHelper.deleteDirectory(created);
+ Path created = fileHelper.createDirectory(fileSystem.getPath(getCurrentTestName()));
+ Path deleted = fileHelper.deleteDirectory(created);
notExists(deleted);
}
- @Test(expected = InvalidPathException.class)
+ @Test(expected = NoSuchFileException.class)
public void testRmdirInvalid() throws IOException {
- Path deleted = FileHelper.deleteDirectory(fileSystem.getPath("../" + getCurrentTestName()));
+ Path deleted = fileHelper.deleteDirectory(fileSystem.getPath(DOESNT_EXIST + getCurrentTestName()));
fail(String.format("Unexpected success in removing directory %s", deleted.toString()));
}
/* chdir */
@Test
public void testChdir() throws IOException {
- Path created = FileHelper.createDirectory(fileSystem.getPath(getCurrentTestName()));
- Path createdFile = FileHelper.createFile(created.resolve(getCurrentTestName()));
- boolean hasFile = false;
- try (DirectoryStream ds = FileHelper.readDirectory(created)) {
- for (Path p : ds) {
- hasFile |= FileHelper.isSameFile(createdFile,
- fileSystem.getPath(created.getFileName() + "/" + p.getFileName()));
+ Path created = fileHelper.createDirectory(fileSystem.getPath(getCurrentTestName()));
+ Path createdFile = fileHelper.createFile(created.resolve(getCurrentTestName()));
+ try {
+ boolean hasFile = false;
+ try (DirectoryStream ds = fileHelper.readDirectory(created)) {
+ for (Path p : ds) {
+ hasFile |= fileHelper.isSameFile(createdFile,
+ fileSystem.getPath(created.getFileName() + "/" + p.getFileName()));
+ }
}
+ assertTrue(createdFile + " found in ch directory", hasFile);
+ } finally {
+ Files.delete(createdFile);
+ Files.delete(created);
}
- assertTrue(createdFile + " found in ch directory", hasFile);
- }
-
- @Test(expected = InvalidPathException.class)
- public void testChdirInvalid() throws IOException {
- Path chdir = FileHelper.createDirectory(fileSystem.getPath("../" + getCurrentTestName()));
- fail(String.format("Unexpected success in changing directory %s", chdir.toString()));
}
/* write */
@Test
public void testWriteFile() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
assertTrue(exists(created) && isReadable(created));
}
- @Test(expected = InvalidPathException.class)
+ @Test
public void testWriteFileInvalid() throws IOException {
- Path written = FileHelper.createFile(fileSystem.getPath("../" + getCurrentTestName()));
- fail(String.format("Unexpected success in writing file %s", written.toString()));
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+
+ String written = DOESNT_EXIST + getCurrentTestName();
+ assertThrows(String.format("Unexpected success in writing file %s", written), NoSuchFileException.class,
+ () -> fileHelper.createFile(fileSystem.getPath(written)));
}
/* read */
@Test
public void testReadFile() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
- isNonEmpty(FileHelper.readFile(created));
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ isNonEmpty(fileHelper.readFile(created));
}
- @Test(expected = InvalidPathException.class)
+ @Test(expected = NoSuchFileException.class)
public void testReadFileInvalid() throws IOException {
- Path read = fileSystem.getPath("../" + getCurrentTestName());
- FileHelper.readFile(read);
+ Path read = fileSystem.getPath(DOESNT_EXIST + getCurrentTestName());
+ fileHelper.readFile(read);
fail(String.format("Unexpected success in reading file %s", read.toString()));
}
/* rm */
@Test
public void testDeleteFile() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
- Path deleted = FileHelper.deleteFile(created);
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ Path deleted = fileHelper.deleteFile(created);
notExists(deleted);
}
- @Test(expected = InvalidPathException.class)
+ @Test(expected = NoSuchFileException.class)
public void testDeleteFileInvalid() throws IOException {
- Path deleted = FileHelper.deleteFile(fileSystem.getPath("../" + getCurrentTestName()));
+ Path deleted = fileHelper.deleteFile(fileSystem.getPath(DOESNT_EXIST + getCurrentTestName()));
fail(String.format("Unexpected success in deleting file %s", deleted.toString()));
}
/* cp */
@Test
public void testCopyFile() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
Path destination = fileSystem.getPath(getCurrentTestName() + "dest");
- FileHelper.copyFile(created, destination);
- assertTrue(exists(destination) && isReadable(destination));
+ try {
+ fileHelper.copyFile(created, destination);
+ assertTrue(exists(destination) && isReadable(destination));
+ } finally {
+ Files.delete(destination);
+ Files.delete(created);
+ }
}
- @Test(expected = InvalidPathException.class)
+ @Test
public void testCopyFileInvalid() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
- Path copy = FileHelper.copyFile(created, fileSystem.getPath("../" + getCurrentTestName()));
- fail(String.format("Unexpected success in copying file to %s", copy.toString()));
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ String copy = DOESNT_EXIST + getCurrentTestName();
+ assertThrows(String.format("Unexpected success in copying file to %s", copy),
+ NoSuchFileException.class,
+ () -> fileHelper.copyFile(created, fileSystem.getPath(copy)));
}
/* mv */
@Test
public void testMoveFile() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
Path destination = fileSystem.getPath(getCurrentTestName() + "dest");
- FileHelper.moveFile(created, destination);
+ fileHelper.moveFile(created, destination);
assertTrue(notExists(created) && exists(destination) && isReadable(destination));
}
- @Test(expected = InvalidPathException.class)
+ @Test
public void testMoveFileInvalid() throws IOException {
- Path created = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
- Path moved = FileHelper.moveFile(created, fileSystem.getPath("../" + getCurrentTestName()));
- fail(String.format("Unexpected success in moving file to %s", moved.toString()));
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+
+ Path created = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ String moved = DOESNT_EXIST + getCurrentTestName();
+ assertThrows(String.format("Unexpected success in moving file to %s", moved), NoSuchFileException.class,
+ () -> fileHelper.moveFile(created, fileSystem.getPath(moved)));
}
/* link */
@Test
public void testCreateLink() throws IOException {
- Path existing = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ Path existing = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
Path link = fileSystem.getPath(getCurrentTestName() + "link");
- FileHelper.createLink(link, existing);
- assertTrue(exists(link) && isReadable(link));
+ try {
+ fileHelper.createLink(link, existing);
+ assertTrue(exists(link) && isReadable(link));
+ } finally {
+ Files.delete(link);
+ Files.delete(existing);
+ }
+ }
+
+ @Test
+ public void testJailbreakLink() throws IOException {
+ testJailbreakLink("../");
+ }
+
+ @Test
+ public void testJailbreakLink2() throws IOException {
+ testJailbreakLink("../test/");
+ }
+
+ @Test
+ public void testJailbreakLink3() throws IOException {
+ testJailbreakLink("/..");
+ }
+
+ @Test
+ public void testJailbreakLink4() throws IOException {
+ testJailbreakLink("/./..");
+ }
+
+ @Test
+ public void testJailbreakLink5() throws IOException {
+ testJailbreakLink("/./../");
+ }
+
+ @Test
+ public void testJailbreakLink6() throws IOException {
+ testJailbreakLink("./../");
+ }
+
+ @Test
+ public void testJailbreakLink7() throws IOException {
+ String fileName = "/testdir/testdir2/../../..";
+ testJailbreakLink(fileName);
}
- @Test(expected = InvalidPathException.class)
+ private void testJailbreakLink(String jailbrokenTarget) throws IOException {
+ Path target = fileSystem.getPath(jailbrokenTarget);
+ Path linkPath = fileSystem.getPath("/testLink");
+ Assert.assertThrows(InvalidPathException.class, () -> fileSystem.provider().createSymbolicLink(linkPath, target));
+ Assert.assertFalse(Files.exists(linkPath));
+ }
+
+ @Test
public void testCreateLinkInvalid() throws IOException {
- Path existing = FileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
- Path link = FileHelper.createLink(fileSystem.getPath("../" + getCurrentTestName() + "link"), existing);
- fail(String.format("Unexpected success in linking file %s", link.toString()));
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+
+ Path existing = fileHelper.createFile(fileSystem.getPath(getCurrentTestName()));
+ String link = DOESNT_EXIST + getCurrentTestName() + "link";
+ assertThrows(String.format("Unexpected success in linking file %s", link), NoSuchFileException.class,
+ () -> fileHelper.createLink(fileSystem.getPath(link), existing));
}
@Test
public void testNewByteChannelProviderMismatchException() throws IOException {
RootedFileSystemProvider provider = new RootedFileSystemProvider();
- Path tempFolder = assertHierarchyTargetFolderExists(getTempTargetFolder());
+ Path tempFolder = getTempTargetFolder();
Path file = Files.createTempFile(tempFolder, getCurrentTestName(), ".txt");
try (FileSystem fs = provider.newFileSystem(tempFolder, Collections.emptyMap());
Channel channel = provider.newByteChannel(fs.getPath(file.getFileName().toString()), Collections.emptySet())) {
@@ -235,23 +310,212 @@ public void testNewByteChannelProviderMismatchException() throws IOException {
public void testResolveRoot() throws IOException {
Path root = GenericUtils.head(fileSystem.getRootDirectories());
Path dir = root.resolve("tsd");
- FileHelper.createDirectory(dir);
- Path f1 = FileHelper.createFile(dir.resolve("test.txt"));
- Path f2 = Files.newDirectoryStream(dir).iterator().next();
- assertTrue("Unrooted path found", f2 instanceof RootedPath);
- assertEquals(f1, f2);
- FileHelper.deleteFile(f1);
- FileHelper.deleteDirectory(dir);
+ fileHelper.createDirectory(dir);
+ Path f1 = fileHelper.createFile(dir.resolve("test.txt"));
+ try {
+ Path f2;
+ try (DirectoryStream ds = Files.newDirectoryStream(dir)) {
+ f2 = ds.iterator().next();
+ }
+ assertTrue("Unrooted path found", f2 instanceof RootedPath);
+ assertEquals(f1, f2);
+ } finally {
+ fileHelper.deleteFile(f1);
+ fileHelper.deleteDirectory(dir);
+ }
+ }
+
+ @Test
+ public void testBreakOutOfChroot1() throws IOException {
+ String fileName = "../" + getCurrentTestName();
+ testBreakOutOfChroot(fileName, fileName);
+ }
+
+ @Test
+ public void testBreakOutOfChroot2() throws IOException {
+ String fileName = "./../" + getCurrentTestName();
+ testBreakOutOfChroot(fileName, fileName);
+ }
+
+ @Test
+ public void testBreakOutOfChroot3() throws IOException {
+ String fileName = "/../" + getCurrentTestName();
+ testBreakOutOfChroot(fileName, fileName);
+ }
+
+ @Test
+ public void testBreakOutOfChroot4() throws IOException {
+ String fileName = "/.././" + getCurrentTestName();
+ testBreakOutOfChroot(fileName, fileName);
+ }
+
+ @Test
+ public void testBreakOutOfChroot5() throws IOException {
+ String fileName = "/./../" + getCurrentTestName();
+ testBreakOutOfChroot(fileName, fileName);
+ }
+
+ @Test
+ public void testBreakOutOfChroot6() throws IOException {
+ String fileName = "//../" + getCurrentTestName();
+ testBreakOutOfChroot(fileName, "/../" + getCurrentTestName());
+ }
+
+ /**
+ * Tests to make sure that the attempted break out of the chroot does not work with the specified filename
+ *
+ * @param fileName the filename to attempt to break out of the chroot with
+ * @throws IOException on test failure
+ */
+ private void testBreakOutOfChroot(String fileName, String expected) throws IOException {
+ RootedPath breakoutAttempt = fileSystem.getPath(fileName);
+
+ // make sure that our rooted fs behaves like a proper unix fs
+ Assert.assertEquals(expected, breakoutAttempt.toString());
+
+ Path expectedDir = fileSystem.getRoot().resolve(getCurrentTestName());
+ Path newDir = fileHelper.createDirectory(breakoutAttempt);
+ try {
+ assertTrue(Files.isDirectory(expectedDir));
+
+ String baseName = breakoutAttempt.getName(breakoutAttempt.getNameCount() - 1).toString();
+ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath(baseName)));
+
+ // make sure we didn't create it one directory out of the jail
+ assertFalse(Files.exists(fileSystem.getRoot().resolve("../" + breakoutAttempt.getFileName().toString())));
+
+ // make sure various methods of referencing the file work.
+ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath("/" + baseName)));
+ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath("/../../" + baseName)));
+ assertTrue(fileHelper.isSameFile(newDir, fileSystem.getPath("./../" + baseName)));
+ } finally {
+ // cleanup the directory.
+ fileHelper.deleteDirectory(newDir);
+ }
+
+ assertFalse(Files.isDirectory(expectedDir));
+ assertFalse(Files.isDirectory(newDir));
+ }
+
+ @Test
+ public void testValidSymlink1() throws IOException {
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+ String fileName = "/testdir/../";
+ testValidSymlink(fileName, true);
+ }
+
+ @Test
+ public void testValidSymlink2() throws IOException {
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+ String fileName = "/testdir/testdir2/../";
+ testValidSymlink(fileName, true);
+ }
+
+ @Test
+ public void testValidSymlink3() throws IOException {
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+ String fileName = "/testdir/../testdir3/";
+ testValidSymlink(fileName, true);
+ }
+
+ @Test
+ public void testValidSymlink4() throws IOException {
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+ String fileName = "testdir/../testdir3/../";
+ testValidSymlink(fileName, true);
+ }
+
+ @Test
+ public void testValidSymlink5() throws IOException {
+ Assume.assumeFalse(SKIP_ON_WINDOWS, OsUtils.isWin32());
+ String fileName = "testdir/../testdir3/../testfile";
+ testValidSymlink(fileName, false);
+ }
+
+ public void testValidSymlink(String symlink, boolean targetIsDirectory) throws IOException {
+ Path target = fileSystem.getPath(symlink);
+ Path linkPath = fileSystem.getPath("/testLink");
+ final List toDelete = new ArrayList<>();
+ try {
+ fileSystem.provider().createSymbolicLink(linkPath, target);
+ toDelete.add(linkPath);
+
+ // ensure that nothing processed the symlink.
+ Assert.assertEquals(Paths.get(symlink).toString(),
+ fileSystem.provider().readSymbolicLink(linkPath).toString());
+ Assert.assertFalse(Files.exists(target));
+ Assert.assertEquals(Files.exists(linkPath), Files.exists(target));
+
+ // If we don't follow the link, we simply check that the link exists, which it does as we created it.
+ Assert.assertTrue(Files.exists(linkPath, LinkOption.NOFOLLOW_LINKS));
+
+ createParentDirs(targetIsDirectory ? target : target.getParent(), toDelete);
+
+ if (!targetIsDirectory) {
+ Files.createFile(target);
+ toDelete.add(target);
+ }
+
+ Assert.assertTrue(Files.exists(linkPath));
+ } finally {
+ for (int i = toDelete.size() - 1; i >= 0; i--) {
+ Path path = toDelete.get(i);
+ try {
+ Files.delete(path);
+ } catch (IOException ex) {
+ // ignore as we might try to delete "/dir/.." which will fail as it contains dir..
+ }
+ }
+ }
+ }
+
+ private static void createParentDirs(Path target, List toDelete) throws IOException {
+ if (target.getParent() != null) {
+ createParentDirs(target.getParent(), toDelete);
+ }
+
+ if (!Files.isDirectory(target)) {
+ Files.createDirectories(target);
+ toDelete.add(target);
+ }
+ }
+
+ @Test
+ public void testFileNamedSlashOnUnixBasedOS() throws IOException {
+ // skip ths test on Win32
+ if (!"\\".equals(File.separator)) {
+ Path slashFile = fileSystem.getPath("\\");
+ Path created = fileHelper.createFile(slashFile);
+ try {
+ assertTrue(Files.isRegularFile(created));
+ } finally {
+ fileHelper.deleteFile(created);
+ }
+ }
+ }
+
+ @Test
+ public void testStreams() throws IOException {
+ byte[] data = "This is test data".getBytes(StandardCharsets.UTF_8);
+ RootedPath testPath = fileSystem.getPath("testfile.txt");
+ try (OutputStream is = Files.newOutputStream(testPath)) {
+ is.write(data);
+ }
+ byte[] read = new byte[data.length];
+ try (InputStream is = Files.newInputStream(testPath)) {
+ is.read(read);
+ }
+ assertArrayEquals(data, read);
}
/* Private helper */
/**
* Wrapper around the FileSystemProvider to test generic FS related commands. All created temp directories and files
- * used for testing are deleted upon JVM exit.
+ * used for testing must be deleted in the test which creates them
*/
@SuppressWarnings("synthetic-access")
- private static final class FileHelper {
+ private final class FileHelper {
private FileHelper() {
super();
}
@@ -263,71 +527,66 @@ private FileHelper() {
* @return the created sandbox Path
* @throws IOException on failure to create
*/
- public static Path createTestSandbox(Path tempDir) throws IOException {
- Path created = Files.createDirectories(tempDir.resolve(RootedFileSystemProviderTest.class.getSimpleName()));
- created.toFile().deleteOnExit();
+ public Path createTestSandbox(Path tempDir) throws IOException {
+ Path path = tempDir.resolve(RootedFileSystemProviderTest.class.getSimpleName());
+ Path created = Files.createDirectories(path);
return created;
}
- public static Path createFile(Path source) throws InvalidPathException, IOException {
+ public Path createFile(Path source) throws InvalidPathException, IOException {
try (FileChannel fc = fileSystem.provider().newFileChannel(source,
new TreeSet(Arrays.asList(StandardOpenOption.CREATE, StandardOpenOption.WRITE)))) {
byte[] randomBytes = new byte[1000];
new Random().nextBytes(randomBytes);
fc.write(ByteBuffer.wrap(randomBytes));
- source.toFile().deleteOnExit();
return source;
}
}
- public static Path createLink(Path link, Path existing) throws IOException {
+ public Path createLink(Path link, Path existing) throws IOException {
fileSystem.provider().createLink(link, existing);
- link.toFile().deleteOnExit();
return link;
}
- public static Path createDirectory(Path dir) throws InvalidPathException, IOException {
+ public Path createDirectory(Path dir) throws InvalidPathException, IOException {
fileSystem.provider().createDirectory(dir);
- dir.toFile().deleteOnExit();
return dir;
}
- public static Path deleteDirectory(Path dir) throws InvalidPathException, IOException {
+ public Path deleteDirectory(Path dir) throws InvalidPathException, IOException {
return deleteFile(dir);
}
- public static Path deleteFile(Path source) throws InvalidPathException, IOException {
+ public Path deleteFile(Path source) throws InvalidPathException, IOException {
fileSystem.provider().delete(source);
return source;
}
- public static byte[] readFile(Path source) throws IOException {
+ public byte[] readFile(Path source) throws IOException {
try (FileChannel fc = fileSystem.provider().newFileChannel(source,
- new TreeSet(Arrays.asList(StandardOpenOption.READ)))) {
- byte[] readBytes = new byte[(int) source.toFile().length()];
+ Collections.singleton(StandardOpenOption.READ))) {
+ byte[] readBytes = new byte[(int) Files.size(source)];
fc.read(ByteBuffer.wrap(readBytes));
return readBytes;
}
}
- public static Path copyFile(Path source, Path destination) throws InvalidPathException, IOException {
+ public Path copyFile(Path source, Path destination) throws InvalidPathException, IOException {
fileSystem.provider().copy(source, destination, StandardCopyOption.COPY_ATTRIBUTES);
- destination.toFile().deleteOnExit();
return destination;
}
- public static Path moveFile(Path source, Path destination) throws InvalidPathException, IOException {
+ public Path moveFile(Path source, Path destination) throws InvalidPathException, IOException {
fileSystem.provider().move(source, destination, StandardCopyOption.ATOMIC_MOVE);
- destination.toFile().deleteOnExit();
return destination;
}
- public static DirectoryStream readDirectory(Path dir) throws InvalidPathException, IOException {
+ public DirectoryStream readDirectory(Path dir) throws InvalidPathException, IOException {
DirectoryStream dirStream = fileSystem.provider().newDirectoryStream(dir, entry -> true);
return dirStream;
}
- public static boolean isSameFile(Path source, Path destination) throws IOException {
+ public boolean isSameFile(Path source, Path destination) throws IOException {
return fileSystem.provider().isSameFile(source, destination);
}
}
diff --git a/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java b/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
index a94811dc5..247e3d655 100644
--- a/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/common/util/io/IoUtilsTest.java
@@ -19,11 +19,17 @@
package org.apache.sshd.common.util.io;
+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 org.apache.sshd.common.util.NumberUtils;
+import org.apache.sshd.util.test.CommonTestSupportUtils;
import org.apache.sshd.util.test.JUnitTestSupport;
import org.apache.sshd.util.test.NoIoTestCase;
+import org.junit.Assert;
import org.junit.FixMethodOrder;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -58,4 +64,46 @@ public void testGetEOLBytes() {
}
}
+ /**
+ * Tests to make sure check exists does not follow symlinks.
+ *
+ * @throws IOException on failure
+ */
+ @Test
+ public void testCheckExists() throws IOException {
+ testCheckExists(Paths.get("target/IoUtilsTest").toAbsolutePath());
+ }
+
+ public void testCheckExists(Path baseDir) throws IOException {
+ CommonTestSupportUtils.deleteRecursive(baseDir, LinkOption.NOFOLLOW_LINKS);
+
+ Path folder = baseDir.resolve("folder1/folder2/");
+ Files.createDirectories(folder);
+
+ Path target = baseDir.resolve("folder1/target");
+ Files.createDirectories(target);
+
+ Path dirInTarget = baseDir.resolve("folder1/target/dirintarget");
+ Files.createDirectories(dirInTarget);
+
+ Files.createDirectories(target);
+ Path link = baseDir.resolve("folder1/folder2/link");
+ Files.createSymbolicLink(link, target);
+
+ Path link2 = baseDir.resolve("link");
+ Files.createSymbolicLink(link2, target);
+
+ Path targetWithLink = baseDir.resolve("folder1/folder2/link/dirintarget");
+
+ Assert.assertTrue("symlink follow should work", IoUtils.checkFileExists(targetWithLink));
+ Assert.assertTrue("symlink follow should work", IoUtils.checkFileExistsAnySymlinks(targetWithLink, false));
+
+ Assert.assertFalse("Link at end shouldn't be followed", IoUtils.checkFileExistsAnySymlinks(link, true));
+ Assert.assertFalse("Nofollow shouldn't follow directory",
+ IoUtils.checkFileExistsAnySymlinks(targetWithLink, true));
+ Assert.assertFalse("Link at beginning shouldn't be followed",
+ IoUtils.checkFileExistsAnySymlinks(link2, true));
+ Assert.assertTrue("Root directory must exist",
+ IoUtils.checkFileExistsAnySymlinks(baseDir, true));
+ }
}
diff --git a/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java b/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java
index 829f25bcd..57f43fbe3 100644
--- a/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java
+++ b/sshd-common/src/test/java/org/apache/sshd/util/test/CommonTestSupportUtils.java
@@ -495,11 +495,11 @@ private static Path getFile(String resource) {
* @throws IOException If failed to access/remove some file(s)
*/
public static Path deleteRecursive(Path path, LinkOption... options) throws IOException {
- if ((path == null) || (!Files.exists(path))) {
+ if ((path == null) || (!Files.exists(path, options))) {
return path;
}
- if (Files.isDirectory(path)) {
+ if (Files.isDirectory(path, options)) {
try (DirectoryStream ds = Files.newDirectoryStream(path)) {
for (Path child : ds) {
deleteRecursive(child, options);
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
index 9638ee9c8..607cd6f5e 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java
@@ -645,7 +645,7 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep
}
// delete target if it exists and REPLACE_EXISTING is specified
- Boolean status = IoUtils.checkFileExists(target, linkOptions);
+ Boolean status = IoUtils.checkFileExistsAnySymlinks(target, noFollowLinks);
if (status == null) {
throw new AccessDeniedException("Existence cannot be determined for copy target: " + target);
}
@@ -724,7 +724,7 @@ public void move(Path source, Path target, CopyOption... options) throws IOExcep
}
// delete target if it exists and REPLACE_EXISTING is specified
- Boolean status = IoUtils.checkFileExists(target, linkOptions);
+ Boolean status = IoUtils.checkFileExistsAnySymlinks(target, noFollowLinks);
if (status == null) {
throw new AccessDeniedException("Existence cannot be determined for move target " + target);
}
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
index 844842f84..6392a588c 100755
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java
@@ -36,6 +36,7 @@
import java.nio.file.LinkOption;
import java.nio.file.NoSuchFileException;
import java.nio.file.NotDirectoryException;
+import java.nio.file.OpenOption;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
@@ -51,7 +52,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
-import java.util.EnumSet;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedList;
@@ -645,9 +645,11 @@ protected Map doLStat(int id, String path, int flags) throws IOE
* SSH_FXP_LSTAT does not.
*/
SftpFileSystemAccessor accessor = getFileSystemAccessor();
+
+ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_LSTAT, "", p);
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
this, p, SftpConstants.SSH_FXP_LSTAT, "", false);
- return resolveFileAttributes(p, flags, options);
+ return resolveFileAttributes(p, flags, !followLinks, options);
}
protected void doSetStat(
@@ -675,7 +677,7 @@ protected void doSetStat(
Path p = resolveFile(path);
if (followLinks == null) {
- followLinks = resolvePathResolutionFollowLinks(cmd, extension, p);
+ followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_SETSTAT, extension, p);
}
doSetAttributes(cmd, extension, p, attrs, followLinks);
}
@@ -1408,12 +1410,14 @@ protected Map doStat(int id, String path, int flags) throws IOEx
*/
Path p = resolveFile(path);
SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_STAT, "", p);
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
- this, p, SftpConstants.SSH_FXP_STAT, "", true);
- return resolveFileAttributes(p, flags, options);
+ this, p, SftpConstants.SSH_FXP_STAT, "", followLinks);
+ return resolveFileAttributes(p, flags, !followLinks, options);
}
protected void doRealPath(Buffer buffer, int id) throws IOException {
+ // do things here.
String path = buffer.getString();
boolean debugEnabled = log.isDebugEnabled();
ServerSession session = getServerSession();
@@ -1467,7 +1471,6 @@ protected void doRealPath(Buffer buffer, int id) throws IOException {
result = doRealPathV6(id, path, extraPaths, p, options);
p = result.getKey();
- options = getPathResolutionLinkOption(SftpConstants.SSH_FXP_REALPATH, "", p);
Boolean status = result.getValue();
switch (control) {
case SftpConstants.SSH_FXP_REALPATH_STAT_IF:
@@ -1557,14 +1560,14 @@ protected SimpleImmutableEntry validateRealPath(
int id, String path, Path f, LinkOption... options)
throws IOException {
Path p = normalize(f);
- Boolean status = IoUtils.checkFileExists(p, options);
+ Boolean status = IoUtils.checkFileExistsAnySymlinks(p, !IoUtils.followLinks(options));
return new SimpleImmutableEntry<>(p, status);
}
protected void doRemoveDirectory(Buffer buffer, int id) throws IOException {
String path = buffer.getString();
try {
- doRemoveDirectory(id, path, false);
+ doRemoveDirectory(id, path);
} catch (IOException | RuntimeException e) {
sendStatus(prepareReply(buffer), id, e,
SftpConstants.SSH_FXP_RMDIR, path);
@@ -1574,15 +1577,23 @@ protected void doRemoveDirectory(Buffer buffer, int id) throws IOException {
sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OK, "");
}
- protected void doRemoveDirectory(int id, String path, boolean followLinks) throws IOException {
+ protected void doRemoveDirectory(int id, String path) throws IOException {
Path p = resolveFile(path);
if (log.isDebugEnabled()) {
log.debug("doRemoveDirectory({})[id={}] SSH_FXP_RMDIR (path={})[{}]", getServerSession(), id, path, p);
}
SftpFileSystemAccessor accessor = getFileSystemAccessor();
+
+ final boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_RMDIR, "", p);
+ Boolean symlinkCheck = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(p, !followLinks);
+ if (!Boolean.TRUE.equals(symlinkCheck)) {
+ throw new AccessDeniedException(p.toString(), p.toString(),
+ "Parent directories do not exist ore are prohibited symlinks");
+ }
+
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
- this, p, SftpConstants.SSH_FXP_RMDIR, "", followLinks);
+ this, p, SftpConstants.SSH_FXP_RMDIR, "", false);
if (Files.isDirectory(p, options)) {
doRemove(id, p, true);
} else {
@@ -1618,7 +1629,7 @@ protected void doMakeDirectory(Buffer buffer, int id) throws IOException {
String path = buffer.getString();
Map attrs = readAttrs(buffer);
try {
- doMakeDirectory(id, path, attrs, false);
+ doMakeDirectory(id, path, attrs);
} catch (IOException | RuntimeException e) {
sendStatus(prepareReply(buffer), id, e,
SftpConstants.SSH_FXP_MKDIR, path, attrs);
@@ -1629,7 +1640,7 @@ protected void doMakeDirectory(Buffer buffer, int id) throws IOException {
}
protected void doMakeDirectory(
- int id, String path, Map attrs, boolean followLinks)
+ int id, String path, Map attrs)
throws IOException {
Path resolvedPath = resolveFile(path);
ServerSession session = getServerSession();
@@ -1640,14 +1651,21 @@ protected void doMakeDirectory(
SftpFileSystemAccessor accessor = getFileSystemAccessor();
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
- this, resolvedPath, SftpConstants.SSH_FXP_MKDIR, "", followLinks);
+ this, resolvedPath, SftpConstants.SSH_FXP_MKDIR, "", false);
+ final boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath);
SftpPathImpl.withAttributeCache(resolvedPath, p -> {
- Boolean status = IoUtils.checkFileExists(p, options);
- if (status == null) {
+ Boolean symlinkCheck = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(p, !followLinks);
+ if (!Boolean.TRUE.equals(symlinkCheck)) {
+ throw new AccessDeniedException(p.toString(), p.toString(),
+ "Parent directories do not exist ore are prohibited symlinks");
+ }
+
+ Boolean fileExists = IoUtils.checkFileExists(p, options);
+ if (fileExists == null) {
throw new AccessDeniedException(p.toString(), p.toString(), "Cannot validate make-directory existence");
}
- if (status) {
+ if (fileExists) {
if (Files.isDirectory(p, options)) {
throw new FileAlreadyExistsException(p.toString(), p.toString(), "Target directory already exists");
} else {
@@ -1661,7 +1679,6 @@ protected void doMakeDirectory(
listener.creating(session, resolvedPath, attrs);
try {
accessor.createDirectory(this, resolvedPath);
- followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath);
doSetAttributes(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath, attrs, followLinks);
} catch (IOException | RuntimeException | Error e) {
listener.created(session, resolvedPath, attrs, e);
@@ -1676,7 +1693,7 @@ protected void doRemove(Buffer buffer, int id) throws IOException {
/*
* If 'filename' is a symbolic link, the link is removed, not the file it points to.
*/
- doRemoveFile(id, path, false);
+ doRemoveFile(id, path);
} catch (IOException | RuntimeException e) {
sendStatus(prepareReply(buffer), id, e, SftpConstants.SSH_FXP_REMOVE, path);
return;
@@ -1685,17 +1702,20 @@ protected void doRemove(Buffer buffer, int id) throws IOException {
sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OK, "");
}
- protected void doRemoveFile(int id, String path, boolean followLinks) throws IOException {
+ protected void doRemoveFile(int id, String path) throws IOException {
Path resolvedPath = resolveFile(path);
if (log.isDebugEnabled()) {
log.debug("doRemoveFile({})[id={}] SSH_FXP_REMOVE (path={}[{}])", getServerSession(), id, path, resolvedPath);
}
+ // whether to follow links in the dir up to the final file
+ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_REMOVE, "", resolvedPath);
SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ // never resolve links in the final path to remove as we want to remove the symlink, not the target
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
- this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", followLinks);
+ this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", false);
SftpPathImpl.withAttributeCache(resolvedPath, p -> {
- Boolean status = IoUtils.checkFileExists(p, options);
+ Boolean status = checkSymlinkState(p, followLinks, options);
if (status == null) {
throw signalRemovalPreConditionFailure(id, path, p,
new AccessDeniedException(p.toString(), p.toString(), "Cannot determine existence of remove candidate"),
@@ -2293,8 +2313,9 @@ protected void writeDirEntry(
int id, DirectoryHandle dir, Map entries, Buffer buffer,
int index, Path f, String shortName, LinkOption... options)
throws IOException {
+ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_READDIR, "", f);
Map attrs = resolveFileAttributes(
- f, SftpConstants.SSH_FILEXFER_ATTR_ALL, options);
+ f, SftpConstants.SSH_FILEXFER_ATTR_ALL, !followLinks, options);
entries.put(shortName, f);
SftpFileSystemAccessor accessor = getFileSystemAccessor();
@@ -2392,10 +2413,10 @@ protected String getShortName(Path f) throws IOException {
}
protected NavigableMap resolveFileAttributes(
- Path path, int flags, LinkOption... options)
+ Path path, int flags, boolean neverFollowSymLinks, LinkOption... options)
throws IOException {
return SftpPathImpl.withAttributeCache(path, file -> {
- Boolean status = IoUtils.checkFileExists(file, options);
+ Boolean status = checkSymlinkState(file, neverFollowSymLinks, options);
if (status == null) {
return handleUnknownStatusFileAttributes(file, flags, options);
} else if (!status) {
@@ -2406,7 +2427,31 @@ protected NavigableMap resolveFileAttributes(
});
}
- protected void writeAttrs(Buffer buffer, Map attributes) throws IOException {
+ /**
+ * A utility function to validate that the directories leading up to a file are not symlinks
+ *
+ * @param path the file to check for symlink presence
+ * @param neverFollowSymLinks whether to never follow symlinks in the parent paths
+ * @param options whether the file itself can be a symlink
+ * @return whether there are symlinks in the path to this file, or null if unknown
+ */
+ public Boolean checkSymlinkState(Path path, boolean neverFollowSymLinks, LinkOption[] options) {
+ Boolean status = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(path, neverFollowSymLinks);
+ if (!Boolean.FALSE.equals(status)) {
+ status = IoUtils.checkFileExists(path, options);
+ }
+ return status;
+ }
+
+ public Boolean validateParentExistWithNoSymlinksIfNeverFollowSymlinks(Path path, boolean neverFollowSymLinks) {
+ Boolean status = true;
+ if (neverFollowSymLinks && path.getParent() != null) {
+ status = IoUtils.checkFileExistsAnySymlinks(path.getParent(), true);
+ }
+ return status;
+ }
+
+ protected void writeAttrs(Buffer buffer, Map attributes) {
SftpHelper.writeAttrs(buffer, getVersion(), attributes);
}
@@ -2653,7 +2698,11 @@ protected void setFileAttributes(
case IoUtils.SIZE_VIEW_ATTR: {
long newSize = ((Number) value).longValue();
SftpFileSystemAccessor accessor = getFileSystemAccessor();
- Set openOptions = EnumSet.of(StandardOpenOption.WRITE);
+ Set openOptions = new HashSet<>();
+ openOptions.add(StandardOpenOption.WRITE);
+ if (!IoUtils.followLinks(options)) {
+ openOptions.add(LinkOption.NOFOLLOW_LINKS);
+ }
try (SeekableByteChannel channel = accessor.openFile(this, null, file, null, openOptions)) {
channel.truncate(newSize);
accessor.closeFile(this, null, file, null, channel, openOptions);
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java
index 6d7c18fc1..2f6ab0553 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpFileSystemAccessor.java
@@ -19,20 +19,14 @@
package org.apache.sshd.sftp.server;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.StreamCorruptedException;
import java.nio.channels.Channel;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.channels.SeekableByteChannel;
-import java.nio.file.CopyOption;
-import java.nio.file.DirectoryStream;
-import java.nio.file.FileSystem;
-import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
-import java.nio.file.LinkOption;
-import java.nio.file.OpenOption;
-import java.nio.file.Path;
+import java.nio.file.*;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.AclFileAttributeView;
import java.nio.file.attribute.FileAttribute;
@@ -44,8 +38,10 @@
import java.nio.file.attribute.UserPrincipalLookupService;
import java.nio.file.attribute.UserPrincipalNotFoundException;
import java.security.Principal;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.NavigableMap;
@@ -214,7 +210,11 @@ default SeekableByteChannel openFile(
attrs = IoUtils.EMPTY_FILE_ATTRIBUTES;
}
- return FileChannel.open(file, options, attrs);
+ // Don't use Set contains as this can fail with TreeSet
+ if (!Arrays.asList(options.toArray(new OpenOption[0])).contains(LinkOption.NOFOLLOW_LINKS)) {
+ return FileChannel.open(file, options, attrs);
+ }
+ return seekableByteChannelNoLinkFollow(file, options, attrs);
}
/**
@@ -317,9 +317,12 @@ default void closeFile(
* @throws IOException If failed to open
*/
default DirectoryStream openDirectory(
- SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle)
+ SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle, LinkOption... linkOptions)
throws IOException {
- return Files.newDirectoryStream(dir);
+ if (IoUtils.followLinks(linkOptions)) {
+ return Files.newDirectoryStream(dir);
+ }
+ return secureResolveDirectoryStream(dir);
}
/**
@@ -523,15 +526,84 @@ default void renameFile(
default void copyFile(
SftpSubsystemProxy subsystem, Path src, Path dst, Collection opts)
throws IOException {
+
+ if (noFollow(opts)) {
+ try (SeekableByteChannel srcFile
+ = seekableByteChannelNoLinkFollow(src, Collections.singleton(StandardOpenOption.READ))) {
+ if (!(srcFile instanceof FileChannel)) {
+ throw new UnsupportedOperationException("Host file system must return a file channel");
+ }
+
+ try (SeekableByteChannel dstFile = seekableByteChannelNoLinkFollow(src, new HashSet<>(Arrays
+ .asList(StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE)))) {
+ ((FileChannel) srcFile).transferTo(0, srcFile.size(), dstFile);
+ }
+ }
+ return;
+ }
Files.copy(src, dst,
GenericUtils.isEmpty(opts)
? IoUtils.EMPTY_COPY_OPTIONS
: opts.toArray(new CopyOption[opts.size()]));
}
+ static SeekableByteChannel seekableByteChannelNoLinkFollow(
+ Path src, Set extends OpenOption> opts, FileAttribute>... fileAttributes)
+ throws IOException {
+ if (src.getNameCount() < 1) {
+ // opening root directory isn't supported.
+ throw new IllegalArgumentException();
+ }
+ Path toResolve = src.isAbsolute() ? src : src.getFileSystem().getPath(src.toString());
+
+ if (!Files.isDirectory(src.getParent(), LinkOption.NOFOLLOW_LINKS)) {
+ throw new FileNotFoundException(src.getParent().toString());
+ }
+
+ toResolve = toResolve.normalize();
+ try (SecureDirectoryStream ds = secureResolveDirectoryStream(toResolve.getParent())) {
+ Set newOpts = new HashSet<>(opts);
+ newOpts.add(LinkOption.NOFOLLOW_LINKS);
+ return ds.newByteChannel(toResolve.getName(toResolve.getNameCount() - 1), newOpts, fileAttributes);
+ }
+ }
+
+ static SecureDirectoryStream secureResolveDirectoryStream(Path toResolve) throws IOException {
+ toResolve = IoUtils.removeCdUpAboveRoot(toResolve);
+ DirectoryStream ds = Files.newDirectoryStream(toResolve.getRoot());
+ for (int i = 0; i < toResolve.getNameCount(); i++) {
+ DirectoryStream dsOld = ds;
+ try {
+ ds = secure(ds).newDirectoryStream(toResolve.getName(i), LinkOption.NOFOLLOW_LINKS);
+ dsOld.close();
+ } catch (IOException ex) {
+ ds.close();
+ throw ex;
+ }
+ }
+ return secure(ds);
+ }
+
+ static SecureDirectoryStream secure(DirectoryStream ds) {
+ if (ds instanceof SecureDirectoryStream) {
+ return (SecureDirectoryStream) ds;
+ }
+ // do we want to bomb? do we want a different fallback option?
+ throw new UnsupportedOperationException("FS Does not support secure directory streams.");
+ }
+
default void removeFile(
SftpSubsystemProxy subsystem, Path path, boolean isDirectory)
throws IOException {
Files.delete(path);
}
+
+ default boolean noFollow(Collection> opts) {
+ for (Object opt : opts) {
+ if (LinkOption.NOFOLLOW_LINKS.equals(opt)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
index 7dc391d1f..8011cc810 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java
@@ -687,7 +687,7 @@ protected void doReadDir(Buffer buffer, int id) throws IOException {
// the upstream server decide.
if (!(file instanceof SftpPath)) {
LinkOption[] options = getPathResolutionLinkOption(SftpConstants.SSH_FXP_READDIR, "", file);
- Boolean status = IoUtils.checkFileExists(file, options);
+ Boolean status = IoUtils.checkFileExistsAnySymlinks(file, !IoUtils.followLinks(options));
if (status == null) {
throw new AccessDeniedException(file.toString(), file.toString(), "Cannot determine existence of read-dir");
}
@@ -748,7 +748,7 @@ protected void doReadDir(Buffer buffer, int id) throws IOException {
@Override
protected String doOpenDir(int id, String path, Path dir, LinkOption... options) throws IOException {
SftpPathImpl.withAttributeCache(dir, p -> {
- Boolean status = IoUtils.checkFileExists(p, options);
+ Boolean status = IoUtils.checkFileExistsAnySymlinks(p, !IoUtils.followLinks(options));
if (status == null) {
throw signalOpenFailure(id, path, p, true,
new AccessDeniedException(p.toString(), p.toString(), "Cannot determine open-dir existence"));
@@ -808,7 +808,9 @@ protected Map doFStat(int id, String handle, int flags) throws I
Path file = fileHandle.getFile();
LinkOption[] options = accessor.resolveFileAccessLinkOptions(
this, file, SftpConstants.SSH_FXP_FSTAT, "", true);
- return resolveFileAttributes(file, flags, options);
+
+ boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_FSTAT, handle, file);
+ return resolveFileAttributes(file, flags, followLinks, options);
}
@Override
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
index b150e3aa0..a0b408d1b 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpTest.java
@@ -383,7 +383,7 @@ private void testCannotEscapeRoot(boolean useAbsolutePath) throws Exception {
SftpClient.Attributes attrs = sftp.stat(escapePath);
fail("Unexpected escape success for path=" + escapePath + ": " + attrs);
} catch (SftpException e) {
- int expected = OsUtils.isWin32() || (!useAbsolutePath)
+ int expected = OsUtils.isWin32() && useAbsolutePath
? SftpConstants.SSH_FX_INVALID_FILENAME
: SftpConstants.SSH_FX_NO_SUCH_FILE;
assertEquals("Mismatched status for " + escapePath,
@@ -711,10 +711,11 @@ public SeekableByteChannel openFile(
@Override
public DirectoryStream openDirectory(
- SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle)
+ SftpSubsystemProxy subsystem, DirectoryHandle dirHandle, Path dir, String handle,
+ LinkOption... linkOptions)
throws IOException {
dirHolder.set(dir);
- return SftpFileSystemAccessor.super.openDirectory(subsystem, dirHandle, dir, handle);
+ return SftpFileSystemAccessor.super.openDirectory(subsystem, dirHandle, dir, handle, linkOptions);
}
@Override
diff --git a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
index d277bbc88..c18a3d7cd 100644
--- a/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
+++ b/sshd-sftp/src/test/java/org/apache/sshd/sftp/client/SftpVersionsTest.java
@@ -291,9 +291,11 @@ public void testSftpACLEncodeDecode() throws Exception {
public Command createSubsystem(ChannelSession channel) throws IOException {
SftpSubsystem subsystem = new SftpSubsystem(channel, this) {
@Override
- protected NavigableMap resolveFileAttributes(Path file, int flags, LinkOption... options)
+ protected NavigableMap resolveFileAttributes(
+ Path file, int flags, boolean neverFollowSymLinks, LinkOption... options)
throws IOException {
- NavigableMap attrs = super.resolveFileAttributes(file, flags, options);
+ NavigableMap attrs
+ = super.resolveFileAttributes(file, flags, neverFollowSymLinks, options);
if (MapEntryUtils.isEmpty(attrs)) {
attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}
@@ -412,9 +414,11 @@ public void testSftpExtensionsEncodeDecode() throws Exception {
public Command createSubsystem(ChannelSession channel) throws IOException {
SftpSubsystem subsystem = new SftpSubsystem(channel, this) {
@Override
- protected NavigableMap resolveFileAttributes(Path file, int flags, LinkOption... options)
+ protected NavigableMap resolveFileAttributes(
+ Path file, int flags, boolean neverFollowLinks, LinkOption... options)
throws IOException {
- NavigableMap attrs = super.resolveFileAttributes(file, flags, options);
+ NavigableMap attrs
+ = super.resolveFileAttributes(file, flags, neverFollowLinks, options);
if (MapEntryUtils.isEmpty(attrs)) {
attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}