mina-sshd git commit: [SSHD-850] Added retry index argument to FilePasswordProvider

l***@apache.org

2018-11-13 05:28:38 UTC

Repository: mina-sshd
Updated Branches:
refs/heads/master 3bea9ff05 -> 750afe77c

[SSHD-850] Added retry index argument to FilePasswordProvider

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

Branch: refs/heads/master
Commit: 750afe77c7c5c8e45980b3b63b355aef41a84aba
Parents: 3bea9ff
Author: Lyor Goldstein <***@apache.org>
Authored: Tue Nov 13 07:28:15 2018 +0200
Committer: Lyor Goldstein <***@apache.org>
Committed: Tue Nov 13 07:28:30 2018 +0200

----------------------------------------------------------------------
CHANGES.md | 4 +++-
README.md | 2 +-
.../apache/sshd/cli/client/SshClientCliSupport.java | 2 +-
.../sshd/common/config/keys/FilePasswordProvider.java | 14 ++++++++++----
.../loader/pem/AbstractPEMResourceKeyPairParser.java | 8 ++++----
.../BouncyCastleKeyPairResourceParser.java | 8 ++++----
.../sshd/common/util/security/SecurityUtilsTest.java | 2 +-
.../keys/loader/openpgp/PGPKeyPairResourceParser.java | 10 +++++-----
.../loader/openpgp/PGPKeyPairResourceParserTest.java | 5 +++--
.../keys/loader/putty/AbstractPuttyKeyDecoder.java | 8 ++++----
.../config/keys/loader/putty/PuttyKeyUtilsTest.java | 7 ++++---
11 files changed, 40 insertions(+), 30 deletions(-)
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/CHANGES.md
----------------------------------------------------------------------
diff --git a/CHANGES.md b/CHANGES.md
index 2a338cc..cc604ce 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -23,7 +23,9 @@ accept also an `AttributeRepository` connection context argument (propagated fro
`ClientSessionCreator#connect` invocation).

* `FilePasswordProvider` has an extra method (`handleDecodeAttemptResult`) that enables
-user to try and repeat an encrypted private key decoding using a different password.
+user to try and repeat an encrypted private key decoding using a different password. Furthermore,
+the interface methods are also provided with a retry index that indicates the number of the time
+they have ben re-invoked for the same resource (including on success).

* `SshAgent#getIdentities` returns an `Iterable` rather than a `List`

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/README.md
----------------------------------------------------------------------
diff --git a/README.md b/README.md
index 720c3e9..1231528 100644
--- a/README.md
+++ b/README.md
@@ -255,7 +255,7 @@ encrypted _ed25519_ private key files are not supported.
The `FilePasswordProvider` has support for a retry mechanism via its `handleDecodeAttemptResult`. When the code detects an encrypted private key,
it will start a loop where it prompts for the password, attempts to decode the key using the provided password and then informs the provider of
the outcome - success or failure. If failure is signaled, then the provider can decide whether to retry using a new password, abort (with exception)
-or ignore. If the provider chooses to ignore the failure, then the code will make a best effort to proceed without the key.
+or ignore. If the provider chooses to ignore the failure, then the code will make a best effort to proceed without the (undecoded) key.

### UserInteraction

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
----------------------------------------------------------------------
diff --git a/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java b/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
index 4d3644c..2978524 100644
--- a/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
+++ b/sshd-cli/src/main/java/org/apache/sshd/cli/client/SshClientCliSupport.java
@@ -346,7 +346,7 @@ public abstract class SshClientCliSupport extends CliSupport {
public static FileKeyPairProvider setupSessionIdentities(ClientFactoryManager client, Collection<? extends Path> identities,
BufferedReader stdin, PrintStream stdout, PrintStream stderr)
throws Throwable {
- client.setFilePasswordProvider(file -> {
+ client.setFilePasswordProvider((file, index) -> {
stdout.print("Enter password for private key file=" + file + ": ");
return stdin.readLine();
});

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-common/src/main/java/org/apache/sshd/common/config/keys/FilePasswordProvider.java
----------------------------------------------------------------------
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/FilePasswordProvider.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/FilePasswordProvider.java
index ad434ba..54e8465 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/FilePasswordProvider.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/FilePasswordProvider.java
@@ -39,14 +39,17 @@ public interface FilePasswordProvider {
/**
* An "empty" provider that returns {@code null} - i.e., unprotected key file
*/
- FilePasswordProvider EMPTY = resourceKey -> null;
+ FilePasswordProvider EMPTY = (resourceKey, retryIndex) -> null;

/**
* @param resourceKey The resource key representing the private file
+ * @param retryIndex The zero-based index of the invocation for the specific
+ * resource (in case invoked several times for the same resource)
* @return The password - if {@code null}/empty then no password is required
* @throws IOException if cannot resolve password
+ * @see #handleDecodeAttemptResult(String, int, String, Exception)
*/
- String getPassword(String resourceKey) throws IOException;
+ String getPassword(String resourceKey, int retryIndex) throws IOException;

/**
* Invoked to inform the password provide about the decoding result. Note:
@@ -54,6 +57,9 @@ public interface FilePasswordProvider {
* success) will be propagated instead of the original (if any was reported)
*
* @param resourceKey The resource key representing the private file
+ * @param retryIndex The zero-based index of the invocation for the specific
+ * resource (in case invoked several times for the same resource). If success
+ * report, it indicates the number of retries it took to succeed
* @param password The password that was attempted
* @param err The attempt result - {@code null} for success
* @return How to proceed in case of error - ignored if invoked in order
@@ -62,12 +68,12 @@ public interface FilePasswordProvider {
* @throws GeneralSecurityException If not attempting to resolve a new password
*/
default ResourceDecodeResult handleDecodeAttemptResult(
- String resourceKey, String password, Exception err)
+ String resourceKey, int retryIndex, String password, Exception err)
throws IOException, GeneralSecurityException {
return ResourceDecodeResult.TERMINATE;
}

static FilePasswordProvider of(String password) {
- return r -> password;
+ return (r, index) -> password;
}
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/AbstractPEMResourceKeyPairParser.java
----------------------------------------------------------------------
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/AbstractPEMResourceKeyPairParser.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/AbstractPEMResourceKeyPairParser.java
index 1528790..6ac9e2f 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/AbstractPEMResourceKeyPairParser.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/pem/AbstractPEMResourceKeyPairParser.java
@@ -132,8 +132,8 @@ public abstract class AbstractPEMResourceKeyPairParser
throw new CredentialException("Missing password provider for encrypted resource=" + resourceKey);
}

- while (true) {
- String password = passwordProvider.getPassword(resourceKey);
+ for (int retryIndex = 0;; retryIndex++) {
+ String password = passwordProvider.getPassword(resourceKey, retryIndex);
Collection<KeyPair> keys;
try {
if (GenericUtils.isEmpty(password)) {
@@ -150,7 +150,7 @@ public abstract class AbstractPEMResourceKeyPairParser
}
} catch (IOException | GeneralSecurityException | RuntimeException e) {
ResourceDecodeResult result =
- passwordProvider.handleDecodeAttemptResult(resourceKey, password, e);
+ passwordProvider.handleDecodeAttemptResult(resourceKey, retryIndex, password, e);
if (result == null) {
result = ResourceDecodeResult.TERMINATE;
}
@@ -166,7 +166,7 @@ public abstract class AbstractPEMResourceKeyPairParser
}
}

- passwordProvider.handleDecodeAttemptResult(resourceKey, password, null);
+ passwordProvider.handleDecodeAttemptResult(resourceKey, retryIndex, password, null);
return keys;
}
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleKeyPairResourceParser.java
----------------------------------------------------------------------
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleKeyPairResourceParser.java b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleKeyPairResourceParser.java
index 1433de0..5c8befb 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleKeyPairResourceParser.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/security/bouncycastle/BouncyCastleKeyPairResourceParser.java
@@ -121,8 +121,8 @@ public class BouncyCastleKeyPairResourceParser extends AbstractKeyPairResourcePa
throw new CredentialException("Missing password provider for encrypted resource=" + resourceKey);
}

- while (true) {
- String password = provider.getPassword(resourceKey);
+ for (int retryIndex = 0;; retryIndex++) {
+ String password = provider.getPassword(resourceKey, retryIndex);
PEMKeyPair decoded;
try {
if (GenericUtils.isEmpty(password)) {
@@ -134,7 +134,7 @@ public class BouncyCastleKeyPairResourceParser extends AbstractKeyPairResourcePa
decoded = ((PEMEncryptedKeyPair) o).decryptKeyPair(pemDecryptor);
} catch (IOException | GeneralSecurityException | RuntimeException e) {
ResourceDecodeResult result =
- provider.handleDecodeAttemptResult(resourceKey, password, e);
+ provider.handleDecodeAttemptResult(resourceKey, retryIndex, password, e);
if (result == null) {
result = ResourceDecodeResult.TERMINATE;
}
@@ -151,7 +151,7 @@ public class BouncyCastleKeyPairResourceParser extends AbstractKeyPairResourcePa
}

o = decoded;
- provider.handleDecodeAttemptResult(resourceKey, password, null);
+ provider.handleDecodeAttemptResult(resourceKey, retryIndex, password, null);
break;
}
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-common/src/test/java/org/apache/sshd/common/util/security/SecurityUtilsTest.java
----------------------------------------------------------------------
diff --git a/sshd-common/src/test/java/org/apache/sshd/common/util/security/SecurityUtilsTest.java b/sshd-common/src/test/java/org/apache/sshd/common/util/security/SecurityUtilsTest.java
index 46ca075..f244fd4 100644
--- a/sshd-common/src/test/java/org/apache/sshd/common/util/security/SecurityUtilsTest.java
+++ b/sshd-common/src/test/java/org/apache/sshd/common/util/security/SecurityUtilsTest.java
@@ -67,7 +67,7 @@ public class SecurityUtilsTest extends JUnitTestSupport {
+ "." + SecurityProviderRegistrar.NAMED_PROVIDER_PROPERTY;

private static final String DEFAULT_PASSWORD = "super secret passphrase";
- private static final FilePasswordProvider TEST_PASSWORD_PROVIDER = file -> DEFAULT_PASSWORD;
+ private static final FilePasswordProvider TEST_PASSWORD_PROVIDER = (file, index) -> DEFAULT_PASSWORD;

public SecurityUtilsTest() {
super();

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-openpgp/src/main/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParser.java
----------------------------------------------------------------------
diff --git a/sshd-openpgp/src/main/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParser.java b/sshd-openpgp/src/main/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParser.java
index 8bfb8e4..b4f7051 100644
--- a/sshd-openpgp/src/main/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParser.java
+++ b/sshd-openpgp/src/main/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParser.java
@@ -126,11 +126,11 @@ public class PGPKeyPairResourceParser extends AbstractKeyPairResourceParser {
public Collection<KeyPair> extractKeyPairs(
String resourceKey, String beginMarker, String endMarker, FilePasswordProvider passwordProvider, InputStream stream)
throws IOException, GeneralSecurityException {
- for (int retryCount = 1;; retryCount++) {
- String password = (passwordProvider == null) ? null : passwordProvider.getPassword(resourceKey);
+ for (int retryCount = 0;; retryCount++) {
+ String password = (passwordProvider == null) ? null : passwordProvider.getPassword(resourceKey, retryCount);
Collection<KeyPair> keys;
try {
- if (retryCount > 1) {
+ if (retryCount > 0) {
stream.reset();
}

@@ -144,7 +144,7 @@ public class PGPKeyPairResourceParser extends AbstractKeyPairResourceParser {
keys = extractKeyPairs(resourceKey, key.getSubkeys());
} catch (IOException | GeneralSecurityException | PGPException | RuntimeException e) {
ResourceDecodeResult result = (passwordProvider != null)
- ? passwordProvider.handleDecodeAttemptResult(resourceKey, password, e)
+ ? passwordProvider.handleDecodeAttemptResult(resourceKey, retryCount, password, e)
: ResourceDecodeResult.TERMINATE;
if (result == null) {
result = ResourceDecodeResult.TERMINATE;
@@ -173,7 +173,7 @@ public class PGPKeyPairResourceParser extends AbstractKeyPairResourceParser {
}

if (passwordProvider != null) {
- passwordProvider.handleDecodeAttemptResult(resourceKey, password, null);
+ passwordProvider.handleDecodeAttemptResult(resourceKey, retryCount, password, null);
}
return keys;
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-openpgp/src/test/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParserTest.java
----------------------------------------------------------------------
diff --git a/sshd-openpgp/src/test/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParserTest.java b/sshd-openpgp/src/test/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParserTest.java
index 3a121d9..bf2b8bb 100644
--- a/sshd-openpgp/src/test/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParserTest.java
+++ b/sshd-openpgp/src/test/java/org/apache/sshd/common/config/keys/loader/openpgp/PGPKeyPairResourceParserTest.java
@@ -70,13 +70,14 @@ public class PGPKeyPairResourceParserTest extends JUnitTestSupport {

@Override
@SuppressWarnings("synthetic-access")
- public String getPassword(String resourceKey) throws IOException {
+ public String getPassword(String resourceKey, int retryIndex) throws IOException {
switch (result) {
case IGNORE:
case TERMINATE:
return "qweryuiop123456!@#$%^";
case RETRY: {
int count = retriesCount.incrementAndGet();
+ assertEquals("Mismatched retries count", count, retryIndex + 1);
if (count == maxRetries) {
return PASSWORD;
} else {
@@ -91,7 +92,7 @@ public class PGPKeyPairResourceParserTest extends JUnitTestSupport {
@Override
@SuppressWarnings("synthetic-access")
public ResourceDecodeResult handleDecodeAttemptResult(
- String resourceKey, String password, Exception err)
+ String resourceKey, int retryIndex, String password, Exception err)
throws IOException, GeneralSecurityException {
if (err == null) {
return null;

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-putty/src/main/java/org/apache/sshd/common/config/keys/loader/putty/AbstractPuttyKeyDecoder.java
----------------------------------------------------------------------
diff --git a/sshd-putty/src/main/java/org/apache/sshd/common/config/keys/loader/putty/AbstractPuttyKeyDecoder.java b/sshd-putty/src/main/java/org/apache/sshd/common/config/keys/loader/putty/AbstractPuttyKeyDecoder.java
index 681422a..05a3c6f 100644
--- a/sshd-putty/src/main/java/org/apache/sshd/common/config/keys/loader/putty/AbstractPuttyKeyDecoder.java
+++ b/sshd-putty/src/main/java/org/apache/sshd/common/config/keys/loader/putty/AbstractPuttyKeyDecoder.java
@@ -186,8 +186,8 @@ public abstract class AbstractPuttyKeyDecoder<PUB extends PublicKey, PRV extends
throw new StreamCorruptedException("Missing private key encryption algorithm details in " + prvEncryption);
}

- while (true) {
- String password = passwordProvider.getPassword(resourceKey);
+ for (int retryIndex = 0;; retryIndex++) {
+ String password = passwordProvider.getPassword(resourceKey, retryIndex);

Collection<KeyPair> keys;
try {
@@ -199,7 +199,7 @@ public abstract class AbstractPuttyKeyDecoder<PUB extends PublicKey, PRV extends
keys = loadKeyPairs(resourceKey, pubBytes, decBytes);
} catch (IOException | GeneralSecurityException | RuntimeException e) {
ResourceDecodeResult result =
- passwordProvider.handleDecodeAttemptResult(resourceKey, password, e);
+ passwordProvider.handleDecodeAttemptResult(resourceKey, retryIndex, password, e);
if (result == null) {
result = ResourceDecodeResult.TERMINATE;
}
@@ -215,7 +215,7 @@ public abstract class AbstractPuttyKeyDecoder<PUB extends PublicKey, PRV extends
}
}

- passwordProvider.handleDecodeAttemptResult(resourceKey, password, null);
+ passwordProvider.handleDecodeAttemptResult(resourceKey, retryIndex, password, null);
return keys;
}
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/750afe77/sshd-putty/src/test/java/org/apache/sshd/common/config/keys/loader/putty/PuttyKeyUtilsTest.java
----------------------------------------------------------------------
diff --git a/sshd-putty/src/test/java/org/apache/sshd/common/config/keys/loader/putty/PuttyKeyUtilsTest.java b/sshd-putty/src/test/java/org/apache/sshd/common/config/keys/loader/putty/PuttyKeyUtilsTest.java
index 94a05d7..3cf2b4f 100644
--- a/sshd-putty/src/test/java/org/apache/sshd/common/config/keys/loader/putty/PuttyKeyUtilsTest.java
+++ b/sshd-putty/src/test/java/org/apache/sshd/common/config/keys/loader/putty/PuttyKeyUtilsTest.java
@@ -125,7 +125,7 @@ public class PuttyKeyUtilsTest extends JUnitTestSupport {
Assume.assumeTrue("Skip non-existent encrypted file: " + encryptedFile, url != null);
assertNotNull("Missing test resource: " + encryptedFile, url);

- Collection<KeyPair> keys = parser.loadKeyPairs(url, r -> PASSWORD);
+ Collection<KeyPair> keys = parser.loadKeyPairs(url, (r, index) -> PASSWORD);
assertEquals("Mismatched loaded keys count from " + encryptedFile, 1, GenericUtils.size(keys));

assertLoadedKeyPair(encryptedFile, keys.iterator().next());
@@ -143,13 +143,14 @@ public class PuttyKeyUtilsTest extends JUnitTestSupport {
AtomicInteger retriesCount = new AtomicInteger(0);
FilePasswordProvider provider = new FilePasswordProvider() {
@Override
- public String getPassword(String resourceKey) throws IOException {
+ public String getPassword(String resourceKey, int retryIndex) throws IOException {
switch (result) {
case IGNORE:
case TERMINATE:
return "qweryuiop123456!@#$%^";
case RETRY: {
int count = retriesCount.incrementAndGet();
+ assertEquals("Mismatched retries count", retryIndex + 1, count);
if (count == maxRetries) {
return PASSWORD;
} else {
@@ -163,7 +164,7 @@ public class PuttyKeyUtilsTest extends JUnitTestSupport {

@Override
public ResourceDecodeResult handleDecodeAttemptResult(
- String resourceKey, String password, Exception err)
+ String resourceKey, int retryIndex, String password, Exception err)
throws IOException, GeneralSecurityException {
if (err == null) {
return null;