You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by gn...@apache.org on 2013/07/19 08:33:25 UTC
[2/2] git commit: [SSHD-220] Return a usable AuthFuture when
authenticating on the client side
[SSHD-220] Return a usable AuthFuture when authenticating on the client side
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/2382d606
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/2382d606
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/2382d606
Branch: refs/heads/master
Commit: 2382d6060d6e490fe8cdba670e053f4b4bc4c710
Parents: eaef1c3
Author: Guillaume Nodet <gn...@apache.org>
Authored: Fri Jul 19 08:33:17 2013 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Fri Jul 19 08:33:17 2013 +0200
----------------------------------------------------------------------
.../main/java/org/apache/sshd/SshClient.java | 15 +-
.../sshd/client/session/ClientSessionImpl.java | 249 ++++++++-----------
.../test/java/org/apache/sshd/ServerTest.java | 34 ++-
3 files changed, 139 insertions(+), 159 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2382d606/sshd-core/src/main/java/org/apache/sshd/SshClient.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/SshClient.java b/sshd-core/src/main/java/org/apache/sshd/SshClient.java
index 9d0bfcb..797d6a0 100644
--- a/sshd-core/src/main/java/org/apache/sshd/SshClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/SshClient.java
@@ -42,6 +42,7 @@ import org.apache.sshd.client.ServerKeyVerifier;
import org.apache.sshd.client.SessionFactory;
import org.apache.sshd.client.UserInteraction;
import org.apache.sshd.client.channel.ChannelShell;
+import org.apache.sshd.client.future.AuthFuture;
import org.apache.sshd.client.future.ConnectFuture;
import org.apache.sshd.client.future.DefaultConnectFuture;
import org.apache.sshd.client.kex.DHG1;
@@ -452,19 +453,19 @@ public class SshClient extends AbstractFactoryManager implements ClientFactoryMa
ClientSession session = client.connect(host, port).await().getSession();
int ret = ClientSession.WAIT_AUTH;
- while ((ret & ClientSession.WAIT_AUTH) != 0) {
+ AuthFuture authFuture;
+ do {
if (hasKeys) {
- session.authAgent(login);
- ret = session.waitFor(ClientSession.WAIT_AUTH | ClientSession.CLOSED | ClientSession.AUTHED, 0);
+ authFuture = session.authAgent(login);
} else {
System.out.print("Password:");
BufferedReader r = new BufferedReader(new InputStreamReader(System.in));
String password = r.readLine();
- session.authPassword(login, password);
- ret = session.waitFor(ClientSession.WAIT_AUTH | ClientSession.CLOSED | ClientSession.AUTHED, 0);
+ authFuture = session.authPassword(login, password);
}
- }
- if ((ret & ClientSession.CLOSED) != 0) {
+ authFuture.await();
+ } while (authFuture.isFailure());
+ if (!authFuture.isSuccess()) {
System.err.println("error");
System.exit(-1);
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2382d606/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
index de12cd4..76a2f54 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
@@ -66,7 +66,11 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
private static final String AUTHENTICATION_SERVICE = "ssh-connection";
private UserAuth userAuth;
- private AuthFuture authFuture;
+ /**
+ * The AuthFuture that is being used by the current auth request. This encodes the state.
+ * isSuccess -> authenticated, else if isDone -> server waiting for user auth, else authenticating.
+ */
+ private volatile AuthFuture authFuture;
/**
* For clients to store their own metadata
@@ -78,6 +82,8 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
log.info("Session created...");
sendClientIdentification();
sendKexInit();
+ // Maintain the current auth status in the authFuture.
+ authFuture = new DefaultAuthFuture(lock);
}
public ClientFactoryManager getClientFactoryManager() {
@@ -88,156 +94,117 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
return kex;
}
+ /**
+ * return true if/when ready for auth; false if never ready.
+ * @return server is ready and waiting for auth
+ */
+ private boolean readyForAuth() {
+ // isDone indicates that the last auth finished and a new one can commence.
+ while (!this.authFuture.isDone()) {
+ log.debug("waiting to send authentication");
+ try {
+ this.authFuture.await();
+ } catch (InterruptedException e) {
+ log.debug("Unexpected interrupt", e);
+ throw new RuntimeException(e);
+ }
+ }
+ if (this.authFuture.isSuccess()) {
+ log.debug("already authenticated");
+ throw new IllegalStateException("Already authenticated");
+ }
+ if (this.authFuture.getException() != null) {
+ log.debug("probably closed", this.authFuture.getException());
+ return false;
+ }
+ if (!this.authFuture.isFailure()) {
+ log.debug("unexpected state");
+ throw new IllegalStateException("Unexpected authentication state");
+ }
+ if (this.userAuth != null) {
+ log.debug("authentication already in progress");
+ throw new IllegalStateException("Authentication already in progress?");
+ }
+ log.debug("ready to try authentication with new lock");
+ // The new future !isDone() - i.e., in progress blocking out other waits.
+ this.authFuture = new DefaultAuthFuture(lock);
+ return true;
+ }
+
+ /**
+ * execute one step in user authentication.
+ * @param buffer
+ * @throws IOException
+ */
+ private void processUserAuth(Buffer buffer) throws IOException {
+ log.debug("processing {}", userAuth);
+ switch (userAuth.next(buffer)) {
+ case Success:
+ log.debug("succeeded with {}", userAuth);
+ this.authed = true;
+ this.username = userAuth.getUsername();
+ setState(State.Running);
+ // Will wake up anyone sitting in waitFor
+ authFuture.setAuthed(true);
+ startHeartBeat();
+ break;
+ case Failure:
+ log.debug("failed with {}", userAuth);
+ this.userAuth = null;
+ setState(State.WaitForAuth);
+ // Will wake up anyone sitting in waitFor
+ this.authFuture.setAuthed(false);
+ break;
+ case Continued:
+ // Will wake up anyone sitting in waitFor
+ setState(State.UserAuth);
+ log.debug("continuing with {}", userAuth);
+ break;
+ }
+ }
+
public AuthFuture authAgent(String user) throws IOException {
+ log.debug("Trying agent authentication");
+ if (getFactoryManager().getAgentFactory() == null) {
+ throw new IllegalStateException("No ssh agent factory has been configured");
+ }
synchronized (lock) {
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- if (authed) {
- throw new IllegalStateException("User authentication has already been performed");
- }
- if (userAuth != null) {
- throw new IllegalStateException("A user authentication request is already pending");
- }
- if (getFactoryManager().getAgentFactory() == null) {
- throw new IllegalStateException("No ssh agent factory has been configured");
- }
- waitFor(CLOSED | WAIT_AUTH, 0);
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- authFuture = new DefaultAuthFuture(lock);
- userAuth = new UserAuthAgent(this, AUTHENTICATION_SERVICE, user);
- setState(State.UserAuth);
-
- switch (userAuth.next(null)) {
- case Success:
- authFuture.setAuthed(true);
- username = userAuth.getUsername();
- authed = true;
- setState(State.Running);
- break;
- case Failure:
- authFuture.setAuthed(false);
- userAuth = null;
- setState(State.WaitForAuth);
- break;
- case Continued:
- break;
+ if (readyForAuth()) {
+ userAuth = new UserAuthAgent(this, AUTHENTICATION_SERVICE, user);
+ processUserAuth(null);
}
return authFuture;
}
}
public AuthFuture authPassword(String user, String password) throws IOException {
+ log.debug("Trying password authentication");
synchronized (lock) {
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- if (authed) {
- throw new IllegalStateException("User authentication has already been performed");
- }
- if (userAuth != null) {
- throw new IllegalStateException("A user authentication request is already pending");
- }
- waitFor(CLOSED | WAIT_AUTH, 0);
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- authFuture = new DefaultAuthFuture(lock);
- userAuth = new UserAuthPassword(this, AUTHENTICATION_SERVICE, user, password);
- setState(State.UserAuth);
-
- switch (userAuth.next(null)) {
- case Success:
- authFuture.setAuthed(true);
- username = userAuth.getUsername();
- authed = true;
- setState(State.Running);
- break;
- case Failure:
- authFuture.setAuthed(false);
- userAuth = null;
- setState(State.WaitForAuth);
- break;
- case Continued:
- break;
+ if (readyForAuth()) {
+ userAuth = new UserAuthPassword(this, AUTHENTICATION_SERVICE, user, password);
+ processUserAuth(null);
}
return authFuture;
}
}
public AuthFuture authInteractive(String user, String password) throws IOException {
+ log.debug("Trying keyboard-interactive authentication");
synchronized (lock) {
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- if (authed) {
- throw new IllegalStateException("User authentication has already been performed");
- }
- if (userAuth != null) {
- throw new IllegalStateException("A user authentication request is already pending");
- }
- waitFor(CLOSED | WAIT_AUTH, 0);
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- authFuture = new DefaultAuthFuture(lock);
- userAuth = new UserAuthKeyboardInteractive(this, AUTHENTICATION_SERVICE, user, password);
- setState(State.UserAuth);
-
- switch (userAuth.next(null)) {
- case Success:
- authFuture.setAuthed(true);
- username = userAuth.getUsername();
- authed = true;
- setState(State.Running);
- break;
- case Failure:
- authFuture.setAuthed(false);
- userAuth = null;
- setState(State.WaitForAuth);
- break;
- case Continued:
- break;
+ if (readyForAuth()) {
+ userAuth = new UserAuthKeyboardInteractive(this, AUTHENTICATION_SERVICE, user, password);
+ processUserAuth(null);
}
return authFuture;
}
- }
+ }
public AuthFuture authPublicKey(String user, KeyPair key) throws IOException {
+ log.debug("Trying publickey authentication");
synchronized (lock) {
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- if (authed) {
- throw new IllegalStateException("User authentication has already been performed");
- }
- if (userAuth != null) {
- throw new IllegalStateException("A user authentication request is already pending");
- }
- waitFor(CLOSED | WAIT_AUTH, 0);
- if (closeFuture.isClosed()) {
- throw new IllegalStateException("Session is closed");
- }
- authFuture = new DefaultAuthFuture(lock);
- userAuth = new UserAuthPublicKey(this, AUTHENTICATION_SERVICE, user, key);
- setState(State.UserAuth);
-
- switch (userAuth.next(null)) {
- case Success:
- authFuture.setAuthed(true);
- username = userAuth.getUsername();
- authed = true;
- setState(State.Running);
- break;
- case Failure:
- authFuture.setAuthed(false);
- userAuth = null;
- setState(State.WaitForAuth);
- break;
- case Continued:
- break;
+ if (readyForAuth()) {
+ userAuth = new UserAuthPublicKey(this, AUTHENTICATION_SERVICE, user, key);
+ processUserAuth(null);
}
return authFuture;
}
@@ -302,7 +269,7 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
@Override
public CloseFuture close(boolean immediately) {
synchronized (lock) {
- if (authFuture != null && !authFuture.isDone()) {
+ if (!authFuture.isDone()) {
authFuture.setException(new SshException("Session is closed"));
}
return super.close(immediately);
@@ -377,6 +344,7 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
disconnect(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR, "Protocol error: expected packet SSH_MSG_SERVICE_ACCEPT, got " + cmd);
return;
}
+ authFuture.setAuthed(false);
setState(State.WaitForAuth);
break;
case WaitForAuth:
@@ -397,22 +365,7 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
}
} else {
buffer.rpos(buffer.rpos() - 1);
- switch (userAuth.next(buffer)) {
- case Success:
- authFuture.setAuthed(true);
- username = userAuth.getUsername();
- authed = true;
- setState(State.Running);
- startHeartBeat();
- break;
- case Failure:
- authFuture.setAuthed(false);
- userAuth = null;
- setState(State.WaitForAuth);
- break;
- case Continued:
- break;
- }
+ processUserAuth(buffer);
}
break;
case Running:
@@ -471,10 +424,10 @@ public class ClientSessionImpl extends AbstractSession implements ClientSession
if (closeFuture.isClosed()) {
cond |= CLOSED;
}
- if (authed) {
+ if (authed) { // authFuture.isSuccess()
cond |= AUTHED;
}
- if (getState() == State.WaitForAuth) {
+ if (authFuture.isFailure()) {
cond |= WAIT_AUTH;
}
if ((cond & mask) != 0) {
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/2382d606/sshd-core/src/test/java/org/apache/sshd/ServerTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/ServerTest.java b/sshd-core/src/test/java/org/apache/sshd/ServerTest.java
index ebdb569..9b7253f 100644
--- a/sshd-core/src/test/java/org/apache/sshd/ServerTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/ServerTest.java
@@ -24,6 +24,7 @@ import java.util.concurrent.TimeoutException;
import org.apache.mina.core.session.IoSession;
import org.apache.sshd.client.SessionFactory;
+import org.apache.sshd.client.future.AuthFuture;
import org.apache.sshd.client.session.ClientSessionImpl;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.SshConstants;
@@ -35,10 +36,13 @@ import org.apache.sshd.util.BogusPasswordAuthenticator;
import org.apache.sshd.util.EchoShellFactory;
import org.apache.sshd.util.Utils;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
/**
* TODO Add javadoc
*
@@ -79,7 +83,7 @@ public class ServerTest {
* @throws Exception
*/
@Test
- public void testFailAuthentication() throws Exception {
+ public void testFailAuthenticationWithWaitFor() throws Exception {
sshd.getProperties().put(SshServer.MAX_AUTH_REQUESTS, "10");
client = SshClient.setUpDefaultClient();
@@ -95,7 +99,29 @@ public class ServerTest {
throw new TimeoutException();
}
}
- Assert.assertTrue(nbTrials > 10);
+ assertTrue(nbTrials > 10);
+ }
+
+ @Test
+ public void testFailAuthenticationWithFuture() throws Exception {
+ sshd.getProperties().put(SshServer.MAX_AUTH_REQUESTS, "10");
+
+ client = SshClient.setUpDefaultClient();
+ client.start();
+ ClientSession s = client.connect("localhost", port).await().getSession();
+ int nbTrials = 0;
+ AuthFuture authFuture;
+ do {
+ nbTrials++;
+ assertTrue(nbTrials < 100);
+ authFuture = s.authPassword("smx", "buggy");
+ assertTrue(authFuture.await(5000));
+ assertTrue(authFuture.isDone());
+ assertFalse(authFuture.isSuccess());
+ }
+ while (authFuture.isFailure());
+ assertNotNull(authFuture.getException());
+ assertTrue(nbTrials > 10);
}
@Test
@@ -106,7 +132,7 @@ public class ServerTest {
client.start();
ClientSession s = client.connect("localhost", port).await().getSession();
int res = s.waitFor(ClientSession.CLOSED, 5000);
- Assert.assertTrue((res & ClientSession.CLOSED) != 0);
+ assertTrue((res & ClientSession.CLOSED) != 0);
}
@Test