You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2020/11/26 07:32:11 UTC

[mina-sshd] 02/03: [SSHD-1104] Stricter signature parameters validation when using public key authentication

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 04bcca9cae8ac397a4321b4655ecd216bd207dd6
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Fri Nov 20 12:39:20 2020 +0200

    [SSHD-1104] Stricter signature parameters validation when using public key authentication
---
 .../sshd/client/auth/pubkey/PublicKeyIdentity.java |  8 +++-
 .../sshd/common/signature/BuiltinSignatures.java   | 13 ++++--
 .../sshd/common/signature/SignatureFactory.java    | 52 ++++++++++++++++++++++
 .../main/java/org/apache/sshd/agent/SshAgent.java  | 13 +++++-
 .../sshd/agent/common/AbstractAgentClient.java     |  8 ++--
 .../sshd/agent/common/AbstractAgentProxy.java      | 15 ++++---
 .../apache/sshd/agent/common/AgentDelegate.java    |  4 +-
 .../org/apache/sshd/agent/local/AgentImpl.java     | 39 +++++++---------
 .../sshd/client/auth/pubkey/KeyAgentIdentity.java  |  5 ++-
 .../sshd/client/auth/pubkey/KeyPairIdentity.java   | 24 +++++++---
 .../sshd/client/auth/pubkey/UserAuthPublicKey.java | 12 +++--
 .../org/apache/sshd/deprecated/UserAuthAgent.java  | 16 ++++---
 12 files changed, 151 insertions(+), 58 deletions(-)

diff --git a/sshd-common/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyIdentity.java b/sshd-common/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyIdentity.java
index 97f9554..e794986 100644
--- a/sshd-common/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyIdentity.java
+++ b/sshd-common/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyIdentity.java
@@ -19,6 +19,7 @@
 package org.apache.sshd.client.auth.pubkey;
 
 import java.security.PublicKey;
+import java.util.Map;
 
 import org.apache.sshd.common.session.SessionContext;
 
@@ -38,9 +39,12 @@ public interface PublicKeyIdentity {
      *
      * @param  session   The {@link SessionContext} for calling this method - may be {@code null} if not called within a
      *                   session context
+     * @param  algo      Recommended signature algorithm - if {@code null}/empty then one will be selected based on the
+     *                   key type and/or signature factories. <B>Note:</B> even if specific algorithm specified, the
+     *                   implementation may disregard and choose another
      * @param  data      Data to sign
-     * @return           Signed data - using the identity
+     * @return           used algorithm + signed data - using the identity
      * @throws Exception If failed to sign the data
      */
-    byte[] sign(SessionContext session, byte[] data) throws Exception;
+    Map.Entry<String, byte[]> sign(SessionContext session, String algo, byte[] data) throws Exception;
 }
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/BuiltinSignatures.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/BuiltinSignatures.java
index dacb79f..de919a7 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/BuiltinSignatures.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/BuiltinSignatures.java
@@ -273,17 +273,22 @@ public enum BuiltinSignatures implements SignatureFactory {
         factoryName = facName;
     }
 
-    public static Signature getByCurveSize(ECParameterSpec params) {
+    public static BuiltinSignatures getFactoryByCurveSize(ECParameterSpec params) {
         int curveSize = ECCurves.getCurveSize(params);
         if (curveSize <= 256) {
-            return nistp256.create();
+            return nistp256;
         } else if (curveSize <= 384) {
-            return nistp384.create();
+            return nistp384;
         } else {
-            return nistp521.create();
+            return nistp521;
         }
     }
 
+    public static Signature getSignerByCurveSize(ECParameterSpec params) {
+        NamedFactory<Signature> factory = getFactoryByCurveSize(params);
+        return (factory == null) ? null : factory.create();
+    }
+
     @Override
     public final String getName() {
         return factoryName;
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureFactory.java b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureFactory.java
index e3dcf4f..5a7e86c 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureFactory.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureFactory.java
@@ -19,6 +19,11 @@
 
 package org.apache.sshd.common.signature;
 
+import java.security.PublicKey;
+import java.security.interfaces.DSAPublicKey;
+import java.security.interfaces.ECPublicKey;
+import java.security.interfaces.RSAPublicKey;
+import java.security.spec.InvalidKeySpecException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -35,6 +40,7 @@ import org.apache.sshd.common.NamedResource;
 import org.apache.sshd.common.config.keys.KeyUtils;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
 import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.security.SecurityUtils;
 
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
@@ -201,4 +207,50 @@ public interface SignatureFactory extends BuiltinFactory<Signature> {
             return NamedResource.findFirstMatchByName(aliases, String.CASE_INSENSITIVE_ORDER, factories);
         }
     }
+
+    /**
+     * @param  pubKey                  The intended {@link PublicKey} - ignored if {@code null}
+     * @param  algo                    The intended signature algorithm - if {@code null}/empty and multiple signatures
+     *                                 available for the key type then a default will be used. Otherwise, it is
+     *                                 validated to make sure it matches the public key type
+     * @return                         The {@link Signature} factory or {@code null} if no match found
+     * @throws InvalidKeySpecException If specified algorithm does not match the selected public key
+     */
+    static NamedFactory<Signature> resolveSignatureFactoryByPublicKey(PublicKey pubKey, String algo)
+            throws InvalidKeySpecException {
+        if (pubKey == null) {
+            return null;
+        }
+
+        NamedFactory<Signature> factory = null;
+        if (pubKey instanceof DSAPublicKey) {
+            factory = BuiltinSignatures.dsa;
+        } else if (pubKey instanceof ECPublicKey) {
+            ECPublicKey ecKey = (ECPublicKey) pubKey;
+            factory = BuiltinSignatures.getFactoryByCurveSize(ecKey.getParams());
+        } else if (pubKey instanceof RSAPublicKey) {
+            // SSHD-1104 take into account key aliases
+            if (GenericUtils.isEmpty(algo)) {
+                factory = BuiltinSignatures.rsa;
+            } else if (algo.contains("rsa")) {
+                factory = BuiltinSignatures.fromFactoryName(algo);
+            }
+        } else if (SecurityUtils.EDDSA.equalsIgnoreCase(pubKey.getAlgorithm())) {
+            factory = BuiltinSignatures.ed25519;
+        }
+
+        if (GenericUtils.isEmpty(algo) || (factory == null)) {
+            return factory;
+        }
+
+        String name = factory.getName();
+        if (!algo.equalsIgnoreCase(name)) {
+            throw new InvalidKeySpecException(
+                    "Mismatched factory name (" + name + ")"
+                                              + " for algorithm=" + algo + " when using key type"
+                                              + KeyUtils.getKeyType(pubKey));
+        }
+
+        return factory;
+    }
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/agent/SshAgent.java b/sshd-core/src/main/java/org/apache/sshd/agent/SshAgent.java
index 7e286f5..1c25f3a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/agent/SshAgent.java
+++ b/sshd-core/src/main/java/org/apache/sshd/agent/SshAgent.java
@@ -34,7 +34,18 @@ public interface SshAgent extends java.nio.channels.Channel {
 
     Iterable<? extends Map.Entry<PublicKey, String>> getIdentities() throws IOException;
 
-    byte[] sign(SessionContext session, PublicKey key, byte[] data) throws IOException;
+    /**
+     *
+     * @param  session     The current {@link SessionContext}
+     * @param  key         The {@link PublicKey} to use for signing
+     * @param  algo        Recommended signature algorithm - if {@code null}/empty then one will be selected based on
+     *                     the key type and/or signature factories. <B>Note:</B> even if specific algorithm specified,
+     *                     the implementation may disregard and choose another
+     * @param  data        Data to sign
+     * @return             used algorithm + signed data - using the identity
+     * @throws IOException If failed to sign
+     */
+    Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, String algo, byte[] data) throws IOException;
 
     void addIdentity(KeyPair key, String comment) throws IOException;
 
diff --git a/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentClient.java b/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentClient.java
index d93f6d2..520068c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentClient.java
@@ -130,9 +130,11 @@ public abstract class AbstractAgentClient extends AbstractLoggingBean {
                         KeyUtils.getKeyType(signingKey),
                         "Cannot resolve key type of %s",
                         signingKey.getClass().getSimpleName());
-                byte[] signature = agent.sign(null, signingKey, data);
-                Buffer sig = new ByteArrayBuffer(keyType.length() + signature.length + Long.SIZE, false);
-                sig.putString(keyType);
+                Map.Entry<String, byte[]> result = agent.sign(null, signingKey, keyType, data);
+                String algo = result.getKey();
+                byte[] signature = result.getValue();
+                Buffer sig = new ByteArrayBuffer(algo.length() + signature.length + Long.SIZE, false);
+                sig.putString(algo);
                 sig.putBytes(signature);
                 rep.putByte(SshAgentConstants.SSH2_AGENT_SIGN_RESPONSE);
                 rep.putBytes(sig.array(), sig.rpos(), sig.available());
diff --git a/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentProxy.java b/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentProxy.java
index 614bee6..3add243 100644
--- a/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentProxy.java
+++ b/sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentProxy.java
@@ -103,7 +103,7 @@ public abstract class AbstractAgentProxy extends AbstractLoggingBean implements
     }
 
     @Override
-    public byte[] sign(SessionContext session, PublicKey key, byte[] data) throws IOException {
+    public Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, String algo, byte[] data) throws IOException {
         int cmd = SshAgentConstants.SSH2_AGENTC_SIGN_REQUEST;
         int okcmd = SshAgentConstants.SSH2_AGENT_SIGN_RESPONSE;
         if (CoreModuleProperties.AGENT_FORWARDING_TYPE_IETF.equals(channelType)) {
@@ -127,22 +127,23 @@ public abstract class AbstractAgentProxy extends AbstractLoggingBean implements
 
         byte[] signature = buffer.getBytes();
         boolean debugEnabled = log.isDebugEnabled();
+        String keyType = KeyUtils.getKeyType(key);
         if (CoreModuleProperties.AGENT_FORWARDING_TYPE_IETF.equals(channelType)) {
             if (debugEnabled) {
-                log.debug("sign({})[{}] : {}",
-                        KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key), BufferUtils.toHex(':', signature));
+                log.debug("sign({}/{})[{}] : {}",
+                        algo, keyType, KeyUtils.getFingerPrint(key), BufferUtils.toHex(':', signature));
             }
+            return new SimpleImmutableEntry<>(keyType, signature);
         } else {
             Buffer buf = new ByteArrayBuffer(signature);
             String algorithm = buf.getString();
             signature = buf.getBytes();
             if (debugEnabled) {
-                log.debug("sign({})[{}] {}: {}",
-                        KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key), algorithm, BufferUtils.toHex(':', signature));
+                log.debug("sign({}/{})[{}] {}: {}",
+                        algo, keyType, KeyUtils.getFingerPrint(key), algorithm, BufferUtils.toHex(':', signature));
             }
+            return new SimpleImmutableEntry<>(algorithm, signature);
         }
-
-        return signature;
     }
 
     @Override
diff --git a/sshd-core/src/main/java/org/apache/sshd/agent/common/AgentDelegate.java b/sshd-core/src/main/java/org/apache/sshd/agent/common/AgentDelegate.java
index 4d55ca5..a9f4997 100644
--- a/sshd-core/src/main/java/org/apache/sshd/agent/common/AgentDelegate.java
+++ b/sshd-core/src/main/java/org/apache/sshd/agent/common/AgentDelegate.java
@@ -50,8 +50,8 @@ public class AgentDelegate implements SshAgent {
     }
 
     @Override
-    public byte[] sign(SessionContext session, PublicKey key, byte[] data) throws IOException {
-        return agent.sign(session, key, data);
+    public Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, String algo, byte[] data) throws IOException {
+        return agent.sign(session, key, algo, data);
     }
 
     @Override
diff --git a/sshd-core/src/main/java/org/apache/sshd/agent/local/AgentImpl.java b/sshd-core/src/main/java/org/apache/sshd/agent/local/AgentImpl.java
index 8fd07b5..2b80370 100644
--- a/sshd-core/src/main/java/org/apache/sshd/agent/local/AgentImpl.java
+++ b/sshd-core/src/main/java/org/apache/sshd/agent/local/AgentImpl.java
@@ -21,9 +21,6 @@ package org.apache.sshd.agent.local;
 import java.io.IOException;
 import java.security.KeyPair;
 import java.security.PublicKey;
-import java.security.interfaces.DSAPublicKey;
-import java.security.interfaces.ECPublicKey;
-import java.security.interfaces.RSAPublicKey;
 import java.security.spec.InvalidKeySpecException;
 import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.ArrayList;
@@ -34,20 +31,19 @@ import java.util.Objects;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.sshd.agent.SshAgent;
+import org.apache.sshd.common.NamedFactory;
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.common.config.keys.KeyUtils;
 import org.apache.sshd.common.session.SessionContext;
-import org.apache.sshd.common.signature.BuiltinSignatures;
 import org.apache.sshd.common.signature.Signature;
+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.security.SecurityUtils;
 
 /**
  * A local SSH agent implementation
  */
 public class AgentImpl implements SshAgent {
-
     private final List<Map.Entry<KeyPair, String>> keys = new ArrayList<>();
     private final AtomicBoolean open = new AtomicBoolean(true);
 
@@ -70,32 +66,29 @@ public class AgentImpl implements SshAgent {
     }
 
     @Override
-    public byte[] sign(SessionContext session, PublicKey key, byte[] data) throws IOException {
+    public Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, String algo, byte[] data) throws IOException {
         if (!isOpen()) {
             throw new SshException("Agent closed");
         }
 
         try {
             Map.Entry<KeyPair, String> pp = Objects.requireNonNull(getKeyPair(keys, key), "Key not found");
-            KeyPair kp = ValidateUtils.checkNotNull(pp.getKey(), "No key pair for agent=%s", pp.getValue());
-            PublicKey pubKey = ValidateUtils.checkNotNull(kp.getPublic(), "No public key for agent=%s", pp.getValue());
-
-            Signature verif;
-            if (pubKey instanceof DSAPublicKey) {
-                verif = BuiltinSignatures.dsa.create();
-            } else if (pubKey instanceof ECPublicKey) {
-                ECPublicKey ecKey = (ECPublicKey) pubKey;
-                verif = BuiltinSignatures.getByCurveSize(ecKey.getParams());
-            } else if (pubKey instanceof RSAPublicKey) {
-                verif = BuiltinSignatures.rsa.create();
-            } else if (SecurityUtils.EDDSA.equalsIgnoreCase(pubKey.getAlgorithm())) {
-                verif = BuiltinSignatures.ed25519.create();
-            } else {
-                throw new InvalidKeySpecException("Unsupported key type: " + pubKey.getClass().getSimpleName());
+            String agentName = pp.getValue();
+            KeyPair kp = ValidateUtils.checkNotNull(pp.getKey(), "No key pair for agent=%s", agentName);
+            PublicKey pubKey = ValidateUtils.checkNotNull(kp.getPublic(), "No public key for agent=%s", agentName);
+            NamedFactory<Signature> factory = SignatureFactory.resolveSignatureFactoryByPublicKey(pubKey, algo);
+            Signature verif = (factory == null) ? null : factory.create();
+            if (verif == null) {
+                throw new InvalidKeySpecException(
+                        "No signer found for " + pubKey.getClass().getSimpleName()
+                                                  + " when algorithm=" + algo + " requested for "
+                                                  + KeyUtils.getKeyType(pubKey));
             }
             verif.initSigner(session, kp.getPrivate());
             verif.update(session, data);
-            return verif.sign(session);
+
+            byte[] signature = verif.sign(session);
+            return new SimpleImmutableEntry<>(factory.getName(), signature);
         } catch (IOException e) {
             throw e;
         } catch (Exception e) {
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyAgentIdentity.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyAgentIdentity.java
index 1e1a891..53b37b2 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyAgentIdentity.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyAgentIdentity.java
@@ -19,6 +19,7 @@
 package org.apache.sshd.client.auth.pubkey;
 
 import java.security.PublicKey;
+import java.util.Map;
 import java.util.Objects;
 
 import org.apache.sshd.agent.SshAgent;
@@ -51,8 +52,8 @@ public class KeyAgentIdentity implements PublicKeyIdentity {
     }
 
     @Override
-    public byte[] sign(SessionContext session, byte[] data) throws Exception {
-        return agent.sign(session, getPublicKey(), data);
+    public Map.Entry<String, byte[]> sign(SessionContext session, String algo, byte[] data) throws Exception {
+        return agent.sign(session, getPublicKey(), algo, data);
     }
 
     @Override
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyPairIdentity.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyPairIdentity.java
index 65812e1..b90c369 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyPairIdentity.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/KeyPairIdentity.java
@@ -20,17 +20,21 @@ package org.apache.sshd.client.auth.pubkey;
 
 import java.security.KeyPair;
 import java.security.PublicKey;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 
 import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.common.NamedResource;
 import org.apache.sshd.common.config.keys.KeyUtils;
 import org.apache.sshd.common.session.SessionContext;
 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;
 
 /**
@@ -61,15 +65,23 @@ public class KeyPairIdentity implements PublicKeyIdentity, SignatureFactoriesHol
     }
 
     @Override
-    public byte[] sign(SessionContext session, byte[] data) throws Exception {
-        String keyType = KeyUtils.getKeyType(getPublicKey());
-        // SSHD-1104 check if the key type is aliased
-        NamedFactory<? extends Signature> factory = SignatureFactory.resolveSignatureFactory(keyType, getSignatureFactories());
+    public Map.Entry<String, byte[]> sign(SessionContext session, String algo, byte[] data) throws Exception {
+        NamedFactory<? extends Signature> factory;
+        if (GenericUtils.isEmpty(algo)) {
+            algo = KeyUtils.getKeyType(getPublicKey());
+            // SSHD-1104 check if the key type is aliased
+            factory = SignatureFactory.resolveSignatureFactory(algo, getSignatureFactories());
+        } else {
+            factory = NamedResource.findByName(algo, String.CASE_INSENSITIVE_ORDER, getSignatureFactories());
+        }
+
         Signature verifier = (factory == null) ? null : factory.create();
-        ValidateUtils.checkNotNull(verifier, "No signer could be located for key type=%s", keyType);
+        ValidateUtils.checkNotNull(verifier, "No signer could be located for key type=%s", algo);
         verifier.initSigner(session, pair.getPrivate());
         verifier.update(session, data);
-        return verifier.sign(session);
+
+        byte[] signature = verifier.sign(session);
+        return new SimpleImmutableEntry<>(factory.getName(), signature);
     }
 
     @Override
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 01143e6..28e575e 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
@@ -25,6 +25,7 @@ import java.security.spec.InvalidKeySpecException;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.sshd.client.auth.AbstractUserAuth;
 import org.apache.sshd.client.session.ClientSession;
@@ -37,6 +38,7 @@ 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;
 import org.apache.sshd.common.util.buffer.BufferUtils;
 import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
@@ -241,10 +243,14 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact
         byte[] contents = bs.getCompactData();
         byte[] sig;
         try {
-            sig = current.sign(session, contents);
+            Map.Entry<String, byte[]> result = current.sign(session, algo, contents);
+            String factoryName = result.getKey();
+            ValidateUtils.checkState(algo.equalsIgnoreCase(factoryName),
+                    "Mismatched signature type generated: requested=%s, used=%s", algo, factoryName);
+            sig = result.getValue();
         } catch (Error e) {
-            warn("appendSignature({})[{}][{}] failed ({}) to sign contents: {}",
-                    session, service, name, e.getClass().getSimpleName(), e.getMessage(), e);
+            warn("appendSignature({})[{}][{}] failed ({}) to sign contents using {}: {}",
+                    session, service, name, e.getClass().getSimpleName(), algo, e.getMessage(), e);
             throw new RuntimeSshException(e);
         }
 
diff --git a/sshd-core/src/test/java/org/apache/sshd/deprecated/UserAuthAgent.java b/sshd-core/src/test/java/org/apache/sshd/deprecated/UserAuthAgent.java
index b529056..52f11a2 100644
--- a/sshd-core/src/test/java/org/apache/sshd/deprecated/UserAuthAgent.java
+++ b/sshd-core/src/test/java/org/apache/sshd/deprecated/UserAuthAgent.java
@@ -25,6 +25,8 @@ import java.util.Iterator;
 import java.util.Map;
 
 import org.apache.sshd.agent.SshAgent;
+import org.apache.sshd.agent.SshAgentFactory;
+import org.apache.sshd.client.ClientFactoryManager;
 import org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory;
 import org.apache.sshd.client.session.ClientSession;
 import org.apache.sshd.client.session.ClientSessionImpl;
@@ -45,10 +47,12 @@ public class UserAuthAgent extends AbstractUserAuth {
 
     public UserAuthAgent(ClientSessionImpl session, String service) throws IOException {
         super(session, service);
-        if (session.getFactoryManager().getAgentFactory() == null) {
+        ClientFactoryManager factoryManager = session.getFactoryManager();
+        SshAgentFactory agentFactory = factoryManager.getAgentFactory();
+        if (agentFactory == null) {
             throw new IllegalStateException("No ssh agent factory has been configured");
         }
-        this.agent = session.getFactoryManager().getAgentFactory().createClient(session.getFactoryManager());
+        this.agent = agentFactory.createClient(factoryManager);
         this.keys = agent.getIdentities().iterator();
     }
 
@@ -79,9 +83,11 @@ public class UserAuthAgent extends AbstractUserAuth {
 
             String keyType = KeyUtils.getKeyType(key);
             byte[] contents = bs.getCompactData();
-            byte[] signature = agent.sign(session, key, contents);
-            Buffer bs2 = new ByteArrayBuffer(keyType.length() + signature.length + Long.SIZE, false);
-            bs2.putString(keyType);
+            Map.Entry<String, byte[]> result = agent.sign(session, key, keyType, contents);
+            String algo = result.getKey();
+            byte[] signature = result.getValue();
+            Buffer bs2 = new ByteArrayBuffer(algo.length() + signature.length + Long.SIZE, false);
+            bs2.putString(algo);
             bs2.putBytes(signature);
             buffer.putBytes(bs2.array(), bs2.rpos(), bs2.available());