Replacing trilead-ssh2 with Apache sshd

We have a lot of problems depending on something as outdated and abandoned as trilead-ssh2, one of the problems is that it still relies on SHA1 for KEX, so the only fix is to use the jenkins-ci fork: GitHub - jenkinsci/trilead-ssh2: Patched trilead-ssh2 used in Jenkins which is what everybody else seems to be doing, but that’s a poor replacement for using a maintained library.

Even the jsch fork of trilead-ssh2 is abandoned with several years since the last release, so that’s clearly not an option.

In the last couple of years Apache sshd has emerged as the java ssh client and server and it’s very actively maintained, so we’ve switched from jsch to Apache sshd in all of our other projects and it has proven to be feature rich, easy to work with and very robust.

I’ve tried my hand at ripping out trilead-ssh2 and replacing it with Apache sshd and it seems to work just fine in the first attempt.

My work sits in the switch-to-apache-sshd branch of my svnkit 1.10.4 fork on github:

I’m sure we could maintain a fork for our projects, but I’d rather get the changes up-stream so others can benefit as well.

This is the very first attempt, so I’m sure there are things I’ve forgotten and I’ve certainly left a very local test case behind, but if you’re interested in merging it, then I’ll make an effort to make it useful beyond our own usages.

I’ll continue to improve the patch when receiving feedback.

Hello Flemming,
I would gladly incorporate any improvements into the mainstream!

I wouldn’t throw away the current trilead-based implementation but introduce a JVM property to switch between implementations.

Until Apache sshd-based solution becomes stable enough, the default implementation would be trilead-based.

Once Apache sshd-based solution becomes stable, it will become the default implementation. But still there will be an ability to swtich back to the old trilead-based solution as a quick work-around if the new implementation would fail for some reason.

Another thing is that we would like to keep our current licensing scheme. We’re using dual-licensing scheme with commercial variant for closed source users and GPL-like variant for open source projects.

I’ll discuss the topic with my team, but I’m sure they’ll agree to what I wrote above.

I have re-implemented the sshd changes so it’s possible to switch implementation at runtime:

Most the changes are confined to this package and there’s a readme that documents the changes:

To try running with the Apache MINA SSHD ssh client add this jvmarg:
-Dsvnkit.ssh.client=apache

Feedback welcome

Hello Flemming,
you’re awesome! Sorry for not responding for a while.
I’ve added your changes to SVNKit locally.

Now I would like to ask you to confirm explicitly that you’re willing to give up your ownership of the code you’ve written and thus to allow us to put our usual header to the Java files you’ve added or changed:

/*
 * ====================================================================
 * Copyright (c) 2004-2022 TMate Software Ltd.  All rights reserved.
 *
 * This software is licensed as described in the file COPYING, which
 * you should have received as part of this distribution.  The terms
 * are also available at http://svnkit.com/license.html.
 * If newer versions of this license are posted there, you may use a
 * newer version instead, at your option.
 * ====================================================================
 */

Once (and if) you confirm that, I’ll commit your changes to SVNKit trunk.

Also I see that you’ve raised Java requirements from 1.6 to 1.8 but it’s ok because we use the same requirements as JavaHL interface and the latest JavaHL requires 1.8.

Sure, have at it, you’re welcome to the code, if that means that we can get rid of trilead-ssh2 at some point.

Note that I have improved the code yesterday, so you need the latest version.

I also upgraded to gradle 3.0, which is the oldest version of gradle that IDEA supports, but that’s also ancient, so perhaps you can take the opportunity to upgrade to gradle 7.3.

Note that I have just added ed25519 key type support by adding net.i2p.crypto:eddsa:0.3.0

We’re currently running the apache-sshd based fork in production and it seems stable, so I hope the change makes its way into a release.

Thank you very much. I’ll apply all your changes and once you don’t have changes anymore, we’ll release a new SVNKit version.

@dren.dk We recently implemented this on our servers. We are seeing some threading issues, potentially from connections not closing properly.

This stuck thread actually prevents the server from shutting down:

"Piping consumer" #2519 prio=5 os_prio=0 tid=0x00007f6a20270800 nid=0x65c waiting on condition [0x00007f6a104e8000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
        at java.lang.Thread.sleep(Native Method)
        at org.tmatesoft.svn.core.internal.io.svn.StreamLogger.lambda$new$0(StreamLogger.java:56)
        at org.tmatesoft.svn.core.internal.io.svn.StreamLogger$$Lambda$559/1479132319.run(Unknown Source)
        at java.lang.Thread.run(Thread.java:748)

There are a few more that indicate connections not closing:

"sshd-SshClient[9ced0fc]-nio2-thread-3" #15032 daemon prio=5 os_prio=0 tid=0x000055f6be15b000 nid=0x6761 waiting on condition [0x00007f770fe70000]
   java.lang.Thread.State: WAITING (parking)
        at sun.misc.Unsafe.park(Native Method)
        - parking to wait for  <0x0000000781c0f7d8> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
        at java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:2039)
        at java.util.concurrent.LinkedBlockingQueue.take(LinkedBlockingQueue.java:442)
        at java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1074)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1134)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

We only see these issues when using the new ssh implementation.

Thank you for the feedback, I’ll try to take a stab at it, I suspect it’s a relatively simple fix.

No problem. Let me know what you find

I have just committed what I think will fix your problem: 18a6ea050f9b4f27563f6eb27dfb66f47c727a4f

For some reason this forum doesn’t allow me to post a direct link to the commit on github???

/dren-dk/svnkit/commit/18a6ea050f9b4f27563f6eb27dfb66f47c727a4f

Please let me know if it helps.

@dren.dk Applied and tested that change and it does look like that resolves the issues

If @dren.dk agrees, I’ll include the fix into SVNKit, so it will become a part of the next release (1.10.6).

@dren.dk All good for the next svnkit release (1.10.6) ?

@dmitry.pavlenko We have a customer who is pushing hard to resolve this issue. The change has been confirmed on our side for two weeks. I realize that @dren.dk contributed this change but we are paying for support from TMate. I do not expect that this sits idle until the community blesses it for release. Please confirm when version 1.10.6 will be released with this change so that we can communicate appropriately with our customer.

Erick

Hello Erick,
We will release 1.10.6 with this fix on the next week.

Thanks Dmitry. We appreciate that!

Do you know if it will be first half or second half of the week? The reason why I am asking is that we are releasing the next version of our product on 5/21, so would really like 2-3 days to run it through our regression tests.

Thanks,
Erick

Taking that into account, we’ll release it during the first half of week, as soon as possible.
Sorry for that delay, I just had an impression that you had your the problem solved so it wasn’t so urgent.

Dmitry,

Perfect! Thanks for accommodating.

It appears to be resolved in the version we have built from source, we just want to ship the released version to the customer, and run the regression tests knowing there could be other changes in there as well.