apacheGH-403: Work-around for WS_FTP client bug

If the file handle size in Apache MINA sshd is > 4, WS_FTP client <=
12.9 sends (fileHandleSize - 4) too many bytes in SSH_FXP_WRITE
requests. If that exceeds the maximum packet size of the channel's
local window, the channel packet is invalid and Apache MINA sshd
correctly throws an exception and closes the channel.

Work around this by allowing for this special case on the server side
in channels used for SFTP.

Bug: apache#403

CHANGES.md

@@ -32,6 +32,7 @@
 * [GH-384](https://github.com/apache/mina-sshd/issues/384) Use correct lock modes for SFTP `FileChannel.lock()`.
 * [GH-388](https://github.com/apache/mina-sshd/issues/388) `ScpClient`: support issuing commands to a server that uses a non-UTF-8 locale.
 * [GH-398](https://github.com/apache/mina-sshd/issues/398) `SftpInputStreamAsync`: fix reporting EOF on zero-length reads.
+* [GH-403](https://github.com/apache/mina-sshd/issues/403) Work-around a bug in WS_FTP <= 12.9 SFTP clients.
 
 * [SSHD-1259](https://issues.apache.org/jira/browse/SSHD-1259) Consider all applicable host keys from the known_hosts files.
 * [SSHD-1310](https://issues.apache.org/jira/browse/SSHD-1310) `SftpFileSystem`: do not close user session.

sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java

@@ -41,6 +41,7 @@
 import org.apache.sshd.common.FactoryManager;
 import org.apache.sshd.common.PropertyResolver;
 import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.exception.SshChannelInvalidPacketException;
 import org.apache.sshd.common.channel.throttle.ChannelStreamWriterResolver;
 import org.apache.sshd.common.channel.throttle.ChannelStreamWriterResolverManager;
 import org.apache.sshd.common.future.CloseFuture;
@@ -77,6 +78,15 @@ public abstract class AbstractChannel extends AbstractInnerCloseable implements
  */
  public static final IntUnaryOperator RESPONSE_BUFFER_GROWTH_FACTOR = Int2IntFunction.add(Byte.SIZE);
 
+ /**
+ * A default {@link PacketValidator} that enforces that the packet size is not greater than the maximum packet size
+ * of the channel's local window.
+ */
+ // Plus 4 because there seems to be some confusion about whether or not the maximum packet size for a channel
+ // includes the packet length field itself.
+ public static final PacketValidator DEFAULT_PACKET_VALIDATOR = (
+ packetSize, maxPacketSize) -> packetSize <= maxPacketSize + 4;
+
  protected enum GracefulState {
  Opened,
  CloseSent,
@@ -85,6 +95,7 @@ protected enum GracefulState {
  }
 
  protected ConnectionService service;
+
  protected final AtomicBoolean initialized = new AtomicBoolean(false);
  protected final AtomicBoolean eofReceived = new AtomicBoolean(false);
  protected final AtomicBoolean eofSent = new AtomicBoolean(false);
@@ -111,6 +122,8 @@ protected enum GracefulState {
  private final RemoteWindow remoteWindow;
  private ChannelStreamWriterResolver channelStreamPacketWriterResolver;
 
+ private PacketValidator packetValidator = DEFAULT_PACKET_VALIDATOR;
+
  /**
  * A {@link Map} of sent requests - key = request name, value = timestamp when request was sent.
  */
@@ -849,20 +862,40 @@ protected long validateIncomingDataSize(
  LocalWindow wLocal = getLocalWindow();
  long maxLocalSize = wLocal.getPacketSize();
 
- /*
- * The reason for the +4 is that there seems to be some confusion
- * whether the max. packet size includes the length field or not
- */
- if (len > (maxLocalSize + 4L)) {
- throw new IllegalStateException("Bad length (" + len + ") " + " for cmd="
- + SshConstants.getCommandMessageName(cmd) + " - max. allowed=" + maxLocalSize);
+ PacketValidator validator = getPacketValidator();
+ if (!validator.isValid(len, maxLocalSize)) {
+ throw new SshChannelInvalidPacketException(getChannelId(), "Bad length (" + len + ") " + " for cmd="
+ + SshConstants.getCommandMessageName(cmd)
+ + " - max. allowed=" + maxLocalSize);
  }
 
  wLocal.consume(len);
 
  return len;
  }
 
+ /**
+ * Retrieves the currently set {@link PacketValidator}.
+ *
+ * @return the validator, never {@code null}
+ */
+ public PacketValidator getPacketValidator() {
+ return packetValidator;
+ }
+
+ /**
+ * Sets a {@link PacketValidator}.
+ *
+ * @param the validator to set, if {@code null} the {@link #DEFAULT_PACKET_VALIDATOR} is set
+ */
+ public void setPacketValidator(PacketValidator validator) {
+ if (validator == null) {
+ packetValidator = DEFAULT_PACKET_VALIDATOR;
+ } else {
+ packetValidator = validator;
+ }
+ }
+
  @Override
  public void handleEof() throws IOException {
  if (eofReceived.getAndSet(true)) {
@@ -1020,4 +1053,21 @@ public String toString() {
  return getClass().getSimpleName() + "[id=" + getChannelId() + ", recipient=" + getRecipient() + "]" + "-"
  + getSession();
  }
+
+ /**
+ * A {@link PacketValidator} can validate packet lengths.
+ */
+ @FunctionalInterface
+ public interface PacketValidator {
+
+ /**
+ * Tells whether a packet received of {@code len} bytes is valid given a channel's {@code maximumPacketSize}.
+ *
+ * @param packetSize from the SSH message
+ * @param maximumPacketSize from the channel's local window
+ * @return {@code true} if the packet is to be considered valid.
+ */
+ boolean isValid(long packetSize, long maximumPacketSize);
+
+ }
 }

sshd-core/src/main/java/org/apache/sshd/common/channel/exception/SshChannelInvalidPacketException.java

@@ -0,0 +1,40 @@
+/*
+ * 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.common.channel.exception;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SshChannelInvalidPacketException extends SshChannelException {
+
+ private static final long serialVersionUID = -4567136397319835717L;
+
+ public SshChannelInvalidPacketException(long channelId, String message) {
+ this(channelId, message, null);
+ }
+
+ public SshChannelInvalidPacketException(long channelId, Throwable cause) {
+ this(channelId, cause.getMessage(), cause);
+ }
+
+ public SshChannelInvalidPacketException(long channelId, String message, Throwable cause) {
+ super(channelId, message, cause);
+ }
+}

sshd-sftp/src/main/java/org/apache/sshd/sftp/SftpModuleProperties.java

@@ -203,7 +203,7 @@ public final class SftpModuleProperties {
 
  /**
  * Properties key for the maximum of available open handles per session. By default we impose virtually no limit and
- * relay on the O/S.
+ * rely on the O/S.
  */
  public static final Property<Integer> MAX_OPEN_HANDLES_PER_SESSION
  = Property.integer("max-open-handles-per-session", Integer.MAX_VALUE - 1);

sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java

@@ -48,6 +48,8 @@
 
 import org.apache.sshd.common.Factory;
 import org.apache.sshd.common.FactoryManager;
+import org.apache.sshd.common.channel.AbstractChannel;
+import org.apache.sshd.common.channel.AbstractChannel.PacketValidator;
 import org.apache.sshd.common.channel.BufferedIoOutputStream;
 import org.apache.sshd.common.channel.LocalWindow;
 import org.apache.sshd.common.digest.BuiltinDigests;
@@ -191,6 +193,22 @@ protected void initializeSessionRelatedMember(ServerSession session, ChannelSess
  this.fileHandleSize = SftpModuleProperties.FILE_HANDLE_SIZE.getRequired(channel);
  this.maxFileHandleRounds = SftpModuleProperties.MAX_FILE_HANDLE_RAND_ROUNDS.getRequired(channel);
 
+ if (this.fileHandleSize > 4) {
+ // WS_FTP <= 12.9 has a bug concerning SFTP file handle sizes. If the handle size is > 4, it sends (handle
+ // size - 4) bytes too many in the SSH_MSG_CHANNEL_DATA packets. To work around this client-side bug,
+ // install a more lenient {@link PacketValidator} on the channel.
+ PacketValidator validator = (packetSize, maximumPacketSize) -> {
+ if (AbstractChannel.DEFAULT_PACKET_VALIDATOR.isValid(packetSize, maximumPacketSize)) {
+ return true;
+ }
+ // Packet size is larger. Accept only if it's exactly the file handle size difference larger.
+ if (packetSize == maximumPacketSize + fileHandleSize * 2 - 4) {
+ return true;
+ }
+ return false;
+ };
+ channel.setPacketValidator(validator);
+ }
  if (workBuf.length < this.fileHandleSize) {
  workBuf = new byte[this.fileHandleSize];
  }