mina-sshd.git
70 min agoChannelAsyncOutputStream: fix closing gracefully master 247/head
Thomas Wolf [Tue, 20 Sep 2022 19:19:07 +0000 (21:19 +0200)] 
ChannelAsyncOutputStream: fix closing gracefully

Window expansions can occur even when the channel is already closing.
Abort writing only if the stream is closed immediately, or is already
closed.

Also: a ChannelAsyncOutputStream must be closed before the channel is
unregistered, otherwise there cannot be any more window expansions. Fix
the order in ChannelSession (server side).

70 min ago[SSHD-1256] Disable one test with the Apache MINA transport
Thomas Wolf [Sat, 1 Oct 2022 13:12:46 +0000 (15:12 +0200)] 
[SSHD-1256] Disable one test with the Apache MINA transport

PortForwardingTest.testRemoteForwardingSecondTimeInSameSession()
frequently fails with the Apache MINA transport back-end because
of DIRMINA-1169.

Disable the test for now; it can be re-enabled once DIRMINA-1169 is
fixed.

4 days agoReleng: make test logs and results in GitHub accessible
Thomas Wolf [Fri, 23 Sep 2022 19:09:25 +0000 (21:09 +0200)] 
Releng: make test logs and results in GitHub accessible

When a GitHub CI build fails, the maven output often only shows an
exception. Store the test logs as artifacts in failed builds, so that
they can be downloaded and examined.

This should help figuring out why some tests fail sometimes in the
GitHub CI builds.

12 days ago[SSHD-1293] Fix unbinding port forwarding for auto-alloc port 241/head
Jan Philipp [Wed, 31 Aug 2022 20:34:07 +0000 (22:34 +0200)] 
[SSHD-1293] Fix unbinding port forwarding for auto-alloc port

Fix unbinding a local TCP/IP port forwarding binding when using a
dynamic auto-allocated port. "localAddress" is controlled by the
user and may contain still a port 0, which means auto-allocating
a local port; the actually bound local port is available in
"boundAddress".

The DefaultForwarder tracks the actual bound port mappings only,
so "boundAddress" must be used to unbind the port.

13 days ago[SSHD-822] Enable ProxyTest for MINA and Netty 244/head
Thomas Wolf [Sat, 17 Sep 2022 13:30:59 +0000 (15:30 +0200)] 
[SSHD-822] Enable ProxyTest for MINA and Netty

These tests run fine with the Netty or Apache MINA transports.

13 days ago[SSHD-822] Fix MINA transport for load tests and enable them
Thomas Wolf [Wed, 14 Sep 2022 15:00:38 +0000 (17:00 +0200)] 
[SSHD-822] Fix MINA transport for load tests and enable them

The load tests do run fine with Apache MINA. But enabling them
showed abysmal performance in port forwarding. Transferring about
400kB locally took about 10 seconds!

The reason is a possible bug in Apache MINA triggered by the recent
re-write of TCP/IP port forwarding. Commit b91a4245 changed the port
forwarding such that it disabled reads on the I/O port session
(IoSession.suspendRead()) while writing to the SSH tunnel, and re-
enabling reads (IoSession.resumeRead()) once the write was complete.
This avoids the need to buffer data inside Apache MINA; it throttles
reading from the port to the speed of writing through the tunnel.

This works fine with the NIO2 and Netty transports, but with Apache
MINA re-enabling reads becomes effective only on the next select call.
If a select call was already on-going, re-enabling reading might become
effective only after one second (the MINA idle time-out on the select).

Fix this by forcing an on-going select call to return immediately when
reading on a MINA IoSession is re-enabled. Apache MINA thus picks up
the change immediately and starts a new select() with the change right
away.

13 days ago[SSHD-822] Enable port forwarding and load tests for Netty
Thomas Wolf [Wed, 14 Sep 2022 14:33:21 +0000 (16:33 +0200)] 
[SSHD-822] Enable port forwarding and load tests for Netty

* LoadTest
* PortForwardingTest
* PortForwardingLoadTest

This only needs a test dependency on mina-core to work. mina-core is
used directly in the test to implement a simple echo server.

Also remove the requirement for Bouncy Castle from the load tests; they
run fine without.

13 days agoTCP/IP port forwarding: fix closing ChannelAsyncOutputStream
Thomas Wolf [Sat, 17 Sep 2022 17:37:35 +0000 (19:37 +0200)] 
TCP/IP port forwarding: fix closing ChannelAsyncOutputStream

The Channels using a ChannelAsyncOutputStream  must allow writing while
being closed, otherwise the ChannelAsyncOutputStream cannot be closed
gracefully because it uses the channel as its packet writer. When the
Channel is closed, it closes its ChannelAsyncOutputStream first, but by
that time the channel state is already "closing" and writes are normally
no longer allowed.

2 weeks agoBump Netty dependency to 4.1.81
Thomas Wolf [Thu, 8 Sep 2022 20:20:58 +0000 (22:20 +0200)] 
Bump Netty dependency to 4.1.81

2 weeks ago[SSHD-822] Enable symbolic link SFTP test also for OS X
Thomas Wolf [Wed, 22 Jun 2022 21:24:34 +0000 (23:24 +0200)] 
[SSHD-822] Enable symbolic link SFTP test also for OS X

OS X is a FreeBSD derivate; it does support symbolic links.

2 weeks ago[SSHD-822] Enable all ScpTests for MINA and Netty
Thomas Wolf [Wed, 22 Jun 2022 21:16:34 +0000 (23:16 +0200)] 
[SSHD-822] Enable all ScpTests for MINA and Netty

The three disabled tests work fine on MINA and Netty, too.

2 weeks ago[SSHD-822] Fix default PGP directory detection for OS X
Thomas Wolf [Tue, 21 Jun 2022 21:48:04 +0000 (23:48 +0200)] 
[SSHD-822] Fix default PGP directory detection for OS X

OS X uses the Unix/Linux name ".gnupg". Windows uses "gnupg".

Note that the real story is a bit more complicated. The directory not
necessarily is in the user's home directory (in particular on Windows;
where it is by default at %APPDATA%\gnupg). It can be set via
environment variable GNUPGHOME, and GPG has various other special
cases. See [1] for the gory details.

All these special cases have _not_ been implemented in this change.

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=575327

2 weeks ago[SSHD-822] Common tests: fix skipped tests
Thomas Wolf [Tue, 21 Jun 2022 21:09:16 +0000 (23:09 +0200)] 
[SSHD-822] Common tests: fix skipped tests

Use Assume.assumeTrue() only for platform-specific tests. Skipped tests
generate warnings in the output. There is exactly one platform-specific
test in the sshd-common tests in SelectorUtilsTests that tests behavior
specific to Windows.

Fix the other uses of assumptions such that the tests work in any case.
Supply the missing PKCS#8 encoded DSA key test resources for the PKCS#8
decoding test.

2 weeks agoMINA: provide a way to control the read buffer size
Thomas Wolf [Sun, 19 Jun 2022 20:44:28 +0000 (22:44 +0200)] 
MINA: provide a way to control the read buffer size

Apache MINA manages the read buffer size dynamically and adapts the
buffer size in the range from 64 bytes to 64kB, starting at 2kB. Give
users a way to influence this a little: use the NIO2_READ_BUFFER_SIZE
also in MINA for the initial buffer size, and add a new property
MIN_READ_BUFFER_SIZE to change the lower limit.

Then use these properties to make the tests previously disabled for
MINA in ApacheServerApacheClientTest and in ApacheServerJSchClientTest
work also for MINA by enforcing a minimum read buffer size of 32kB.

The NIO2_READ_BUFFER_SIZE property has _not_ been renamed, and its
property key has been left unchanged to not cause surprises for
existing clients using it, possibly via a system property.

Note that the NIO2 transport uses the default defined in
CoreModuleProperties (32kB) if the property is not set explicitly.
The MINA transport uses these properties only if set explicitly,
otherwise the MINA default behavior is effective.

The Netty transport uses neither property; read buffer management in
Netty works completely differently (via RecvByteBufAllocator).

2 weeks agoMake Netty respect our settings for SO_BACKLOG and SO_REUSEADDR
Thomas Wolf [Sun, 19 Jun 2022 16:03:23 +0000 (18:03 +0200)] 
Make Netty respect our settings for SO_BACKLOG and SO_REUSEADDR

Make the FactoryManager accessible to the NettyIoAcceptor and use it
to obtain the values configured in Apache MINA sshd, then set them
on the Netty ServerBootstrap.

2 weeks ago[Tests] Properly close SshClient and stop port forwardings
Thomas Wolf [Sun, 19 Jun 2022 15:39:37 +0000 (17:39 +0200)] 
[Tests] Properly close SshClient and stop port forwardings

Do not let a port forwarding from one test spill over into other tests.

2 weeks ago[SSHD-822] Enable more tests for MINA
Thomas Wolf [Sat, 18 Jun 2022 12:46:35 +0000 (14:46 +0200)] 
[SSHD-822] Enable more tests for MINA

The following tests work fine with MINA:

* AsyncAuthTest
* AsyncAuthInteractiveTest
* CipherTest
* CompressionTest
* NoServerNoClientTest (doesn't use MINA or SSH at all)
* SpringConfigTest

2 weeks ago[SSHD-822] Enable ApacheServer*Test for MINA
Thomas Wolf [Sat, 18 Jun 2022 12:43:37 +0000 (14:43 +0200)] 
[SSHD-822] Enable ApacheServer*Test for MINA

Enable the following two tests for MINA:

* ApacheServerApacheClientTest
* ApacheServerJSchClientTest

The classes contain fundamentally flawed tests though that assume that
large-ish data can be read in a single read() call (or in two read()
calls).

This can only work if all buffers in all layers are large enough to
hold the data. With MINA, this is not the case: MINA manages the read
buffer size for a channel/session *dynamically*, and it starts out at
2048 bytes. Therefore these tests *cannot* work with MINA, and are
disabled for MINA.

The other tests in these classes are enabled for MINA.

2 weeks ago[SSHD-822] Enable more tests for Netty
Thomas Wolf [Sat, 18 Jun 2022 08:21:03 +0000 (10:21 +0200)] 
[SSHD-822] Enable more tests for Netty

The following tests work fine with Netty:

* ApacheServerApacheClientTest
* ApacheServerJSchClientTest
* BuiltinIoServiceFactoryFactoriesTest
* SignatureFactoriesTest
* Sshd1033Test

2 weeks ago[SSHD-822] Enable ClientTest for Netty and MINA
Thomas Wolf [Sat, 18 Jun 2022 10:17:47 +0000 (12:17 +0200)] 
[SSHD-822] Enable ClientTest for Netty and MINA

Provide an implementation of IoSession.suspend() for Netty and MINA.
For Netty, make accesses to the ChannelHandlerContext thread safe.

Remove the `@BeforeClass` check that the BouncyCastle provider
was present: these tests runs fine if BC is not there.

2 weeks ago[SSHD-822] Netty fix for ClientDeadlockTest
Thomas Wolf [Sat, 18 Jun 2022 08:15:15 +0000 (10:15 +0200)] 
[SSHD-822] Netty fix for ClientDeadlockTest

This test failed on Netty because Netty swallows and logs exceptions
propagated out of exceptionCaught(). The server in this test was left
with a NettyIoSession without SSH session.

The fix is to close the NettyIoSession if we get an exception inside
channelActivated() when trying to create the SSH session for it.

Also remove the ChannelOption.ALLOW_HALF_CLOSURE in NettyIoSession.
It was added in commit d2f0b13e, but it's actually wrong. It prevents
closing a channel when the peer closes its output. It is not needed
if we want to shut down *our* output, which is what commit d2f0b13e was
about. With the option set, the ClientDeadlockTest would sometimes fail
because the client would not close the ClientSession when the server
closed the channel.

3 weeks ago[SSHD-822] Enable AsyncAuthInteractiveTest for Netty
Thomas Wolf [Sat, 18 Jun 2022 08:18:45 +0000 (10:18 +0200)] 
[SSHD-822] Enable AsyncAuthInteractiveTest for Netty

This test works, but runs about 4 times slower than with NIO2.

3 weeks ago[SSHD-822] Enable all SFTP tests also for Netty
Thomas Wolf [Fri, 17 Jun 2022 21:20:13 +0000 (23:20 +0200)] 
[SSHD-822] Enable all SFTP tests also for Netty

There's no reason to exclude some of them; they all work fine.

3 weeks agoSCP: Call the receive listener before sending back OK
Thomas Wolf [Sun, 19 Jun 2022 16:47:55 +0000 (18:47 +0200)] 
SCP: Call the receive listener before sending back OK

The listener may interdict the operation by throwing an exception. If
we've already sent an OK, it's possible that the client goes ahead and
tries to write the file data, and then gets an exception because the
channel is closed.

By calling the listener earlier we ensure that we send back a NACK, and
the client won't even try to upload anything.

3 weeks ago[SSHD-1294] Close MinaServiceFactory instances properly 243/head 246/head 248/head
Thomas Wolf [Thu, 8 Sep 2022 17:13:29 +0000 (19:13 +0200)] 
[SSHD-1294] Close MinaServiceFactory instances properly

Ensure that the IoProcessor pool is disposed, otherwise a selector may
not get closed, leading to a file handle leak.

5 weeks ago[maven-release-plugin] prepare for next development iteration
Guillaume Nodet [Mon, 22 Aug 2022 12:09:31 +0000 (14:09 +0200)] 
[maven-release-plugin] prepare for next development iteration

5 weeks ago[maven-release-plugin] prepare release sshd-2.9.1 sshd-2.9.1
Guillaume Nodet [Mon, 22 Aug 2022 12:09:24 +0000 (14:09 +0200)] 
[maven-release-plugin] prepare release sshd-2.9.1

5 weeks agoFix changelog
Guillaume Nodet [Mon, 22 Aug 2022 07:09:57 +0000 (09:09 +0200)] 
Fix changelog

5 weeks ago[maven-release-plugin] prepare for next development iteration
Guillaume Nodet [Mon, 22 Aug 2022 06:19:04 +0000 (08:19 +0200)] 
[maven-release-plugin] prepare for next development iteration

5 weeks ago[maven-release-plugin] prepare release sshd-2.9.1
Guillaume Nodet [Mon, 22 Aug 2022 06:09:29 +0000 (08:09 +0200)] 
[maven-release-plugin] prepare release sshd-2.9.1

5 weeks agoPrepare sshd 2.9.1 release
Guillaume Nodet [Mon, 22 Aug 2022 05:47:18 +0000 (07:47 +0200)] 
Prepare sshd 2.9.1 release

6 weeks agoAdded documentation regarding CLI properties values
Lyor Goldstein [Sat, 6 Aug 2022 04:58:58 +0000 (07:58 +0300)] 
Added documentation regarding CLI properties values

6 weeks ago[SSHD-1283] Added configuration property to control whether ScpShell is enabled
Lyor Goldstein [Thu, 28 Jul 2022 17:50:08 +0000 (20:50 +0300)] 
[SSHD-1283] Added configuration property to control whether ScpShell is enabled

7 weeks ago[SSHD-1290] ChannelAsyncOutputStream: improve logging 240/head
Thomas Wolf [Fri, 12 Aug 2022 21:28:08 +0000 (23:28 +0200)] 
[SSHD-1290] ChannelAsyncOutputStream: improve logging

More precise logging in onWritten() to distinguish the cases better.

7 weeks ago[SSHD-1288] SFTP: fix reading files that are being written 239/head
Thomas Wolf [Sun, 7 Aug 2022 17:48:49 +0000 (19:48 +0200)] 
[SSHD-1288] SFTP: fix reading files that are being written

If data is appended while a file is read via SFTP, SftpInputStreamAsync
would enter an infinite loop if requestOffset >= fileSize + bufferSize.
Fix this and issue only sequential read requests once we detect that
we're beyond the expected EOF.

7 weeks ago[SSHD-1289] Fix lock handling in KeyExchangeMessageHandler 238/head
Thomas Wolf [Thu, 11 Aug 2022 18:30:17 +0000 (20:30 +0200)] 
[SSHD-1289] Fix lock handling in KeyExchangeMessageHandler

Make sure that a thread does not try to acquire the write lock if
it already holds the read lock. This could happen if a write is not
enqueued and there is an exception during writing, and we then try
to close the session on the same thread.

The read/write lock is used for three purposes: first, it gives the
flushing thread trying to empty the queue of pending packets priority
over other threads trying to enqueue more packets, and second, it is
held during writeOrEnqueue() while writing a packet directly to prevent
that the KEX state changes between being checked and the write being
done, and third, to prevent that the KEX state changes asynchronously
while the flushing thread is checking it. The read/write lock itself
does not serve to ensure mutual exclusion on the KEX state itself.

These three functions can also be fulfilled if update() is executed
when only the read lock is held. If a thread in update() holds the read
lock, this can only occur if it wrote the buffer directly in
writeOrEnqueue(), in which case it is fine to proceed, and the flushing
thread for sure is not in its critical region where it holds the write
lock. Otherwise, any other thread either is the flushing thread and
holds the write lock already, or it's a thread not holding the lock at
all. In both cases it is fine to acquire the write lock.

8 weeks ago[SSHD-1281] Close Nio2Session properly on failure after connecting 236/head
Thomas Wolf [Tue, 2 Aug 2022 20:32:49 +0000 (22:32 +0200)] 
[SSHD-1281] Close Nio2Session properly on failure after connecting

If there's an exception in Nio2Connector after the session was set on
the DefaultIoConnectFuture, a program might just hang on that session
until the next read or write attempt, or until a timeout expired.

This was caused by closing the underlying connection, but not properly
closing the already existing session, and because the session was
already set on the future, setting the exception had no effect anymore.

Fix this by immediately closing the Nio2Session properly in that case.

2 months ago[SSHD-1285] Fix runtime linkage errors with Java 8 when building with Java versions > 8
Jeremy Norris [Fri, 29 Jul 2022 12:22:40 +0000 (07:22 -0500)] 
[SSHD-1285] Fix runtime linkage errors with Java 8 when building with Java versions > 8

2 months agoAdd link to version 2.9.0 release notes
Lyor Goldstein [Thu, 28 Jul 2022 17:22:33 +0000 (20:22 +0300)] 
Add link to version 2.9.0 release notes

2 months ago[maven-release-plugin] prepare for next development iteration
Guillaume Nodet [Mon, 18 Jul 2022 06:41:30 +0000 (08:41 +0200)] 
[maven-release-plugin] prepare for next development iteration

2 months ago[maven-release-plugin] prepare release sshd-2.9.0 sshd-2.9.0
Guillaume Nodet [Mon, 18 Jul 2022 06:41:22 +0000 (08:41 +0200)] 
[maven-release-plugin] prepare release sshd-2.9.0

2 months agoFix container tests when run from the src tgz in the distribution
Thomas Wolf [Thu, 14 Jul 2022 19:02:32 +0000 (21:02 +0200)] 
Fix container tests when run from the src tgz in the distribution

Running a "mvn clean install" from the source tar archive failed for
some container tests because unpacking the tar might not preserve
executable bits. Testcontainers need an entrypoint that is executable,
or otherwise the entrypoint script must not be run directly but via
a shell explicitly.

Rewrite the two problematic tests to ensure the entrypoint script is
always executable, irrespective of whether the test resource has the
bit set.

2 months agoUpgraded logback version to 1.2.11
Lyor Goldstein [Wed, 13 Jul 2022 17:31:38 +0000 (20:31 +0300)] 
Upgraded logback version to 1.2.11

2 months agoUpgraded Maven Surefire plugin version to 3.0.0-M7
Lyor Goldstein [Wed, 13 Jul 2022 16:40:01 +0000 (19:40 +0300)] 
Upgraded Maven Surefire plugin version to 3.0.0-M7

2 months agoUpgraded Groovy version to 3.0.11
Lyor Goldstein [Wed, 13 Jul 2022 16:35:06 +0000 (19:35 +0300)] 
Upgraded Groovy version to 3.0.11

2 months agoUpgraded Checkstyle version to 9.3
Lyor Goldstein [Wed, 13 Jul 2022 16:33:35 +0000 (19:33 +0300)] 
Upgraded Checkstyle version to 9.3

2 months agoUpgraded Maven OWASP check plugin version to 7.1.1
Lyor Goldstein [Wed, 13 Jul 2022 16:30:37 +0000 (19:30 +0300)] 
Upgraded Maven OWASP check plugin version to 7.1.1

2 months agoUpgraded Maven PMD plugin version to 3.17.0
Lyor Goldstein [Wed, 13 Jul 2022 16:29:15 +0000 (19:29 +0300)] 
Upgraded Maven PMD plugin version to 3.17.0

2 months agoUpgraded PMD version to 6.47.0
Lyor Goldstein [Wed, 13 Jul 2022 16:28:32 +0000 (19:28 +0300)] 
Upgraded PMD version to 6.47.0

2 months ago[maven-release-plugin] prepare for next development iteration
Guillaume Nodet [Mon, 11 Jul 2022 13:14:16 +0000 (15:14 +0200)] 
[maven-release-plugin] prepare for next development iteration

2 months ago[maven-release-plugin] prepare release sshd-2.9.0
Guillaume Nodet [Mon, 11 Jul 2022 13:14:02 +0000 (15:14 +0200)] 
[maven-release-plugin] prepare release sshd-2.9.0

2 months agoPrepare sshd 2.9.0 release
Guillaume Nodet [Mon, 11 Jul 2022 13:09:27 +0000 (15:09 +0200)] 
Prepare sshd 2.9.0 release

2 months ago[SSHD-1276] Added capability to redirect command/shell STDERR stream to STDOUT one
Lyor Goldstein [Thu, 7 Jul 2022 05:58:59 +0000 (08:58 +0300)] 
[SSHD-1276] Added capability to redirect command/shell STDERR stream to STDOUT one

3 months agoUpdated CHANGES.md to include SSHD-1273
Lyor Goldstein [Sat, 2 Jul 2022 06:19:41 +0000 (09:19 +0300)] 
Updated CHANGES.md to include SSHD-1273

3 months ago[SSHD-1273] Add support to use env vars together with subsystem
adanilenka [Wed, 29 Jun 2022 10:17:58 +0000 (13:17 +0300)] 
[SSHD-1273] Add support to use env vars together with subsystem

3 months agoREADME.md: Change implementation detail lists to header items
David Ostrovsky [Sun, 26 Jun 2022 09:40:03 +0000 (11:40 +0200)] 
README.md: Change implementation detail lists to header items

Doing this would allow projects to include external links to the
implementation specific section in upstream documentation.

3 months agoREADME.md: Fix names of 3dec-cbc and blowfish-cbc
David Ostrovsky [Sun, 26 Jun 2022 09:00:41 +0000 (11:00 +0200)] 
README.md: Fix names of 3dec-cbc and blowfish-cbc

Use the external algorithm names, not the internal Java identifiers
used in Apache MINA sshd.

s/tripledescbc/3des-cbc
s/blowfishcbc/blowfish-cbc

3 months ago[SSHD-1272] Mention in CHANGES.md
Thomas Wolf [Sat, 25 Jun 2022 14:24:09 +0000 (16:24 +0200)] 
[SSHD-1272] Mention in CHANGES.md

3 months ago[SSHD-1272] Client-side pubkey auth: use session's signature factories
Thomas Wolf [Thu, 23 Jun 2022 16:34:58 +0000 (18:34 +0200)] 
[SSHD-1272] Client-side pubkey auth: use session's signature factories

When determining the signature algorithm for a public key, fall back
properly to the session's signature factories if none can be found
earlier. Skip keys for which no signature algorithm can be determined.
Provide a way for client code to supply a last-resort default signature
name, if desired.

3 months agoBump Netty dependency to 4.1.78
Thomas Wolf [Sat, 18 Jun 2022 09:59:55 +0000 (11:59 +0200)] 
Bump Netty dependency to 4.1.78

3 months agoSSHD-1269: Fix TCP/IP remote port forwarding with wildcard addresses
Thomas Wolf [Mon, 13 Jun 2022 07:13:15 +0000 (09:13 +0200)] 
SSHD-1269: Fix TCP/IP remote port forwarding with wildcard addresses

Remote port forwarding using an OpenSSH client and an Apache MINA sshd
server didn't work with wildcard addresses or with "localhost". For
instance,

ssh ... -R 0.0.0.0:0:somewhere:1234

would fail: the Apache MINA sshd server would send a forwarded-tcpip
request with 127.0.0.1:55555 (if port 55555 was chosen) to OpenSSH,
but OpenSSH expected 0.0.0.0:55555.

ssh ... -R 0:somewhere:1234

also failed in the same way.

Fix this by sending back in the forwarded-tcpip request the original
address with the bound port.

(Note: this was already fixed by SSHD-792 as of Apache MINA sshd 2.2.0,
but was broken again in 2.6.0. Unit tests using an Apache MINA sshd
client to initiate the remote port forwarding didn't detect the problem
because Apache MINA sshd only checks the port but not the hostname.)

3 months agoNetty: use proper API to shut down output on a socket 227/head
Thomas Wolf [Wed, 15 Jun 2022 20:27:35 +0000 (22:27 +0200)] 
Netty: use proper API to shut down output on a socket

Use the proper Netty API DuplexChannel.shutdownOutput() instead of a
reflective hack.

3 months agoSSHD-1055: Half-close port-forwarded socket on EOF in tunnel
Thomas Wolf [Sat, 11 Jun 2022 10:04:03 +0000 (12:04 +0200)] 
SSHD-1055: Half-close port-forwarded socket on EOF in tunnel

Propagate an EOF on a tunnel for port forwarding to the socket by
shutting down its output; otherwise the client connected to that
socket may hang.

For the MINA transport back-end, calling socket.shutdownOutput(), which
is possible only via reflection, leads to problems as it confuses the
main polling loop inside MINA. So don't do that but schedule the MINA
session to close once all pending writes have been written.

Enable half-closure in the Netty transport back-end.

Fix the Nio2 transport back-end to not re-try accepting connections
when the channel is already closing.

See also SSHD-964 and SSHD-902.

3 months agoBump MINA dependency to 2.0.23
Thomas Wolf [Sat, 11 Jun 2022 09:55:46 +0000 (11:55 +0200)] 
Bump MINA dependency to 2.0.23

3 months agoTCP/IP forwarding: always resume reading, even in error cases
Thomas Wolf [Fri, 10 Jun 2022 07:30:17 +0000 (09:30 +0200)] 
TCP/IP forwarding: always resume reading, even in error cases

Follow-up on commit 3c2e8b6f: missed a case in the DefaultForwarder.

3 months agoBump spring-core from 5.3.14 to 5.3.20
dependabot[bot] [Wed, 25 May 2022 06:03:42 +0000 (06:03 +0000)] 
Bump spring-core from 5.3.14 to 5.3.20

Bumps [spring-core](https://github.com/spring-projects/spring-framework) from 5.3.14 to 5.3.20.
- [Release notes](https://github.com/spring-projects/spring-framework/releases)
- [Commits](https://github.com/spring-projects/spring-framework/compare/v5.3.14...v5.3.20)

---
updated-dependencies:
- dependency-name: org.springframework:spring-core
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
3 months agoTCP/IP forwarding: always resume reading, even in error cases 226/head
Thomas Wolf [Fri, 3 Jun 2022 12:45:26 +0000 (14:45 +0200)] 
TCP/IP forwarding: always resume reading, even in error cases

PortForwardingTest did hang sometimes in GitHub CI (Windows/Java 1.8).

Also: don't redirect test output to a file; these files are not
accessible once a GitHUb CI pod has terminated. By not redirecting,
the GitHUb CI build log has much more information useful to analyze
potential problems.

3 months agoAsynchronous handling of global requests 224/head
Thomas Wolf [Thu, 19 May 2022 19:34:10 +0000 (21:34 +0200)] 
Asynchronous handling of global requests

Handling of global requests with want-reply=true was synchronous. This
is fine if the call is made on a non-I/O thread, but to be able to use
global requests in I/O threads, an asynchronous handling is needed.

Provide an asynchronous variant of Session.request() that returns a
future and that takes an optional reply handler as argument. If a
handler is given, it will be invoked by the framework once the reply
is received, on the I/O thread that received the reply. The future can
in this case be used to wait until the request has indeed be sent.
Without handler, the future can be used to await the reply on some
other thread, or to handle it via a future listener.

This makes global request handling much more flexible, avoids that a
synchronous implementation blocks an I/O thread, and it avoids problems
with making global requests while a key exchange is ongoing.

It also removes the restriction that there must be only one pending
global request per SSH session. The implementation keeps a FIFO list
of requests sent, and associates a reply with the front-most request
in that list. Only requests with want-reply=true enter that list.

Because of SSHD-968 we must also know the SSH message sequence number
of each global request made with want-reply=true. The previous
implementation assumed that the when a request was made, the next
packet written would be that request. But that may not be true when a
KEX is ongoing. Improve that implementation to report the sequence
number via a callback when the request's packet is indeed sent.

In AbstractSession.preClose(), fail all pending global requests.

Additionally, make Session.request() also work for global requests with
want-reply=false. Such requests can also be written directly with
Session.writePacket(), but it seems natural and convenient for users
of the library to also be able to use Session.request() for this.

Add some fairly advanced unit tests to verify the implementation. The
new unit tests showcase different implementations, somewhat simplified,
of a client-side OpenSshHostKeysHandler.

Add some technical documentation on global requests.

4 months ago[SSHD-1254] Client side implementation of host-bound pubkey auth 213/head
Thomas Wolf [Sat, 19 Mar 2022 15:39:30 +0000 (16:39 +0100)] 
[SSHD-1254] Client side implementation of host-bound pubkey auth

Provide a KexExtensionParser for it; make the default client KEX
extension handler understand the "publickey-hostbound@openssh.com"
extension, and use it in UserAuthPublicKey.

The extension changes the auth name in the SSH_MSG_USERAUTH_REQUEST
from "pubkey" to "publickey-hostbound-v00@openssh.com" and includes
the server's public key in the message and its signature.

Includes a container test using publickey authentication to log in to
an OpenSSH 9.0 server, verifying that the Apache MINA sshd client got
the KEX extension and checking the server log from the container to
verify that the server got a host-based signature.

See https://github.com/openssh/openssh-portable/blob/807be686/PROTOCOL#L347

4 months ago[SSHD-1262] Eliminate buffering in TCP/IP port forwarding 223/head
Thomas Wolf [Mon, 9 May 2022 20:32:59 +0000 (22:32 +0200)] 
[SSHD-1262] Eliminate buffering in TCP/IP port forwarding

TCP/IP port forwarding started reading on the local port before the SSH
tunnel was established, i.e., before SSH_MSG_CHANNEL_OPEN_CONFIRMATION
was received. It buffered data until then, and then tried to flush the
buffer once the confirmation was received.

This didn't work if the channel window was exhausted while that buffered
data was to be written, and in particular it failed when the channel was
opened with an initial window size of zero.

Change the whole forwarding setup: first set up the SSH tunnel, only
then start reading from the local port. That way, no data needs to be
buffered at all.

Use a ChannelAsyncOutputStream to forward data read into the SSH tunnel;
this nicely adapts to the channel window. To avoid concurrent writes on
this channel, suspend reading from the local port before writing, and
resume it only after the data was written. This automatically throttles
reading to the speed of writing, which is what one wants in this case.

Implement IOSession.suspendRead() and IOSession.resumeRead() also for
the MINA and Netty back-ends. Fix the implementation for the NIO2
back-end: the former implementation of resumeRead() could cause
concurrent reads if the channel indeed did an asynchronous read, and if
the channel did a synchronous read it led to deep completion handler
chains on the stack, and could delay the next read on some other
session's completion handler. Resolve this for our use-case by
scheduling an asynchronous read explicitly if necessary, and letting
the normal read completion handler do the read if possible.

Do this also for the TcpipServerChannel: it started reading from the
connected port before it even sent SSH_MSG_CHANNEL_OPEN_CONFIRMATION.
Remove the BufferedIoOutputStream and use a ChannelAsyncOutputStream
always.

Run the forwarding writes through ThreadUtils.runAsInternal(): the
write might otherwise block if a KEX was on-going, but we should never
block NIO threads. With ThreadUtils.runAsInternal(), the write may
enqueue a packet, but the write future will be fulfilled only once this
enqueued packet was flushed when KEX is done, so we won't read from the
port until then and thus don't run the risk that a fast producer puts
thousands of packets onto the internal queue during KEX.

Fix a bug in AbstractChannel that became apparent with this change: when
a peer disconnects before having sent SSH_MSG_OPEN_CONFIRMATION for a
channel, that channel cannot be closed gracefully because the peer's id
for it is unknown.

Include technical documentation on TCP/IP port forwarding.

4 months agoMention SSHD-1266 in CHANGES.md
Thomas Wolf [Sat, 14 May 2022 16:26:09 +0000 (18:26 +0200)] 
Mention SSHD-1266 in CHANGES.md

4 months agoFix typo in README
Thomas Wolf [Sat, 14 May 2022 11:22:36 +0000 (13:22 +0200)] 
Fix typo in README

4 months agoMark more tests as NoIoTestCases
Thomas Wolf [Fri, 13 May 2022 19:00:53 +0000 (21:00 +0200)] 
Mark more tests as NoIoTestCases

4 months agoFix the Netty connection back-end
Thomas Wolf [Fri, 13 May 2022 18:12:20 +0000 (20:12 +0200)] 
Fix the Netty connection back-end

Running the tests with Netty frequently resulted in failures due to
connection time-outs. The root cause of this was a race condition
in NettyIoConnector: if the connection succeeded and the NettyIoSession
was initialized via channelActive() before the connector had set
the DefaultIoConnectFuture attribute on the channel, then that future
would never be signalled.

Rewrite the NettyIoConnector to not use a global Bootstrap for all
connections. Use a new and dedicated Bootstrap per connection. This
makes is possible to use Bootstrap.attr() to have the channel attribute
be set early on, before even the connection is attempted. For the
context AttributeRepository, one then doesn't even need an attribute;
it's available directly.

Also don't set SO_BACKLOG in the connector; this socket option only
makes sense for server sockets, i.e., in the acceptor.

Enable running the sshd-core tests also with Netty.

4 months ago[SSHD-1266] OpenSSH certificates: fix encoding of critical options 222/head
Zeljko Vukovic [Tue, 10 May 2022 11:29:23 +0000 (13:29 +0200)] 
[SSHD-1266] OpenSSH certificates: fix encoding of critical options

Option values are not encoded directly, but must be wrapped in an
additional buffer. This would allow for an option to have multiple
data items.

Also enforce the ordering and uniqueness constraints on options.

4 months agoRevert "Upgraded Checkstyle plugin version to 10.2"
Thomas Wolf [Thu, 12 May 2022 17:37:19 +0000 (19:37 +0200)] 
Revert "Upgraded Checkstyle plugin version to 10.2"

This reverts commit b8bcdce95a4e4f46a6ab5aebd4508112982fe9fa.

This version of the checkstyle plug-in does not work with Java 1.8.

4 months agoRevert "Upgraded formatter plugin version to 2.18.0"
Lyor Goldstein [Thu, 12 May 2022 16:36:18 +0000 (19:36 +0300)] 
Revert "Upgraded formatter plugin version to 2.18.0"

This reverts commit 9f8aa3dbafe3934bad661d36a1d6bf20e80baada.

4 months agoUpgraded Maven surefire plugin to 3.0.0-M6
Lyor Goldstein [Mon, 9 May 2022 17:48:10 +0000 (20:48 +0300)] 
Upgraded Maven surefire plugin to 3.0.0-M6

4 months agoUpgraded Maven PMD plugin version to 3.16.0
Lyor Goldstein [Mon, 9 May 2022 17:45:08 +0000 (20:45 +0300)] 
Upgraded Maven PMD plugin version to 3.16.0

4 months agoUpgraded PMD version to 6.45.0
Lyor Goldstein [Mon, 9 May 2022 17:44:17 +0000 (20:44 +0300)] 
Upgraded PMD version to 6.45.0

4 months agoUpgraded GMaven+ plugin to version 1.13.1
Lyor Goldstein [Mon, 9 May 2022 17:43:04 +0000 (20:43 +0300)] 
Upgraded GMaven+ plugin to version 1.13.1

4 months agoUpgraded Groovy version to 3.0.10
Lyor Goldstein [Mon, 9 May 2022 17:42:06 +0000 (20:42 +0300)] 
Upgraded Groovy version to 3.0.10

4 months agoUpgraded formatter plugin version to 2.18.0
Lyor Goldstein [Mon, 9 May 2022 17:40:49 +0000 (20:40 +0300)] 
Upgraded formatter plugin version to 2.18.0

4 months agoUpgraded Checkstyle plugin version to 10.2
Lyor Goldstein [Mon, 9 May 2022 17:40:17 +0000 (20:40 +0300)] 
Upgraded Checkstyle plugin version to 10.2

4 months agoUpgraded OWASP dependency check pluging to version 7.1.0
Lyor Goldstein [Mon, 9 May 2022 17:39:16 +0000 (20:39 +0300)] 
Upgraded OWASP dependency check pluging to version 7.1.0

4 months agoKEX: handle SSH_MSG_SERVICE_REQUEST and SSH_MSG_SERVICE_ACCEPT 217/head
Thomas Wolf [Thu, 21 Apr 2022 18:06:47 +0000 (20:06 +0200)] 
KEX: handle SSH_MSG_SERVICE_REQUEST and SSH_MSG_SERVICE_ACCEPT

According to RFC 4253, these two messages must not be sent during KEX.
Queue them, or block the thread trying to send them, as appropriate.

This also enables us to start flushing queued pending writes already in
KEX state KEYS.

Also accept SSH_MSG_SERVICE_REQUEST when in KEX state INIT. After having
sent our KEX_INIT, it's possible that we receive service requests "in
flight" before the peer's KEX_INIT. A potential service accept message
to send back will be delayed until after the KEX. If no KEX was run yet,
refuse the service request.

Do the same also for SSH_MSG_SERVICE_ACCEPT.

This change uncovered a bug in ClientUserAuthService, which suddenly
sent the SSH_MSG_USER_AUTH 'none' before the SSH_MSG_SERVICE_REQUEST
'user-auth'. This happened because SSH_MSG_USER_AUTH got written before
the service was even started; the message got queued, and flushing the
queue starts now before KEX state done. Until now this was hidden
because SSH_MSG_SERVICE_REQUEST was written on the KeyEstablished event
and was treated as a low-level message, and thus got sent before
flushing the queue started.

Fix this by delaying sending SSH_MSG_USER_AUTH until the service has
indeed been started (and the SSH_MSG_SERVICE_REQUEST has been sent).

Include some documentation on KEX.

4 months ago[SSHD-966] Flush pending packets asynchronously at end of KEX
Thomas Wolf [Fri, 15 Apr 2022 05:33:52 +0000 (07:33 +0200)] 
[SSHD-966] Flush pending packets asynchronously at end of KEX

Previous code flushed them synchronously before setting KEX state to
DONE. This could lead to deadlocks if there were exceptions and the
session got closed while flushing.

Moreover, AbstractSession.writePacket() had a race condition: after
having determined the kex state, it was still possible that an incoming
SSH_MSG_KEX_INIT would change the KEX state and then get the encodeLock
and send back its answer before the first thread. In that case the
client might have received a high-level message during KEX, which is
forbidden by RFC 4253.

Change the code to flush pending packets asynchronously. In
writePackets, keep enqueuing packets if KEX is DONE but we're still
flushing. To avoid keeping the flushing thread busy forever, we block
new writes, unless they are direct replies to SSH protocol messages.
I.e., channel data messages block the calling thread while flushing
is still in progress.

Some care has to be exercised to avoid new deadlocks. First, obviously
a thread holding the decodeLock must not be blocked. Second, threads
holding the futureLock must never block. Additionally, the code never
blocks writes that occur inside a future listener.

Note that while KEX is in progress (not DONE yet), an application may
put arbitrarily many packets onto the queue (for instance, if the
remote's channel window is large, and there's an asynchronous
application thread pumping data through a channel). This was the case
already before this change, and it is still the case. If there is so
much data in the queue that a new KEX will be triggered, the new code
will actually initiate a new KEX, flushing stops, and is resumed once
the new KEX finishes.

4 months agoFine-grained synchronization in ChannelOutputStream
Thomas Wolf [Sun, 17 Apr 2022 21:04:15 +0000 (23:04 +0200)] 
Fine-grained synchronization in ChannelOutputStream

Allow closing a ChannelOutputStream asynchronously. The broad
synchronization on the stream instance's monitor in both write() and
flush() was the immediate cause of the deadlock reported in SSHD-966.

The Apache MINA sshd framework needs to be able to asynchronously
close a ChannelOutputStream when there are errors and the session
is shut down on exceptions.

The only synchronized method is now write(), and synchronization is
used solely to prevent concurrent writes. The state shared between
write() and flush() (the buffer state) is guarded separately, and is
never locked while waiting for window adjustments or while writing.

4 months agoFine-grained synchronization in ChannelAsyncOutputStream
Thomas Wolf [Sun, 17 Apr 2022 19:21:09 +0000 (21:21 +0200)] 
Fine-grained synchronization in ChannelAsyncOutputStream

Remove the broad locking on the stream instance's monitor. Holding
such broad locks during write operations may easily deadlock. Instead
guard only some state fields, and make sure these are never locked
during write operations, or when futures are fulfilled, which may
notify arbitrary listeners, which might trigger writes.

The idea of this ChannelAsyncOutputStream is to write as much of a
buffer as possible as individual packets. When there is not enough
window space, stop writing and let the write be resumed asynchronously
once the window has been enlarged; then write the rest of the buffer.

The implementation treats the buffer to be written as a token: while
it is being processed, the pendingWrite field is null. This ensures
that always only one thread is processing the buffer to write the next
chunk. Otherwise it might be possible that an asynchronous window
adjustment tried to process the buffer while it already was being
written.

Closing a ChannelAsyncOutputStream gracefully is tricky and may not
always be possible, depending on the reason for closing it. If channel
window adjustments are not handled anymore, writing the last pending
buffer in full may not be possible if the stream is waiting for more
window space. In that case, the stream may close with some unwritten
data, but that is unavoidable. (The previous code had a similar problem;
see SSHD-1261; this implementation should also solve that issue.)

5 months ago[SSHD-1264] Use the same KEX proposal for every KEX in a session 221/head
Thomas Wolf [Mon, 25 Apr 2022 20:03:52 +0000 (22:03 +0200)] 
[SSHD-1264] Use the same KEX proposal for every KEX in a session

The KEX proposal (algorithms and so on) is created once upon the first
KEX, and subsequently must not be changed anymore during the session.
Re-KEX must use the same proposal in this session, otherwise it may
fail because suddenly a different host key might be used (if the server
has several), or it might not be possible to negotiate algorithms when
it was possible on the initial KEX.

OpenSSH also evaluates the proposal only once per session and then
re-uses it; only the cookie in the SSH_MSG_KEX_INIT message changes
every time it is sent.

5 months ago[SSHD-1264] Add a test case
James Nord [Mon, 25 Apr 2022 12:36:59 +0000 (13:36 +0100)] 
[SSHD-1264] Add a test case

Use a CentOS container running OpenSSH 7.4 configured to re-key every
second. Connect with an Apache MINA sshd client and write some data
every second for 20 seconds. Check that we got multiple key exchanges.

Install a ServerKeyVerifier that fails if the host key on re-keying is
different from the key presented in the initial KEX.

Note that the problem is not reproducible with newer OpenSSH (as
available for instance for Alpine containers).

5 months agoRelax locking on CurrentService
Thomas Wolf [Sun, 17 Apr 2022 17:20:19 +0000 (19:20 +0200)] 
Relax locking on CurrentService

I don't think we have to run Service.process() while holding a lock on
the current service instance.

5 months agoBump Netty dependency to 4.1.76
Thomas Wolf [Sun, 17 Apr 2022 17:17:40 +0000 (19:17 +0200)] 
Bump Netty dependency to 4.1.76

5 months agoKEX: change output encoding atomically
Thomas Wolf [Sun, 10 Apr 2022 16:53:34 +0000 (18:53 +0200)] 
KEX: change output encoding atomically

Ensure the `encodeLock` is held for sending the `SSH_MSG_NEWKEYS`
message and changing the output encoding.

For the input encoding a similar change is not needed since the
`decodeLock` is held anyway.

Strictly speaking acquiring the `encodeLock` should not be necessary
since any other potentially concurrent writes should be queued because
the KEX state is not yet `DONE`.

5 months agoFix ClientTest
Thomas Wolf [Fri, 15 Apr 2022 15:51:05 +0000 (17:51 +0200)] 
Fix ClientTest

Don't use `PublicKey.equals()` to check two `PublicKey` instances for
equality. This may fail depending on the implementation class. Use our
own `KeyUtils.compareKeys()` instead.

5 months agoSimplify uses of the KEX future
Thomas Wolf [Sat, 9 Apr 2022 13:25:51 +0000 (15:25 +0200)] 
Simplify uses of the KEX future

`DefaultKeyExchangeFuture.setValue()` allows setting the value only
once. Subsequent invocations are no-ops. It is thus not necessary to
synchronize on that future and set the value only if not set yet.

5 months agoClean up service support 216/head
Thomas Wolf [Fri, 8 Apr 2022 21:53:12 +0000 (23:53 +0200)] 
Clean up service support

Previous code declared a `SessionHelper.sessionLock` and used it in
`AbstractSession` around `doHandleMessage()` and in `ClientSessionImpl`
to partially guard service-related info (the service names and the next
service) against concurrent accesses.

The use of this `sessionLock` around handling messages was a way to
synchronize access to the `currentService` field.

Encapsulate the current service in a synchronized per-session
`CurrentService` object. In `ClientSessionImpl`, install a customized
object that also can handle switching the service (from authentication
to connections), and set the `initialRequestSent` flag atomically.

Message handling is serialized per session already via the `decodeLock`.
Remove the locking in `AbstractSession.handleMessage()`.

Note that like previous code `AbstractServerSession.startService()`
actually does _not_ call `start()` on the service. This looks like an
oversight, but doing so leads to flaky tests in my test runs.

5 months ago[SSHD-1257] Updated CHANGES.md file with issue description
Lyor Goldstein [Wed, 6 Apr 2022 16:24:58 +0000 (19:24 +0300)] 
[SSHD-1257] Updated CHANGES.md file with issue description

5 months ago[SSHD-1257] ChannelSession: don't flush out stream if already closed
Vincent Latombe [Thu, 31 Mar 2022 13:40:15 +0000 (15:40 +0200)] 
[SSHD-1257] ChannelSession: don't flush out stream if already closed

Since OutputStream.close() is a no-op when the stream is already closed
and does flush if not, use close() instead of flush().

This use in ChannelSession is the last use of this OutputStream. It
may get closed again when the ChannelSession itself closes, but that
is then just a no-op.

7 months agoOrganized POM properties in a more consistent manner
Lyor Goldstein [Fri, 18 Feb 2022 10:11:54 +0000 (12:11 +0200)] 
Organized POM properties in a more consistent manner