Replacing trilead-ssh2 with Apache sshd

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.

Hello Erick,
I would like to let you know that we’ve uploaded 1.10.6 to Maven Central. Once it passes their review process, it’ll become publicly available. Meanwhile you can use our mirror of Maven Central https://maven.tmatesoft.com/ for testing purposes (the new SVNKit is already there). Later in the day we will announce 1.10.6 release on svnkit.com website.

SVNKIT-764: a work-around for the case if the channel fails to be opened (the original SshConnection#reOpen wasn’t called in that case). On my tests discarding the whole pool with SVNSSHConnector#recreateSessionPoolForTest makes things faster though.

I tried to extract SSH code to tests it alone (without SVNKit):

        final File privateKey = new File("/path/to/id_rsa");

        try (final SshClient sshClient = SshClient.setUpDefaultClient()) {
            sshClient.setIoServiceFactoryFactory(new Nio2ServiceFactoryFactory());

            final byte[] privateKeyBytes = SVNFileUtil.readFully(privateKey);
            assert privateKeyBytes != null;
            final Iterable<KeyPair> keyPairs = SecurityUtils.loadKeyPairIdentities(null, () -> "private key",
                    new ByteArrayInputStream(privateKeyBytes), FilePasswordProvider.EMPTY);
            if (!keyPairs.iterator().hasNext()) {
                throw new RuntimeException("Did not find a keypair in the provided private key string: " + privateKey);
            }
            sshClient.addPublicKeyIdentity(keyPairs.iterator().next());
            sshClient.setServerKeyVerifier((clientSession, remoteAddress, serverKey) -> true);
            sshClient.setHostConfigEntryResolver(DefaultConfigFileHostEntryResolver.INSTANCE);

            sshClient.start();

            ConnectFuture connectFuture = sshClient.connect("username", "hostname", 22);
            connectFuture.await();

            try (final ClientSession clientSession = connectFuture.getClientSession()) {
                clientSession.auth().verify(10000);

                for (int i = 0; i < 20; i++) {
                    System.out.println("i = " + i);

                    try (final ChannelExec channel = clientSession.createExecChannel("svnserve -t")) {
                        OpenFuture openFuture = channel.open();
                        openFuture.await();
                        if (openFuture.isOpened()) {
                            System.out.println("opened");
                        } else {
                            System.out.println("not opened");
                        }
                        
                        final InputStream in = new ByteArrayInputStream(new byte[0]);
                        final OutputStream out = new ByteArrayOutputStream();
                        final OutputStream err = new ByteArrayOutputStream();

                        channel.setIn(in);
                        channel.setOut(out);
                        channel.setErr(err);
                        
                        SVNFileUtil.closeFile(err);
                        SVNFileUtil.closeFile(out);
                        SVNFileUtil.closeFile(in);
                        
                        channel.close(true);
                        channel.waitFor(Collections.singletonList(ClientChannelEvent.CLOSED), Duration.ofDays(30));
                    }
                }
            }
        }

it outputs

i = 0
opened
i = 1
opened
i = 2
opened
i = 3
opened
i = 4
opened
i = 5
opened
i = 6
opened
i = 7
opened
i = 8
opened
i = 9
opened
i = 10
not opened
i = 11
not opened
i = 12
not opened
i = 13
not opened
i = 14
not opened
i = 15
not opened
i = 16
not opened
i = 17
not opened
i = 18
not opened
i = 19
not opened

This can be perfectly explained by the fact that by default in OpenSSH MaxSessions option is 10. If I understand coerrectly, this corresponds to 10 Apache SSHD channels. (And the TCP connection corresponds to ClientSession class).

The code above shows that either:

  • the counter doesn’t reset when closing the channel;
  • the code above doesn’t close the channel correctly.

In SVNKit there’s a sessions counter SshConnection#sessionCount which is incremented when the channel is opened and decremented when it is closed. But for now it seems that after several channel open/close cycles exhaust the connection and another one should be opened in the pool. Or there should be a pool of open channels and they shouldn’t be closed (though it’s suspicious from authentication point of view). Or maybe there’s a way to close the channel to decrement the counter on the server side.

Another difficulty is to detect "MaxSessions limit hit" because openFuture.getException() contains generic “open failed” error with no further details.

  • the code above doesn’t close the channel correctly.

Maybe a session should be re-created after 10 channels. Or there is a bug inside the channel.close() call or it is not enough…

UPD: Or channels could be re-used?

I’ve tried to modify examples/exec.c file from libssh project to
create and destroy the channel 20 times:

/* simple exec example */
#include <stdio.h>

#include <libssh/libssh.h>
#include "examples_common.h"

int main(void) {
    ssh_session session;
    ssh_channel channel;
    char buffer[256];
    int nbytes;
    int rc;

    session = connect_ssh("hostname", "username", 22);
    if (session == NULL) {
        ssh_finalize();
        return 1;
    }
    
    for (int i = 0; i < 20; i++) {
      fprintf(stderr, "\ni = %d\n\n", i);
      
      channel = ssh_channel_new(session);;
      if (channel == NULL) {
          ssh_disconnect(session);
          ssh_free(session);
          ssh_finalize();
          return 1;
      }

      rc = ssh_channel_open_session(channel);
      if (rc < 0) {
          goto failed;
      }

      rc = ssh_channel_request_exec(channel, "lsof");
      if (rc < 0) {
          goto failed;
      }

      nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
      while (nbytes > 0) {
          if (fwrite(buffer, 1, nbytes, stdout) != (unsigned int) nbytes) {
              goto failed;
          }
          nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
      }

      if (nbytes < 0) {
          goto failed;
      }

      ssh_channel_send_eof(channel); // if I comment out these 3 lines, everything stops after i = 10
      ssh_channel_close(channel);    // if I comment out these 3 lines, everything stops after i = 10
      ssh_channel_free(channel);     // if I comment out these 3 lines, everything stops after i = 10
    }
    ssh_disconnect(session);
    ssh_free(session);
    ssh_finalize();

    return 0;
failed:
    ssh_channel_close(channel);
    ssh_channel_free(channel);
    ssh_disconnect(session);
    ssh_free(session);
    ssh_finalize();

    return 1;
}

The channel is opened and closed successfully 20 times.

Then I try to comment out lines destroying the channel:

      //ssh_channel_send_eof(channel);
      //ssh_channel_close(channel);
      //ssh_channel_free(channel); 

Then the example stops after printing i = 10. This proves that either

  • I’m using Apache SSHD incorrectly to close the channel; OR
  • There’s a bug in Apache SSHD’s channel closing code.

Also I’ve noticed that actually Apache SSHD’s

channel.close(true);

returns a future (CloseFuture). But if I call

                        CloseFuture closeFuture = channel.close(true);
                        closeFuture.await();

or

                        CloseFuture closeFuture = channel.close(true);
                        closeFuture.awaitUninterruptibly();

and check closeFuture.isClosed(), it doesn’t solve the problem: closeFuture.isClosed() returns true and the channels fail to reopen after 10th.

I’ll continue digging, maybe I should send something through the channel as it is done in libssh:

ssh_channel_send_eof(channel);
1 Like

Finally I’ve managed to do it right:

                        final CloseFuture closeFuture = channel.close(false);
                        closeFuture.await();
                        assert closeFuture.isClosed();

false – here means “not immediately”. If I understand correctly “not immediately” means with sending EOF (AbstractChannel#sendEof); “immediately” – without sending EOF, thus exhausting the MaxSessions limit. I’ll fix SVNKit now accordingly.

1 Like

@dmitry.pavlenko
Should those of us on 1.10.6 upgrade to a newer version with this fix in it?

Hello Karl,
absolutely, an important security issue is fixed in 1.10.7:
https://issues.tmatesoft.com/issue/SVNKIT-765

And this bug (“E210004: Handshake failed, data stream ended unexpectedly”) is also fixed in 1.10.7:
https://issues.tmatesoft.com/issue/SVNKIT-764

One more related fix:

https://issues.tmatesoft.com/issue/SVNKIT-767

Just wanted to say “Thanks!” for posting about this issue. I’m the maintainer of the svn plugin for jEdit, which uses svnkit. I’ve been getting the dreaded E210002 error for a couple of weeks now and finally got around to looking into it. Setting the system property “svnkit.ssh.client=apache” fixed the problem, so thanks! Any idea when the “apache” setting will be the default rather than “trilead”? I didn’t find any documentation about this except for this thread, and running jsvn from the command line pointed me to this solution.

Hello,
thanks and all glory to @dren.dk for implementing this feature! Sorry for the documentation, it is mostly outdated now and we had no resources to keep it up to date. But we always answer here.

What about making “svnkit.ssh.client=apache” default, we have it on our roadmap, but we would like to do that once this implementation is stable. As you can see from this thread, until recently different issues related to the new implementation have been popping up.

We plan to release 1.10.9 soon and probably it won’t have “svnkit.ssh.client=apache” by default yet. But if we get no new reports, probably 1.10.10 (or maybe it will be 1.11.0) will switch to “apache”.

I installed svnkit 1.10.9 by the eclipse update site zip (the real update site seems to still point to some older version, btw).

However, when doing some SVN operations (via Subclipse) I get the following exception:

java.lang.NoClassDefFoundError: org/apache/sshd/common/NamedResource
	at org.tmatesoft.svn.core.internal.io.svn.SVNSSHPrivateKeyUtil.isValidPrivateKey(SVNSSHPrivateKeyUtil.java:99)
	at org.tmatesoft.svn.core.internal.io.svn.SVNSSHConnector.open(SVNSSHConnector.java:102)
	at org.tmatesoft.svn.core.internal.io.svn.SVNConnection.open(SVNConnection.java:80)
	at org.tmatesoft.svn.core.internal.io.svn.SVNRepositoryImpl.openConnection(SVNRepositoryImpl.java:1282)
	at org.tmatesoft.svn.core.internal.io.svn.SVNRepositoryImpl.getLatestRevision(SVNRepositoryImpl.java:173)
	at org.tmatesoft.svn.core.internal.wc2.ng.SvnNgRepositoryAccess.getRevisionNumber(SvnNgRepositoryAccess.java:119)
	at org.tmatesoft.svn.core.internal.wc2.SvnRepositoryAccess.getLocations(SvnRepositoryAccess.java:183)
	at org.tmatesoft.svn.core.internal.wc2.ng.SvnNgRepositoryAccess.createRepositoryFor(SvnNgRepositoryAccess.java:43)
	at org.tmatesoft.svn.core.internal.wc2.remote.SvnRemoteLog.run(SvnRemoteLog.java:116)
	at org.tmatesoft.svn.core.internal.wc2.remote.SvnRemoteLog.run(SvnRemoteLog.java:35)
	at org.tmatesoft.svn.core.internal.wc2.SvnOperationRunner.run(SvnOperationRunner.java:21)
	at org.tmatesoft.svn.core.wc2.SvnOperationFactory.run(SvnOperationFactory.java:1239)
	at org.tmatesoft.svn.core.wc2.SvnOperation.run(SvnOperation.java:294)
	at org.tmatesoft.svn.core.javahl17.SVNClientImpl.logMessages(SVNClientImpl.java:408)
	at org.tigris.subversion.svnclientadapter.javahl.AbstractJhlClientAdapter.getLogMessages(AbstractJhlClientAdapter.java:2615)
	at org.tigris.subversion.svnclientadapter.javahl.AbstractJhlClientAdapter.getLogMessages(AbstractJhlClientAdapter.java:2647)
	at org.tigris.subversion.subclipse.core.commands.GetLogsCommand.run(GetLogsCommand.java:141)
	at org.tigris.subversion.subclipse.ui.history.SVNHistoryPage$AbstractFetchJob.getLogEntries(SVNHistoryPage.java:3157)
	at org.tigris.subversion.subclipse.ui.history.SVNHistoryPage$FetchLogEntriesJob.run(SVNHistoryPage.java:2953)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63)

I did not set any system properties to enable apache sshd. But apache sshd is installed within my eclipse.
How can I resolve this issue?

Thanks,
Patric

Hello Patric,
I have little experience with Eclipse. Probably the new dependencies are missing here

https://svn.svnkit.com/repos/svnkit/trunk/svnkit-osgi/build.gradle

but I’m not sure. I’ll consult with my colleagues and give you the answer. Do you have an ability to build SVNKit from sources and test how it works with Eclipse? In that case we could try to specify there dependencies from

https://svn.svnkit.com/repos/svnkit/trunk/build.gradle

and see if it solves the problem.

Hello Dmitry,
Thanks for your response.

Even though the class cannot be found (“NoClassDefFoundError”) the apache sshd plugin jar is installed in my eclipse installation.
I’m not an OSGI expert but for me it looks like the svnkit plugin misses the apache sshd “Require-Bundle:” attribution in its jar MANIFEST and therefore, as a rough guess, the OSGI classloading prevents any usage/visibility to apache sshd.

I can try building svnkit by myself. What is the output of the build - an eclipse update site? If not, it would be helpful how to deploy the custom built artifacts to eclipse.

On the other hand it might make sense to wait for the response from your colleagues.

Thank you,
Patric

‘svnkit-osgi’ can be built with

./gradlew svnkit:clean svnkit:build -x svnkit:javadoc -x svnkit:test svnkit-osgi:clean svnkit-osgi:build

command and it generates

svnkit-osgi/build/libs/org.tmatesoft.svnkit-1.10.10-SNAPSHOT.jar

When I unzip it and read its manifest, indeed, the manifest doesn’t mention ‘sshd’ while it mentions other dependencies like ‘sqljet’ library. So I think you’re right about it.

As I understand, you can’t just test the JAR file built from the command line with dependencies fixed in svnkit-osgi/build.gradle, right? Then I’ll update svnkit-osgi/build.gradle and ask my colleague to upload the resulting artifact to Eclipse if this is possible at all without issuing a new SVNKit release.

r10857 of trunk: added

    embedOsgi 'org.apache.sshd:sshd-core:2.8.0'
    embedOsgi 'org.apache.sshd:sshd-common:2.8.0'
    embedOsgi 'net.i2p.crypto:eddsa:0.3.0'

to

svnkit-osgi/build.gradle

I’ve created an intermediate build with the fix on our Teamcity. Could you try to get the new version directly from it?