From 071084818cae26fd3f1075d4e6763218197c94d5 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Mon, 27 Dec 2021 19:50:24 +0100 Subject: [PATCH] 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 --- .../transport/sshd/SshdText.properties | 1 + .../sshd/JGitPublicKeyAuthentication.java | 166 ++++++++++++++---- .../internal/transport/sshd/SshdText.java | 1 + 3 files changed, 137 insertions(+), 31 deletions(-) diff --git a/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties b/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties index 2bba736aa..4b12db5d5 100644 --- a/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties +++ b/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} diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPublicKeyAuthentication.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPublicKeyAuthentication.java index 2996a221c..bfe11cb74 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPublicKeyAuthentication.java +++ b/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> 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 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 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> 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 identityFiles; + + public KeyIterator(ClientSession session, + SignatureFactoriesManager manager) + throws Exception { + super(session, manager); + } + + private List getExplicitKeys( + Collection 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 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> iter = agentKeys + .iterator(); + + private Map.Entry 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 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; } }; } diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java index 00ee62d6d..f7b6f6aca 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/SshdText.java +++ b/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;