You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2021/03/28 06:55:57 UTC
[mina-sshd] branch master updated: [SSHD-1137] Added capability to
override used LinkOption(s) when accessing a file/folder via SFTP
This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new 9d3443c [SSHD-1137] Added capability to override used LinkOption(s) when accessing a file/folder via SFTP
9d3443c is described below
commit 9d3443c14eb5c1df24a182bed4b1ec3e5d6c15fd
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Sun Mar 28 09:55:17 2021 +0300
[SSHD-1137] Added capability to override used LinkOption(s) when accessing a file/folder via SFTP
---
CHANGES.md | 1 +
docs/sftp.md | 8 +--
.../sftp/server/AbstractSftpSubsystemHelper.java | 84 +++++++++++++---------
.../org/apache/sshd/sftp/server/FileHandle.java | 2 +-
.../sshd/sftp/server/SftpFileSystemAccessor.java | 20 ++++++
.../org/apache/sshd/sftp/server/SftpSubsystem.java | 39 ++++++----
6 files changed, 102 insertions(+), 52 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 3d9e0e2..8cf7dc8 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -54,4 +54,5 @@
* [SSHD-1133](https://issues.apache.org/jira/browse/SSHD-1133) Added capability to specify a custom charset for parsing incoming commands to the `ScpShell`
* [SSHD-1133](https://issues.apache.org/jira/browse/SSHD-1133) Added capability to specify a custom charset for returning environment variables related data from the `ScpShell`
* [SSHD-1133](https://issues.apache.org/jira/browse/SSHD-1133) Added capability to specify a custom charset for handling the SCP protocol textual commands and responses
+* [SSHD-1137](https://issues.apache.org/jira/browse/SSHD-1137) Added capability to override LinkOption(s) when accessing a file/folder via SFTP
* [SSHD-1147](https://issues.apache.org/jira/browse/SSHD-1147) SftpInputStreamAsync: get file size before SSH_FXP_OPEN
\ No newline at end of file
diff --git a/docs/sftp.md b/docs/sftp.md
index 3a5c083..4d60f86 100644
--- a/docs/sftp.md
+++ b/docs/sftp.md
@@ -124,13 +124,9 @@ Same logic as the STDERR incoming data applies to the outgoing error I/O streams
### Symbolic links handling
-Whenever the server needs to execute a command that may behave differently if applied to a symbolic link instead of its target
-it consults the `AbstractSftpSubsystemHelper#resolvePathResolutionFollowLinks` method. By default, this method simply consults
-the value of the `sftp-auto-follow-links` configuration property (default=*true*).
+Whenever the server needs to execute a command that may behave differently if applied to a symbolic link instead of its target it consults the `AbstractSftpSubsystemHelper#resolvePathResolutionFollowLinks` method. By default, this method simply consultsthe value of the `sftp-auto-follow-links` configuration property (default=*true*).
-**Note:** the property is consulted only for cases where there is no clear indication in the standard how to behave for the
-specific command. E.g., the `lsetstat@openssh.com` command specifically specifies that symbolic links should not be followed,
-so the implementation does not consult the aforementioned property.
+**Note:** the property is consulted only for cases where there is no clear indication in the standard how to behave for the specific command. E.g., the `lsetstat@openssh.com` command specifically specifies that symbolic links should not be followed, so the implementation does not consult the aforementioned property. However, the final decision what `LinkOption`(s) to use is left to the `SftpFileSystemAccessor#resolveFileAccessLinkOptions` method (which by default does not interfere in th [...]
## Client-side SFTP
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 a7f0c75..05f6e74 100644
--- 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
@@ -617,16 +617,20 @@ public abstract class AbstractSftpSubsystemHelper
protected Map<String, Object> doLStat(int id, String path, int flags) throws IOException {
Path p = resolveFile(path);
+ ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
log.debug("doLStat({})[id={}] SSH_FXP_LSTAT (path={}[{}], flags=0x{})",
- getServerSession(), id, path, p, Integer.toHexString(flags));
+ session, id, path, p, Integer.toHexString(flags));
}
/*
* SSH_FXP_STAT and SSH_FXP_LSTAT only differ in that SSH_FXP_STAT follows symbolic links on the server, whereas
* SSH_FXP_LSTAT does not.
*/
- return resolveFileAttributes(p, flags, IoUtils.getLinkOptions(false));
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, p, SftpConstants.SSH_FXP_LSTAT, "", false);
+ return resolveFileAttributes(p, flags, options);
}
protected void doSetStat(
@@ -645,21 +649,19 @@ public abstract class AbstractSftpSubsystemHelper
}
protected void doSetStat(
- int id, String path, int cmd, String extension, Map<String, ?> attrs, Boolean followLinks /*
- * null =
- * auto-resolve
- */)
+ int id, String path, int cmd, String extension, Map<String, ?> attrs, Boolean followLinks)
throws IOException {
if (log.isDebugEnabled()) {
+ ServerSession session = getServerSession();
log.debug("doSetStat({})[id={}, cmd={}, extension={}] (path={}, attrs={}, followLinks={})",
- getServerSession(), id, cmd, extension, path, attrs, followLinks);
+ session, id, cmd, extension, path, attrs, followLinks);
}
Path p = resolveFile(path);
if (followLinks == null) {
followLinks = resolvePathResolutionFollowLinks(cmd, extension, p);
}
- doSetAttributes(p, attrs, followLinks);
+ doSetAttributes(cmd, extension, p, attrs, followLinks);
}
protected void doFStat(Buffer buffer, int id) throws IOException {
@@ -1348,9 +1350,10 @@ public abstract class AbstractSftpSubsystemHelper
}
protected Map<String, Object> doStat(int id, String path, int flags) throws IOException {
+ ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
log.debug("doStat({})[id={}] SSH_FXP_STAT (path={}, flags=0x{})",
- getServerSession(), id, path, Integer.toHexString(flags));
+ session, id, path, Integer.toHexString(flags));
}
/*
@@ -1358,7 +1361,10 @@ public abstract class AbstractSftpSubsystemHelper
* SSH_FXP_LSTAT does not.
*/
Path p = resolveFile(path);
- return resolveFileAttributes(p, flags, IoUtils.getLinkOptions(true));
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, p, SftpConstants.SSH_FXP_STAT, "", true);
+ return resolveFileAttributes(p, flags, options);
}
protected void doRealPath(Buffer buffer, int id) throws IOException {
@@ -1513,7 +1519,7 @@ public abstract class AbstractSftpSubsystemHelper
protected void doRemoveDirectory(Buffer buffer, int id) throws IOException {
String path = buffer.getString();
try {
- doRemoveDirectory(id, path, IoUtils.getLinkOptions(false));
+ doRemoveDirectory(id, path, false);
} catch (IOException | RuntimeException e) {
sendStatus(prepareReply(buffer), id, e,
SftpConstants.SSH_FXP_RMDIR, path);
@@ -1523,14 +1529,16 @@ public abstract class AbstractSftpSubsystemHelper
sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OK, "");
}
- protected void doRemoveDirectory(
- int id, String path, LinkOption... options)
- throws IOException {
+ protected void doRemoveDirectory(int id, String path, boolean followLinks) throws IOException {
Path p = resolveFile(path);
+ ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
- log.debug("doRemoveDirectory({})[id={}] SSH_FXP_RMDIR (path={})[{}]",
- getServerSession(), id, path, p);
+ log.debug("doRemoveDirectory({})[id={}] SSH_FXP_RMDIR (path={})[{}]", session, id, path, p);
}
+
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, p, SftpConstants.SSH_FXP_RMDIR, "", followLinks);
if (Files.isDirectory(p, options)) {
doRemove(id, p, true);
} else {
@@ -1566,7 +1574,7 @@ public abstract class AbstractSftpSubsystemHelper
String path = buffer.getString();
Map<String, ?> attrs = readAttrs(buffer);
try {
- doMakeDirectory(id, path, attrs, IoUtils.getLinkOptions(false));
+ doMakeDirectory(id, path, attrs, false);
} catch (IOException | RuntimeException e) {
sendStatus(prepareReply(buffer), id, e,
SftpConstants.SSH_FXP_MKDIR, path, attrs);
@@ -1577,7 +1585,7 @@ public abstract class AbstractSftpSubsystemHelper
}
protected void doMakeDirectory(
- int id, String path, Map<String, ?> attrs, LinkOption... options)
+ int id, String path, Map<String, ?> attrs, boolean followLinks)
throws IOException {
Path p = resolveFile(path);
ServerSession session = getServerSession();
@@ -1586,6 +1594,9 @@ public abstract class AbstractSftpSubsystemHelper
session, id, path, p, attrs);
}
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, p, SftpConstants.SSH_FXP_MKDIR, "", followLinks);
Boolean status = IoUtils.checkFileExists(p, options);
if (status == null) {
throw new AccessDeniedException(
@@ -1604,11 +1615,9 @@ public abstract class AbstractSftpSubsystemHelper
SftpEventListener listener = getSftpEventListenerProxy();
listener.creating(session, p, attrs);
try {
- SftpFileSystemAccessor accessor = getFileSystemAccessor();
accessor.createDirectory(session, this, p);
- boolean followLinks = resolvePathResolutionFollowLinks(
- SftpConstants.SSH_FXP_MKDIR, "", p);
- doSetAttributes(p, attrs, followLinks);
+ followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_MKDIR, "", p);
+ doSetAttributes(SftpConstants.SSH_FXP_MKDIR, "", p, attrs, followLinks);
} catch (IOException | RuntimeException | Error e) {
listener.created(session, p, attrs, e);
throw e;
@@ -1623,7 +1632,7 @@ public abstract class AbstractSftpSubsystemHelper
/*
* If 'filename' is a symbolic link, the link is removed, not the file it points to.
*/
- doRemove(id, path, IoUtils.getLinkOptions(false));
+ doRemoveFile(id, path, false);
} catch (IOException | RuntimeException e) {
sendStatus(prepareReply(buffer), id, e, SftpConstants.SSH_FXP_REMOVE, path);
return;
@@ -1632,13 +1641,16 @@ public abstract class AbstractSftpSubsystemHelper
sendStatus(prepareReply(buffer), id, SftpConstants.SSH_FX_OK, "");
}
- protected void doRemove(int id, String path, LinkOption... options) throws IOException {
+ protected void doRemoveFile(int id, String path, boolean followLinks) throws IOException {
Path p = resolveFile(path);
+ ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
- log.debug("doRemove({})[id={}] SSH_FXP_REMOVE (path={}[{}])",
- getServerSession(), id, path, p);
+ log.debug("doRemoveFile({})[id={}] SSH_FXP_REMOVE (path={}[{}])", session, id, path, p);
}
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, p, SftpConstants.SSH_FXP_REMOVE, "", followLinks);
Boolean status = IoUtils.checkFileExists(p, options);
if (status == null) {
throw signalRemovalPreConditionFailure(id, path, p,
@@ -2138,13 +2150,17 @@ public abstract class AbstractSftpSubsystemHelper
* @param dir The {@link DirectoryHandle}
* @param buffer The {@link Buffer} to write the results
* @param maxSize Max. buffer size
- * @param options The {@link LinkOption}-s to use when querying the directory contents
+ * @param followLinks Whether to follow symbolic links when querying the directory contents
* @return Number of written entries
* @throws IOException If failed to generate an entry
*/
protected int doReadDir(
- int id, String handle, DirectoryHandle dir, Buffer buffer, int maxSize, LinkOption... options)
+ int id, String handle, DirectoryHandle dir, Buffer buffer, int maxSize, boolean followLinks)
throws IOException {
+ ServerSession session = getServerSession();
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, dir.getFile(), SftpConstants.SSH_FXP_READDIR, "", followLinks);
int nb = 0;
Map<String, Path> entries = new TreeMap<>(Comparator.naturalOrder());
while ((dir.isSendDot() || dir.isSendDotDot() || dir.hasNext()) && (buffer.wpos() < maxSize)) {
@@ -2164,7 +2180,7 @@ public abstract class AbstractSftpSubsystemHelper
}
SftpEventListener listener = getSftpEventListenerProxy();
- listener.readEntries(getServerSession(), handle, dir, entries);
+ listener.readEntries(session, handle, dir, entries);
return nb;
}
@@ -2489,13 +2505,16 @@ public abstract class AbstractSftpSubsystemHelper
}
protected void doSetAttributes(
- Path file, Map<String, ?> attributes, boolean followLinks)
+ int cmd, String extension, Path file, Map<String, ?> attributes, boolean followLinks)
throws IOException {
SftpEventListener listener = getSftpEventListenerProxy();
ServerSession session = getServerSession();
listener.modifyingAttributes(session, file, attributes);
try {
- setFileAttributes(file, attributes, IoUtils.getLinkOptions(followLinks));
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, file, cmd, extension, followLinks);
+ setFileAttributes(file, attributes, options);
} catch (IOException | RuntimeException | Error e) {
listener.modifiedAttributes(session, file, attributes, e);
throw e;
@@ -2505,7 +2524,8 @@ public abstract class AbstractSftpSubsystemHelper
protected LinkOption[] getPathResolutionLinkOption(int cmd, String extension, Path path) throws IOException {
boolean followLinks = resolvePathResolutionFollowLinks(cmd, extension, path);
- return IoUtils.getLinkOptions(followLinks);
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ return accessor.resolveFileAccessLinkOptions(getServerSession(), this, path, cmd, extension, followLinks);
}
protected boolean resolvePathResolutionFollowLinks(int cmd, String extension, Path path) throws IOException {
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
index a88f7b5..d86ded4 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/FileHandle.java
@@ -75,7 +75,7 @@ public class FileHandle extends Handle {
} catch (UnsupportedOperationException e) {
channel = accessor.openFile(
session, subsystem, this, file, handle, openOptions, IoUtils.EMPTY_FILE_ATTRIBUTES);
- subsystem.doSetAttributes(file, attrs, false);
+ subsystem.doSetAttributes(SftpConstants.SSH_FXP_OPEN, "", file, attrs, false);
}
this.fileChannel = channel;
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 0d2d3ce..70c1638 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
@@ -118,6 +118,26 @@ public interface SftpFileSystemAccessor {
}
/**
+ * Invoked in order to determine the symbolic link follow options
+ *
+ * @param session The {@link ServerSession} through which the request was received
+ * @param subsystem The SFTP subsystem instance that manages the session
+ * @param file The referenced file
+ * @param cmd The SFTP command that triggered this access
+ * @param extension The SFTP extension that triggered this access - non-empty only for {SSH_FXP_EXTENDED} command
+ * @param followLinks Whether to follow symbolic links
+ * @return The {@link LinkOption}-s to use - invokes {@link IoUtils#getLinkOptions(boolean)} by default
+ * @throws IOException if failed to resolve the required options
+ * @see <A HREF="https://issues.apache.org/jira/browse/SSHD-1137">SSHD-1137</A>
+ */
+ default LinkOption[] resolveFileAccessLinkOptions(
+ ServerSession session, SftpSubsystemProxy subsystem, Path file,
+ int cmd, String extension, boolean followLinks)
+ throws IOException {
+ return IoUtils.getLinkOptions(followLinks);
+ }
+
+ /**
* Invoked in order to encode the outgoing referenced file name/path
*
* @param session The {@link ServerSession} through which the request was received
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 c18714d..85c3fab 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
@@ -109,9 +109,10 @@ public class SftpSubsystem
protected Path defaultDir = fileSystem.getPath("").toAbsolutePath().normalize();
protected int version;
- protected ServerSession serverSession;
protected CloseableExecutorService executorService;
+ private final ServerSession serverSession;
+
/**
* @param channel The {@link ChannelSession} through which the command was received
* @param configurator The {@link SftpSubsystemConfigurator} to use
@@ -128,7 +129,8 @@ public class SftpSubsystem
this.executorService = executorService;
}
- initializeSessionRelatedMember(channel);
+ serverSession = Objects.requireNonNull(channel.getServerSession(), "No session associated with the channel");
+ initializeSessionRelatedMember(serverSession, channel);
ChannelDataReceiver errorDataChannelReceiver
= resolveErrorDataChannelReceiver(channel, configurator.getErrorChannelDataReceiver());
@@ -178,10 +180,8 @@ public class SftpSubsystem
return executorService;
}
- protected void initializeSessionRelatedMember(ChannelSession channel) {
- serverSession = Objects.requireNonNull(channel.getServerSession(), "No session associated with the channel");
-
- FactoryManager manager = serverSession.getFactoryManager();
+ protected void initializeSessionRelatedMember(ServerSession session, ChannelSession channel) {
+ FactoryManager manager = session.getFactoryManager();
Factory<? extends Random> factory = manager.getRandomFactory();
this.randomizer = factory.create();
@@ -420,7 +420,10 @@ public class SftpSubsystem
throw new FileSystemLoopException(target);
}
- if (Files.isDirectory(path, IoUtils.getLinkOptions(false))) {
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ getServerSession(), this, path, SftpConstants.SSH_FXP_EXTENDED, targetType, false);
+ if (Files.isDirectory(path, options)) {
throw new NotDirectoryException(path.toString());
}
}
@@ -443,9 +446,10 @@ public class SftpSubsystem
protected byte[] doMD5Hash(
int id, String targetType, String target, long startOffset, long length, byte[] quickCheckHash)
throws Exception {
+ ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
log.debug("doMD5Hash({})({})[{}] offset={}, length={}, quick-hash={}",
- getServerSession(), targetType, target, startOffset, length,
+ session, targetType, target, startOffset, length,
BufferUtils.toHex(':', quickCheckHash));
}
@@ -468,7 +472,11 @@ public class SftpSubsystem
}
} else {
path = resolveFile(target);
- if (Files.isDirectory(path, IoUtils.getLinkOptions(true))) {
+
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, path, SftpConstants.SSH_FXP_EXTENDED, targetType, true);
+ if (Files.isDirectory(path, options)) {
throw new NotDirectoryException(path.toString());
}
}
@@ -698,7 +706,7 @@ public class SftpSubsystem
reply.putInt(0);
int maxDataSize = SftpModuleProperties.MAX_READDIR_DATA_SIZE.getRequired(session);
- int count = doReadDir(id, handle, dh, reply, maxDataSize, IoUtils.getLinkOptions(false));
+ int count = doReadDir(id, handle, dh, reply, maxDataSize, false);
BufferUtils.updateLengthPlaceholder(reply, lenPos, count);
if ((!dh.isSendDot()) && (!dh.isSendDotDot()) && (!dh.hasNext())) {
dh.markDone();
@@ -768,19 +776,24 @@ public class SftpSubsystem
Path path = fileHandle.getFile();
boolean followLinks = resolvePathResolutionFollowLinks(
SftpConstants.SSH_FXP_FSETSTAT, "", path);
- doSetAttributes(fileHandle.getFile(), attrs, followLinks);
+ doSetAttributes(SftpConstants.SSH_FXP_FSETSTAT, "", path, attrs, followLinks);
}
@Override
protected Map<String, Object> doFStat(int id, String handle, int flags) throws IOException {
Handle h = handles.get(handle);
+ ServerSession session = getServerSession();
if (log.isDebugEnabled()) {
log.debug("doFStat({})[id={}] SSH_FXP_FSTAT (handle={}[{}], flags=0x{})",
- getServerSession(), id, handle, h, Integer.toHexString(flags));
+ session, id, handle, h, Integer.toHexString(flags));
}
Handle fileHandle = validateHandle(handle, h, Handle.class);
- return resolveFileAttributes(fileHandle.getFile(), flags, IoUtils.getLinkOptions(true));
+ SftpFileSystemAccessor accessor = getFileSystemAccessor();
+ Path file = fileHandle.getFile();
+ LinkOption[] options = accessor.resolveFileAccessLinkOptions(
+ session, this, file, SftpConstants.SSH_FXP_FSTAT, "", true);
+ return resolveFileAttributes(file, flags, options);
}
@Override