SshdSessionFactory: generalize providing default keys

Provide a mechanism for a subclass to provide its own set
of default identities from anywhere as an Iterable<KeyPair>.

The default implementation is functionally unchanged and uses
the known default identity files in the ~/.ssh directory. A subclass
can override the getDefaultKeys() function and return whatever keys
are appropriate.

Bug: 543152
Change-Id: I500d63146bc67e20e051f617790eb87c7cb500b6
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

4 files changed, 71 insertions, 24 deletions

diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/CachingKeyPairProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/CachingKeyPairProvider.java
index 06a0a5f07f..1072f32548 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/CachingKeyPairProvider.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/CachingKeyPairProvider.java
@@ -63,7 +63,8 @@ import org.eclipse.jgit.transport.sshd.KeyCache;
  * A {@link EncryptedFileKeyPairProvider} that uses an external
  * {@link KeyCache}.
  */
-public class CachingKeyPairProvider extends EncryptedFileKeyPairProvider {
+public class CachingKeyPairProvider extends EncryptedFileKeyPairProvider
+		implements Iterable<KeyPair> {
 
 	private final KeyCache cache;
 
@@ -83,11 +84,17 @@ public class CachingKeyPairProvider extends EncryptedFileKeyPairProvider {
 	}
 
 	@Override
-	protected Iterable<KeyPair> loadKeys(Collection<? extends Path> resources) {
+	public Iterator<KeyPair> iterator() {
+		Collection<? extends Path> resources = getPaths();
 		if (resources.isEmpty()) {
-			return Collections.emptyList();
+			return Collections.emptyListIterator();
 		}
-		return () -> new CancellingKeyPairIterator(resources);
+		return new CancellingKeyPairIterator(resources);
+	}
+
+	@Override
+	public Iterable<KeyPair> loadKeys() {
+		return this;
 	}
 
 	@Override
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java
index ff81989991..ef8e611811 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java
@@ -70,7 +70,7 @@ import org.eclipse.jgit.internal.transport.sshd.RepeatingFilePasswordProvider.Re
  * encrypted private key if the {@link FilePasswordProvider} is a
  * {@link RepeatingFilePasswordProvider}.
  */
-public class EncryptedFileKeyPairProvider extends FileKeyPairProvider {
+public abstract class EncryptedFileKeyPairProvider extends FileKeyPairProvider {
 
 	// TODO: remove this class once we're based on sshd > 2.1.0. See upstream
 	// issue SSHD-850 https://issues.apache.org/jira/browse/SSHD-850 and commit
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java
index 212b67fe33..b9ff5e5208 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java
@@ -70,7 +70,7 @@ import org.apache.sshd.common.config.keys.FilePasswordProvider;
 import org.apache.sshd.common.future.SshFutureListener;
 import org.apache.sshd.common.io.IoConnectFuture;
 import org.apache.sshd.common.io.IoSession;
-import org.apache.sshd.common.keyprovider.FileKeyPairProvider;
+import org.apache.sshd.common.keyprovider.AbstractResourceKeyPairProvider;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
 import org.apache.sshd.common.session.helpers.AbstractSession;
 import org.apache.sshd.common.util.ValidateUtils;
@@ -243,12 +243,11 @@ public class JGitSshClient extends SshClient {
 		int numberOfPasswordPrompts = getNumberOfPasswordPrompts(hostConfig);
 		session.getProperties().put(PASSWORD_PROMPTS,
 				Integer.valueOf(numberOfPasswordPrompts));
-		FilePasswordProvider provider = getFilePasswordProvider();
-		if (provider instanceof RepeatingFilePasswordProvider) {
-			((RepeatingFilePasswordProvider) provider)
+		FilePasswordProvider passwordProvider = getFilePasswordProvider();
+		if (passwordProvider instanceof RepeatingFilePasswordProvider) {
+			((RepeatingFilePasswordProvider) passwordProvider)
 					.setAttempts(numberOfPasswordPrompts);
 		}
-		FileKeyPairProvider ourConfiguredKeysProvider = null;
 		List<Path> identities = hostConfig.getIdentities().stream()
 				.map(s -> {
 					try {
@@ -260,16 +259,16 @@ public class JGitSshClient extends SshClient {
 					}
 				}).filter(p -> p != null && Files.exists(p))
 				.collect(Collectors.toList());
-		ourConfiguredKeysProvider = new CachingKeyPairProvider(identities,
-				keyCache);
-		ourConfiguredKeysProvider.setPasswordFinder(getFilePasswordProvider());
+		CachingKeyPairProvider ourConfiguredKeysProvider = new CachingKeyPairProvider(
+				identities, keyCache);
+		ourConfiguredKeysProvider.setPasswordFinder(passwordProvider);
 		if (hostConfig.isIdentitiesOnly()) {
 			session.setKeyPairProvider(ourConfiguredKeysProvider);
 		} else {
 			KeyPairProvider defaultKeysProvider = getKeyPairProvider();
-			if (defaultKeysProvider instanceof FileKeyPairProvider) {
-				((FileKeyPairProvider) defaultKeysProvider)
-						.setPasswordFinder(getFilePasswordProvider());
+			if (defaultKeysProvider instanceof AbstractResourceKeyPairProvider<?>) {
+				((AbstractResourceKeyPairProvider<?>) defaultKeysProvider)
+						.setPasswordFinder(passwordProvider);
 			}
 			KeyPairProvider combinedProvider = new CombinedKeyPairProvider(
 					ourConfiguredKeysProvider, defaultKeysProvider);
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java
index 275cf5824d..cdd47bf323 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java
@@ -47,6 +47,7 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.security.KeyPair;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -68,7 +69,6 @@ import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
 import org.apache.sshd.common.NamedFactory;
 import org.apache.sshd.common.compression.BuiltinCompressions;
 import org.apache.sshd.common.config.keys.FilePasswordProvider;
-import org.apache.sshd.common.keyprovider.FileKeyPairProvider;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.errors.TransportException;
@@ -89,7 +89,9 @@ import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.util.FS;
 
 /**
- * A {@link SshSessionFactory} that uses Apache MINA sshd.
+ * A {@link SshSessionFactory} that uses Apache MINA sshd. Classes from Apache
+ * MINA sshd are kept private to avoid API evolution problems when Apache MINA
+ * sshd interfaces change.
  *
  * @since 5.2
  */
@@ -103,7 +105,7 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable {
 
 	private final Map<Tuple, ServerKeyVerifier> defaultServerKeyVerifier = new ConcurrentHashMap<>();
 
-	private final Map<Tuple, FileKeyPairProvider> defaultKeys = new ConcurrentHashMap<>();
+	private final Map<Tuple, Iterable<KeyPair>> defaultKeys = new ConcurrentHashMap<>();
 
 	private final KeyCache keyCache;
 
@@ -209,8 +211,8 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable {
 				}
 				HostConfigEntryResolver configFile = getHostConfigEntryResolver(
 						home, sshDir);
-				KeyPairProvider defaultKeysProvider = getDefaultKeysProvider(
-						sshDir);
+				KeyPairProvider defaultKeysProvider = toKeyPairProvider(
+						getDefaultKeys(sshDir));
 				KeyPasswordProvider passphrases = createKeyPasswordProvider(
 						credentialsProvider);
 				SshClient client = ClientBuilder.builder()
@@ -395,14 +397,38 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable {
 	}
 
 	/**
-	 * Determines a {@link KeyPairProvider} to use to load the default keys.
+	 * Determines the default keys. The default implementation will lazy load
+	 * the {@link #getDefaultIdentities(File) default identity files}.
+	 * <p>
+	 * Subclasses may override and return an {@link Iterable} of whatever keys
+	 * are appropriate. If the returned iterable lazily loads keys, it should be
+	 * an instance of
+	 * {@link org.apache.sshd.common.keyprovider.AbstractResourceKeyPairProvider
+	 * AbstractResourceKeyPairProvider} so that the session can later pass it
+	 * the {@link #createKeyPasswordProvider(CredentialsProvider) password
+	 * provider} wrapped as a {@link FilePasswordProvider} via
+	 * {@link org.apache.sshd.common.keyprovider.AbstractResourceKeyPairProvider#setPasswordFinder(FilePasswordProvider)
+	 * AbstractResourceKeyPairProvider#setPasswordFinder(FilePasswordProvider)}
+	 * so that encrypted, password-protected keys can be loaded.
+	 * </p>
+	 * <p>
+	 * The default implementation uses exactly this mechanism; class
+	 * {@link CachingKeyPairProvider} may serve as a model for a customized
+	 * lazy-loading {@link Iterable} implementation
+	 * </p>
+	 * <p>
+	 * If the {@link Iterable} returned has the keys already pre-loaded or
+	 * otherwise doesn't need to decrypt encrypted keys, it can be any
+	 * {@link Iterable}, for instance a simple {@link java.util.List List}.
+	 * </p>
 	 *
 	 * @param sshDir
 	 *            to look in for keys
-	 * @return the {@link KeyPairProvider}
+	 * @return an {@link Iterable} over the default keys
+	 * @since 5.3
 	 */
 	@NonNull
-	private KeyPairProvider getDefaultKeysProvider(@NonNull File sshDir) {
+	protected Iterable<KeyPair> getDefaultKeys(@NonNull File sshDir) {
 		List<Path> defaultIdentities = getDefaultIdentities(sshDir);
 		return defaultKeys.computeIfAbsent(
 				new Tuple(defaultIdentities.toArray(new Path[0])),
@@ -411,6 +437,21 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable {
 	}
 
 	/**
+	 * Converts an {@link Iterable} of {link KeyPair}s into a
+	 * {@link KeyPairProvider}.
+	 *
+	 * @param keys
+	 *            to provide via the returned {@link KeyPairProvider}
+	 * @return a {@link KeyPairProvider} that provides the given {@code keys}
+	 */
+	private KeyPairProvider toKeyPairProvider(Iterable<KeyPair> keys) {
+		if (keys instanceof KeyPairProvider) {
+			return (KeyPairProvider) keys;
+		}
+		return () -> keys;
+	}
+
+	/**
 	 * Gets a list of default identities, i.e., private key files that shall
 	 * always be tried for public key authentication. Typically those are
 	 * ~/.ssh/id_dsa, ~/.ssh/id_rsa, and so on. The default implementation