This is an automated email from the ASF dual-hosted git repository. twolf pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push: new cb2cc8e78 [SSHD-1272] Client-side pubkey auth: use session's signature factories cb2cc8e78 is described below commit cb2cc8e78c5b94e58fe88dbd3867e40c618f86db Author: Thomas Wolf <tw...@apache.org> AuthorDate: Thu Jun 23 18:34:58 2022 +0200 [SSHD-1272] Client-side pubkey auth: use session's signature factories When determining the signature algorithm for a public key, fall back properly to the session's signature factories if none can be found earlier. Skip keys for which no signature algorithm can be determined. Provide a way for client code to supply a last-resort default signature name, if desired. --- .../pubkey/PublicKeyAuthenticationReporter.java | 12 ++ .../sshd/client/auth/pubkey/UserAuthPublicKey.java | 167 ++++++++++++++------- 2 files changed, 127 insertions(+), 52 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyAuthenticationReporter.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyAuthenticationReporter.java index 77197213e..da4eaa054 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyAuthenticationReporter.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/PublicKeyAuthenticationReporter.java @@ -61,6 +61,18 @@ public interface PublicKeyAuthenticationReporter { // ignored } + /** + * A {@link KeyPair} is is present, but is not attempted because no signature factory for it could be found. + * + * @param session The {@link ClientSession} + * @param service The requesting service name + * @param identity The {@link KeyPair} identity being skipped - <B>Note:</B> for agent based authentications the + * private key may be {@code null} + */ + default void signalIdentitySkipped(ClientSession session, String service, KeyPair identity) throws Exception { + // ignored + } + /** * Sending the signed response to the server's challenge * diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java index 274a16f77..52cbc0e0b 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java @@ -147,66 +147,83 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact } } PublicKeyAuthenticationReporter reporter = session.getPublicKeyAuthenticationReporter(); - if (currentAlgorithm == null) { + KeyPair keyPair; + PublicKey pubKey; + do { + if (currentAlgorithm == null) { + try { + current = resolveAttemptedPublicKeyIdentity(session, service, reporter); + } catch (Error e) { + warn("sendAuthDataRequest({})[{}] failed ({}) to get next key: {}", session, service, + e.getClass().getSimpleName(), e.getMessage(), e); + throw new RuntimeSshException(e); + } + currentAlgorithms.clear(); + chosenAlgorithm = null; + if (current == null) { + if (debugEnabled) { + log.debug("resolveAttemptedPublicKeyIdentity({})[{}] no more keys to send", session, service); + } + + if (reporter != null) { + reporter.signalAuthenticationExhausted(session, service); + } + + return false; + } + } + if (log.isTraceEnabled()) { + log.trace("sendAuthDataRequest({})[{}] current key details: {}", session, service, current); + } + try { - current = resolveAttemptedPublicKeyIdentity(session, service); + keyPair = current.getKeyIdentity(); } catch (Error e) { - warn("sendAuthDataRequest({})[{}] failed ({}) to get next key: {}", + warn("sendAuthDataRequest({})[{}] failed ({}) to retrieve key identity: {}", session, service, e.getClass().getSimpleName(), e.getMessage(), e); throw new RuntimeSshException(e); } - currentAlgorithms.clear(); - chosenAlgorithm = null; - if (current == null) { - if (debugEnabled) { - log.debug("resolveAttemptedPublicKeyIdentity({})[{}] no more keys to send", session, service); + pubKey = keyPair.getPublic(); + + if (currentAlgorithm == null) { + String keyType = KeyUtils.getKeyType(pubKey); + Set<String> aliases = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + aliases.addAll(KeyUtils.getAllEquivalentKeyTypes(keyType)); + aliases.add(keyType); + List<NamedFactory<Signature>> existingFactories = null; + if (current instanceof SignatureFactoriesHolder) { + existingFactories = ((SignatureFactoriesHolder) current).getSignatureFactories(); } - - if (reporter != null) { - reporter.signalAuthenticationExhausted(session, service); + if (GenericUtils.isEmpty(existingFactories)) { + existingFactories = getSignatureFactories(); } - - return false; - } - } - if (log.isTraceEnabled()) { - log.trace("sendAuthDataRequest({})[{}] current key details: {}", session, service, current); - } - - KeyPair keyPair; - try { - keyPair = current.getKeyIdentity(); - } catch (Error e) { - warn("sendAuthDataRequest({})[{}] failed ({}) to retrieve key identity: {}", - session, service, e.getClass().getSimpleName(), e.getMessage(), e); - throw new RuntimeSshException(e); - } - PublicKey pubKey = keyPair.getPublic(); - - if (currentAlgorithm == null) { - String keyType = KeyUtils.getKeyType(pubKey); - Set<String> aliases = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); - aliases.addAll(KeyUtils.getAllEquivalentKeyTypes(keyType)); - aliases.add(keyType); - List<NamedFactory<Signature>> existingFactories; - if (current instanceof SignatureFactoriesHolder) { - existingFactories = ((SignatureFactoriesHolder) current) - .getSignatureFactories(); - } else { - existingFactories = getSignatureFactories(); - } - if (existingFactories != null) { - // Select the factories by name and in order - existingFactories.forEach(f -> { - if (aliases.contains(f.getName())) { - currentAlgorithms.add(f.getName()); + if (GenericUtils.isEmpty(existingFactories)) { + existingFactories = session.getSignatureFactories(); + } + if (existingFactories != null) { + // Select the factories by name and in order + existingFactories.forEach(f -> { + if (aliases.contains(f.getName())) { + currentAlgorithms.add(f.getName()); + } + }); + } + currentAlgorithm = currentAlgorithms.poll(); + if (GenericUtils.isEmpty(currentAlgorithm)) { + currentAlgorithm = getDefaultSignatureAlgorithm(session, service, current, keyPair, keyType); + if (GenericUtils.isEmpty(currentAlgorithm)) { + currentAlgorithm = null; + if (debugEnabled) { + log.debug("sendAuthDataRequest({})[{}] skipping {} key {}; no signature algorithm", session, + service, keyType, KeyUtils.getFingerPrint(pubKey)); + } + if (reporter != null) { + reporter.signalIdentitySkipped(session, service, keyPair); + } } - }); + } } - currentAlgorithm = currentAlgorithms.isEmpty() - ? keyType - : currentAlgorithms.poll(); - } + } while (currentAlgorithm == null); String name = getName(); Integer hostBoundPubKeyVersion = session.getAttribute(DefaultClientKexExtensionHandler.HOSTBOUND_AUTHENTICATION); @@ -239,10 +256,18 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact } protected PublicKeyIdentity resolveAttemptedPublicKeyIdentity(ClientSession session, String service) throws Exception { + return resolveAttemptedPublicKeyIdentity(session, service, null); + } + + protected PublicKeyIdentity resolveAttemptedPublicKeyIdentity( + ClientSession session, String service, + PublicKeyAuthenticationReporter reporter) + throws Exception { if (keys != null) { while (keys.hasNext()) { PublicKeyIdentity nextKey = keys.next(); - PublicKey pk = nextKey.getKeyIdentity().getPublic(); + KeyPair identity = nextKey.getKeyIdentity(); + PublicKey pk = identity.getPublic(); if (pk instanceof OpenSshCertificate) { OpenSshCertificate cert = (OpenSshCertificate) pk; if (!OpenSshCertificate.Type.USER.equals(cert.getType())) { @@ -250,6 +275,9 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact "resolveAttemptedPublicKeyIdentity({})[{}]: public key certificate {} {} (id={}) is not a user certificate", session, service, KeyUtils.getKeyType(cert), KeyUtils.getFingerPrint(cert), cert.getId()); + if (reporter != null) { + reporter.signalIdentitySkipped(session, service, identity); + } continue; } if (!OpenSshCertificate.isValidNow(cert)) { @@ -257,6 +285,9 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact "resolveAttemptedPublicKeyIdentity({})[{}]: public key certificate {} {} (id={}) is not valid now", session, service, KeyUtils.getKeyType(cert), KeyUtils.getFingerPrint(cert), cert.getId()); + if (reporter != null) { + reporter.signalIdentitySkipped(session, service, identity); + } continue; } } @@ -277,6 +308,38 @@ public class UserAuthPublicKey extends AbstractUserAuth implements SignatureFact return new KeyPairIdentity(this, session, kp); } + /** + * Determines a signature algorithm name to use for the authentication request if none could be determined from the + * installed signature factories. If a non-{@code null} non-empty string is returned, it is used <em>as is</em> in + * the authentication. + * <p> + * This is mainly intended for use with identities from an SSH agent, where the SSH agent may be able to sign the + * request even if there is no appropriate signature factory present in Java. Whether it makes sense to allow this + * depends on the application logic and how it handles e.g. SSH config {@code PubkeyAcceptedKeyTypes} (or + * {@code PubkeyAcceptedAlgorithms}}. + * </p> + * <p> + * This default implementation always returns {@code null}, skipping the key. + * </p> + * + * @param session {@link ClientSession} trying to authenticate + * @param service SSH service name + * @param identity {@link PublicKeyIdentity} considered to be used for authentication + * @param keyPair {@link KeyPair} from {@code identity} + * @param keyType the key type of {@code keyPair} + * @return {@code null} or an empty string to skip this key and consider another key, if any, to use for + * authentication, or a non-empty signature algorithm name to use for the authentication attempt + * using the given {@code identity} + * @throws Exception if an error occurs + * @see KeyAgentIdentity + */ + protected String getDefaultSignatureAlgorithm( + ClientSession session, String service, PublicKeyIdentity identity, + KeyPair keyPair, String keyType) + throws Exception { + return null; + } + @Override protected boolean processAuthDataRequest(ClientSession session, String service, Buffer buffer) throws Exception { String name = getName();