This is an automated email from the ASF dual-hosted git repository. twolf pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push: new e7f5def [SSHD-1105] Try all configured signature algorithms for a public key (#183) e7f5def is described below commit e7f5def501cd9f609873082da5a7711fb5931b9e Author: tomaswolf <tho...@paranor.ch> AuthorDate: Wed Mar 24 08:40:07 2021 +0100 [SSHD-1105] Try all configured signature algorithms for a public key (#183) Some keys (RSA) may have several signature algorithms (rsa-sha2-512, rsa-sha2-256, ssh-rsa). Try them all in the order defined and try the next key only if no attempt was successful. --- .../sshd/client/auth/pubkey/UserAuthPublicKey.java | 86 ++++++++++++++-------- .../common/auth/PublicKeyAuthenticationTest.java | 41 +++++++++++ 2 files changed, 98 insertions(+), 29 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java index f55fc4e..d34a3df 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java @@ -24,9 +24,13 @@ import java.security.KeyPair; import java.security.PublicKey; import java.security.spec.InvalidKeySpecException; import java.util.Collection; +import java.util.Deque; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import org.apache.sshd.client.auth.AbstractUserAuth; import org.apache.sshd.client.auth.keyboard.UserInteraction; @@ -38,7 +42,6 @@ import org.apache.sshd.common.config.keys.KeyUtils; import org.apache.sshd.common.signature.Signature; import org.apache.sshd.common.signature.SignatureFactoriesHolder; import org.apache.sshd.common.signature.SignatureFactoriesManager; -import org.apache.sshd.common.signature.SignatureFactory; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.ValidateUtils; import org.apache.sshd.common.util.buffer.Buffer; @@ -53,6 +56,8 @@ import org.apache.sshd.common.util.buffer.ByteArrayBuffer; public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFactoriesManager { public static final String NAME = UserAuthPublicKeyFactory.NAME; + protected final Deque<String> currentAlgorithms = new LinkedList<>(); + protected Iterator<PublicKeyIdentity> keys; protected PublicKeyIdentity current; protected List<NamedFactory<Signature>> factories; @@ -93,27 +98,35 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact @Override protected boolean sendAuthDataRequest(ClientSession session, String service) throws Exception { boolean debugEnabled = log.isDebugEnabled(); - try { - current = resolveAttemptedPublicKeyIdentity(session, service); - } catch (Error e) { - warn("sendAuthDataRequest({})[{}] failed ({}) to get next key: {}", - session, service, e.getClass().getSimpleName(), e.getMessage(), e); - throw new RuntimeSshException(e); + String currentAlgorithm = null; + if (current == null) { + // Just to be safe. (Paranoia) + currentAlgorithms.clear(); + } else if (!currentAlgorithms.isEmpty()) { + currentAlgorithm = currentAlgorithms.poll(); } - PublicKeyAuthenticationReporter reporter = session.getPublicKeyAuthenticationReporter(); - if (current == null) { - if (debugEnabled) { - log.debug("resolveAttemptedPublicKeyIdentity({})[{}] no more keys to send", session, service); + if (currentAlgorithm == null) { + try { + current = resolveAttemptedPublicKeyIdentity(session, service); + } catch (Error e) { + warn("sendAuthDataRequest({})[{}] failed ({}) to get next key: {}", + session, service, e.getClass().getSimpleName(), e.getMessage(), e); + throw new RuntimeSshException(e); } - if (reporter != null) { - reporter.signalAuthenticationExhausted(session, service); - } + if (current == null) { + if (debugEnabled) { + log.debug("resolveAttemptedPublicKeyIdentity({})[{}] no more keys to send", session, service); + } - return false; - } + if (reporter != null) { + reporter.signalAuthenticationExhausted(session, service); + } + return false; + } + } if (log.isTraceEnabled()) { log.trace("sendAuthDataRequest({})[{}] current key details: {}", session, service, current); } @@ -126,27 +139,40 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact session, service, e.getClass().getSimpleName(), e.getMessage(), e); throw new RuntimeSshException(e); } - PublicKey pubKey = keyPair.getPublic(); - String keyType = KeyUtils.getKeyType(pubKey); - NamedFactory<? extends Signature> factory; - // SSHD-1104 check if the key type is aliased - if (current instanceof SignatureFactoriesHolder) { - factory = SignatureFactory.resolveSignatureFactory( - keyType, ((SignatureFactoriesHolder) current).getSignatureFactories()); - } else { - factory = SignatureFactory.resolveSignatureFactory(keyType, getSignatureFactories()); + + if (currentAlgorithm == null) { + String keyType = KeyUtils.getKeyType(pubKey); + Set<String> aliases = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + aliases.addAll(KeyUtils.getAllEquivalentKeyTypes(keyType)); + aliases.add(keyType); + List<NamedFactory<Signature>> existingFactories; + if (current instanceof SignatureFactoriesHolder) { + existingFactories = ((SignatureFactoriesHolder) current) + .getSignatureFactories(); + } else { + existingFactories = getSignatureFactories(); + } + if (existingFactories != null) { + // Select the factories by name and in order + existingFactories.forEach(f -> { + if (aliases.contains(f.getName())) { + currentAlgorithms.add(f.getName()); + } + }); + } + currentAlgorithm = currentAlgorithms.isEmpty() ? keyType + : currentAlgorithms.poll(); } - String algo = (factory == null) ? keyType : factory.getName(); String name = getName(); if (debugEnabled) { log.debug("sendAuthDataRequest({})[{}] send SSH_MSG_USERAUTH_REQUEST request {} type={} - fingerprint={}", - session, service, name, algo, KeyUtils.getFingerPrint(pubKey)); + session, service, name, currentAlgorithm, KeyUtils.getFingerPrint(pubKey)); } if (reporter != null) { - reporter.signalAuthenticationAttempt(session, service, keyPair, algo); + reporter.signalAuthenticationAttempt(session, service, keyPair, currentAlgorithm); } Buffer buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST); @@ -154,7 +180,7 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact buffer.putString(service); buffer.putString(name); buffer.putBoolean(false); - buffer.putString(algo); + buffer.putString(currentAlgorithm); buffer.putPublicKey(pubKey); session.writePacket(buffer); return true; @@ -335,6 +361,8 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact } protected void releaseKeys() throws IOException { + currentAlgorithms.clear(); + current = null; try { if (keys instanceof Closeable) { if (log.isTraceEnabled()) { diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java index 3b86eb5..2f23152 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java @@ -29,7 +29,9 @@ import java.security.spec.InvalidKeySpecException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.concurrent.atomic.AtomicInteger; import org.apache.sshd.client.SshClient; @@ -389,4 +391,43 @@ public class PublicKeyAuthenticationTest extends AuthenticationTestSupport { assertEquals("Mismatched invocation count", 1, exhaustedCount.getAndSet(0)); assertEquals("Mismatched retries count", 4 /* 3 attempts + null */, attemptsCount.getAndSet(0)); } + + @Test + public void testRsaAuthenticationOldServer() throws Exception { + KeyPair userkey = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 2048); + List<String> factoryNames = sshd.getSignatureFactoriesNames(); + // Remove anything that has "rsa" in the name, except "ssh-rsa". Make sure "ssh-rsa" is there. + // We need to keep the others; the test server uses an EC host key, and sshd uses the same + // factory list for host key algorithms and public key signature algorithms. So we can't just + // set the list to only "ssh-rsa". + boolean sshRsaFound = false; + for (Iterator<String> i = factoryNames.iterator(); i.hasNext(); ) { + String name = i.next(); + if (name.equalsIgnoreCase("ssh-rsa")) { + sshRsaFound = true; + } else if (name.toLowerCase(Locale.ROOT).contains("rsa")) { + i.remove(); + } + } + if (!sshRsaFound) { + factoryNames.add("ssh-rsa"); + } + sshd.setSignatureFactoriesNames(factoryNames); + sshd.setPublickeyAuthenticator((username, key, session) -> { + return KeyUtils.compareKeys(userkey.getPublic(), key); + }); + try (SshClient client = setupTestClient()) { + client.setUserAuthFactories( + Collections.singletonList(new org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory())); + client.start(); + + try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port) + .verify(CONNECT_TIMEOUT).getSession()) { + session.addPublicKeyIdentity(userkey); + assertTrue("Successful authentication expected", session.auth().verify(AUTH_TIMEOUT).isSuccess()); + } finally { + client.stop(); + } + } + } }