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 2015/12/14 07:22:02 UTC

[3/4] mina-sshd git commit: [SSHD-611] Client incorrectly handles rejected keyboard-interactive authentication by server

[SSHD-611] Client incorrectly handles rejected keyboard-interactive authentication by server


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/1fab5c0e
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/1fab5c0e
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/1fab5c0e

Branch: refs/heads/master
Commit: 1fab5c0e9766540e939b7f6a8a587eb788a16351
Parents: 5b3ebf6
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Mon Dec 14 08:14:02 2015 +0200
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Mon Dec 14 08:14:02 2015 +0200

----------------------------------------------------------------------
 .../sshd/client/auth/AbstractUserAuth.java      |  28 ++-
 .../org/apache/sshd/client/auth/UserAuth.java   |  18 +-
 .../auth/UserAuthKeyboardInteractive.java       | 198 +++++++++++--------
 .../sshd/client/auth/UserAuthPassword.java      |  86 ++++----
 .../sshd/client/auth/UserAuthPublicKey.java     | 132 ++++++-------
 .../client/session/ClientUserAuthService.java   |   2 +-
 .../server/ServerAuthenticationManager.java     |   4 +-
 .../java/org/apache/sshd/server/ServerTest.java | 115 ++++++++++-
 8 files changed, 373 insertions(+), 210 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/client/auth/AbstractUserAuth.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/AbstractUserAuth.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/AbstractUserAuth.java
index 119310f..7cc3348 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/AbstractUserAuth.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/AbstractUserAuth.java
@@ -23,6 +23,7 @@ import java.util.Collection;
 
 import org.apache.sshd.client.session.ClientSession;
 import org.apache.sshd.common.util.ValidateUtils;
+import org.apache.sshd.common.util.buffer.Buffer;
 import org.apache.sshd.common.util.logging.AbstractLoggingBean;
 
 /**
@@ -30,7 +31,7 @@ import org.apache.sshd.common.util.logging.AbstractLoggingBean;
  */
 public abstract class AbstractUserAuth extends AbstractLoggingBean implements UserAuth {
     private final String name;
-    private ClientSession session;
+    private ClientSession clientSession;
     private String service;
 
     protected AbstractUserAuth(String name) {
@@ -39,7 +40,7 @@ public abstract class AbstractUserAuth extends AbstractLoggingBean implements Us
 
     @Override
     public ClientSession getClientSession() {
-        return session;
+        return clientSession;
     }
 
     @Override
@@ -58,11 +59,32 @@ public abstract class AbstractUserAuth extends AbstractLoggingBean implements Us
 
     @Override
     public void init(ClientSession session, String service, Collection<?> identities) throws Exception {
-        this.session = ValidateUtils.checkNotNull(session, "No client session");
+        this.clientSession = ValidateUtils.checkNotNull(session, "No client session");
         this.service = ValidateUtils.checkNotNullAndNotEmpty(service, "No service");
     }
 
     @Override
+    public boolean process(Buffer buffer) throws Exception {
+        ClientSession session = getClientSession();
+        String service = getService();
+        if (buffer == null) {
+            return sendAuthDataRequest(session, service);
+        } else {
+            return processAuthDataRequest(session, service, buffer);
+        }
+    }
+
+    protected abstract boolean sendAuthDataRequest(ClientSession session, String service) throws Exception;
+    protected abstract boolean processAuthDataRequest(ClientSession session, String service, Buffer buffer) throws Exception;
+
+    @Override
+    public void destroy() {
+        if (log.isDebugEnabled()) {
+            log.debug("destroy({})[{}]", getClientSession(), getService());
+        }
+    }
+
+    @Override
     public String toString() {
         return getName() + ": " + getSession() + "[" + getService() + "]";
     }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuth.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuth.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuth.java
index 09fca32..44a111a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuth.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuth.java
@@ -26,22 +26,32 @@ import org.apache.sshd.common.auth.UserAuthInstance;
 import org.apache.sshd.common.util.buffer.Buffer;
 
 /**
- * TODO Add javadoc
- *
+ * Represents a user authentication mechanism
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public interface UserAuth extends ClientSessionHolder, UserAuthInstance<ClientSession> {
 
+    /**
+     * @param session The {@link ClientSession}
+     * @param service The requesting service name
+     * @param identities The currently available identities - e.g., password, keys, etc.
+     * @throws Exception If failed to initialize the mechanism
+     */
     void init(ClientSession session, String service, Collection<?> identities) throws Exception;
 
     /**
-     * @param buffer The {@link Buffer} to process - {@code null} if not a response buffer
+     * @param buffer The {@link Buffer} to process - {@code null} if not a response buffer,
+     * i.e., the underlying authentication mechanism should initiate whatever challenge/response
+     * mechanism is required
      * @return {@code true} if request handled - {@code false} if the next authentication
      * mechanism should be used
-     * @throws Exception
+     * @throws Exception If failed to process the request
      */
     boolean process(Buffer buffer) throws Exception;
 
+    /**
+     * Called to release any allocated resources
+     */
     void destroy();
 
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthKeyboardInteractive.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthKeyboardInteractive.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthKeyboardInteractive.java
index 203c3d1..1863379 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthKeyboardInteractive.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthKeyboardInteractive.java
@@ -23,6 +23,8 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.sshd.client.ClientAuthenticationManager;
 import org.apache.sshd.client.session.ClientSession;
@@ -75,9 +77,10 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth {
      */
     public static final String DEFAULT_INTERACTIVE_SUBMETHODS = "";
 
+    private final AtomicBoolean requestPending = new AtomicBoolean(false);
+    private final AtomicInteger trialsCount = new AtomicInteger(0);
     private Iterator<String> passwords;
     private int maxTrials;
-    private int nbTrials;
 
     public UserAuthKeyboardInteractive() {
         super(NAME);
@@ -99,102 +102,112 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth {
     }
 
     @Override
-    public boolean process(Buffer buffer) throws Exception {
-        ClientSession session = getClientSession();
-        String username = session.getUsername();
-        String service = getService();
-
-        if (buffer == null) {
-            String name = getName();
+    protected boolean sendAuthDataRequest(ClientSession session, String service) throws Exception {
+        String name = getName();
+        if (requestPending.get()) {
             if (log.isDebugEnabled()) {
-                log.debug("process({}@{})[{}] Send SSH_MSG_USERAUTH_REQUEST for {}",
-                          username, session, service, name);
+                log.debug("sendAuthDataRequest({})[{}] no reply for previous request for {}",
+                          session, service, name);
             }
-            buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST,
-                                username.length() + service.length() + name.length() + Integer.SIZE);
-            buffer.putString(username);
-            buffer.putString(service);
-            buffer.putString(name);
-            buffer.putString(getExchangeLanguageTag(session));
-            buffer.putString(getExchangeSubMethods(session));
-            session.writePacket(buffer);
-            return true;
+            return false;
+        }
+
+        if (!verifyTrialsCount(session, service, SshConstants.SSH_MSG_USERAUTH_REQUEST, trialsCount.get(), maxTrials)) {
+            return false;
+        }
+
+        String username = session.getUsername();
+        String lang = getExchangeLanguageTag(session);
+        String subMethods = getExchangeSubMethods(session);
+        if (log.isDebugEnabled()) {
+            log.debug("sendAuthDataRequest({})[{}] send SSH_MSG_USERAUTH_REQUEST for {}: lang={}, methods={}",
+                      session, service, name, lang, subMethods);
         }
 
+        Buffer buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST,
+                            username.length() + service.length() + name.length()
+                          + GenericUtils.length(lang) + GenericUtils.length(subMethods)
+                          + Long.SIZE /* a bit extra for the lengths */);
+        buffer.putString(username);
+        buffer.putString(service);
+        buffer.putString(name);
+        buffer.putString(lang);
+        buffer.putString(subMethods);
+        requestPending.set(true);
+        session.writePacket(buffer);
+        return true;
+    }
+
+    @Override
+    protected boolean processAuthDataRequest(ClientSession session, String service, Buffer buffer) throws Exception {
         int cmd = buffer.getUByte();
-        if (cmd == SshConstants.SSH_MSG_USERAUTH_INFO_REQUEST) {
-            nbTrials++;
-            if (nbTrials > maxTrials) {
-                if (log.isDebugEnabled()) {
-                    log.debug("process({})[{}] Reject SSH_MSG_USERAUTH_INFO_REQUEST for {} num. trials ({}) exceeds max({})",
-                              session, service, getName(), nbTrials, maxTrials);
-                }
-                return false;
-            }
+        if (cmd != SshConstants.SSH_MSG_USERAUTH_INFO_REQUEST) {
+            throw new IllegalStateException("processAuthDataRequest(" + session + ")[" + service + "]"
+                            + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd));
+        }
 
-            if (log.isDebugEnabled()) {
-                log.debug("process({})[{}] Received SSH_MSG_USERAUTH_INFO_REQUEST for {}", session, service, getName());
-            }
+        requestPending.set(false);
 
-            String name = buffer.getString();
-            String instruction = buffer.getString();
-            String language_tag = buffer.getString();
-            if (log.isTraceEnabled()) {
-                log.trace("process({})[{}] SSH_MSG_USERAUTH_INFO_REQUEST name={} instruction={} language={}",
-                          session, service, name, instruction, language_tag);
-            }
+        if (!verifyTrialsCount(session, service, cmd, trialsCount.incrementAndGet(), maxTrials)) {
+            return false;
+        }
 
-            int num = buffer.getInt();
-            String[] prompt = new String[num];
-            boolean[] echo = new boolean[num];
-            for (int i = 0; i < num; i++) {
-                // according to RFC4256: "The prompt field(s) MUST NOT be empty strings."
-                prompt[i] = buffer.getString();
-                echo[i] = buffer.getBoolean();
-            }
+        String name = buffer.getString();
+        String instruction = buffer.getString();
+        String lang = buffer.getString();
+        int num = buffer.getInt();
+        if (log.isDebugEnabled()) {
+            log.debug("processAuthDataRequest({})[{}] SSH_MSG_USERAUTH_INFO_REQUEST name={}, instruction={}, language={}, num-prompts={}",
+                      session, service, name, instruction, lang, num);
+        }
 
-            if (log.isTraceEnabled()) {
-                log.trace("process({})[{}] Prompt: {}", session, service, Arrays.toString(prompt));
-                log.trace("process({})[{}] Echo: {}", session, service, Arrays.toString(echo));
-            }
+        String[] prompt = new String[num];
+        boolean[] echo = new boolean[num];
+        for (int i = 0; i < num; i++) {
+            // according to RFC4256: "The prompt field(s) MUST NOT be empty strings."
+            prompt[i] = buffer.getString();
+            echo[i] = buffer.getBoolean();
+        }
 
-            String[] rep = getUserResponses(name, instruction, language_tag, prompt, echo);
-            if (rep == null) {
-                if (log.isDebugEnabled()) {
-                    log.debug("process({})[{}] no responses for {}", session, service, name);
-                }
-                return false;
-            }
+        if (log.isTraceEnabled()) {
+            log.trace("processAuthDataRequest({})[{}] Prompt: {}", session, service, Arrays.toString(prompt));
+            log.trace("processAuthDataRequest({})[{}] Echo: {}", session, service, Arrays.toString(echo));
+        }
 
-            /*
-             * According to RFC4256:
-             *
-             *      If the num-responses field does not match the num-prompts
-             *      field in the request message, the server MUST send a failure
-             *      message.
-             *
-             * However it is the server's (!) responsibility to fail, so we only warn...
-             */
-            if (num != rep.length) {
-                log.warn("process({})[{}] Mismatched prompts ({}) vs. responses count ({})",
-                         session, service, num, rep.length);
+        String[] rep = getUserResponses(name, instruction, lang, prompt, echo);
+        if (rep == null) {
+            if (log.isDebugEnabled()) {
+                log.debug("processAuthDataRequest({})[{}] no responses for {}", session, service, name);
             }
+            return false;
+        }
 
-            buffer = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_INFO_RESPONSE, BufferUtils.clear(buffer));
-            buffer.putInt(rep.length);
-            for (int index = 0; index < rep.length; index++) {
-                String r = rep[index];
-                if (log.isTraceEnabled()) {
-                    log.trace("process({})[{}] response #{}: {}", session, service, index + 1, r);
-                }
-                buffer.putString(r);
+        /*
+         * According to RFC4256:
+         *
+         *      If the num-responses field does not match the num-prompts
+         *      field in the request message, the server MUST send a failure
+         *      message.
+         *
+         * However it is the server's (!) responsibility to fail, so we only warn...
+         */
+        if (num != rep.length) {
+            log.warn("processAuthDataRequest({})[{}] Mismatched prompts ({}) vs. responses count ({})",
+                     session, service, num, rep.length);
+        }
+
+        buffer = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_INFO_RESPONSE, BufferUtils.clear(buffer));
+        buffer.putInt(rep.length);
+        for (int index = 0; index < rep.length; index++) {
+            String r = rep[index];
+            if (log.isTraceEnabled()) {
+                log.trace("processAuthDataRequest({})[{}] response #{}: {}", session, service, index + 1, r);
             }
-            session.writePacket(buffer);
-            return true;
+            buffer.putString(r);
         }
 
-        throw new IllegalStateException("process(" + session + ")[" + service + "]"
-                + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd));
+        session.writePacket(buffer);
+        return true;
     }
 
     protected String getExchangeLanguageTag(ClientSession session) {
@@ -213,6 +226,15 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth {
         }
     }
 
+    protected boolean verifyTrialsCount(ClientSession session, String service, int cmd, int nbTrials, int maxAllowed) {
+        if (log.isDebugEnabled()) {
+            log.debug("verifyTrialsCount({})[{}] cmd={} - {} out of {}",
+                      session, service, getAuthCommandName(cmd), nbTrials, maxAllowed);
+        }
+
+        return nbTrials <= maxAllowed;
+    }
+
     /**
      * @param name        The interaction name - may be empty
      * @param instruction The instruction - may be empty
@@ -265,7 +287,7 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth {
         }
 
         // check that prompt is something like "XXX password YYY:"
-        String value = prompt[0].toLowerCase();
+        String value = GenericUtils.trimToEmpty(prompt[0]).toLowerCase();
         int passPos = value.lastIndexOf("password");
         if (passPos < 0) {  // no password keyword in prompt
             return false;
@@ -279,8 +301,14 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth {
         return true;
     }
 
-    @Override
-    public void destroy() {
-        // nothing
+    public static String getAuthCommandName(int cmd) {
+        switch(cmd) {
+            case SshConstants.SSH_MSG_USERAUTH_REQUEST:
+                return "SSH_MSG_USERAUTH_REQUEST";
+            case SshConstants.SSH_MSG_USERAUTH_INFO_REQUEST:
+                return "SSH_MSG_USERAUTH_INFO_REQUEST";
+            default:
+                return SshConstants.getCommandMessageName(cmd);
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPassword.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPassword.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPassword.java
index 3737b24..449d10a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPassword.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPassword.java
@@ -60,56 +60,53 @@ public class UserAuthPassword extends AbstractUserAuth {
     }
 
     @Override
-    public boolean process(Buffer buffer) throws Exception {
-        ClientSession session = getClientSession();
-        String username = session.getUsername();
-        String service = getService();
+    protected boolean sendAuthDataRequest(ClientSession session, String service) throws Exception {
+        if (passwords.hasNext()) {
+            current = passwords.next();
+            String username = session.getUsername();
+            Buffer buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST,
+                                username.length() + service.length() + getName().length() + current.length() + Integer.SIZE);
+            sendPassword(buffer, session, current, current);
+            return true;
+        }
 
-        // Send next password
-        if (buffer == null) {
-            if (passwords.hasNext()) {
-                current = passwords.next();
-                buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST,
-                                    username.length() + service.length() + getName().length() + current.length() + Integer.SIZE);
-                sendPassword(buffer, session, current, current);
-                return true;
-            }
+        if (log.isDebugEnabled()) {
+            log.debug("sendAuthDataRequest({})[{}] no more passwords to send", session, service);
+        }
 
-            if (log.isDebugEnabled()) {
-                log.debug("process({}@{})[{}] no more passwords to send", username, session, service);
-            }
+        return false;
+    }
 
-            return false;
+    @Override
+    protected boolean processAuthDataRequest(ClientSession session, String service, Buffer buffer) throws Exception {
+        int cmd = buffer.getUByte();
+        if (cmd != SshConstants.SSH_MSG_USERAUTH_PASSWD_CHANGEREQ) {
+            throw new IllegalStateException("processAuthDataRequest(" + session + ")[" + service + "]"
+                            + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd));
         }
 
-        int cmd = buffer.getUByte();
-        if (cmd == SshConstants.SSH_MSG_USERAUTH_PASSWD_CHANGEREQ) {
-            String prompt = buffer.getString();
-            String lang = buffer.getString();
-            UserInteraction ui = session.getUserInteraction();
-            if ((ui != null) && ui.isInteractionAllowed(session)) {
-                String password = ui.getUpdatedPassword(session, prompt, lang);
-                if (GenericUtils.isEmpty(password)) {
-                    if (log.isDebugEnabled()) {
-                        log.debug("process({}@{})[{}] No updated password for prompt={}, lang={}",
-                                  username, session, service, prompt, lang);
-                    }
-                } else {
-                    sendPassword(buffer, session, password, password);
-                    return true;
-                }
-            } else {
+        String prompt = buffer.getString();
+        String lang = buffer.getString();
+        UserInteraction ui = session.getUserInteraction();
+        if ((ui != null) && ui.isInteractionAllowed(session)) {
+            String password = ui.getUpdatedPassword(session, prompt, lang);
+            if (GenericUtils.isEmpty(password)) {
                 if (log.isDebugEnabled()) {
-                    log.debug("process({}@{})[{}] no UI for password change request for prompt={}, lang={}",
-                              username, session, service, prompt, lang);
+                    log.debug("processAuthDataRequest({})[{}] No updated password for prompt={}, lang={}",
+                              session, service, prompt, lang);
                 }
+            } else {
+                sendPassword(buffer, session, password, password);
+                return true;
+            }
+        } else {
+            if (log.isDebugEnabled()) {
+                log.debug("processAuthDataRequest({})[{}] no UI for password change request for prompt={}, lang={}",
+                          session, service, prompt, lang);
             }
-
-            return false;
         }
 
-        throw new IllegalStateException("process(" + username + "@" + session + ")[" + service + "]"
-                + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd));
+        return false;
     }
 
     /**
@@ -130,8 +127,8 @@ public class UserAuthPassword extends AbstractUserAuth {
         String name = getName();
         boolean modified = !Objects.equals(oldPassword, newPassword);
         if (log.isDebugEnabled()) {
-            log.debug("sendPassword({}@{})[{}] send SSH_MSG_USERAUTH_REQUEST for {} - modified={}",
-                      username, session, service, name, modified);
+            log.debug("sendPassword({})[{}] send SSH_MSG_USERAUTH_REQUEST for {} - modified={}",
+                      session, service, name, modified);
         }
 
         buffer = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST, BufferUtils.clear(buffer));
@@ -146,9 +143,4 @@ public class UserAuthPassword extends AbstractUserAuth {
         }
         session.writePacket(buffer);
     }
-
-    @Override
-    public void destroy() {
-        // ignored
-    }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPublicKey.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPublicKey.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPublicKey.java
index 2adbe03..8cbfddf 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPublicKey.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/UserAuthPublicKey.java
@@ -104,81 +104,79 @@ public class UserAuthPublicKey extends AbstractUserAuth {
     }
 
     @Override
-    public boolean process(Buffer buffer) throws Exception {
-        ClientSession session = getClientSession();
-        String username = session.getUsername();
-        String service = getService();
-
-        // Send next key
-        if (buffer == null) {
-            if (keys.hasNext()) {
-                current = keys.next();
-                PublicKey key = current.getPublicKey();
-                String algo = KeyUtils.getKeyType(key);
-                String name = getName();
-                if (log.isDebugEnabled()) {
-                    log.debug("process({}@{})[{}] send SSH_MSG_USERAUTH_REQUEST request {} type={} - fingerprint={}",
-                              username, session, service, name, algo, KeyUtils.getFingerPrint(key));
-                }
-
-                buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST);
-                buffer.putString(username);
-                buffer.putString(service);
-                buffer.putString(name);
-                buffer.putBoolean(false);
-                buffer.putString(algo);
-                buffer.putPublicKey(key);
-                session.writePacket(buffer);
-                return true;
-            }
-
+    protected boolean sendAuthDataRequest(ClientSession session, String service) throws Exception {
+        if (!keys.hasNext()) {
             if (log.isDebugEnabled()) {
-                log.debug("process({}@{})[{}] no more keys to send", username, session, service);
+                log.debug("sendAuthDataRequest({})[{}] no more keys to send", session, service);
             }
+
             return false;
         }
 
+        current = keys.next();
+        PublicKey key = current.getPublicKey();
+        String algo = KeyUtils.getKeyType(key);
+        String name = getName();
+        if (log.isDebugEnabled()) {
+            log.debug("sendAuthDataRequest({})[{}] send SSH_MSG_USERAUTH_REQUEST request {} type={} - fingerprint={}",
+                      session, service, name, algo, KeyUtils.getFingerPrint(key));
+        }
+
+        Buffer buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST);
+        buffer.putString(session.getUsername());
+        buffer.putString(service);
+        buffer.putString(name);
+        buffer.putBoolean(false);
+        buffer.putString(algo);
+        buffer.putPublicKey(key);
+        session.writePacket(buffer);
+        return true;
+    }
+
+    @Override
+    protected boolean processAuthDataRequest(ClientSession session, String service, Buffer buffer) throws Exception {
         int cmd = buffer.getUByte();
-        if (cmd == SshConstants.SSH_MSG_USERAUTH_PK_OK) {
-            PublicKey key = current.getPublicKey();
-            String algo = KeyUtils.getKeyType(key);
-            String name = getName();
-            if (log.isDebugEnabled()) {
-                log.debug("process({}@{})[{}] send SSH_MSG_USERAUTH_REQUEST reply {} type={} - fingerprint={}",
-                          username, session, service, name, algo, KeyUtils.getFingerPrint(key));
-            }
+        if (cmd != SshConstants.SSH_MSG_USERAUTH_PK_OK) {
+            throw new IllegalStateException("processAuthDataRequest(" + session + ")[" + service + "]"
+                    + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd));
+        }
 
-            buffer = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST, BufferUtils.clear(buffer));
-            buffer.putString(username);
-            buffer.putString(service);
-            buffer.putString(name);
-            buffer.putBoolean(true);
-            buffer.putString(algo);
-            buffer.putPublicKey(key);
-
-            Buffer bs = new ByteArrayBuffer();
-            KeyExchange kex = session.getKex();
-            bs.putBytes(kex.getH());
-            bs.putByte(SshConstants.SSH_MSG_USERAUTH_REQUEST);
-            bs.putString(username);
-            bs.putString(service);
-            bs.putString(name);
-            bs.putBoolean(true);
-            bs.putString(algo);
-            bs.putPublicKey(key);
-
-            byte[] sig = current.sign(bs.getCompactData());
-            bs = new ByteArrayBuffer(algo.length() + sig.length + Long.SIZE, false);
-            bs.putString(algo);
-            bs.putBytes(sig);
-            buffer.putBytes(bs.array(), bs.rpos(), bs.available());
-
-            session.writePacket(buffer);
-            return true;
+        PublicKey key = current.getPublicKey();
+        String algo = KeyUtils.getKeyType(key);
+        String name = getName();
+        if (log.isDebugEnabled()) {
+            log.debug("processAuthDataRequest({})[{}] send SSH_MSG_USERAUTH_PK_OK reply for {}: type={}, fingerprint={}",
+                      session, service, name, algo, KeyUtils.getFingerPrint(key));
         }
 
-        throw new IllegalStateException("process(" + username + "@" + session + ")[" + service + "]"
-                + " received unknown packet: cmd=" + SshConstants.getCommandMessageName(cmd));
+        String username = session.getUsername();
+        buffer = session.prepareBuffer(SshConstants.SSH_MSG_USERAUTH_REQUEST, BufferUtils.clear(buffer));
+        buffer.putString(username);
+        buffer.putString(service);
+        buffer.putString(name);
+        buffer.putBoolean(true);
+        buffer.putString(algo);
+        buffer.putPublicKey(key);
+
+        Buffer bs = new ByteArrayBuffer();
+        KeyExchange kex = session.getKex();
+        bs.putBytes(kex.getH());
+        bs.putByte(SshConstants.SSH_MSG_USERAUTH_REQUEST);
+        bs.putString(username);
+        bs.putString(service);
+        bs.putString(name);
+        bs.putBoolean(true);
+        bs.putString(algo);
+        bs.putPublicKey(key);
+
+        byte[] sig = current.sign(bs.getCompactData());
+        bs = new ByteArrayBuffer(algo.length() + sig.length + Long.SIZE, false);
+        bs.putString(algo);
+        bs.putBytes(sig);
+        buffer.putBytes(bs.array(), bs.rpos(), bs.available());
+
+        session.writePacket(buffer);
+        return true;
     }
 
     @Override
@@ -191,6 +189,8 @@ public class UserAuthPublicKey extends AbstractUserAuth {
             } finally {
                 agent = null;
             }
+
+            super.destroy(); // for logging
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
index 49bd727..34c6cf5 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientUserAuthService.java
@@ -199,6 +199,7 @@ public class ClientUserAuthService extends AbstractCloseable implements Service,
             }
             if (partial || (serverMethods == null)) {
                 serverMethods = Arrays.asList(GenericUtils.split(mths, ','));
+                currentMethod = 0;
                 if (userAuth != null) {
                     try {
                         userAuth.destroy();
@@ -236,7 +237,6 @@ public class ClientUserAuthService extends AbstractCloseable implements Service,
                     log.debug("tryNext({}) starting authentication mechanisms: client={}, server={}",
                               session, clientMethods, serverMethods);
                 }
-                currentMethod = 0;
             } else if (!userAuth.process(null)) {
                 if (log.isDebugEnabled()) {
                     log.debug("tryNext({}) no initial request sent by method={}", session, userAuth.getName());

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/main/java/org/apache/sshd/server/ServerAuthenticationManager.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/ServerAuthenticationManager.java b/sshd-core/src/main/java/org/apache/sshd/server/ServerAuthenticationManager.java
index e889392..99c075a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/ServerAuthenticationManager.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/ServerAuthenticationManager.java
@@ -143,13 +143,15 @@ public interface ServerAuthenticationManager {
         public static List<NamedFactory<UserAuth>> resolveUserAuthFactories(
                 ServerAuthenticationManager manager, List<NamedFactory<UserAuth>> userFactories) {
             if (GenericUtils.size(userFactories) > 0) {
-                return userFactories;
+                return userFactories;   // use whatever the user decided
             }
 
             List<NamedFactory<UserAuth>> factories = new ArrayList<>();
             if (manager.getPasswordAuthenticator() != null) {
                 factories.add(DEFAULT_USER_AUTH_PASSWORD_FACTORY);
                 factories.add(DEFAULT_USER_AUTH_KB_INTERACTIVE_FACTORY);
+            } else if (manager.getKeyboardInteractiveAuthenticator() != null) {
+                factories.add(DEFAULT_USER_AUTH_KB_INTERACTIVE_FACTORY);
             }
 
             if (manager.getPublickeyAuthenticator() != null) {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1fab5c0e/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
index 8bf16db..ebacd90 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
@@ -30,6 +30,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.concurrent.CountDownLatch;
@@ -38,7 +39,9 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.sshd.client.ClientAuthenticationManager;
 import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.auth.UserInteraction;
 import org.apache.sshd.client.channel.ChannelExec;
 import org.apache.sshd.client.channel.ChannelShell;
 import org.apache.sshd.client.channel.ClientChannel;
@@ -49,6 +52,7 @@ import org.apache.sshd.client.session.ClientSessionImpl;
 import org.apache.sshd.common.FactoryManager;
 import org.apache.sshd.common.NamedFactory;
 import org.apache.sshd.common.PropertyResolverUtils;
+import org.apache.sshd.common.auth.UserAuthMethodFactory;
 import org.apache.sshd.common.channel.Channel;
 import org.apache.sshd.common.channel.TestChannelListener;
 import org.apache.sshd.common.channel.WindowClosedException;
@@ -62,7 +66,13 @@ import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.OsUtils;
 import org.apache.sshd.common.util.ValidateUtils;
 import org.apache.sshd.deprecated.ClientUserAuthServiceOld;
+import org.apache.sshd.server.auth.keyboard.InteractiveChallenge;
+import org.apache.sshd.server.auth.keyboard.KeyboardInteractiveAuthenticator;
+import org.apache.sshd.server.auth.keyboard.PromptEntry;
+import org.apache.sshd.server.auth.password.RejectAllPasswordAuthenticator;
+import org.apache.sshd.server.auth.pubkey.RejectAllPublickeyAuthenticator;
 import org.apache.sshd.server.command.ScpCommandFactory;
+import org.apache.sshd.server.session.ServerSession;
 import org.apache.sshd.server.session.ServerSessionImpl;
 import org.apache.sshd.server.subsystem.sftp.SftpSubsystemFactory;
 import org.apache.sshd.util.test.BaseTestSupport;
@@ -107,10 +117,8 @@ public class ServerTest extends BaseTestSupport {
         }
     }
 
-    /**
+    /*
      * Send bad password.  The server should disconnect after a few attempts
-     *
-     * @throws Exception
      */
     @Test
     public void testFailAuthenticationWithWaitFor() throws Exception {
@@ -600,6 +608,107 @@ public class ServerTest extends BaseTestSupport {
         }
     }
 
+    @Test   // see SSHD-611
+    public void testImmediateAuthFailureOpcode() throws Exception {
+        sshd.setPasswordAuthenticator(RejectAllPasswordAuthenticator.INSTANCE);
+        sshd.setPublickeyAuthenticator(RejectAllPublickeyAuthenticator.INSTANCE);
+        final AtomicInteger challengeCount = new AtomicInteger(0);
+        sshd.setKeyboardInteractiveAuthenticator(new KeyboardInteractiveAuthenticator() {
+            @Override
+            public InteractiveChallenge generateChallenge(ServerSession session, String username, String lang, String subMethods) {
+                challengeCount.incrementAndGet();
+                outputDebugMessage("generateChallenge(%s@%s) count=%s", username, session, challengeCount);
+                return null;
+            }
+
+            @Override
+            public boolean authenticate(ServerSession session, String username, List<String> responses) throws Exception {
+                return false;
+            }
+        });
+        sshd.start();
+
+        String authMethods = GenericUtils.join( // order is important
+                Arrays.asList(UserAuthMethodFactory.KB_INTERACTIVE, UserAuthMethodFactory.PUBLIC_KEY, UserAuthMethodFactory.PUBLIC_KEY), ',');
+        PropertyResolverUtils.updateProperty(client, ClientAuthenticationManager.PREFERRED_AUTHS, authMethods);
+
+        client.start();
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, sshd.getPort()).verify(7L, TimeUnit.SECONDS).getSession()) {
+            AuthFuture auth = session.auth();
+            assertTrue("Failed to complete authentication on time", auth.await(17L, TimeUnit.SECONDS));
+            assertFalse("Unexpected authentication success", auth.isSuccess());
+            assertEquals("Mismatched interactive challenge calls", 1, challengeCount.get());
+        } finally {
+            client.stop();
+        }
+    }
+
+    @Test
+    public void testMaxKeyboardInteractiveTrialsSetting() throws Exception {
+        sshd.setPasswordAuthenticator(RejectAllPasswordAuthenticator.INSTANCE);
+        sshd.setPublickeyAuthenticator(RejectAllPublickeyAuthenticator.INSTANCE);
+
+        final InteractiveChallenge challenge = new InteractiveChallenge();
+        challenge.setInteractionInstruction(getCurrentTestName());
+        challenge.setInteractionName(getClass().getSimpleName());
+        challenge.setLanguageTag("il-heb");
+        challenge.addPrompt(new PromptEntry("Password", false));
+        final AtomicInteger serverCount = new AtomicInteger(0);
+        sshd.setKeyboardInteractiveAuthenticator(new KeyboardInteractiveAuthenticator() {
+            @Override
+            public InteractiveChallenge generateChallenge(ServerSession session, String username, String lang, String subMethods) {
+                return challenge;
+            }
+
+            @Override
+            public boolean authenticate(ServerSession session, String username, List<String> responses) throws Exception {
+                outputDebugMessage("authenticate(%s@%s) count=%s", username, session, serverCount);
+                serverCount.incrementAndGet();
+                return false;
+            }
+        });
+        sshd.start();
+
+        String authMethods = GenericUtils.join( // order is important
+                Arrays.asList(UserAuthMethodFactory.KB_INTERACTIVE, UserAuthMethodFactory.PUBLIC_KEY, UserAuthMethodFactory.PUBLIC_KEY), ',');
+        PropertyResolverUtils.updateProperty(client, ClientAuthenticationManager.PREFERRED_AUTHS, authMethods);
+        final AtomicInteger clientCount = new AtomicInteger(0);
+        final String[] replies = { getCurrentTestName() };
+        client.setUserInteraction(new UserInteraction() {
+            @Override
+            public void welcome(ClientSession session, String banner, String lang) {
+                // ignored
+            }
+
+            @Override
+            public boolean isInteractionAllowed(ClientSession session) {
+                return true;
+            }
+
+            @Override
+            public String[] interactive(ClientSession session, String name, String instruction, String lang, String[] prompt, boolean[] echo) {
+                clientCount.incrementAndGet();
+                return replies;
+            }
+
+            @Override
+            public String getUpdatedPassword(ClientSession session, String prompt, String lang) {
+                throw new UnsupportedOperationException("Unexpected updated password request");
+            }
+        });
+        client.start();
+
+        try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, sshd.getPort()).verify(7L, TimeUnit.SECONDS).getSession()) {
+            AuthFuture auth = session.auth();
+            assertTrue("Failed to complete authentication on time", auth.await(17L, TimeUnit.SECONDS));
+            assertFalse("Unexpected authentication success", auth.isSuccess());
+            assertEquals("Mismatched interactive server challenge calls", ClientAuthenticationManager.DEFAULT_PASSWORD_PROMPTS, serverCount.get());
+            assertEquals("Mismatched interactive client challenge calls", ClientAuthenticationManager.DEFAULT_PASSWORD_PROMPTS, clientCount.get());
+        } finally {
+            client.stop();
+        }
+    }
+
     private ClientSession createTestClientSession(SshServer server) throws Exception {
         ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, server.getPort()).verify(7L, TimeUnit.SECONDS).getSession();
         try {