You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2021/03/24 07:42:57 UTC
[mina-sshd] branch master updated: [SSHD-1141] Fix client-side
server-sig-algs handling
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 b08a90a [SSHD-1141] Fix client-side server-sig-algs handling
b08a90a is described below
commit b08a90a5a3f68df592b39ce977028bf5d9211db6
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Tue Mar 23 12:34:11 2021 +0100
[SSHD-1141] Fix client-side server-sig-algs handling
Unconditionally announce that the client wants to get the server's
SSH_MSG_EXT_INFO. Otherwise the server will never send it. Add the
"ext-info-c" marker only on the very first key exchange proposal,
and make sure the client doesn't send by mistake "ext-info-s".
KexExtensions are available in all phases except PREKEX.
When server-sig-algs is received, reorder the client session's
signature factories such that algorithms the server announced as
supported come first, followed by those not announced, both in client
order.
The client determines the order and the server just says what it
supports.
Note that per RFC 8308 [1] it's possible that a server doesn't announce
_all_ the algorithms it supports, and a client is allowed to try
unsupported algorithms, but may face authentication penalties such
as back-off delays, authentication failures, or disconnections.
[1] https://tools.ietf.org/html/rfc8308
---
.../DefaultClientKexExtensionHandler.java | 283 +++++----------------
1 file changed, 69 insertions(+), 214 deletions(-)
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
index 09690a8..9085def 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/extension/DefaultClientKexExtensionHandler.java
@@ -22,28 +22,19 @@ package org.apache.sshd.common.kex.extension;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.EnumMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.NavigableSet;
-import java.util.Objects;
-import java.util.stream.Stream;
+import java.util.Set;
+import java.util.TreeSet;
import org.apache.sshd.common.AttributeRepository.AttributeKey;
import org.apache.sshd.common.NamedFactory;
-import org.apache.sshd.common.NamedResource;
-import org.apache.sshd.common.OptionalFeature;
-import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.kex.KexProposalOption;
import org.apache.sshd.common.kex.extension.parser.ServerSignatureAlgorithms;
import org.apache.sshd.common.session.Session;
-import org.apache.sshd.common.signature.BuiltinSignatures;
import org.apache.sshd.common.signature.Signature;
-import org.apache.sshd.common.signature.SignatureFactory;
import org.apache.sshd.common.util.GenericUtils;
-import org.apache.sshd.common.util.MapEntryUtils;
-import org.apache.sshd.common.util.functors.UnaryEquator;
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
/**
@@ -52,27 +43,17 @@ import org.apache.sshd.common.util.logging.AbstractLoggingBean;
* session by adding the <A HREF="https://tools.ietf.org/html/rfc8332">"rsa-sha2-256/512"</A> signature
* factories (if not already added).
*
- * <B>Note:</B> experimental - used for development purposes and as an example
- *
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implements KexExtensionHandler {
- /**
- * Session {@link AttributeKey} used to store the client's proposal
- */
- public static final AttributeKey<Map<KexProposalOption, String>> CLIENT_PROPOSAL_KEY = new AttributeKey<>();
+
+ /** Default singleton instance. */
+ public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
/**
- * Session {@link AttributeKey} used to store the server's proposal
+ * Session {@link AttributeKey} used to store whether the extension indicator was already sent.
*/
- public static final AttributeKey<Map<KexProposalOption, String>> SERVER_PROPOSAL_KEY = new AttributeKey<>();
-
- public static final NavigableSet<String> DEFAULT_EXTRA_SIGNATURES = Collections.unmodifiableNavigableSet(
- GenericUtils.asSortedSet(String.CASE_INSENSITIVE_ORDER,
- KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS,
- KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS));
-
- public static final DefaultClientKexExtensionHandler INSTANCE = new DefaultClientKexExtensionHandler();
+ public static final AttributeKey<Boolean> CLIENT_PROPOSAL_MADE = new AttributeKey<>();
public DefaultClientKexExtensionHandler() {
super();
@@ -80,219 +61,93 @@ public class DefaultClientKexExtensionHandler extends AbstractLoggingBean implem
@Override
public boolean isKexExtensionsAvailable(Session session, AvailabilityPhase phase) throws IOException {
- if ((session == null) || session.isServerSession()) {
- return false;
- }
-
- // We only need to take special care during the proposal build phase
- if (phase != AvailabilityPhase.PROPOSAL) {
- return true;
- }
-
- boolean debugEnabled = log.isDebugEnabled();
- // Check if client already sent its proposal - if not, we can still influence it
- Map<KexProposalOption, String> clientProposal = session.getAttribute(CLIENT_PROPOSAL_KEY);
- Map<KexProposalOption, String> serverProposal = session.getAttribute(SERVER_PROPOSAL_KEY);
- if (MapEntryUtils.isNotEmpty(clientProposal)) {
- if (debugEnabled) {
- log.debug("isKexExtensionsAvailable({})[{}] already sent proposal={} (server={})",
- session, phase, clientProposal, serverProposal);
- }
- return false;
- }
-
- /*
- * According to https://tools.ietf.org/html/rfc8308#section-3.1:
- *
- *
- * Note that implementations are known to exist that apply authentication penalties if the client attempts to
- * use an unexpected public key algorithm.
- *
- * Therefore we want to be sure the server declared its support for extensions before we declare ours.
- */
- if (MapEntryUtils.isEmpty(serverProposal)) {
- if (debugEnabled) {
- log.debug("isKexExtensionsAvailable({})[{}] no server proposal", session, phase);
- }
- return false;
- }
-
- String algos = serverProposal.get(KexProposalOption.ALGORITHMS);
- String extDeclared = Stream.of(GenericUtils.split(algos, ','))
- .filter(s -> KexExtensions.SERVER_KEX_EXTENSION.equalsIgnoreCase(s))
- .findFirst()
- .orElse(null);
- if (GenericUtils.isEmpty(extDeclared)) {
- if (debugEnabled) {
- log.debug("isKexExtensionsAvailable({})[{}] server proposal does not include extension indicator: {}",
- session, phase, algos);
- }
- return false;
- }
-
- return true;
+ return !AvailabilityPhase.PREKEX.equals(phase);
}
@Override
public void handleKexInitProposal(
Session session, boolean initiator, Map<KexProposalOption, String> proposal)
throws IOException {
- if (session.isServerSession()) {
- return; // just in case
+ // If it's the very first time, we may add the marker telling the server that we are ready to
+ // handle SSH_MSG_EXT_INFO.
+ if (session == null || session.isServerSession() || !initiator) {
+ return;
}
-
- session.setAttribute(initiator ? CLIENT_PROPOSAL_KEY : SERVER_PROPOSAL_KEY, new EnumMap<>(proposal));
+ if (session.getAttribute(CLIENT_PROPOSAL_MADE) != null) {
+ return;
+ }
+ String kexAlgorithms = proposal.get(KexProposalOption.SERVERKEYS);
+ if (GenericUtils.isEmpty(kexAlgorithms)) {
+ return;
+ }
+ List<String> algorithms = new ArrayList<>();
+ // We're a client. We mustn't send the server extension, and we should send the client extension only once.
+ for (String algo : kexAlgorithms.split(",")) { //$NON-NLS-1$
+ if (KexExtensions.CLIENT_KEX_EXTENSION.equalsIgnoreCase(algo)
+ || KexExtensions.SERVER_KEX_EXTENSION.equalsIgnoreCase(algo)) {
+ continue;
+ }
+ algorithms.add(algo);
+ }
+ // Tell the server that we want to receive SSH_MSG_EXT_INFO
+ algorithms.add(KexExtensions.CLIENT_KEX_EXTENSION);
if (log.isDebugEnabled()) {
- log.debug("handleKexInitProposal({})[initiator={}] proposal={}", session, initiator, proposal);
+ log.debug("handleKexInitProposal({}): proposing HostKeyAlgorithms {}", //$NON-NLS-1$
+ session, algorithms);
}
- return;
+ proposal.put(KexProposalOption.SERVERKEYS, String.join(",", algorithms)); //$NON-NLS-1$
+ session.setAttribute(CLIENT_PROPOSAL_MADE, Boolean.TRUE);
}
@Override
public boolean handleKexExtensionRequest(
Session session, int index, int count, String name, byte[] data)
throws IOException {
- if (!ServerSignatureAlgorithms.NAME.equalsIgnoreCase(name)) {
- return true; // process next extension (if available)
+ if (ServerSignatureAlgorithms.NAME.equals(name)) {
+ handleServerSignatureAlgorithms(session, ServerSignatureAlgorithms.INSTANCE.parseExtension(data));
}
-
- Collection<String> sigAlgos = ServerSignatureAlgorithms.INSTANCE.parseExtension(data);
- updateAvailableSignatureFactories(session, sigAlgos);
- return false; // don't care about any more extensions (for now)
- }
-
- public List<NamedFactory<Signature>> updateAvailableSignatureFactories(
- Session session, Collection<String> extraAlgos)
- throws IOException {
- List<NamedFactory<Signature>> available = session.getSignatureFactories();
- List<NamedFactory<Signature>> updated = resolveUpdatedSignatureFactories(session, available, extraAlgos);
- if (!UnaryEquator.isSameReference(available, updated)) {
- if (log.isDebugEnabled()) {
- log.debug("updateAvailableSignatureFactories({}) available={}, updated={}",
- session, available, updated);
- }
- session.setSignatureFactories(updated);
- }
-
- return updated;
+ return true;
}
/**
- * Checks if the extra signature algorithms are already included in the available ones, and adds the extra ones (if
- * supported).
+ * Perform updates after a server-sig-algs extension has been received.
*
- * @param session The {@link Session} for which the resolution occurs
- * @param available The available signature factories
- * @param extraAlgos The extra requested signatures - ignored if {@code null}/empty
- * @return The resolved signature factories - same as input if nothing added
- * @throws IOException If failed to resolve the factories
+ * @param session
+ * the message was received for
+ * @param serverAlgorithms
+ * signature algorithm names announced by the server
*/
- public List<NamedFactory<Signature>> resolveUpdatedSignatureFactories(
- Session session, List<NamedFactory<Signature>> available, Collection<String> extraAlgos)
- throws IOException {
- boolean debugEnabled = log.isDebugEnabled();
- List<NamedFactory<Signature>> toAdd = resolveRequestedSignatureFactories(session, extraAlgos);
- if (GenericUtils.isEmpty(toAdd)) {
- if (debugEnabled) {
- log.debug("resolveUpdatedSignatureFactories({}) Nothing to add to {} out of {}",
- session, NamedResource.getNames(available), extraAlgos);
- }
- return available;
- }
-
- for (int index = 0; index < toAdd.size(); index++) {
- NamedFactory<Signature> f = toAdd.get(index);
- String name = f.getName();
- NamedFactory<Signature> a = available.stream()
- .filter(s -> Objects.equals(name, s.getName()))
- .findFirst()
- .orElse(null);
- if (a == null) {
- continue;
- }
-
- if (debugEnabled) {
- log.debug("resolveUpdatedSignatureFactories({}) skip {} - already available", session, name);
- }
-
- toAdd.remove(index);
- index--; // compensate for loop auto-increment
- }
-
- return updateAvailableSignatureFactories(session, available, toAdd);
- }
-
- public List<NamedFactory<Signature>> updateAvailableSignatureFactories(
- Session session, List<NamedFactory<Signature>> available, Collection<? extends NamedFactory<Signature>> toAdd)
- throws IOException {
- boolean debugEnabled = log.isDebugEnabled();
- if (GenericUtils.isEmpty(toAdd)) {
- if (debugEnabled) {
- log.debug("updateAvailableSignatureFactories({}) nothing to add to {}",
- session, NamedResource.getNames(available));
- }
- return available;
- }
-
- List<NamedFactory<Signature>> updated = new ArrayList<>(available.size() + toAdd.size());
- updated.addAll(available);
-
- for (NamedFactory<Signature> f : toAdd) {
- int index = resolvePreferredSignaturePosition(session, updated, f);
- if (debugEnabled) {
- log.debug("updateAvailableSignatureFactories({}) add {} at position={}", session, f, index);
- }
- if ((index < 0) || (index >= updated.size())) {
- updated.add(f);
- } else {
- updated.add(index, f);
- }
- }
-
- return updated;
- }
-
- public int resolvePreferredSignaturePosition(
- Session session, List<? extends NamedFactory<Signature>> factories, NamedFactory<Signature> factory)
- throws IOException {
- return SignatureFactory.resolvePreferredSignaturePosition(factories, factory);
- }
-
- public List<NamedFactory<Signature>> resolveRequestedSignatureFactories(
- Session session, Collection<String> extraAlgos)
- throws IOException {
- if (GenericUtils.isEmpty(extraAlgos)) {
- return Collections.emptyList();
+ protected void handleServerSignatureAlgorithms(Session session, Collection<String> serverAlgorithms) {
+ if (log.isDebugEnabled()) {
+ log.debug("handleServerSignatureAlgorithms({}): {}", session, //$NON-NLS-1$
+ serverAlgorithms);
}
-
- List<NamedFactory<Signature>> toAdd = Collections.emptyList();
- boolean debugEnabled = log.isDebugEnabled();
- for (String algo : extraAlgos) {
- NamedFactory<Signature> factory = resolveRequestedSignatureFactory(session, algo);
- if (factory == null) {
- if (debugEnabled) {
- log.debug("resolveRequestedSignatureFactories({}) skip {} - no factory found", session, algo);
- }
- continue;
+ // Client determines order; server says what it supports. Re-order such that supported ones are
+ // at the front, in client order, followed by unsupported ones, also in client order.
+ if (serverAlgorithms != null && !serverAlgorithms.isEmpty()) {
+ List<NamedFactory<Signature>> clientAlgorithms = session.getSignatureFactories();
+ if (log.isDebugEnabled()) {
+ log.debug("handleServerSignatureAlgorithms({}): PubkeyAcceptedAlgorithms before: {}", //$NON-NLS-1$
+ session, clientAlgorithms);
}
-
- if ((factory instanceof OptionalFeature) && (!((OptionalFeature) factory).isSupported())) {
- if (debugEnabled) {
- log.debug("resolveRequestedSignatureFactories({}) skip {} - not supported", session, algo);
+ List<NamedFactory<Signature>> unknown = new ArrayList<>();
+ Set<String> known = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
+ known.addAll(serverAlgorithms);
+ for (Iterator<NamedFactory<Signature>> i = clientAlgorithms.iterator(); i.hasNext(); ) {
+ NamedFactory<Signature> algo = i.next();
+ if (!known.contains(algo.getName())) {
+ unknown.add(algo);
+ i.remove();
}
- continue;
}
-
- if (toAdd.isEmpty()) {
- toAdd = new ArrayList<>(extraAlgos.size());
+ // Re-add the unknown ones at the end. Per RFC 8308, some servers may not announce _all_ their
+ // supported algorithms, and a client may use unknown algorithms.
+ clientAlgorithms.addAll(unknown);
+ if (log.isDebugEnabled()) {
+ log.debug("handleServerSignatureAlgorithms({}): PubkeyAcceptedAlgorithms after: {}", //$NON-NLS-1$
+ session, clientAlgorithms);
}
- toAdd.add(factory);
+ session.setSignatureFactories(clientAlgorithms);
}
-
- return toAdd;
- }
-
- public NamedFactory<Signature> resolveRequestedSignatureFactory(Session session, String name) throws IOException {
- return BuiltinSignatures.fromFactoryName(name);
}
}