Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exceptionCaught(ServerSessionImpl[user@/10.x.x.x:23232])[state=Opened] IllegalStateException: Bad length (32796) for cmd=SSH_MSG_CHANNEL_DATA - max. allowed=32768 #403

Closed
dragonknight88 opened this issue Aug 10, 2023 · 18 comments
Assignees
Milestone

Comments

@dragonknight88
Copy link

Client: WS_FTP 1.26/1.27
Server: SFTP server (mina 2.9.2)

Operation: Uploading a file

Mina version - 2.9.2

As sftp client tries to upload a file channel is closed as below exception is caught.

2023-08-10 10:16:31 2023-08-10T17:16:31.119Z WARN 337 --- [] [] [] [)-nio2-thread-3] o.a.s.server.session.ServerSessionImpl : exceptionCaught(ServerSessionImpl[user@/10.x.x.x:23232])[state=Opened] IllegalStateException: Bad length (32796) for cmd=SSH_MSG_CHANNEL_DATA - max. allowed=32768

Is there a way or config via core module props to increase the channel packet size limit?

@tomaswolf
Copy link
Member

You can increase the maximum packet size for channel windows by changing property CoreModuleProperties.MAX_PACKET_SIZE (for instance on the session). But strange that this client sends more uncompressed payload data than the maximum packet size the server did advertise in its channel window.

@dragonknight88
Copy link
Author

Ty @tomaswolf I did bump up the packet size and it seemed to have worked increasing the limit on server.

But client is still sending 28 bytes more than the increased limits. Wondering if additional bytes are related to certain openssh extension/s. I will be debugging more with the server setup config for extensions.

@dragonknight88
Copy link
Author

Its been happening with other clients as well. eg. maverick_legacy_1.6.0

@dragonknight88
Copy link
Author

@tomaswolf
Copy link
Member

I suppose you mean not the whole mailing list but just the thread starting at https://www.mail-archive.com/users@mina.apache.org/msg06936.html . Yes, that's the same issue.

If that client consistently sends 28 bytes more it's likely a client-side bug. From the SSH message structures as defined by the SSH RFC and the SFTP draft RFCs I can't readily see why it should send 28 extra bytes, though. 41 or 4 I could understand.

@tomaswolf
Copy link
Member

Since you mentioned maverick_legacy, I've tried to reproduce this. Simple SFTP test server using Apache MINA sshd, and a simple SFTP client using maverick uploading a 100KB file.

I cannot reproduce this. I tried both j2ssh-maverick:1.5.5 and maverick-legacy:1.6.0 (with maverick-common:1.0.1) against both Apache MINA 2.9.2 and 2.10.0. Apart from the fact that I had to enable some long deprecated KEX algorithms and allow small keys in my test server for maverick-legacy: the SFTP file upload worked fine in all combinations. (All running on localhost on OS X.)

@tomaswolf
Copy link
Member

tomaswolf commented Aug 16, 2023

Related:

reported in 2022 for WS_FTP 12.6, and in 2023 for 12.7 and 12.8. Seems to be specific to WS_FTP, which I don't have.

@dragonknight88 : if you can live debug the server, a breakpoint where that exception is raised in AbstractChannel.validateIncomingDataSize() should let you examine the buffer contents. Then we could look at what the actual bytes of this message are. Perhaps that gives a clue. (I'd also be interested to know what the total length of that buffer is at that point -- i.e., its number of available bytes.)

Also, it would be interesting to know when exactly it occurs. What messages were exchanged over the channel already when the exception occurs? A debug log of the server would help.

@dragonknight88
Copy link
Author

dragonknight88 commented Aug 17, 2023

hi @tomaswolf it seem to be failing in decode method on SSH_MSG_CHANNEL_DATA size validation. pfa: trace log of server events.

2023-08-16 11:13:23
2023-08-16T18:13:23.280Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.s.session.ServerConnectionService  : unregisterChannel(ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]) result=ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z TRACE 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.u.closeable.SequentialCloseable  : doClose(org.apache.sshd.common.util.closeable.SequentialCloseable$1@37a4791b) closing [DefaultCloseFuture[id=ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]][value=null]] immediately=true
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] AbstractChannel$GracefulChannelCloseable : close(ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279])[immediately=true] processing
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z TRACE 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.u.closeable.SequentialCloseable  : doClose(org.apache.sshd.common.util.closeable.SequentialCloseable$1@37a4791b) closing GracefulChannelCloseable[ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]] immediately=true
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.c.ChannelAsyncOutputStream       : close(ChannelAsyncOutputStream[ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]] cmd=SSH_MSG_CHANNEL_EXTENDED_DATA)[Immediately] closed
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.c.ChannelAsyncOutputStream       : close(ChannelAsyncOutputStream[ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]] cmd=SSH_MSG_CHANNEL_EXTENDED_DATA) Closing immediately
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.c.ChannelAsyncOutputStream       : close(ChannelAsyncOutputStream[ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]] cmd=SSH_MSG_CHANNEL_DATA)[Immediately] closed
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.c.ChannelAsyncOutputStream       : close(ChannelAsyncOutputStream[ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]] cmd=SSH_MSG_CHANNEL_DATA) Closing immediately
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.sshd.common.channel.RemoteWindow     : Closing RemoteWindow[server](ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279])
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.sshd.common.channel.LocalWindow      : Closing LocalWindow[server](ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279])
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z TRACE 338 --- [] [] [] [)-nio2-thread-6] o.a.sshd.server.channel.ChannelSession   : signalChannelClosed(ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279])[signalChannelClosed]
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.sshd.server.channel.ChannelSession   : close(ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]) no EOF sent
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.sshd.server.channel.ChannelSession   : close(ChannelSession[id=0, recipient=123773646]-ServerSessionImpl[sftpusername@/10.100.92.51:38279]) Closing immediately
2023-08-16 11:13:23
2023-08-16T18:13:23.279Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.s.session.ServerConnectionService  : stopHeartBeat(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) no heartbeat to stop
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.s.session.ServerConnectionService  : close(ServerConnectionService[ServerSessionImpl[sftpusername@/10.100.92.51:38279]]) Closing immediately
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z TRACE 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.u.closeable.SequentialCloseable  : doClose(org.apache.sshd.common.util.closeable.SequentialCloseable$1@5246f14f) closing ParallelCloseable[DefaultCloseFuture[id=ServerSessionImpl[sftpusername@/10.100.92.51:38279]][value=null]] immediately=true
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z TRACE 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.s.h.SessionTimeoutListener       : sessionClosed(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) not tracked
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z  INFO 338 --- [/34.210.81.97:17155] [699a6c79-2ea6-4a18-bbfa-4945e0402da4] [sftpusername] [)-nio2-thread-6] c.XXX.service.sftp.SessionListener  : Session closed
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.server.session.ServerSessionImpl   : close(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) Closing immediately
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.s.h.SessionTimeoutListener       : sessionClosed(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) un-tracked
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z DEBUG 338 --- [] [] [] [)-nio2-thread-6] o.a.s.c.s.h.SessionTimeoutListener       : sessionException(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) IllegalStateException: Bad length (65600)  for cmd=SSH_MSG_CHANNEL_DATA - max. allowed=65572
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z ERROR 338 --- [/34.210.81.97:17155] [699a6c79-2ea6-4a18-bbfa-4945e0402da4] [sftpusername] [)-nio2-thread-6] c.XXX.service.sftp.SessionListener  : Session exception: java.lang.IllegalStateException: Bad length (65600)  for cmd=SSH_MSG_CHANNEL_DATA - max. allowed=65572
2023-08-16 11:13:23
2023-08-16T18:13:23.278Z  WARN 338 --- [/34.210.81.97:17155] [699a6c79-2ea6-4a18-bbfa-4945e0402da4] [sftpusername] [)-nio2-thread-6] c.XXX.service.sftp.SessionListener  : Reporting the failure metric: java.lang.IllegalStateException: Bad length (65600)  for cmd=SSH_MSG_CHANNEL_DATA - max. allowed=65572
2023-08-16 11:13:23
2023-08-16T18:13:23.277Z  WARN 338 --- [/10.205.225.32:2572] [06e94709-083c-4920-b6e5-a358fb59229e] [sftpusername] [)-nio2-thread-6] o.a.s.server.session.ServerSessionImpl   : exceptionCaught(ServerSessionImpl[sftpusername@/10.100.92.51:38279])[state=Opened] IllegalStateException: Bad length (65600)  for cmd=SSH_MSG_CHANNEL_DATA - max. allowed=65572
2023-08-16 11:13:23
2023-08-16T18:13:23.277Z DEBUG 338 --- [/10.205.225.32:2572] [06e94709-083c-4920-b6e5-a358fb59229e] [sftpusername] [)-nio2-thread-6] o.a.s.server.session.ServerSessionImpl   : doHandleMessage(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) process #14 SSH_MSG_CHANNEL_DATA
2023-08-16 11:13:23
2023-08-16T18:13:23.277Z TRACE 338 --- [/10.205.225.32:2572] [06e94709-083c-4920-b6e5-a358fb59229e] [sftpusername] [)-nio2-thread-6] o.a.s.server.session.ServerSessionImpl   : decode(ServerSessionImpl[sftpusername@/10.100.92.51:38279]) packet #15 [chunk #1026](65609/65609) 00 00 00 00 00 00 00 00 00

Surprisingly client works without an issue with OpenSSH server. I have been testing server behavior with different packet limits for below props on server init. Unsure if mina buffer validation is active by default or this enforces the buffer size validation check.

    CoreModuleProperties.LIMIT_PACKET_SIZE.set(server, packetSizeLimit);
    CoreModuleProperties.MAX_PACKET_SIZE.set(server, maxPacketSizeLimit);

@tomaswolf
Copy link
Member

Sorry, this log doesn't help. It seems to report stuff only after the exception has already occurred and the channel has been closed.

Does this occur also with Apache MINA sshd 2.10.0?

How big is the file being sent? (Or if you don't know: how much data was already received?)

What's the log before that? From the start of the SFTP connection.

@dragonknight88
Copy link
Author

Occurs on 2.10.0 as well.
Size of the file being uploaded - 40MB
mina_logs.txt

Lmk if this helps or if you have a sftp server running over the internet; which I could use to connect with the sftp client and test user for the whole logs on server side.

@tomaswolf
Copy link
Member

tomaswolf commented Aug 17, 2023

Thanks, that helps. I was a bit confused for a moment by the log being chronologically descending, but that's OK. The interesting bit are these two lines:

2023-08-17T03:46:06-07:00	2023-08-17T10:46:06.268Z TRACE 345 --- [] [] [] [)-nio2-thread-5] o.a.s.server.session.ServerSessionImpl   : decode(ServerSessionImpl[user@/10.209.10.5:23011]) packet #15 [chunk #2](128/65573) ff e3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................................................................
2023-08-17T03:46:06-07:00	2023-08-17T10:46:06.268Z TRACE 345 --- [] [] [] [)-nio2-thread-5] o.a.s.server.session.ServerSessionImpl   : decode(ServerSessionImpl[user@/10.209.10.5:23011]) packet #15 [chunk #1](64/65573) 5e 00 00 00 00 00 01 00 1c 00 01 00 18 06 00 00 04 d2 00 00 00 20 38 32 35 64 35 64 64 33 61 38 30 35 36 34 38 65 65 66 35 35 33 62 64 66 33 61 64 39 37 66 36 66 00 00 00 00 00 00 00 00 00 00    ^.....................825d5dd3a805648eef553bdf3ad97f6f..........

This shows the SSH message itself is internally consistent. We have

  • 5e SSH_MSG_CHANNEL_DATA
  • 00 00 00 00 Channel ID = 0
  • 00 01 00 1c Length of data following: 65564 bytes, which is higher than the packet size you configured (65536)

Next comes the data itself, which is an SFTP write request:

  • 00 01 00 18 Length of data following: 65560 bytes
  • 06 SSH_FXP_WRITE
  • 00 00 04 d2 SFTP request ID
  • 00 00 00 20 SFTP file handle length (32)
  • 32 bytes containing the SFTP file handle "825d5dd3a805648eef553bdf3ad97f6f"
  • 00 00 00 00 00 00 00 00 File position to write to: offset 0
  • 00 00 ff e3 number of bytes to write: 65507 bytes
  • 65507 data bytes following (incidentally all zero bytes)

Also visible from the log: it's the first SSH_FXP_WRITE request for this file (there's an SSH_FXP_OPEN request just before).

So whatever that client is doing, it really is sending 28 bytes too many. I do notice that 28 = 32 - 4. OpenSSH returns handles that have only 4 bytes. Apache MINA sshd uses by default handles of 16 bytes AFAIK, and it stringifies these 16 bytes, so we end up with 32 bytes (one byte per hex character).

The SFTP draft RFCs say that SFTP file handles may be up to 256 bytes long.

If you set the handle size in the Apache MINA sshd server to 8, we should end up with 16 hex chars. Does that client then send 12 extra bytes? And if the handle size is 20, we should have 40 bytes, does it then send 36 extra bytes? (For the configuration, see SftpModuleProperties.FILE_HANDLE_SIZE. Apache MINA sshd allows values between 4 and 64.)

If so, we'd have established that this SFTP client can only handle SFTP file handles of 4 bytes. For larger handles, its length logic for SFTP write requests (SSH_FXP_WRITE) is wrong.

This would also explain why it works with OpenSSH: an OpenSSH server will use a handle size of 4 bytes.

@tomaswolf
Copy link
Member

All that said: I find the file handle implementation in Apache MINA sshd a little bit strange. I don't know why it is done the way it is done; it strikes me as overly complicated. I see absolutely no reason why a file handle should be longer than 4 bytes, be it in memory or in the network packet.

So even if I think we may have found a bug in WS_FTP, I also think the Apache MINA sshd implementation could be simplified and use by default a handle size of 4 bytes.

@dragonknight88
Copy link
Author

dragonknight88 commented Aug 17, 2023

Ack! I was able to successfully test by setting FILE_HANDLE_SIZE to 4; and testing it for 16 and 40.

Other thing I noticed is server dint have it configured to 32 before but mina was somehow defaulting file-handle to be 32. Going through code it is still unclear where it defaults to 32 vs 16 bytes. file-handle-limits-default

Initially I suspected client may be sending corrupted file-handle back; but that dint seem to be the case.

@tomaswolf
Copy link
Member

tomaswolf commented Aug 17, 2023

Yes, that was my mistake when reading the code. File handle size 16 means it uses 16 bytes, but then converts these to a hex string, which gives 32 characters, and thus we have with SftpModuleProperties.FILE_HANDLE_SIZE = 16 32 bytes for the actual file handle and thus in the SSH network messages. As I wrote, the implementation is strange. :-(

Now that this is confirmed:

@dragonknight88
Copy link
Author

dragonknight88 commented Aug 18, 2023

As it definitely is a bug in WS_FTP, can you file a bug report there? Or comment on https://community.progress.com/s/question/0D54Q0000ASQm9FSQT/bad-length-32796-for-cmdsshmsgchanneldata-max-allowed32768 .

It was reported by me :) I will add more info on RFC (file handle, 4-258 byte limits and buffer calc.)

I'll see if I can rewrite this file handle stuff in Apache MINA sshd to use 4 bytes by default. Shouldn't be too hard.

Meanwhile, a possible quick fix could be made on Validation condition - AbstractChannel.java#L856 as it appears +4L confusion here is related to fileHandle * 2 byte count you mentioned above. And partly the reason why file handle size of 4 succeeds without validation failure.

if (len > (maxLocalSize + fileHandleSize * 2 - 4L)) { 

Thank you so much Tomas for taking the time and helping out on the issue.

@tomaswolf
Copy link
Member

a possible quick fix could be...

Yes, I'll also do something like that. Except that this validation is in the connection protocol (RFC 4254), and fileHandleSize is known only in the higher-layer SFTP protocol, so a bit more is needed to implement this in a proper way without layer breaks. We'd want to accept this extra size only in SFTP channels, and only if it's exactly this expected extra amount of bytes.

@tomaswolf
Copy link
Member

BTW, the "+4 confusion" is this: https://mailarchive.ietf.org/arch/msg/secsh/mIvfsnrukzaIvUBah5RJjDb3yyQ/

It's basically unrelated to this handle size problem. But it makes the SFTP write from WS_FTP work if the handle size in Apache MINA sshd is set to 4: then we get 8 bytes of actual handle, and WS_FTP will send exactly 4 bytes too many, which Apache MINA sshd will accept, but for the wrong reason as it's not this "+4 confusion" but a bug at another level that just happens to also cause exactly 4 extra bytes.

tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 18, 2023
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
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 18, 2023
Change the file handle implementation: use raw bytes, not hexified
bytes. Representing file handles internally as hexified strings doubled
the size, so with the default setting of 16, an Apache MINA file handle
was actually 32 bytes on the network.

SftpModuleProperties.FILE_HANDLE_SIZE specified that its the size in
bytes of the file handle; so the final handle as sent to the client
should not be twice the configured size.

Because many APIs in Apache MINA sshd use String as the type to pass
around such file handles, we cannot easily change that without
potentially breaking a lot of user code. So keep storing file handles
as strings, but use ISO-8859-1 to convert: this lets us store any byte
value as a character.

When printing file handles, for instance in log messages, hexify them
explicitly.

Bug: apache#403
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 18, 2023
Change the default size for SFTP file handles to 4 bytes, and change
the implementation such that it produces collision-free 4-byte handles.

The algorithm for longer handles is left unchanged.

Using smaller SFTP file handles wastes less space in the SSH packets,
which can thus be used to send actual data. OpenSSH also uses 4-byte
SFTP file handles; using the same size also helps avoid strange bugs in
SFTP clients that might not always handle larger file handles correctly.

Bug: apache#403
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 18, 2023
Change the file handle implementation: use raw bytes, not hexified
bytes. Representing file handles internally as hexified strings doubled
the size, so with the default setting of 16, an Apache MINA file handle
was actually 32 bytes on the network.

SftpModuleProperties.FILE_HANDLE_SIZE specified that its the size in
bytes of the file handle; so the final handle as sent to the client
should not be twice the configured size.

Because many APIs in Apache MINA sshd use String as the type to pass
around such file handles, we cannot easily change that without
potentially breaking a lot of user code. So keep storing file handles
as strings, but use ISO-8859-1 to convert: this lets us store any byte
value as a character.

When printing file handles, for instance in log messages, hexify them
explicitly.

Bug: apache#403
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 18, 2023
Change the default size for SFTP file handles to 4 bytes, and change
the implementation such that it produces collision-free 4-byte handles.

The algorithm for longer handles is left unchanged.

Using smaller SFTP file handles wastes less space in the SSH packets,
which can thus be used to send actual data. OpenSSH also uses 4-byte
SFTP file handles; using the same size also helps avoid strange bugs in
SFTP clients that might not always handle larger file handles correctly.

Bug: apache#403
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 22, 2023
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
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 22, 2023
Change the file handle implementation: use raw bytes, not hexified
bytes. Representing file handles internally as hexified strings doubled
the size, so with the default setting of 16, an Apache MINA file handle
was actually 32 bytes on the network.

SftpModuleProperties.FILE_HANDLE_SIZE specified that its the size in
bytes of the file handle; so the final handle as sent to the client
should not be twice the configured size.

Because many APIs in Apache MINA sshd use String as the type to pass
around such file handles, we cannot easily change that without
potentially breaking a lot of user code. So keep storing file handles
as strings, but use ISO-8859-1 to convert: this lets us store any byte
value as a character.

When printing file handles, for instance in log messages, hexify them
explicitly.

Bug: apache#403
tomaswolf added a commit to tomaswolf/mina-sshd that referenced this issue Aug 22, 2023
Change the default size for SFTP file handles to 4 bytes, and change
the implementation such that it produces collision-free 4-byte handles.

The algorithm for longer handles is left unchanged.

Using smaller SFTP file handles wastes less space in the SSH packets,
which can thus be used to send actual data. OpenSSH also uses 4-byte
SFTP file handles; using the same size also helps avoid strange bugs in
SFTP clients that might not always handle larger file handles correctly.

Bug: apache#403
@tomaswolf tomaswolf added this to the 2.10.1 milestone Aug 24, 2023
@tomaswolf tomaswolf self-assigned this Aug 24, 2023
@tomaswolf
Copy link
Member

@dragonknight88 : PR #405 has been merged. Using the 2.10.1-SNAPSHOT release from the Apache Snapshot maven repository you could test whether it really works with WS_FTP with different handle sizes. If not, feel free to re-open this.

Ultimately it would be good if the vendor of WS_FTP fixed that bug in their SFTP client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants