This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch dev_3.0
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 7ec8197e9751cde2ecaaf27532532ab1ad42ace6
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Tue Apr 22 20:05:28 2025 +0200
Simplify CoreModuleProperties.SEND_IMMEDIATE_KEXINIT
Setting this property to false should not only delay sending the
client's initial KEX_INIT until has received the server. It should
delay the whole KEX, including determining the client's proposal,
until that point, so that the client can adapt its own proposal
based on what the server sent (ident and server proposal).
So remove the DelayKexFilter, and just don't do anything in
KexFilter.startKex() if the property is false and it's the very first
call. (Well, we send a null message through the filter chain to tell
lower filters that they should initialize. In particular, this triggers
the IdentFilter to send the client's SSH protocol version string.)
Add a test to verify that we do have the server's ident and proposal
when the client finally builds its own proposal.
---
docs/technical/filters.md | 7 +-
.../common/session/filters/DelayKexInitFilter.java | 132 ---------------------
.../common/session/filters/SshTransportFilter.java | 4 -
.../sshd/common/session/filters/kex/KexFilter.java | 27 +++--
.../java/org/apache/sshd/client/ClientTest.java | 28 +++++
5 files changed, 50 insertions(+), 148 deletions(-)
diff --git a/docs/technical/filters.md b/docs/technical/filters.md
index 28123d50f..9d4899371 100644
--- a/docs/technical/filters.md
+++ b/docs/technical/filters.md
@@ -150,7 +150,7 @@ chain should not be modified anymore. Adding filters to or
removing filters from
are coming in or going out would be very tricky to get right without race
conditions.
If there are filters that should not be active after some time, they can set
their incoming or outgoing
-handlers to `null` (use an `AtomicReference`).- An example would be the
aforementioned proxy filter: once the
+handlers to `null` (use an `AtomicReference`). An example would be the
aforementioned proxy filter: once the
proxy tunnel is established (or the HAproxy protocol header has been
received), such a proxy filter will
not do anything anymore but pass on messages. Such a filter can then set its
handlers to `null`. The filter
chain will skip any filter that has a `null` handler for the direction a
particular message is going.
@@ -180,9 +180,8 @@ server's messages:
* `CoreModuleProperties.SEND_IMMEDIATE_IDENTIFICATION`: by default `true`. If
`false`, the client sends its SSH
protocol version only after it has received the server's. This is implemented
transparently in `IdentFilter`.
* `CoreModuleProperties.SEND_IMMEDIATE_KEXINIT`: by default `true`. If
`false`, the client sends its `SSH_MSG_KEXINIT`
-only after it has received the server's. This is handled in a special
`DelayKexFilter` that sits below the
-`KexFilter` and that is omitted on purpose from the above diagrams since it is
a really special special case
-and including it would only have confused the presentation.
+only after it has received the server's. The client thus has a chance to adapt
its own proposal depending on
+the server's ident and key exchange proposal.
These two options were introduced long ago to handle connecting to old SSH
servers. In SSH version 1, the
server sent its protocol version string first, and the client always came
second. In SSH version 2, the
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/DelayKexInitFilter.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/DelayKexInitFilter.java
deleted file mode 100644
index b63e028ff..000000000
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/DelayKexInitFilter.java
+++ /dev/null
@@ -1,132 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.sshd.common.session.filters;
-
-import java.io.IOException;
-import java.util.Objects;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
-
-import org.apache.sshd.common.SshConstants;
-import org.apache.sshd.common.filter.BufferInputHandler;
-import org.apache.sshd.common.filter.InputHandler;
-import org.apache.sshd.common.filter.IoFilter;
-import org.apache.sshd.common.filter.OutputHandler;
-import org.apache.sshd.common.io.DefaultIoWriteFuture;
-import org.apache.sshd.common.io.IoWriteFuture;
-import org.apache.sshd.common.session.Session;
-import org.apache.sshd.common.util.buffer.Buffer;
-import org.apache.sshd.core.CoreModuleProperties;
-
-/**
- * A filter implementing "delayed KEX-INIT" where the client waits for the
server's initial KEX-INIT to arrive first
- * before sending its own.
- */
-public class DelayKexInitFilter extends IoFilter {
-
- private AtomicBoolean isFirst = new AtomicBoolean(true);
-
- private final DefaultIoWriteFuture initReceived = new
DefaultIoWriteFuture(this, null);
-
- private final AtomicReference<InputHandler> input = new
AtomicReference<>();
-
- private final AtomicReference<OutputHandler> output = new
AtomicReference<>();
-
- private Session session;
-
- public DelayKexInitFilter() {
- input.set(new KexInputHandler());
- output.set(new KexOutputHandler());
- }
-
- @Override
- public InputHandler in() {
- return input.get();
- }
-
- @Override
- public OutputHandler out() {
- return output.get();
- }
-
- public void setSession(Session session) {
- this.session = Objects.requireNonNull(session);
- }
-
- private class KexInputHandler implements BufferInputHandler {
-
- KexInputHandler() {
- super();
- }
-
- @Override
- public void handleMessage(Buffer message) throws Exception {
- if (input.get() != null) {
- int cmd = message.rawByte(message.rpos());
- if (cmd == SshConstants.SSH_MSG_KEXINIT) {
- initReceived.setValue(Boolean.TRUE);
- input.set(null);
- }
- }
- owner().passOn(message);
- }
-
- }
-
- private class KexOutputHandler implements OutputHandler {
-
- KexOutputHandler() {
- super();
- }
-
- @Override
- public IoWriteFuture send(int cmd, Buffer message) throws IOException {
- if (cmd != SshConstants.SSH_MSG_KEXINIT || output.get() == null ||
message == null) {
- return owner().send(cmd, message);
- }
- boolean first = isFirst.getAndSet(false);
- if (!first || session.isServerSession()
- ||
CoreModuleProperties.SEND_IMMEDIATE_KEXINIT.getRequired(session).booleanValue())
{
- return owner().send(cmd, message).addListener(f ->
output.set(null));
- }
- // We're a client, and we delay sending the initial KEX-INIT until
we have received the peer's KEX-INIT
- IoWriteFuture initial = owner().send(-1, null);
- DefaultIoWriteFuture result = new
DefaultIoWriteFuture(KexOutputHandler.this, null);
- initial.addListener(init -> {
- Throwable t = init.getException();
- if (t != null) {
- result.setValue(t);
- return;
- }
- initReceived.addListener(f -> {
- try {
- owner().send(cmd, message).addListener(g -> {
- output.set(null);
- result.setValue(g.isWritten() ? Boolean.TRUE :
g.getException());
- });
- } catch (IOException e) {
- result.setValue(e);
- }
- });
- });
- return result;
- }
-
- }
-}
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/SshTransportFilter.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/SshTransportFilter.java
index 2223b29a8..273b00b5f 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/SshTransportFilter.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/SshTransportFilter.java
@@ -80,10 +80,6 @@ public class SshTransportFilter extends IoFilter {
filters.addLast(new PacketLoggingFilter(session, cryptFilter));
- DelayKexInitFilter delayKexFilter = new DelayKexInitFilter();
- delayKexFilter.setSession(session);
- filters.addLast(delayKexFilter);
-
filters.addLast(new InjectIgnoreFilter(session, random));
kexFilter = new KexFilter(session, random, cryptFilter,
compressionFilter, events, proposer, checker);
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/kex/KexFilter.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/kex/KexFilter.java
index 4be26057e..e41cdabee 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/kex/KexFilter.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/kex/KexFilter.java
@@ -33,6 +33,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
@@ -144,6 +145,8 @@ public class KexFilter extends IoFilter {
private final CopyOnWriteArrayList<KexListener> listeners = new
CopyOnWriteArrayList<>();
+ private final AtomicBoolean firstKex = new AtomicBoolean();
+
private final AtomicReference<KexState> kexState = new
AtomicReference<>(KexState.DONE);
private final AtomicReference<DefaultKeyExchangeFuture> kexFuture = new
AtomicReference<>();
@@ -278,19 +281,18 @@ public class KexFilter extends IoFilter {
}
public Map<KexProposalOption, String> getNegotiated() {
- return Collections.unmodifiableMap(negotiated.get());
+ Map<KexProposalOption, String> result = negotiated.get();
+ return result == null ? null : Collections.unmodifiableMap(result);
}
public Map<KexProposalOption, String> getClientProposal() {
- return session.isServerSession()
- ? Collections.unmodifiableMap(peerProposal.get())
- : Collections.unmodifiableMap(myProposal.get());
+ Map<KexProposalOption, String> result = session.isServerSession() ?
peerProposal.get() : myProposal.get();
+ return result == null ? null : Collections.unmodifiableMap(result);
}
public Map<KexProposalOption, String> getServerProposal() {
- return session.isServerSession()
- ? Collections.unmodifiableMap(myProposal.get())
- : Collections.unmodifiableMap(peerProposal.get());
+ Map<KexProposalOption, String> result = session.isServerSession() ?
myProposal.get() : peerProposal.get();
+ return result == null ? null : Collections.unmodifiableMap(result);
}
public void setClientIdent(String ident) {
@@ -1061,6 +1063,16 @@ public class KexFilter extends IoFilter {
}
public KeyExchangeFuture startKex() throws Exception {
+ DefaultKeyExchangeFuture result = new
DefaultKeyExchangeFuture(session.toString(), session.getFutureLock());
+ if (firstKex.compareAndSet(false, true) && !session.isServerSession()
+ &&
!CoreModuleProperties.SEND_IMMEDIATE_KEXINIT.getRequired(session).booleanValue())
{
+ // We're a client, and we're supposed to wait for the server's
proposal. Just trigger sending the SSH
+ // protocol ident, but don't do anything else. When we get the
server's KEX-INIT, we'll evaluate and send
+ // our proposal, then do the negotiation.
+ owner().send(-1, null);
+ result.setValue(Boolean.FALSE);
+ return result;
+ }
boolean start = output.updateState(() -> {
if (kexState.compareAndSet(KexState.DONE, KexState.INIT)) {
output.initNewKeyExchange();
@@ -1068,7 +1080,6 @@ public class KexFilter extends IoFilter {
}
return false;
});
- DefaultKeyExchangeFuture result = new
DefaultKeyExchangeFuture(session.toString(), session.getFutureLock());
if (start) {
listeners.forEach(listener -> listener.event(true));
kexFuture.set(result);
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
index f82eb40c4..d2dfa3ea7 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
@@ -101,6 +101,8 @@ import org.apache.sshd.common.io.IoOutputStream;
import org.apache.sshd.common.io.IoReadFuture;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.io.IoWriteFuture;
+import org.apache.sshd.common.kex.KexProposalOption;
+import org.apache.sshd.common.kex.extension.DefaultClientKexExtensionHandler;
import org.apache.sshd.common.keyprovider.FileKeyPairProvider;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.session.ConnectionService;
@@ -743,6 +745,32 @@ public class ClientTest extends BaseTestSupport {
assertNull(clientSessionHolder.get(), "Session closure not signalled");
}
+ @Test
+ void delayedKexInit() throws Exception {
+ CoreModuleProperties.SEND_IMMEDIATE_KEXINIT.set(client, false);
+ AtomicReference<String> serverId = new AtomicReference<>();
+ AtomicReference<Map<KexProposalOption, String>> serverProposal = new
AtomicReference<>();
+ client.setKexExtensionHandler(new DefaultClientKexExtensionHandler() {
+
+ @Override
+ public void handleKexInitProposal(Session session, boolean
initiator, Map<KexProposalOption, String> proposal)
+ throws Exception {
+ serverId.set(session.getServerVersion());
+ serverProposal.set(session.getServerKexProposals());
+ super.handleKexInitProposal(session, initiator, proposal);
+ }
+ });
+ client.start();
+ try (ClientSession session = client.connect("user1", TEST_LOCALHOST,
port).verify(CONNECT_TIMEOUT).getSession()) {
+ session.addPasswordIdentity("user1");
+ session.auth().verify(AUTH_TIMEOUT);
+ } finally {
+ client.stop();
+ }
+ assertNotNull(serverId.get());
+ assertNotNull(serverProposal.get());
+ }
+
@Test
void client() throws Exception {
client.start();
