You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2018/11/13 05:28:38 UTC

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

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 <lg...@apache.org>
Authored: Tue Nov 13 07:28:15 2018 +0200
Committer: Lyor Goldstein <lg...@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 &quot;empty&quot; 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 <U>private</U> 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. <b>Note:</b>
@@ -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 <U>private</U> 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 - <u>ignored</u> 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;