You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2021/03/24 07:40:13 UTC

[mina-sshd] branch master updated: [SSHD-1105] Try all configured signature algorithms for a public key (#183)

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 <th...@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();
+            }
+        }
+    }
 }