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 2017/10/07 15:58:44 UTC
[5/5] mina-sshd git commit: [SSHD-775] SftpSubSystem::sendStatus
leaks Exception information
[SSHD-775] SftpSubSystem::sendStatus leaks Exception information
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/2529a4c3
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/2529a4c3
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/2529a4c3
Branch: refs/heads/master
Commit: 2529a4c3da8635ca350cd85ae76b0df5ac3b39d0
Parents: 654bb26
Author: Lyor Goldstein <ly...@gmail.com>
Authored: Fri Oct 6 18:42:41 2017 +0300
Committer: Lyor Goldstein <ly...@gmail.com>
Committed: Sat Oct 7 18:59:47 2017 +0300
----------------------------------------------------------------------
README.md | 7 +
.../DetailedSftpErrorStatusDataHandler.java | 60 +
.../sshd/common/subsystem/sftp/SftpHelper.java | 81 +-
.../apache/sshd/common/util/GenericUtils.java | 2 +-
.../sftp/AbstractSftpSubsystemHelper.java | 2582 ++++++++++++++++++
.../sftp/SftpErrorStatusDataHandler.java | 83 +
.../subsystem/sftp/SftpFileSystemAccessor.java | 22 +
.../server/subsystem/sftp/SftpSubsystem.java | 2565 +----------------
.../sftp/SftpSubsystemEnvironment.java | 67 +
.../subsystem/sftp/SftpSubsystemFactory.java | 24 +-
.../subsystem/sftp/SftpFileSystemTest.java | 4 +-
.../sshd/client/subsystem/sftp/SftpTest.java | 33 +-
.../subsystem/sftp/SftpVersionSelectorTest.java | 18 +-
.../client/subsystem/sftp/SftpVersionsTest.java | 23 +-
.../SpaceAvailableExtensionImplTest.java | 3 +-
.../openssh/helpers/OpenSSHExtensionsTest.java | 3 +-
.../subsystem/sftp/SftpConstantsTest.java | 9 +
.../sftp/ApacheSshdSftpSessionFactory.java | 5 +-
18 files changed, 3035 insertions(+), 2556 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2529a4c3/README.md
----------------------------------------------------------------------
diff --git a/README.md b/README.md
index 260c024..2c29d77 100644
--- a/README.md
+++ b/README.md
@@ -746,6 +746,13 @@ One can skip all the conditional code if a specific known extension is required:
```
+### Internal exceptions and error message handling
+
+If an exception is thrown during processing of an SFTP command, then the exception is translated into a `SSH_FXP_STATUS` message
+using a registered `SftpErrorStatusDataHandler`. The default implementation provides a short description of the failure based on the thrown
+exception type. However, users may override it when creating the `SftpSubsystemFactory` and provide their own codes and/or messages - e.g., for debugging one can register a `DetailedSftpErrorStatusDataHandler` (see `sshd-contrib`) that "leaks" more information in the generated message.
+
+
## Port forwarding
### Standard port forwarding
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2529a4c3/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/DetailedSftpErrorStatusDataHandler.java
----------------------------------------------------------------------
diff --git a/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/DetailedSftpErrorStatusDataHandler.java b/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/DetailedSftpErrorStatusDataHandler.java
new file mode 100644
index 0000000..3990eab
--- /dev/null
+++ b/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/DetailedSftpErrorStatusDataHandler.java
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sshd.server.subsystem.sftp;
+
+import java.nio.file.FileSystemException;
+import java.util.Objects;
+
+import org.apache.sshd.common.subsystem.sftp.SftpException;
+
+/**
+ * An {@link SftpErrorStatusDataHandler} implementation that returns an elaborate
+ * message string for the thrown exception - thus potentially "leaking"
+ * information about the internal implementation and/or real paths. Recommended for
+ * debugging or systems where such leakage is not considered a security risk
+ *
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class DetailedSftpErrorStatusDataHandler implements SftpErrorStatusDataHandler {
+ public static final DetailedSftpErrorStatusDataHandler INSTANCE = new DetailedSftpErrorStatusDataHandler();
+
+ public DetailedSftpErrorStatusDataHandler() {
+ super();
+ }
+
+ @Override
+ public String resolveErrorMessage(
+ SftpSubsystemEnvironment sftpSubsystem, int id, Throwable e, int subStatus, int cmd, Object... args) {
+ if (e instanceof FileSystemException) {
+ FileSystemException fse = (FileSystemException) e;
+ String file = fse.getFile();
+ String otherFile = fse.getOtherFile();
+ String message = fse.getReason();
+ return e.getClass().getSimpleName()
+ + "[file=" + file + "]"
+ + (Objects.equals(file, otherFile) ? "" : "[other=" + otherFile + "]")
+ + ": " + message;
+ } else if (e instanceof SftpException) {
+ return e.toString();
+ } else {
+ return "Internal " + e.getClass().getSimpleName() + ": " + e.getMessage();
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2529a4c3/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java
index 5b8b143..ead2764 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/subsystem/sftp/SftpHelper.java
@@ -20,6 +20,7 @@ package org.apache.sshd.common.subsystem.sftp;
import java.io.EOFException;
import java.io.FileNotFoundException;
+import java.net.UnknownServiceException;
import java.nio.channels.OverlappingFileLockException;
import java.nio.charset.StandardCharsets;
import java.nio.file.AccessDeniedException;
@@ -42,9 +43,11 @@ import java.security.Principal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
+import java.util.NavigableMap;
import java.util.Objects;
import java.util.Set;
import java.util.TreeMap;
@@ -77,6 +80,48 @@ public final class SftpHelper {
*/
public static final boolean DEFAULT_APPEND_END_OF_LIST_INDICATOR = true;
+ public static final NavigableMap<Integer, String> DEFAULT_SUBSTATUS_MESSAGE =
+ Collections.unmodifiableNavigableMap(new TreeMap<Integer, String>(Comparator.naturalOrder()) {
+ // Not serializing it
+ private static final long serialVersionUID = 1L;
+
+ {
+ put(SftpConstants.SSH_FX_OK, "Success");
+ put(SftpConstants.SSH_FX_EOF, "End of file");
+ put(SftpConstants.SSH_FX_NO_SUCH_FILE, "No such file or directory");
+ put(SftpConstants.SSH_FX_PERMISSION_DENIED, "Permission denied");
+ put(SftpConstants.SSH_FX_FAILURE, "General failure");
+ put(SftpConstants.SSH_FX_BAD_MESSAGE, "Bad message data");
+ put(SftpConstants.SSH_FX_NO_CONNECTION, "No connection to server");
+ put(SftpConstants.SSH_FX_CONNECTION_LOST, "Connection lost");
+ put(SftpConstants.SSH_FX_OP_UNSUPPORTED, "Unsupported operation requested");
+ put(SftpConstants.SSH_FX_INVALID_HANDLE, "Invalid handle value");
+ put(SftpConstants.SSH_FX_NO_SUCH_PATH, "No such path");
+ put(SftpConstants.SSH_FX_FILE_ALREADY_EXISTS, "File/Directory already exists");
+ put(SftpConstants.SSH_FX_WRITE_PROTECT, "File/Directory is write-protected");
+ put(SftpConstants.SSH_FX_NO_MEDIA, "No such meadia");
+ put(SftpConstants.SSH_FX_NO_SPACE_ON_FILESYSTEM, "No space left on device");
+ put(SftpConstants.SSH_FX_QUOTA_EXCEEDED, "Quota exceeded");
+ put(SftpConstants.SSH_FX_UNKNOWN_PRINCIPAL, "Unknown user/group");
+ put(SftpConstants.SSH_FX_LOCK_CONFLICT, "Lock conflict");
+ put(SftpConstants.SSH_FX_DIR_NOT_EMPTY, "Directory not empty");
+ put(SftpConstants.SSH_FX_NOT_A_DIRECTORY, "Accessed location is not a directory");
+ put(SftpConstants.SSH_FX_INVALID_FILENAME, "Invalid filename");
+ put(SftpConstants.SSH_FX_LINK_LOOP, "Link loop");
+ put(SftpConstants.SSH_FX_CANNOT_DELETE, "Cannot remove");
+ put(SftpConstants.SSH_FX_INVALID_PARAMETER, "Invalid parameter");
+ put(SftpConstants.SSH_FX_FILE_IS_A_DIRECTORY, "Accessed location is a directory");
+ put(SftpConstants.SSH_FX_BYTE_RANGE_LOCK_CONFLICT, "Range lock conflict");
+ put(SftpConstants.SSH_FX_BYTE_RANGE_LOCK_REFUSED, "Range lock refused");
+ put(SftpConstants.SSH_FX_DELETE_PENDING, "Delete pending");
+ put(SftpConstants.SSH_FX_FILE_CORRUPT, "Corrupted file/directory");
+ put(SftpConstants.SSH_FX_OWNER_INVALID, "Invalid file/directory owner");
+ put(SftpConstants.SSH_FX_GROUP_INVALID, "Invalid file/directory group");
+ put(SftpConstants.SSH_FX_NO_MATCHING_BYTE_RANGE_LOCK, "No matching byte range lock");
+ }
+ });
+
+
private SftpHelper() {
throw new UnsupportedOperationException("No instance allowed");
}
@@ -452,14 +497,13 @@ public final class SftpHelper {
return SftpConstants.SSH_FX_EOF;
} else if (t instanceof OverlappingFileLockException) {
return SftpConstants.SSH_FX_LOCK_CONFLICT;
- } else if (t instanceof UnsupportedOperationException) {
+ } else if ((t instanceof UnsupportedOperationException)
+ || (t instanceof UnknownServiceException)) {
return SftpConstants.SSH_FX_OP_UNSUPPORTED;
} else if (t instanceof InvalidPathException) {
return SftpConstants.SSH_FX_INVALID_FILENAME;
} else if (t instanceof IllegalArgumentException) {
return SftpConstants.SSH_FX_INVALID_PARAMETER;
- } else if (t instanceof UnsupportedOperationException) {
- return SftpConstants.SSH_FX_OP_UNSUPPORTED;
} else if (t instanceof UserPrincipalNotFoundException) {
return SftpConstants.SSH_FX_UNKNOWN_PRINCIPAL;
} else if (t instanceof FileSystemLoopException) {
@@ -471,18 +515,13 @@ public final class SftpHelper {
}
}
- public static String resolveStatusMessage(Throwable t) {
- if (t == null) {
- return "";
- } else if (t instanceof SftpException) {
- return t.toString();
- } else {
- return "Internal " + t.getClass().getSimpleName() + ": " + t.getMessage();
- }
+ public static String resolveStatusMessage(int subStatus) {
+ String message = DEFAULT_SUBSTATUS_MESSAGE.get(subStatus);
+ return GenericUtils.isEmpty(message) ? ("Unknown error: " + subStatus) : message;
}
- public static Map<String, Object> readAttrs(Buffer buffer, int version) {
- Map<String, Object> attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ public static NavigableMap<String, Object> readAttrs(Buffer buffer, int version) {
+ NavigableMap<String, Object> attrs = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
int flags = buffer.getInt();
if (version >= SftpConstants.SFTP_V4) {
int type = buffer.getUByte();
@@ -592,10 +631,10 @@ public final class SftpHelper {
return attrs;
}
- public static Map<String, byte[]> readExtensions(Buffer buffer) {
+ public static NavigableMap<String, byte[]> readExtensions(Buffer buffer) {
int count = buffer.getInt();
// NOTE
- Map<String, byte[]> extended = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ NavigableMap<String, byte[]> extended = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
for (int i = 0; i < count; i++) {
String key = buffer.getString();
byte[] val = buffer.getBytes();
@@ -623,13 +662,13 @@ public final class SftpHelper {
}
}
- public static Map<String, String> toStringExtensions(Map<String, ?> extensions) {
+ public static NavigableMap<String, String> toStringExtensions(Map<String, ?> extensions) {
if (GenericUtils.isEmpty(extensions)) {
- return Collections.emptyMap();
+ return Collections.emptyNavigableMap();
}
// NOTE: even though extensions are probably case sensitive we do not allow duplicate name that differs only in case
- Map<String, String> map = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ NavigableMap<String, String> map = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
extensions.forEach((key, value) -> {
ValidateUtils.checkNotNull(value, "No value for extension=%s", key);
String prev = map.put(key, (value instanceof byte[]) ? new String((byte[]) value, StandardCharsets.UTF_8) : value.toString());
@@ -639,13 +678,13 @@ public final class SftpHelper {
return map;
}
- public static Map<String, byte[]> toBinaryExtensions(Map<String, String> extensions) {
+ public static NavigableMap<String, byte[]> toBinaryExtensions(Map<String, String> extensions) {
if (GenericUtils.isEmpty(extensions)) {
- return Collections.emptyMap();
+ return Collections.emptyNavigableMap();
}
// NOTE: even though extensions are probably case sensitive we do not allow duplicate name that differs only in case
- Map<String, byte[]> map = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ NavigableMap<String, byte[]> map = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
extensions.forEach((key, value) -> {
ValidateUtils.checkNotNull(value, "No value for extension=%s", key);
byte[] prev = map.put(key, value.getBytes(StandardCharsets.UTF_8));
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2529a4c3/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java
index 7280288..6bf6d20 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java
@@ -878,7 +878,7 @@ public final class GenericUtils {
this.map = new LinkedHashMap<>();
}
- public MapBuilder(Comparator<K> comparator) {
+ public MapBuilder(Comparator<? super K> comparator) {
this.map = new TreeMap<>(comparator);
}