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 579695bb757c067b1f59335219dc7201c7f2006e Author: Lyor Goldstein <lgoldst...@apache.org> AuthorDate: Thu Dec 31 22:10:45 2020 +0200 [SSHD-1114] Added callbacks for client-side password authentication progress --- CHANGES.md | 1 + docs/event-listeners.md | 6 ++ .../sshd/client/ClientAuthenticationManager.java | 5 ++ .../java/org/apache/sshd/client/SshClient.java | 14 ++++- .../java/org/apache/sshd/client/auth/UserAuth.java | 34 ++++++++++- .../password/PasswordAuthenticationReporter.java | 70 ++++++++++++++++++++++ .../client/auth/password/UserAuthPassword.java | 29 ++++++++- .../sshd/client/session/AbstractClientSession.java | 14 +++++ .../sshd/client/session/ClientUserAuthService.java | 15 ++++- .../client/ClientAuthenticationManagerTest.java | 14 ++++- .../sshd/common/auth/AuthenticationTest.java | 52 +++++++++++++++- 11 files changed, 244 insertions(+), 10 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index da9bb6c..75ae5f7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,3 +26,4 @@ * [SSHD-1085](https://issues.apache.org/jira/browse/SSHD-1085) Added more notifications related to channel state change for detecting channel closing or closed earlier. * [SSHD-1109](https://issues.apache.org/jira/browse/SSHD-1109) Replace log4j with logback as the slf4j logger implementation for tests +* [SSHD-1114](https://issues.apache.org/jira/browse/SSHD-1114) Added callbacks for client-side password authentication progress diff --git a/docs/event-listeners.md b/docs/event-listeners.md index fb31d55..3f9dcf6 100644 --- a/docs/event-listeners.md +++ b/docs/event-listeners.md @@ -193,3 +193,9 @@ the handler decided not to intervene. Informs about signal requests as described in [RFC 4254 - section 6.9](https://tools.ietf.org/html/rfc4254#section-6.9), "break" requests (sent as SIGINT) as described in [RFC 4335](https://tools.ietf.org/html/rfc4335) and "window-change" (sent as SIGWINCH) requests as described in [RFC 4254 - section 6.7](https://tools.ietf.org/html/rfc4254#section-6.7) + +### `PasswordAuthenticationReporter` + +Used to inform about the progress of the client-side password based authentication as described in [RFC-4252 section 8](https://tools.ietf.org/html/rfc4252#section-8). +Can be registered globally on the `SshClient` and also for a specific `ClientSession` after it is established but before its `auth()` method is called - thus +overriding any globally registered instance. diff --git a/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java b/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java index b42eae5..14bca04 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/ClientAuthenticationManager.java @@ -28,6 +28,7 @@ import org.apache.sshd.client.auth.BuiltinUserAuthFactories; import org.apache.sshd.client.auth.UserAuth; import org.apache.sshd.client.auth.UserAuthFactory; import org.apache.sshd.client.auth.keyboard.UserInteraction; +import org.apache.sshd.client.auth.password.PasswordAuthenticationReporter; import org.apache.sshd.client.auth.password.PasswordIdentityProvider; import org.apache.sshd.client.keyverifier.ServerKeyVerifier; import org.apache.sshd.client.session.ClientSession; @@ -106,6 +107,10 @@ public interface ClientAuthenticationManager void setUserInteraction(UserInteraction userInteraction); + PasswordAuthenticationReporter getPasswordAuthenticationReporter(); + + void setPasswordAuthenticationReporter(PasswordAuthenticationReporter reporter); + @Override default void setUserAuthFactoriesNames(Collection<String> names) { BuiltinUserAuthFactories.ParseResult result = BuiltinUserAuthFactories.parseFactoriesList(names); diff --git a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java b/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java index d5873eb..4182bf8 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java @@ -46,6 +46,7 @@ import org.apache.sshd.client.auth.AuthenticationIdentitiesProvider; import org.apache.sshd.client.auth.UserAuthFactory; import org.apache.sshd.client.auth.keyboard.UserAuthKeyboardInteractiveFactory; import org.apache.sshd.client.auth.keyboard.UserInteraction; +import org.apache.sshd.client.auth.password.PasswordAuthenticationReporter; import org.apache.sshd.client.auth.password.PasswordIdentityProvider; import org.apache.sshd.client.auth.password.UserAuthPasswordFactory; import org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory; @@ -170,7 +171,6 @@ public class SshClient extends AbstractFactoryManager implements ClientFactoryMa protected IoConnector connector; protected SessionFactory sessionFactory; - protected UserInteraction userInteraction; protected List<UserAuthFactory> userAuthFactories; private ClientProxyConnector proxyConnector; @@ -180,6 +180,8 @@ public class SshClient extends AbstractFactoryManager implements ClientFactoryMa private KeyIdentityProvider keyIdentityProvider; private FilePasswordProvider filePasswordProvider; private PasswordIdentityProvider passwordIdentityProvider; + private UserInteraction userInteraction; + private PasswordAuthenticationReporter passwordAuthenticationReporter; private final List<Object> identities = new CopyOnWriteArrayList<>(); private final AuthenticationIdentitiesProvider identitiesProvider; @@ -258,6 +260,16 @@ public class SshClient extends AbstractFactoryManager implements ClientFactoryMa } @Override + public PasswordAuthenticationReporter getPasswordAuthenticationReporter() { + return passwordAuthenticationReporter; + } + + @Override + public void setPasswordAuthenticationReporter(PasswordAuthenticationReporter reporter) { + this.passwordAuthenticationReporter = reporter; + } + + @Override public List<UserAuthFactory> getUserAuthFactories() { return userAuthFactories; } 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 4ac1bf9..1822584 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 @@ -18,6 +18,8 @@ */ package org.apache.sshd.client.auth; +import java.util.List; + import org.apache.sshd.client.session.ClientSession; import org.apache.sshd.client.session.ClientSessionHolder; import org.apache.sshd.common.auth.UserAuthInstance; @@ -25,7 +27,7 @@ import org.apache.sshd.common.util.buffer.Buffer; /** * Represents a user authentication mechanism - * + * * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> */ public interface UserAuth extends ClientSessionHolder, UserAuthInstance<ClientSession> { @@ -46,6 +48,36 @@ public interface UserAuth extends ClientSessionHolder, UserAuthInstance<ClientSe boolean process(Buffer buffer) throws Exception; /** + * Signal reception of {@code SSH_MSG_USERAUTH_SUCCESS} message + * + * @param session The {@link ClientSession} + * @param service The requesting service name + * @param buffer The {@link Buffer} containing the success message (after having consumed the relevant data from + * it) + * @throws Exception If failed to handle the callback - <B>Note:</B> may cause session close + */ + default void signalAuthMethodSuccess(ClientSession session, String service, Buffer buffer) throws Exception { + // ignored + } + + /** + * Signals reception of {@code SSH_MSG_USERAUTH_FAILURE} message + * + * @param session The {@link ClientSession} + * @param service The requesting service name + * @param partial {@code true} if some partial authentication success so far + * @param serverMethods The {@link List} of authentication methods that can continue + * @param buffer The {@link Buffer} containing the failure message (after having consumed the relevant data + * from it) + * @throws Exception If failed to handle the callback - <B>Note:</B> may cause session close + */ + default void signalAuthMethodFailure( + ClientSession session, String service, boolean partial, List<String> serverMethods, Buffer buffer) + throws Exception { + // ignored + } + + /** * Called to release any allocated resources */ void destroy(); diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/password/PasswordAuthenticationReporter.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/password/PasswordAuthenticationReporter.java new file mode 100644 index 0000000..3ebdb71 --- /dev/null +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/password/PasswordAuthenticationReporter.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.sshd.client.auth.password; + +import java.util.List; + +import org.apache.sshd.client.session.ClientSession; + +/** + * Used to inform the about the progress of a password authentication + * + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + * @see <a href="https://tools.ietf.org/html/rfc4252#section-8">RFC-4252 section 8</a> + */ +public interface PasswordAuthenticationReporter { + /** + * @param session The {@link ClientSession} + * @param service The requesting service name + * @param oldPassword The password being attempted + * @param modified {@code true} if this is an attempt due to {@code SSH_MSG_USERAUTH_PASSWD_CHANGEREQ} + * @param newPassword The changed password + * @throws Exception If failed to handle the callback - <B>Note:</B> may cause session close + */ + default void signalAuthenticationAttempt( + ClientSession session, String service, String oldPassword, boolean modified, String newPassword) + throws Exception { + // ignored + } + + /** + * @param session The {@link ClientSession} + * @param service The requesting service name + * @param password The password that was attempted + * @throws Exception If failed to handle the callback - <B>Note:</B> may cause session close + */ + default void signalAuthenticationSuccess(ClientSession session, String service, String password) throws Exception { + // ignored + } + + /** + * @param session The {@link ClientSession} + * @param service The requesting service name + * @param password The password that was attempted + * @param partial {@code true} if some partial authentication success so far + * @param serverMethods The {@link List} of authentication methods that can continue + * @throws Exception If failed to handle the callback - <B>Note:</B> may cause session close + */ + default void signalAuthenticationFailure( + ClientSession session, String service, String password, boolean partial, List<String> serverMethods) + throws Exception { + // ignored + } +} diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/password/UserAuthPassword.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/password/UserAuthPassword.java index 6fcad9b..e99dfe8 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/password/UserAuthPassword.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/password/UserAuthPassword.java @@ -18,8 +18,8 @@ */ package org.apache.sshd.client.auth.password; -import java.io.IOException; import java.util.Iterator; +import java.util.List; import java.util.Objects; import org.apache.sshd.client.auth.AbstractUserAuth; @@ -157,11 +157,11 @@ public class UserAuthPassword extends AbstractUserAuth { * @param newPassword The new password * @return An {@link IoWriteFuture} that can be used to wait and check on the success/failure of the * request packet being sent - * @throws IOException If failed to send the message. + * @throws Exception If failed to send the message. */ protected IoWriteFuture sendPassword( Buffer buffer, ClientSession session, String oldPassword, String newPassword) - throws IOException { + throws Exception { String username = session.getUsername(); String service = getService(); String name = getName(); @@ -187,6 +187,29 @@ public class UserAuthPassword extends AbstractUserAuth { buffer.putString(newPassword); } + PasswordAuthenticationReporter reporter = session.getPasswordAuthenticationReporter(); + if (reporter != null) { + reporter.signalAuthenticationAttempt(session, service, oldPassword, modified, newPassword); + } + return session.writePacket(buffer); } + + @Override + public void signalAuthMethodSuccess(ClientSession session, String service, Buffer buffer) throws Exception { + PasswordAuthenticationReporter reporter = session.getPasswordAuthenticationReporter(); + if (reporter != null) { + reporter.signalAuthenticationSuccess(session, service, current); + } + } + + @Override + public void signalAuthMethodFailure( + ClientSession session, String service, boolean partial, List<String> serverMethods, Buffer buffer) + throws Exception { + PasswordAuthenticationReporter reporter = session.getPasswordAuthenticationReporter(); + if (reporter != null) { + reporter.signalAuthenticationFailure(session, service, current, partial, serverMethods); + } + } } diff --git a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java index c7c8d5e..4c5eff4 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/session/AbstractClientSession.java @@ -33,6 +33,7 @@ import org.apache.sshd.client.ClientFactoryManager; import org.apache.sshd.client.auth.AuthenticationIdentitiesProvider; import org.apache.sshd.client.auth.UserAuthFactory; import org.apache.sshd.client.auth.keyboard.UserInteraction; +import org.apache.sshd.client.auth.password.PasswordAuthenticationReporter; import org.apache.sshd.client.auth.password.PasswordIdentityProvider; import org.apache.sshd.client.channel.ChannelDirectTcpip; import org.apache.sshd.client.channel.ChannelExec; @@ -90,6 +91,7 @@ public abstract class AbstractClientSession extends AbstractSession implements C private ServerKeyVerifier serverKeyVerifier; private UserInteraction userInteraction; private PasswordIdentityProvider passwordIdentityProvider; + private PasswordAuthenticationReporter passwordAuthenticationReporter; private KeyIdentityProvider keyIdentityProvider; private List<UserAuthFactory> userAuthFactories; private SocketAddress connectAddress; @@ -161,6 +163,18 @@ public abstract class AbstractClientSession extends AbstractSession implements C } @Override + public PasswordAuthenticationReporter getPasswordAuthenticationReporter() { + ClientFactoryManager manager = getFactoryManager(); + return resolveEffectiveProvider(PasswordAuthenticationReporter.class, passwordAuthenticationReporter, + manager.getPasswordAuthenticationReporter()); + } + + @Override + public void setPasswordAuthenticationReporter(PasswordAuthenticationReporter reporter) { + this.passwordAuthenticationReporter = reporter; + } + + @Override public List<UserAuthFactory> getUserAuthFactories() { ClientFactoryManager manager = getFactoryManager(); return resolveEffectiveFactories(userAuthFactories, manager.getUserAuthFactories()); 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 ab2b55c..fb430b1 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 @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -239,9 +240,14 @@ public class ClientUserAuthService extends AbstractCloseable implements Service, log.debug("processUserAuth({}) SSH_MSG_USERAUTH_SUCCESS Succeeded with {}", session, (userAuth == null) ? "<unknown>" : userAuth.getName()); } + if (userAuth != null) { try { - userAuth.destroy(); + try { + userAuth.signalAuthMethodSuccess(session, service, buffer); + } finally { + userAuth.destroy(); + } } finally { userAuth = null; } @@ -267,7 +273,12 @@ public class ClientUserAuthService extends AbstractCloseable implements Service, currentMethod = 0; if (userAuth != null) { try { - userAuth.destroy(); + try { + userAuth.signalAuthMethodFailure( + session, service, partial, Collections.unmodifiableList(serverMethods), buffer); + } finally { + userAuth.destroy(); + } } finally { userAuth = null; } diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientAuthenticationManagerTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientAuthenticationManagerTest.java index 86adf44..40df790 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/ClientAuthenticationManagerTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientAuthenticationManagerTest.java @@ -31,6 +31,7 @@ import org.apache.sshd.client.auth.AuthenticationIdentitiesProvider; import org.apache.sshd.client.auth.BuiltinUserAuthFactories; import org.apache.sshd.client.auth.UserAuthFactory; import org.apache.sshd.client.auth.keyboard.UserInteraction; +import org.apache.sshd.client.auth.password.PasswordAuthenticationReporter; import org.apache.sshd.client.auth.password.PasswordIdentityProvider; import org.apache.sshd.client.keyverifier.ServerKeyVerifier; import org.apache.sshd.client.session.ClientSession; @@ -100,6 +101,16 @@ public class ClientAuthenticationManagerTest extends BaseTestSupport { } @Override + public PasswordAuthenticationReporter getPasswordAuthenticationReporter() { + return null; + } + + @Override + public void setPasswordAuthenticationReporter(PasswordAuthenticationReporter reporter) { + throw new UnsupportedOperationException("setPasswordAuthenticationReporter(" + reporter + ")"); + } + + @Override public ServerKeyVerifier getServerKeyVerifier() { return null; } @@ -183,7 +194,8 @@ public class ClientAuthenticationManagerTest extends BaseTestSupport { PasswordIdentityProvider.class, ServerKeyVerifier.class, UserInteraction.class, - KeyIdentityProvider.class + KeyIdentityProvider.class, + PasswordAuthenticationReporter.class }) { testClientProvidersPropagation(provider, client, session); } diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java index 0e748d8..2dbe763 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java @@ -25,6 +25,7 @@ import java.security.GeneralSecurityException; import java.security.KeyPair; import java.security.PublicKey; import java.security.spec.InvalidKeySpecException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -39,6 +40,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.apache.sshd.client.SshClient; import org.apache.sshd.client.auth.hostbased.HostKeyIdentityProvider; import org.apache.sshd.client.auth.keyboard.UserInteraction; +import org.apache.sshd.client.auth.password.PasswordAuthenticationReporter; import org.apache.sshd.client.auth.password.PasswordIdentityProvider; import org.apache.sshd.client.future.AuthFuture; import org.apache.sshd.client.session.ClientSession; @@ -236,7 +238,7 @@ public class AuthenticationTest extends BaseTestSupport { @Override protected IoWriteFuture sendPassword( Buffer buffer, ClientSession session, String oldPassword, String newPassword) - throws IOException { + throws Exception { int count = sentCount.incrementAndGet(); // 1st one is the original one (which is denied by the server) // 2nd one is the updated one retrieved from the user interaction @@ -970,7 +972,7 @@ public class AuthenticationTest extends BaseTestSupport { session.auth().verify(AUTH_TIMEOUT); KeyExchange kex = session.getKex(); - assertNull("KEX no nullified after completion", kex); + assertNull("KEX not nullified after completion", kex); actualKey = session.getServerKey(); } finally { @@ -990,6 +992,52 @@ public class AuthenticationTest extends BaseTestSupport { fail("No matching server key found for " + actualKey); } + @Test // see SSHD-1114 + public void testPasswordAuthenticationReporter() throws Exception { + String goodPassword = getCurrentTestName(); + String badPassword = getClass().getSimpleName(); + List<String> actual = new ArrayList<>(); + PasswordAuthenticationReporter reporter = new PasswordAuthenticationReporter() { + @Override + public void signalAuthenticationAttempt( + ClientSession session, String service, String oldPassword, boolean modified, String newPassword) + throws Exception { + actual.add(oldPassword); + } + + @Override + public void signalAuthenticationSuccess(ClientSession session, String service, String password) + throws Exception { + assertEquals("Mismatched succesful password", goodPassword, password); + } + + @Override + public void signalAuthenticationFailure( + ClientSession session, String service, String password, boolean partial, List<String> serverMethods) + throws Exception { + assertEquals("Mismatched failed password", badPassword, password); + } + }; + + try (SshClient client = setupTestClient()) { + client.setUserAuthFactories( + Collections.singletonList(new org.apache.sshd.client.auth.password.UserAuthPasswordFactory())); + client.start(); + + try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port) + .verify(CONNECT_TIMEOUT).getSession()) { + session.addPasswordIdentity(badPassword); + session.addPasswordIdentity(goodPassword); + session.setPasswordAuthenticationReporter(reporter); + session.auth().verify(AUTH_TIMEOUT); + } finally { + client.stop(); + } + } + + assertListEquals("Attempted passwords", Arrays.asList(badPassword, goodPassword), actual); + } + private static void assertAuthenticationResult(String message, AuthFuture future, boolean expected) throws IOException { assertTrue(message + ": failed to get result on time", future.await(AUTH_TIMEOUT)); assertEquals(message + ": mismatched authentication result", expected, future.isSuccess());