[SSHD-860] org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client identities (private key files)

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.0.0, 2.1.0, 2.2.0
• Fix Version/s: 2.2.0
• Assignee: Lyor Goldstein
• Reporter: Thomas Wolf
• Created: 06/Nov/18 12:37
• Updated: 08/Jun/20 13:34
• Resolved: 12/Nov/18 17:48

Description

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:

1. Private keys should be kept in memory only if and when needed. Loading completely unused private keys must be avoided.
2. 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:

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();

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 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!

Issue Links

• incorporates: SSHD-865 Use lazy-loading for keys specified in the HostConfigEntry