From eb67862cbaf8f77a7ebb6e01a0d43366d3a223f0 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 27 Jun 2020 15:01:01 +0200 Subject: [PATCH 1/2] Remove dependency on JSch from SSH test framework Use standard java.security to generate test keys, use sshd to write public key files, and write PKCS#8 PEM files for our non-encrypted test private keys. This is a format that both JSch and Apache MINA sshd can read. Change-Id: I6ec55cfd7346b672a7fb6139d51abfb06d81a394 Signed-off-by: Thomas Wolf --- org.eclipse.jgit.junit.ssh/BUILD | 1 - .../META-INF/MANIFEST.MF | 3 +- org.eclipse.jgit.junit.ssh/pom.xml | 10 --- .../jgit/junit/ssh/SshTestGitServer.java | 71 ++++++++++++--- .../jgit/junit/ssh/SshTestHarness.java | 87 ++++++++++++------- 5 files changed, 116 insertions(+), 56 deletions(-) diff --git a/org.eclipse.jgit.junit.ssh/BUILD b/org.eclipse.jgit.junit.ssh/BUILD index 61b5ce78c..50142fe5a 100644 --- a/org.eclipse.jgit.junit.ssh/BUILD +++ b/org.eclipse.jgit.junit.ssh/BUILD @@ -13,7 +13,6 @@ java_library( "//org.eclipse.jgit.ssh.jsch.test:__pkg__", ], deps = [ - "//lib:jsch", "//lib:junit", "//lib:sshd-osgi", "//lib:sshd-sftp", diff --git a/org.eclipse.jgit.junit.ssh/META-INF/MANIFEST.MF b/org.eclipse.jgit.junit.ssh/META-INF/MANIFEST.MF index 77f65e469..2bf9c1f85 100644 --- a/org.eclipse.jgit.junit.ssh/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.junit.ssh/META-INF/MANIFEST.MF @@ -8,8 +8,7 @@ Bundle-Localization: plugin Bundle-Vendor: %Bundle-Vendor Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: JavaSE-1.8 -Import-Package: com.jcraft.jsch;version="0.1.55", - org.apache.sshd.common;version="[2.4.0,2.5.0)", +Import-Package: org.apache.sshd.common;version="[2.4.0,2.5.0)", org.apache.sshd.common.config.keys;version="[2.4.0,2.5.0)", org.apache.sshd.common.file.virtualfs;version="[2.4.0,2.5.0)", org.apache.sshd.common.helpers;version="[2.4.0,2.5.0)", diff --git a/org.eclipse.jgit.junit.ssh/pom.xml b/org.eclipse.jgit.junit.ssh/pom.xml index c4dae967f..e1d35ceea 100644 --- a/org.eclipse.jgit.junit.ssh/pom.xml +++ b/org.eclipse.jgit.junit.ssh/pom.xml @@ -57,16 +57,6 @@ ${apache-sshd-version} - - com.jcraft - jsch - - - - com.jcraft - jzlib - - junit junit diff --git a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java index 9aa4afcef..1862b0760 100644 --- a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java +++ b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java @@ -91,8 +91,7 @@ public class SshTestGitServer { * @param testUser * user name of the test user * @param testKey - * private key file of the test user; the server will - * only user the public key from it + * public key file of the test user * @param repository * to serve * @param hostKey @@ -103,17 +102,54 @@ public class SshTestGitServer { public SshTestGitServer(@NonNull String testUser, @NonNull Path testKey, @NonNull Repository repository, @NonNull byte[] hostKey) throws IOException, GeneralSecurityException { + this(testUser, readPublicKey(testKey), repository, + readKeyPair(hostKey)); + } + + /** + * Creates a ssh git test server. It serves one single repository, + * and accepts public-key authentication for exactly one test user. + * + * @param testUser + * user name of the test user + * @param testKey + * public key file of the test user + * @param repository + * to serve + * @param hostKey + * the unencrypted private key to use as host key + * @throws IOException + * @throws GeneralSecurityException + * @since 5.9 + */ + public SshTestGitServer(@NonNull String testUser, @NonNull Path testKey, + @NonNull Repository repository, @NonNull KeyPair hostKey) + throws IOException, GeneralSecurityException { + this(testUser, readPublicKey(testKey), repository, hostKey); + } + + /** + * Creates a ssh git test server. It serves one single repository, + * and accepts public-key authentication for exactly one test user. + * + * @param testUser + * user name of the test user + * @param testKey + * the {@link PublicKey} of the test user + * @param repository + * to serve + * @param hostKey + * the {@link KeyPair} to use as host key + * @since 5.9 + */ + public SshTestGitServer(@NonNull String testUser, + @NonNull PublicKey testKey, @NonNull Repository repository, + @NonNull KeyPair hostKey) { this.testUser = testUser; setTestUserPublicKey(testKey); this.repository = repository; server = SshServer.setUpDefaultServer(); - // Set host key - try (ByteArrayInputStream in = new ByteArrayInputStream(hostKey)) { - SecurityUtils.loadKeyPairIdentities(null, null, in, null) - .forEach((k) -> hostKeys.add(k)); - } catch (IOException | GeneralSecurityException e) { - // Ignore. - } + hostKeys.add(hostKey); server.setKeyPairProvider((session) -> hostKeys); configureAuthentication(); @@ -135,6 +171,20 @@ public class SshTestGitServer { }); } + private static PublicKey readPublicKey(Path key) + throws IOException, GeneralSecurityException { + return AuthorizedKeyEntry.readAuthorizedKeys(key).get(0) + .resolvePublicKey(null, PublicKeyEntryResolver.IGNORING); + } + + private static KeyPair readKeyPair(byte[] keyMaterial) + throws IOException, GeneralSecurityException { + try (ByteArrayInputStream in = new ByteArrayInputStream(keyMaterial)) { + return SecurityUtils.loadKeyPairIdentities(null, null, in, null) + .iterator().next(); + } + } + private static class FakeUserAuthGSS extends UserAuthGSS { @Override protected Boolean doAuth(Buffer buffer, boolean initial) @@ -343,8 +393,7 @@ public class SshTestGitServer { */ public void setTestUserPublicKey(Path key) throws IOException, GeneralSecurityException { - this.testKey = AuthorizedKeyEntry.readAuthorizedKeys(key).get(0) - .resolvePublicKey(null, PublicKeyEntryResolver.IGNORING); + this.testKey = readPublicKey(key); } /** diff --git a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestHarness.java b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestHarness.java index 43f9dc4b2..797068543 100644 --- a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestHarness.java +++ b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestHarness.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018, Thomas Wolf and others + * Copyright (C) 2018, 2020 Thomas Wolf and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -9,27 +9,31 @@ */ package org.eclipse.jgit.junit.ssh; -import static java.nio.charset.StandardCharsets.US_ASCII; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import java.io.ByteArrayOutputStream; +import java.io.BufferedWriter; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.PrivateKey; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; import java.util.Iterator; import java.util.List; +import org.apache.sshd.common.config.keys.PublicKeyEntry; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.PushCommand; @@ -48,9 +52,6 @@ import org.eclipse.jgit.transport.URIish; import org.eclipse.jgit.util.FS; import org.junit.After; -import com.jcraft.jsch.JSch; -import com.jcraft.jsch.KeyPair; - /** * Root class for ssh tests. Sets up the ssh test server. A set of pre-computed * keys for testing is provided in the bundle and can be used in test cases via @@ -104,50 +105,71 @@ public abstract class SshTestHarness extends RepositoryTestCase { File serverDir = new File(getTemporaryDirectory(), "srv"); assertTrue(serverDir.mkdir()); // Create two key pairs. Let's not call them "id_rsa". + KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA"); + generator.initialize(2048); privateKey1 = new File(sshDir, "first_key"); privateKey2 = new File(sshDir, "second_key"); - publicKey1 = createKeyPair(privateKey1); - createKeyPair(privateKey2); - ByteArrayOutputStream publicHostKey = new ByteArrayOutputStream(); + publicKey1 = createKeyPair(generator.generateKeyPair(), privateKey1); + createKeyPair(generator.generateKeyPair(), privateKey2); + // Create a host key + KeyPair hostKey = generator.generateKeyPair(); // Start a server with our test user and the first key. server = new SshTestGitServer(TEST_USER, publicKey1.toPath(), db, - createHostKey(publicHostKey)); + hostKey); testPort = server.start(); assertTrue(testPort > 0); knownHosts = new File(sshDir, "known_hosts"); - Files.write(knownHosts.toPath(), Collections.singleton("[localhost]:" - + testPort + ' ' - + publicHostKey.toString(US_ASCII.name()))); + StringBuilder knownHostsLine = new StringBuilder(); + knownHostsLine.append("[localhost]:").append(testPort).append(' '); + PublicKeyEntry.appendPublicKeyEntry(knownHostsLine, + hostKey.getPublic()); + Files.write(knownHosts.toPath(), + Collections.singleton(knownHostsLine.toString())); factory = createSessionFactory(); SshSessionFactory.setInstance(factory); } - private static File createKeyPair(File privateKeyFile) throws Exception { - // Found no way to do this with MINA sshd except rolling it all - // ourselves... - JSch jsch = new JSch(); - KeyPair pair = KeyPair.genKeyPair(jsch, KeyPair.RSA, 2048); - try (OutputStream out = new FileOutputStream(privateKeyFile)) { - pair.writePrivateKey(out); + private static File createKeyPair(KeyPair newKey, File privateKeyFile) + throws Exception { + // Write PKCS#8 PEM unencrypted. Both JSch and sshd can read that. + PrivateKey privateKey = newKey.getPrivate(); + String format = privateKey.getFormat(); + if (!"PKCS#8".equalsIgnoreCase(format)) { + throw new IOException("Cannot write " + privateKey.getAlgorithm() + + " key in " + format + " format"); + } + try (BufferedWriter writer = Files.newBufferedWriter( + privateKeyFile.toPath(), StandardCharsets.US_ASCII)) { + writer.write("-----BEGIN PRIVATE KEY-----"); + writer.newLine(); + write(writer, privateKey.getEncoded(), 64); + writer.write("-----END PRIVATE KEY-----"); + writer.newLine(); } File publicKeyFile = new File(privateKeyFile.getParentFile(), privateKeyFile.getName() + ".pub"); + StringBuilder builder = new StringBuilder(); + PublicKeyEntry.appendPublicKeyEntry(builder, newKey.getPublic()); + builder.append(' ').append(TEST_USER); try (OutputStream out = new FileOutputStream(publicKeyFile)) { - pair.writePublicKey(out, TEST_USER); + out.write(builder.toString().getBytes(StandardCharsets.US_ASCII)); } return publicKeyFile; } - private static byte[] createHostKey(OutputStream publicKey) - throws Exception { - JSch jsch = new JSch(); - KeyPair pair = KeyPair.genKeyPair(jsch, KeyPair.RSA, 2048); - pair.writePublicKey(publicKey, ""); - try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { - pair.writePrivateKey(out); - out.flush(); - return out.toByteArray(); + private static void write(BufferedWriter out, byte[] bytes, int lineLength) + throws IOException { + String data = Base64.getEncoder().encodeToString(bytes); + int last = data.length(); + for (int i = 0; i < last; i += lineLength) { + if (i + lineLength <= last) { + out.write(data.substring(i, i + lineLength)); + } else { + out.write(data.substring(i)); + } + out.newLine(); } + Arrays.fill(bytes, (byte) 0); } /** @@ -167,7 +189,8 @@ public abstract class SshTestHarness extends RepositoryTestCase { */ protected static String createKnownHostsFile(File file, String host, int port, File publicKey) throws IOException { - List lines = Files.readAllLines(publicKey.toPath(), UTF_8); + List lines = Files.readAllLines(publicKey.toPath(), + StandardCharsets.UTF_8); assertEquals("Public key has too many lines", 1, lines.size()); String pubKey = lines.get(0); // Strip off the comment. From 835e3225a83f50dbab64f4591d8fa2a60adad77c Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 28 Jul 2020 09:44:43 +0200 Subject: [PATCH 2/2] sshd: use PropertyResolver in test Improve the SshTestGitServer API for accessing the server properties. Instead of returning the raw property map, return the proper sshd API abstraction PropertyResolver. This makes the interface more resilient against upstream changes. Change-Id: Ie5b685bddc4e59f3eb6c121026d3658d57618ca4 Signed-off-by: Thomas Wolf --- .../org/eclipse/jgit/junit/ssh/SshTestGitServer.java | 12 ++++++------ .../eclipse/jgit/transport/sshd/ApacheSshTest.java | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java index 1862b0760..250a13876 100644 --- a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java +++ b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java @@ -22,9 +22,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.Map; import org.apache.sshd.common.NamedResource; +import org.apache.sshd.common.PropertyResolver; import org.apache.sshd.common.PropertyResolverUtils; import org.apache.sshd.common.SshConstants; import org.apache.sshd.common.config.keys.AuthorizedKeyEntry; @@ -349,14 +349,14 @@ public class SshTestGitServer { } /** - * Retrieves the server's property map. This is a live map; changing it - * affects the server. + * Retrieves the server's {@link PropertyResolver}, giving access to server + * properties. * - * @return a live map of the server's properties + * @return the {@link PropertyResolver} * @since 5.9 */ - public Map getProperties() { - return server.getProperties(); + public PropertyResolver getPropertyResolver() { + return server; } /** diff --git a/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java b/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java index a58ee2e99..f27af6e19 100644 --- a/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java +++ b/org.eclipse.jgit.ssh.apache.test/tst/org/eclipse/jgit/transport/sshd/ApacheSshTest.java @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; import org.apache.sshd.client.config.hosts.KnownHostEntry; +import org.apache.sshd.common.PropertyResolverUtils; import org.apache.sshd.server.ServerFactoryManager; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.TransportException; @@ -189,8 +190,8 @@ public class ApacheSshTest extends SshTestBase { */ @Test public void testCloneAndFetchWithSessionLimit() throws Exception { - server.getProperties().put(ServerFactoryManager.MAX_CONCURRENT_SESSIONS, - Integer.valueOf(2)); + PropertyResolverUtils.updateProperty(server.getPropertyResolver(), + ServerFactoryManager.MAX_CONCURRENT_SESSIONS, 2); File localClone = cloneWith("ssh://localhost/doesntmatter", defaultCloneDir, null, // "Host localhost", //