From 3459239dfc224a5164a07f2afe3ec406a357937b Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 13 Sep 2020 21:46:46 +0200 Subject: [PATCH] sshd: better error report when user cancels authentication Use a dedicated exception class to be able to detect this case in the SshdSessionFactory and skip the generic SshException in that case. Change-Id: I2a0bacf47bae82f154a0f4e79efbb2af2a17d0cf Signed-off-by: Thomas Wolf --- .../transport/sshd/SshdText.properties | 4 +- .../sshd/AuthenticationCanceledException.java | 30 ++++++++ .../sshd/JGitPasswordAuthentication.java | 4 +- .../sshd/IdentityPasswordProvider.java | 72 ++++++++++++++++--- .../transport/sshd/SshdSessionFactory.java | 10 ++- 5 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/AuthenticationCanceledException.java diff --git a/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties b/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties index 504e6001c..f810fd40e 100644 --- a/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties +++ b/org.eclipse.jgit.ssh.apache/resources/org/eclipse/jgit/internal/transport/sshd/SshdText.properties @@ -1,4 +1,4 @@ -authenticationCanceled=Authentication canceled: no password +authenticationCanceled=SSH authentication canceled: no password given authenticationOnClosedSession=Authentication canceled: session is already closing or closed closeListenerFailed=Ssh session close listener failed configInvalidPath=Invalid path in ssh config key {0}: {1} @@ -49,7 +49,7 @@ knownHostsUnknownKeyPrompt=Accept and store this key, and continue connecting? knownHostsUnknownKeyType=Cannot read server key from known hosts file {0}; line {1} knownHostsUserAskCreationMsg=File {0} does not exist. knownHostsUserAskCreationPrompt=Create file {0} ? -loginDenied=Log-in denied at {0}:{1} +loginDenied=Cannot log in at {0}:{1} passwordPrompt=Password proxyCannotAuthenticate=Cannot authenticate to proxy {0} proxyHttpFailure=HTTP Proxy connection to {0} failed with code {1}: {2} diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/AuthenticationCanceledException.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/AuthenticationCanceledException.java new file mode 100644 index 000000000..aa4623571 --- /dev/null +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/AuthenticationCanceledException.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2020, Thomas Wolf and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.internal.transport.sshd; + +import java.util.concurrent.CancellationException; + +/** + * An exception to report that the user canceled the SSH authentication. + */ +public class AuthenticationCanceledException extends CancellationException { + + // If this is not a CancellationException sshd will try other authentication + // mechanisms. + + private static final long serialVersionUID = 1L; + + /** + * Creates a new {@link AuthenticationCanceledException}. + */ + public AuthenticationCanceledException() { + super(SshdText.get().authenticationCanceled); + } +} diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPasswordAuthentication.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPasswordAuthentication.java index 0a7082cef..4abd6e901 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPasswordAuthentication.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPasswordAuthentication.java @@ -9,8 +9,6 @@ */ package org.eclipse.jgit.internal.transport.sshd; -import java.util.concurrent.CancellationException; - import org.apache.sshd.client.ClientAuthenticationManager; import org.apache.sshd.client.auth.keyboard.UserInteraction; import org.apache.sshd.client.auth.password.UserAuthPassword; @@ -49,7 +47,7 @@ protected boolean sendAuthDataRequest(ClientSession session, String service) } String password = getPassword(session, interaction); if (password == null) { - throw new CancellationException(); + throw new AuthenticationCanceledException(); } // sendPassword takes a buffer as first argument, but actually doesn't // use it and creates its own buffer... diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/IdentityPasswordProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/IdentityPasswordProvider.java index 004d3f836..dd6894b66 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/IdentityPasswordProvider.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/IdentityPasswordProvider.java @@ -19,13 +19,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.CancellationException; import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.internal.transport.sshd.AuthenticationCanceledException; import org.eclipse.jgit.internal.transport.sshd.SshdText; import org.eclipse.jgit.transport.CredentialItem; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.util.StringUtils; /** * A {@link KeyPasswordProvider} based on a {@link CredentialsProvider}. @@ -155,34 +156,83 @@ protected char[] getPassword(URIish uri, int attempt, @NonNull State state) state.incCount(); String message = state.count == 1 ? SshdText.get().keyEncryptedMsg : SshdText.get().keyEncryptedRetry; - char[] pass = getPassword(uri, message); + char[] pass = getPassword(uri, format(message, uri)); state.setPassword(pass); return pass; } - private char[] getPassword(URIish uri, String message) { + /** + * Retrieves the JGit {@link CredentialsProvider} to use for user + * interaction. + * + * @return the {@link CredentialsProvider} or {@code null} if none + * configured + * @since 5.10 + */ + protected CredentialsProvider getCredentialsProvider() { + return provider; + } + + /** + * Obtains the passphrase/password for an encrypted private key via the + * {@link #getCredentialsProvider() configured CredentialsProvider}. + * + * @param uri + * identifying the resource to obtain a password for + * @param message + * optional message text to display; may be {@code null} or empty + * if none + * @return the password entered, or {@code null} if no + * {@link CredentialsProvider} is configured or none was entered + * @throws java.util.concurrent.CancellationException + * if the user canceled the operation + * @since 5.10 + */ + protected char[] getPassword(URIish uri, String message) { if (provider == null) { return null; } - List items = new ArrayList<>(2); - items.add(new CredentialItem.InformationalMessage( - format(message, uri))); + boolean haveMessage = !StringUtils.isEmptyOrNull(message); + List items = new ArrayList<>(haveMessage ? 2 : 1); + if (haveMessage) { + items.add(new CredentialItem.InformationalMessage(message)); + } CredentialItem.Password password = new CredentialItem.Password( SshdText.get().keyEncryptedPrompt); items.add(password); try { - provider.get(uri, items); + boolean completed = provider.get(uri, items); char[] pass = password.getValue(); - if (pass == null) { - throw new CancellationException( - SshdText.get().authenticationCanceled); + if (!completed) { + cancelAuthentication(); + return null; } - return pass.clone(); + return pass == null ? null : pass.clone(); } finally { password.clear(); } } + /** + * Cancels the authentication process. Called by + * {@link #getPassword(URIish, String)} when the user interaction has been + * canceled. If this throws a + * {@link java.util.concurrent.CancellationException}, the authentication + * process is aborted; otherwise it may continue with the next configured + * authentication mechanism, if any. + *

+ * This default implementation always throws a + * {@link java.util.concurrent.CancellationException}. + *

+ * + * @throws java.util.concurrent.CancellationException + * always + * @since 5.10 + */ + protected void cancelAuthentication() { + throw new AuthenticationCanceledException(); + } + /** * Invoked to inform the password provider about the decoding result. * diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java index 4ad3c4a4b..df0e1d28a 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java @@ -34,6 +34,7 @@ import org.apache.sshd.client.auth.keyboard.UserAuthKeyboardInteractiveFactory; import org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyFactory; import org.apache.sshd.client.config.hosts.HostConfigEntryResolver; +import org.apache.sshd.common.SshException; import org.apache.sshd.common.compression.BuiltinCompressions; import org.apache.sshd.common.config.keys.FilePasswordProvider; import org.apache.sshd.common.config.keys.loader.openssh.kdf.BCryptKdfOptions; @@ -41,6 +42,7 @@ import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.internal.transport.ssh.OpenSshConfigFile; +import org.eclipse.jgit.internal.transport.sshd.AuthenticationCanceledException; import org.eclipse.jgit.internal.transport.sshd.CachingKeyPairProvider; import org.eclipse.jgit.internal.transport.sshd.GssApiWithMicAuthFactory; import org.eclipse.jgit.internal.transport.sshd.JGitPasswordAuthFactory; @@ -233,7 +235,13 @@ public SshdSession getSession(URIish uri, if (e instanceof TransportException) { throw (TransportException) e; } - throw new TransportException(uri, e.getMessage(), e); + Throwable cause = e; + if (e instanceof SshException && e + .getCause() instanceof AuthenticationCanceledException) { + // Results in a nicer error message + cause = e.getCause(); + } + throw new TransportException(uri, cause.getMessage(), cause); } }