From 7aaeb6489f9819227fa8ebe122a849b6029242b7 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 15 Nov 2018 16:33:04 +0100 Subject: [PATCH] Apache MINA sshd client: don't leak upstream classes and interfaces We will get an API evolution problem if we expose as API classes and interfaces that derive from upstream classes or interfaces. Upstream interfaces also evolve quite erratically and evolution doesn't seem to follow semantic versioning. Introduce a new KeyPasswordProvider interface so that we don't have to depend on the upstream FilePasswordProvider in our API. (We do need _some_ abstraction for getting passwords for encrypted keys in the API; EGit will need to provide its own implementation.) Move some other upstream dependencies (HostConfigEntry, and various previously protected methods in SshdSessionFactory) out of the API: classes moved to internal space, and methods made private. The only dependencies on upstream interfaces are thus in a few method parameter types. Those cannot be avoided, but should also not pose problems. Bug: 520927 Change-Id: Idc9c6b0f237f29f46343c0fe15179242f2007bec Signed-off-by: Thomas Wolf --- .../sshd/EncryptedFileKeyPairProvider.java | 3 +- .../transport/sshd/JGitHostConfigEntry.java | 2 +- .../transport/sshd/JGitSshClient.java | 1 - .../transport/sshd/JGitSshConfig.java | 2 +- .../sshd/OpenSshServerKeyVerifier.java | 11 +- .../sshd/PasswordProviderWrapper.java | 137 ++++++++++++++++++ .../sshd/RepeatingFilePasswordProvider.java | 12 +- .../sshd/IdentityPasswordProvider.java | 94 +++++------- .../transport/sshd/KeyPasswordProvider.java | 117 +++++++++++++++ .../transport/sshd/SshdSessionFactory.java | 58 ++++++-- 10 files changed, 343 insertions(+), 94 deletions(-) rename org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/{ => internal}/transport/sshd/JGitHostConfigEntry.java (98%) rename org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/{ => internal}/transport/sshd/JGitSshConfig.java (99%) create mode 100644 org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java rename org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/{ => internal}/transport/sshd/RepeatingFilePasswordProvider.java (93%) create mode 100644 org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/KeyPasswordProvider.java diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java index 2e201d882..ff8198999 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/EncryptedFileKeyPairProvider.java @@ -63,8 +63,7 @@ import org.apache.sshd.common.config.keys.loader.KeyPairResourceParser; import org.apache.sshd.common.keyprovider.FileKeyPairProvider; import org.apache.sshd.common.util.io.IoUtils; import org.apache.sshd.common.util.security.SecurityUtils; -import org.eclipse.jgit.transport.sshd.RepeatingFilePasswordProvider; -import org.eclipse.jgit.transport.sshd.RepeatingFilePasswordProvider.ResourceDecodeResult; +import org.eclipse.jgit.internal.transport.sshd.RepeatingFilePasswordProvider.ResourceDecodeResult; /** * A {@link FileKeyPairProvider} that asks repeatedly for a passphrase for an diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/JGitHostConfigEntry.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitHostConfigEntry.java similarity index 98% rename from org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/JGitHostConfigEntry.java rename to org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitHostConfigEntry.java index 6bffa2e3e..8e97dad4a 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/JGitHostConfigEntry.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitHostConfigEntry.java @@ -40,7 +40,7 @@ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -package org.eclipse.jgit.transport.sshd; +package org.eclipse.jgit.internal.transport.sshd; import java.util.Collections; import java.util.List; diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java index d3289259e..915b696b9 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java @@ -76,7 +76,6 @@ import org.apache.sshd.common.util.ValidateUtils; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.SshConstants; import org.eclipse.jgit.transport.sshd.KeyCache; -import org.eclipse.jgit.transport.sshd.RepeatingFilePasswordProvider; /** * Customized {@link SshClient} for JGit. It creates specialized diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/JGitSshConfig.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshConfig.java similarity index 99% rename from org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/JGitSshConfig.java rename to org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshConfig.java index 963837425..8ca9d2103 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/JGitSshConfig.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshConfig.java @@ -40,7 +40,7 @@ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -package org.eclipse.jgit.transport.sshd; +package org.eclipse.jgit.internal.transport.sshd; import static org.eclipse.jgit.internal.transport.ssh.OpenSshConfigFile.flag; import static org.eclipse.jgit.internal.transport.ssh.OpenSshConfigFile.positive; diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/OpenSshServerKeyVerifier.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/OpenSshServerKeyVerifier.java index e511be01d..540b586dd 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/OpenSshServerKeyVerifier.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/OpenSshServerKeyVerifier.java @@ -86,7 +86,6 @@ import org.eclipse.jgit.transport.CredentialItem; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.SshConstants; import org.eclipse.jgit.transport.URIish; -import org.eclipse.jgit.transport.sshd.JGitHostConfigEntry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -181,12 +180,12 @@ public class OpenSshServerKeyVerifier * empty or {@code null}, in which case no default files are * installed. The files need not exist. */ - public OpenSshServerKeyVerifier(boolean askAboutNewFile, List defaultFiles) { + public OpenSshServerKeyVerifier(boolean askAboutNewFile, + List defaultFiles) { if (defaultFiles != null) { - for (File file : defaultFiles) { - Path p = file.toPath(); - HostKeyFile newFile = new HostKeyFile(p); - knownHostsFiles.put(p, newFile); + for (Path file : defaultFiles) { + HostKeyFile newFile = new HostKeyFile(file); + knownHostsFiles.put(file, newFile); this.defaultFiles.add(newFile); } } diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java new file mode 100644 index 000000000..93bd10285 --- /dev/null +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java @@ -0,0 +1,137 @@ +/* + * Copyright (C) 2018, Thomas Wolf + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.internal.transport.sshd; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.security.GeneralSecurityException; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.transport.sshd.KeyPasswordProvider; + +/** + * A bridge from sshd's {@link RepeatingFilePasswordProvider} to our + * {@link KeyPasswordProvider} API. + */ +public class PasswordProviderWrapper implements RepeatingFilePasswordProvider { + + private final KeyPasswordProvider delegate; + + private Map counts = new ConcurrentHashMap<>(); + + /** + * @param delegate + */ + public PasswordProviderWrapper(@NonNull KeyPasswordProvider delegate) { + this.delegate = delegate; + } + + @Override + public void setAttempts(int numberOfPasswordPrompts) { + delegate.setAttempts(numberOfPasswordPrompts); + } + + @Override + public int getAttempts() { + return delegate.getAttempts(); + } + + @Override + public String getPassword(String resourceKey) throws IOException { + int attempt = counts + .computeIfAbsent(resourceKey, k -> new AtomicInteger()).get(); + char[] passphrase = delegate.getPassphrase(toUri(resourceKey), attempt); + if (passphrase == null) { + return null; + } + try { + return new String(passphrase); + } finally { + Arrays.fill(passphrase, '\000'); + } + } + + @Override + public ResourceDecodeResult handleDecodeAttemptResult(String resourceKey, + String password, Exception err) + throws IOException, GeneralSecurityException { + AtomicInteger count = counts.get(resourceKey); + int numberOfAttempts = count == null ? 0 : count.incrementAndGet(); + ResourceDecodeResult result = null; + try { + if (delegate.keyLoaded(toUri(resourceKey), numberOfAttempts, err)) { + result = ResourceDecodeResult.RETRY; + } else { + result = ResourceDecodeResult.TERMINATE; + } + } finally { + if (result != ResourceDecodeResult.RETRY) { + counts.remove(resourceKey); + } + } + return result; + } + + /** + * Creates a {@link URIish} from a given string. The + * {@link CredentialsProvider} uses uris as resource identifications. + * + * @param resourceKey + * to convert + * @return the uri + */ + private URIish toUri(String resourceKey) { + try { + return new URIish(resourceKey); + } catch (URISyntaxException e) { + return new URIish().setPath(resourceKey); // Doesn't check!! + } + } + +} diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/RepeatingFilePasswordProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java similarity index 93% rename from org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/RepeatingFilePasswordProvider.java rename to org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java index da8b76844..5d58bd6d7 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/RepeatingFilePasswordProvider.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java @@ -40,7 +40,7 @@ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -package org.eclipse.jgit.transport.sshd; +package org.eclipse.jgit.internal.transport.sshd; import java.io.IOException; import java.security.GeneralSecurityException; @@ -60,14 +60,10 @@ public interface RepeatingFilePasswordProvider extends FilePasswordProvider { * attempted for one identity resource through this provider. * * @param numberOfPasswordPrompts - * number of times to ask for a password, >= 1. + * number of times to ask for a password; + * {@link IllegalArgumentException} may be thrown if <= 0 */ - default void setAttempts(int numberOfPasswordPrompts) { - if (numberOfPasswordPrompts <= 0) { - throw new IllegalArgumentException( - "Number of password prompts must be >= 1"); //$NON-NLS-1$ - } - } + void setAttempts(int numberOfPasswordPrompts); /** * Gets the maximum number of attempts to get a password that should be 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 231d3f4eb..2a5f2ff24 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 @@ -45,7 +45,6 @@ package org.eclipse.jgit.transport.sshd; import static java.text.MessageFormat.format; import java.io.IOException; -import java.net.URISyntaxException; import java.security.GeneralSecurityException; import java.security.InvalidKeyException; import java.util.ArrayList; @@ -62,12 +61,11 @@ import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.URIish; /** - * A {@link RepeatingFilePasswordProvider} based on a - * {@link CredentialsProvider}. + * A {@link KeyPasswordProvider} based on a {@link CredentialsProvider}. * * @since 5.2 */ -public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { +public class IdentityPasswordProvider implements KeyPasswordProvider { private CredentialsProvider provider; @@ -136,7 +134,7 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { /** * Counts per resource key. */ - private final Map current = new HashMap<>(); + private final Map current = new HashMap<>(); /** * Creates a new {@link IdentityPasswordProvider} to get the passphrase for @@ -151,8 +149,10 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { @Override public void setAttempts(int numberOfPasswordPrompts) { - RepeatingFilePasswordProvider.super.setAttempts( - numberOfPasswordPrompts); + if (numberOfPasswordPrompts <= 0) { + throw new IllegalArgumentException( + "Number of password prompts must be >= 1"); //$NON-NLS-1$ + } attempts = numberOfPasswordPrompts; } @@ -162,24 +162,18 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { } @Override - public String getPassword(String resourceKey) throws IOException { - char[] pass = getPassword(resourceKey, - current.computeIfAbsent(resourceKey, r -> new State())); - if (pass == null) { - return null; - } - try { - return new String(pass); - } finally { - Arrays.fill(pass, '\000'); - } + public char[] getPassphrase(URIish uri, int attempt) throws IOException { + return getPassword(uri, attempt, + current.computeIfAbsent(uri, r -> new State())); } /** * Retrieves a password to decrypt a private key. * - * @param resourceKey + * @param uri * identifying the resource to obtain a password for + * @param attempt + * number of previous attempts to get a passphrase * @param state * encapsulating state information about attempts to get the * password @@ -188,46 +182,29 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { * @throws IOException * if an error occurs */ - protected char[] getPassword(String resourceKey, @NonNull State state) + protected char[] getPassword(URIish uri, int attempt, @NonNull State state) throws IOException { state.setPassword(null); state.incCount(); String message = state.count == 1 ? SshdText.get().keyEncryptedMsg : SshdText.get().keyEncryptedRetry; - char[] pass = getPassword(resourceKey, message); + char[] pass = getPassword(uri, message); state.setPassword(pass); return pass; } - /** - * Creates a {@link URIish} from a given string. The - * {@link CredentialsProvider} uses uris as resource identifications. - * - * @param resourceKey - * to convert - * @return the uri - */ - protected URIish toUri(String resourceKey) { - try { - return new URIish(resourceKey); - } catch (URISyntaxException e) { - return new URIish().setPath(resourceKey); // Doesn't check!! - } - } - - private char[] getPassword(String resourceKey, String message) { + private char[] getPassword(URIish uri, String message) { if (provider == null) { return null; } - URIish file = toUri(resourceKey); List items = new ArrayList<>(2); items.add(new CredentialItem.InformationalMessage( - format(message, resourceKey))); + format(message, uri))); CredentialItem.Password password = new CredentialItem.Password( SshdText.get().keyEncryptedPrompt); items.add(password); try { - provider.get(file, items); + provider.get(uri, items); char[] pass = password.getValue(); if (pass == null) { throw new CancellationException( @@ -242,8 +219,9 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { /** * Invoked to inform the password provider about the decoding result. * - * @param resourceKey - * the resource key + * @param uri + * identifying the key resource the key was attempted to be + * loaded from * @param state * associated with this key * @param password @@ -253,18 +231,15 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { * @return how to proceed in case of error * @throws IOException * @throws GeneralSecurityException - * @see #handleDecodeAttemptResult(String, String, Exception) */ - protected ResourceDecodeResult handleDecodeAttemptResult(String resourceKey, + protected boolean keyLoaded(URIish uri, State state, char[] password, Exception err) throws IOException, GeneralSecurityException { if (err == null) { - return null; + return false; // Success, don't retry } else if (err instanceof GeneralSecurityException) { throw new InvalidKeyException( - format(SshdText.get().identityFileCannotDecrypt, - resourceKey), - err); + format(SshdText.get().identityFileCannotDecrypt, uri), err); } else { // Unencrypted key (state == null && password == null), or exception // before having asked for the password (state != null && password @@ -272,30 +247,29 @@ public class IdentityPasswordProvider implements RepeatingFilePasswordProvider { // attempts exhausted. if (state == null || password == null || state.getCount() >= attempts) { - return ResourceDecodeResult.TERMINATE; + return false; } - return ResourceDecodeResult.RETRY; + return true; } } @Override - public ResourceDecodeResult handleDecodeAttemptResult(String resourceKey, - String password, Exception err) + public boolean keyLoaded(URIish uri, int attempt, Exception error) throws IOException, GeneralSecurityException { - ResourceDecodeResult result = null; State state = null; + boolean retry = false; try { - state = current.get(resourceKey); - result = handleDecodeAttemptResult(resourceKey, state, - state == null ? null : state.getPassword(), err); + state = current.get(uri); + retry = keyLoaded(uri, state, + state == null ? null : state.getPassword(), error); } finally { if (state != null) { state.setPassword(null); } - if (result != ResourceDecodeResult.RETRY) { - current.remove(resourceKey); + if (!retry) { + current.remove(uri); } } - return result; + return retry; } } diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/KeyPasswordProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/KeyPasswordProvider.java new file mode 100644 index 000000000..0f315a454 --- /dev/null +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/KeyPasswordProvider.java @@ -0,0 +1,117 @@ +/* + * Copyright (C) 2018, Thomas Wolf + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.transport.sshd; + +import java.io.IOException; +import java.security.GeneralSecurityException; + +import org.eclipse.jgit.transport.URIish; + +/** + * A {@code KeyPasswordProvider} provides passwords for encrypted private keys. + * + * @since 5.2 + */ +public interface KeyPasswordProvider { + + /** + * Obtains a passphrase to use to decrypt an ecrypted private key. Returning + * {@code null} or an empty array will skip this key. To cancel completely, + * the operation should raise + * {@link java.util.concurrent.CancellationException}. + * + * @param uri + * identifying the key resource that is being attempted to be + * loaded + * @param attempt + * the number of previous attempts to get a passphrase; >= 0 + * @return the passphrase + * @throws IOException + * if no password can be obtained + */ + char[] getPassphrase(URIish uri, int attempt) throws IOException; + + /** + * Define the maximum number of attempts to get a passphrase that should be + * attempted for one identity resource through this provider. + * + * @param maxNumberOfAttempts + * number of times to ask for a passphrase; + * {@link IllegalArgumentException} may be thrown if <= 0 + */ + void setAttempts(int maxNumberOfAttempts); + + /** + * Gets the maximum number of attempts to get a passphrase that should be + * attempted for one identity resource through this provider. The default + * return 1. + * + * @return the number of times to ask for a passphrase; should be >= 1. + */ + default int getAttempts() { + return 1; + } + + /** + * Invoked after a key has been loaded. If this raises an exception, the + * original {@code error} is lost unless it is attached to that exception. + * + * @param uri + * identifying the key resource the key was attempted to be + * loaded from + * @param attempt + * the number of times {@link #getPassphrase(URIish, int)} had + * been called; zero indicates that {@code uri} refers to a + * non-encrypted key + * @param error + * {@code null} if the key was loaded successfully; otherwise an + * exception indicating why the key could not be loaded + * @return {@code true} to re-try again; {@code false} to re-raise the + * {@code error} exception; Ignored if the key was loaded + * successfully, i.e., if {@code error == null}. + * @throws IOException + * @throws GeneralSecurityException + */ + boolean keyLoaded(URIish uri, int attempt, Exception error) + throws IOException, GeneralSecurityException; +} 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 08d08090e..302ba09cc 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 @@ -77,8 +77,10 @@ import org.eclipse.jgit.internal.transport.sshd.CachingKeyPairProvider; import org.eclipse.jgit.internal.transport.sshd.GssApiWithMicAuthFactory; import org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthFactory; import org.eclipse.jgit.internal.transport.sshd.JGitSshClient; +import org.eclipse.jgit.internal.transport.sshd.JGitSshConfig; import org.eclipse.jgit.internal.transport.sshd.JGitUserInteraction; import org.eclipse.jgit.internal.transport.sshd.OpenSshServerKeyVerifier; +import org.eclipse.jgit.internal.transport.sshd.PasswordProviderWrapper; import org.eclipse.jgit.internal.transport.sshd.SshdText; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.SshConstants; @@ -127,8 +129,8 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { * {@link KeyCache} is still the right choice, for instance to avoid that a * user gets prompted several times for the same password for the same key. * In general, however, it is preferable not to use a key cache but - * to use a {@link #createFilePasswordProvider(CredentialsProvider) - * FilePasswordProvider} that has access to some secure storage and can save + * to use a {@link #createKeyPasswordProvider(CredentialsProvider) + * KeyPasswordProvider} that has access to some secure storage and can save * and retrieve passwords from there without user interaction. Another * approach is to use an ssh agent. *

@@ -201,10 +203,12 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { home, sshDir); KeyPairProvider defaultKeysProvider = getDefaultKeysProvider( sshDir); + KeyPasswordProvider passphrases = createKeyPasswordProvider( + credentialsProvider); SshClient client = ClientBuilder.builder() .factory(JGitSshClient::new) .filePasswordProvider( - createFilePasswordProvider(credentialsProvider)) + createFilePasswordProvider(passphrases)) .hostConfigEntryResolver(configFile) .serverKeyVerifier(getServerKeyVerifier(home, sshDir)) .compressionFactories( @@ -335,7 +339,7 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { * @return the resolver */ @NonNull - protected HostConfigEntryResolver getHostConfigEntryResolver( + private HostConfigEntryResolver getHostConfigEntryResolver( @NonNull File homeDir, @NonNull File sshDir) { return defaultHostConfigEntryResolver.computeIfAbsent( new Tuple(homeDir, sshDir), @@ -359,15 +363,26 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { * @return the resolver */ @NonNull - protected ServerKeyVerifier getServerKeyVerifier(@NonNull File homeDir, + private ServerKeyVerifier getServerKeyVerifier(@NonNull File homeDir, @NonNull File sshDir) { return defaultServerKeyVerifier.computeIfAbsent( new Tuple(homeDir, sshDir), t -> new OpenSshServerKeyVerifier(true, - Arrays.asList( - new File(sshDir, SshConstants.KNOWN_HOSTS), - new File(sshDir, - SshConstants.KNOWN_HOSTS + '2')))); + getDefaultKnownHostsFiles(sshDir))); + } + + /** + * Gets the list of default user known hosts files. The default returns + * ~/.ssh/known_hosts and ~/.ssh/known_hosts2. The ssh config + * {@code UserKnownHostsFile} overrides this default. + * + * @param sshDir + * @return the possibly empty list of default known host file paths. + */ + @NonNull + protected List getDefaultKnownHostsFiles(@NonNull File sshDir) { + return Arrays.asList(sshDir.toPath().resolve(SshConstants.KNOWN_HOSTS), + sshDir.toPath().resolve(SshConstants.KNOWN_HOSTS + '2')); } /** @@ -378,7 +393,7 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { * @return the {@link KeyPairProvider} */ @NonNull - protected KeyPairProvider getDefaultKeysProvider(@NonNull File sshDir) { + private KeyPairProvider getDefaultKeysProvider(@NonNull File sshDir) { return defaultKeys.computeIfAbsent(new Tuple(sshDir), t -> new CachingKeyPairProvider(getDefaultIdentities(sshDir), getKeyCache())); @@ -413,19 +428,32 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { } /** - * Creates a {@link FilePasswordProvider} for a new session. + * Creates a {@link KeyPasswordProvider} for a new session. * * @param provider - * the {@link CredentialsProvider} to delegate for for user + * the {@link CredentialsProvider} to delegate to for user * interactions - * @return a new {@link FilePasswordProvider} + * @return a new {@link KeyPasswordProvider} */ @NonNull - protected FilePasswordProvider createFilePasswordProvider( + protected KeyPasswordProvider createKeyPasswordProvider( CredentialsProvider provider) { return new IdentityPasswordProvider(provider); } + /** + * Creates a {@link FilePasswordProvider} for a new session. + * + * @param provider + * the {@link KeyPasswordProvider} to delegate to + * @return a new {@link FilePasswordProvider} + */ + @NonNull + private FilePasswordProvider createFilePasswordProvider( + KeyPasswordProvider provider) { + return new PasswordProviderWrapper(provider); + } + /** * Gets the user authentication mechanisms (or rather, factories for them). * By default this returns gssapi-with-mic, public-key, @@ -437,7 +465,7 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable { * @return the non-empty list of factories. */ @NonNull - protected List> getUserAuthFactories() { + private List> getUserAuthFactories() { return Collections.unmodifiableList( Arrays.asList(GssApiWithMicAuthFactory.INSTANCE, JGitPublicKeyAuthFactory.INSTANCE,