[
https://issues.apache.org/jira/browse/SSHD-860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16677170#comment-16677170
]
Goldstein Lyor commented on SSHD-860:
-------------------------------------
I do not necessarily agree with the severity but I can see why some may think
it is not minor (so upgraded it to major). Unfortunately, loading key pairs
lazily is a major overhaul of the code which we can afford at the moment given
the extremely limited R&D resources at our disposal. We have considered it in
the past but I am not sure when we might find the time for such an overhaul.
The good news is that we do accept code contributions (;)). If this is indeed
critical for you I recommend not to wait for us to get around to it, clone the
code and publish a PR. (Again, not because we don't want to, but because in the
foreseeable future we might not be able to invest the time it requires). If you
do go ahead with it, please note that not only this class needs an overhaul,
but perhaps all the other identity loaders and all their code....
> org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client
> identities (private key files)
> ------------------------------------------------------------------------------------------------------------
>
> Key: SSHD-860
> URL: https://issues.apache.org/jira/browse/SSHD-860
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 2.0.0, 2.1.0, 2.1.1
> Reporter: Thomas Wolf
> Priority: Major
>
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} loads and
> caches in memory private keys prematurely. Keys are loaded even if they are
> not used at all in the end. In other words, incremental loading of keys
> doesn't work.
> This is bad for two reasons:
> # Private keys should be kept in memory only if and when needed. Loading
> completely unused private keys must be avoided.
> # With encrypted private keys, the user may end up being asked for
> passphrases for keys that are not used at all in the end, which is highly
> disruptive.
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} does in its
> constructor:
> {code:java}
> Iterator<? extends PublicKeyIdentity> current;
> Collection<Stream<? extends PublicKeyIdentity>> identities = new
> LinkedList<>();
> ...
> identities.add(Stream.of(ClientSession.providerOf(session))
> .map(KeyIdentityProvider::loadKeys)
> .flatMap(GenericUtils::stream)
> .map(kp -> new KeyPairIdentity(signatureFactories, session, kp)));
> current = identities.stream().flatMap(r -> r).iterator();
> {code}
> The final {{current}} iterator uses some internal buffer (size unknown;
> didn't try to determine it) and will pre-fill this buffer completely. So with
> buffer size _N_ it'll pre-load the first _N_ keys from the combined identity
> stream. If the first key authenticates successfully, loading all the others
> must not be done.
> See my [test
> case|https://github.com/tomaswolf/mina-sshd/blob/SSHD-860/sshd-core/src/test/java/org/apache/sshd/client/ClientKeyLoadTest.java]
> showing this faulty behavior. It does exactly the same as the
> {{UserAuthPublicKeyIterator}} constructor, using two iterators with two
> identity files each, and then tests the resulting iterator. The first
> {{hasNext()/next()}} call on the {{current}} iterator _loads all four keys!_
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
