sshd: handle IdentitiesOnly with an SSH agent

If an SSH agent is used but "IdentitiesOnly yes" is set, only those
keys from the agent that correspond to one of the keys explicitly given
via an IdentityFile directive are to be used.

Implement this by filtering the list of keys obtained from the agent
against the list of IdentityFiles, each entry suffixed with ".pub".
Load the public keys from these files, and ignore all other keys from
the agent. Keys without ".pub" file are also ignored.

Apache MINA sshd has no operation to load only the public key from a
private key file, so we have to rely on *.pub files.

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

org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties

@@ -1,5 +1,6 @@
 authenticationCanceled=SSH authentication canceled: no password given
 authenticationOnClosedSession=Authentication canceled: session is already closing or closed
+cannotReadPublicKey=Cannot read public key from file {0}
 closeListenerFailed=Ssh session close listener failed
 configInvalidPath=Invalid path in ssh config key {0}: {1}
 configInvalidPattern=Invalid pattern in ssh config key {0}: {1}

org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPublicKeyAuthentication.java

@@ -12,25 +12,49 @@
 import static java.text.MessageFormat.format;
 import static org.eclipse.jgit.transport.SshConstants.PUBKEY_ACCEPTED_ALGORITHMS;
 
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.GeneralSecurityException;
+import java.security.PublicKey;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.stream.Collectors;
 
+import org.apache.sshd.agent.SshAgent;
+import org.apache.sshd.agent.SshAgentFactory;
 import org.apache.sshd.client.auth.pubkey.KeyAgentIdentity;
 import org.apache.sshd.client.auth.pubkey.PublicKeyIdentity;
 import org.apache.sshd.client.auth.pubkey.UserAuthPublicKey;
+import org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator;
 import org.apache.sshd.client.config.hosts.HostConfigEntry;
 import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.FactoryManager;
 import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
+import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.PublicKeyEntryResolver;
 import org.apache.sshd.common.signature.Signature;
+import org.apache.sshd.common.signature.SignatureFactoriesManager;
 import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Custom {@link UserAuthPublicKey} implementation for handling SSH config
- * PubkeyAcceptedAlgorithms.
+ * PubkeyAcceptedAlgorithms and interaction with the SSH agent.
  */
 public class JGitPublicKeyAuthentication extends UserAuthPublicKey {
 
+	private SshAgent agent;
+
+	private HostConfigEntry hostConfig;
+
 	JGitPublicKeyAuthentication(List<NamedFactory<Signature>> factories) {
 		super(factories);
 	}
@@ -43,7 +67,7 @@ public void init(ClientSession rawSession, String service)
 					+ rawSession.getClass().getCanonicalName());
 		}
 		JGitClientSession session = (JGitClientSession) rawSession;
-		HostConfigEntry hostConfig = session.getHostConfigEntry();
+		hostConfig = session.getHostConfigEntry();
 		// Set signature algorithms for public key authentication
 		String pubkeyAlgos = hostConfig.getProperty(PUBKEY_ACCEPTED_ALGORITHMS);
 		if (!StringUtils.isEmptyOrNull(pubkeyAlgos)) {
@@ -64,46 +88,126 @@ public void init(ClientSession rawSession, String service)
 		// If we don't set signature factories here, the default ones from the
 		// session will be used.
 		super.init(session, service);
-		// In sshd 2.7.0, we end up now with a key iterator that uses keys
-		// provided by an ssh-agent even if IdentitiesOnly is true. So if
-		// needed, filter out any KeyAgentIdentity.
-		if (hostConfig.isIdentitiesOnly()) {
-			Iterator<PublicKeyIdentity> original = keys;
-			// The original iterator will already have gotten the identities
-			// from the agent. Unfortunately there's nothing we can do about
-			// that; it'll have to be fixed upstream. (As will, ultimately,
-			// respecting isIdentitiesOnly().) At least we can simply not
-			// use the keys the agent provided.
-			//
-			// See https://issues.apache.org/jira/browse/SSHD-1218
-			keys = new Iterator<>() {
+	}
 
-				private PublicKeyIdentity value;
+	@Override
+	protected Iterator<PublicKeyIdentity> createPublicKeyIterator(
+			ClientSession session, SignatureFactoriesManager manager)
+			throws Exception {
+		agent = getAgent(session);
+		return new KeyIterator(session, manager);
+	}
+
+	private SshAgent getAgent(ClientSession session) throws Exception {
+		FactoryManager manager = Objects.requireNonNull(
+				session.getFactoryManager(), "No session factory manager"); //$NON-NLS-1$
+		SshAgentFactory factory = manager.getAgentFactory();
+		if (factory == null) {
+			return null;
+		}
+		return factory.createClient(session, manager);
+	}
+
+	@Override
+	protected void releaseKeys() throws IOException {
+		try {
+			if (agent != null) {
+				try {
+					agent.close();
+				} finally {
+					agent = null;
+				}
+			}
+		} finally {
+			super.releaseKeys();
+		}
+	}
+
+	private class KeyIterator extends UserAuthPublicKeyIterator {
+
+		private Iterable<? extends Map.Entry<PublicKey, String>> agentKeys;
+
+		// If non-null, all the public keys from explicitly given key files. Any
+		// agent key not matching one of these public keys will be ignored in
+		// getIdentities().
+		private Collection<PublicKey> identityFiles;
+
+		public KeyIterator(ClientSession session,
+				SignatureFactoriesManager manager)
+				throws Exception {
+			super(session, manager);
+		}
+
+		private List<PublicKey> getExplicitKeys(
+				Collection<String> explicitFiles) {
+			if (explicitFiles == null) {
+				return null;
+			}
+			return explicitFiles.stream().map(s -> {
+				try {
+					Path p = Paths.get(s + ".pub"); //$NON-NLS-1$
+					if (Files.isRegularFile(p, LinkOption.NOFOLLOW_LINKS)) {
+						return AuthorizedKeyEntry.readAuthorizedKeys(p).get(0)
+								.resolvePublicKey(null,
+										PublicKeyEntryResolver.IGNORING);
+					}
+				} catch (InvalidPathException | IOException
+						| GeneralSecurityException e) {
+					log.warn(format(SshdText.get().cannotReadPublicKey, s), e);
+				}
+				return null;
+			}).filter(Objects::nonNull).collect(Collectors.toList());
+		}
+
+		@Override
+		protected Iterable<KeyAgentIdentity> initializeAgentIdentities(
+				ClientSession session) throws IOException {
+			if (agent == null) {
+				return null;
+			}
+			agentKeys = agent.getIdentities();
+			if (hostConfig != null && hostConfig.isIdentitiesOnly()) {
+				identityFiles = getExplicitKeys(hostConfig.getIdentities());
+			}
+			return () -> new Iterator<>() {
+
+				private final Iterator<? extends Map.Entry<PublicKey, String>> iter = agentKeys
+						.iterator();
+
+				private Map.Entry<PublicKey, String> next;
 
 				@Override
 				public boolean hasNext() {
-					if (value != null) {
-						return true;
-					}
-					PublicKeyIdentity next = null;
-					while (original.hasNext()) {
-						next = original.next();
-						if (!(next instanceof KeyAgentIdentity)) {
-							value = next;
+					while (next == null && iter.hasNext()) {
+						Map.Entry<PublicKey, String> val = iter.next();
+						PublicKey pk = val.getKey();
+						// This checks against all explicit keys for any agent
+						// key, but since identityFiles.size() is typically 1,
+						// it should be fine.
+						if (identityFiles == null || identityFiles.stream()
+								.anyMatch(k -> KeyUtils.compareKeys(k, pk))) {
+							next = val;
 							return true;
 						}
+						if (log.isTraceEnabled()) {
+							log.trace(
+									"Ignoring SSH agent {} key not in explicit IdentityFile in SSH config: {}", //$NON-NLS-1$
+									KeyUtils.getKeyType(pk),
+									KeyUtils.getFingerPrint(pk));
+						}
 					}
-					return false;
+					return next != null;
 				}
 
 				@Override
-				public PublicKeyIdentity next() {
-					if (hasNext()) {
-						PublicKeyIdentity result = value;
-						value = null;
-						return result;
+				public KeyAgentIdentity next() {
+					if (!hasNext()) {
+						throw new NoSuchElementException();
 					}
-					throw new NoSuchElementException();
+					KeyAgentIdentity result = new KeyAgentIdentity(agent,
+							next.getKey(), next.getValue());
+					next = null;
+					return result;
 				}
 			};
 		}

org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java

@@ -30,6 +30,7 @@ public static SshdText get() {
 	/***/ public String authenticationCanceled;
 	/***/ public String authenticationOnClosedSession;
 	/***/ public String closeListenerFailed;
+	/***/ public String cannotReadPublicKey;
 	/***/ public String configInvalidPath;
 	/***/ public String configInvalidPattern;
 	/***/ public String configInvalidPositive;