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 <lgoldst...@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 `lsets...@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 `lsets...@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