Pintuzoft opened a new issue, #952:
URL: https://github.com/apache/mina-sshd/issues/952
### Version
3.0.0-M7 (same code on `dev_3.0`)
### Bug description
Rarely, a client sends its first key exchange message (type 30) before its
own `SSH_MSG_KEXINIT`.
`IdentFilter.WriteHandler.send()` queues the KEXINIT behind the
identification line. When that write completes, the listener releases the queue
*before* it passes the KEXINIT on:
```java
queue.addListener(f -> {
lastWrite.compareAndSet(result, null); // queue released
if (f.isWritten()) {
owner().send(cmd, message)... // KEXINIT passed on
```
`KexFilter.sendKexInit()` counts the KEXINIT as sent when `forward.send()`
returns, not when it is written. So the negotiation can send 30 in that gap; it
finds `lastWrite == null` and goes out first.
Reproducer, using only sshd-core, sshd-common and slf4j-api. It prints `[-1,
30, 20]` (-1 is the identification line):
<details><summary>IdentFilterOrder.java</summary>
```java
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.apache.sshd.common.PropertyResolver;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.filter.DefaultFilterChain;
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.session.filters.IdentFilter;
import org.apache.sshd.common.session.filters.SshIdentHandler;
import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
// Chain [wire, moment, IdentFilter, top]. KEXINIT is queued behind the
identification line; when it is
// passed on, "moment" (right below IdentFilter) lets another thread send 30
from the top first.
public class IdentFilterOrder {
static final List<Integer> wire = Collections.synchronizedList(new
ArrayList<>());
static final DefaultIoWriteFuture identWritten = new
DefaultIoWriteFuture("ident", null);
static IoFilter top;
static Buffer packet(int cmd) {
ByteArrayBuffer buffer = new ByteArrayBuffer();
buffer.putByte((byte) cmd);
return buffer;
}
static IoFilter filter(OutputHandler out) {
return new IoFilter() {
@Override
public InputHandler in() {
return null;
}
@Override
public OutputHandler out() {
return out;
}
};
}
public static void main(String[] args) throws Exception {
IoFilter socket = filter((cmd, message) -> {
wire.add(cmd);
if (cmd < 0) {
return identWritten; // the identification line, written
when we say so
}
DefaultIoWriteFuture written = new DefaultIoWriteFuture(cmd,
null);
written.setValue(Boolean.TRUE);
return written;
});
IoFilter[] moment = new IoFilter[1];
moment[0] = filter((cmd, message) -> {
if (cmd == SshConstants.SSH_MSG_KEXINIT) {
Thread other = new Thread(() -> {
try {
top.owner().send(SshConstants.SSH_MSG_KEX_FIRST,
packet(SshConstants.SSH_MSG_KEX_FIRST));
} catch (Exception e) {
throw new RuntimeException(e);
}
});
other.start();
try {
other.join();
} catch (InterruptedException e) {
throw new java.io.InterruptedIOException();
}
}
return moment[0].owner().send(cmd, message);
});
IdentFilter ident = new IdentFilter();
ident.setPropertyResolver(PropertyResolver.EMPTY);
ident.setIdentHandler(new SshIdentHandler() {
@Override
public boolean isServer() {
return false;
}
@Override
public List<String> readIdentification(Buffer buffer) {
return List.of();
}
@Override
public List<String> provideIdentification() {
return List.of("SSH-2.0-IdentFilterOrder");
}
});
top = filter(null);
DefaultFilterChain chain = new DefaultFilterChain();
chain.addLast(socket);
chain.addLast(moment[0]);
chain.addLast(ident);
chain.addLast(top);
top.owner().send(SshConstants.SSH_MSG_KEXINIT,
packet(SshConstants.SSH_MSG_KEXINIT)); // queued
identWritten.setValue(Boolean.TRUE);
System.out.println(wire); // expected [-1, 20, 30]
}
}
```
</details>
### Actual behavior
Identification, 30, KEXINIT. OpenSSH with strict KEX answers the early 30
with `SSH_MSG_UNIMPLEMENTED`, and the client fails:
```
org.apache.sshd.common.SshException: SSH_MSG_UNIMPLEMENTED not allowed
during initial key exchange in strict KEX
at
org.apache.sshd.common.session.filters.kex.KexFilter$KexInputHandler.handleMessage(KexFilter.java:1216)
```
### Expected behavior
KEXINIT is the first packet after the identification line.
### Other information
Peer: OpenSSH 9.2p1 (Debian bookworm). Seen once in a night of CI tests, and
once in 6000 connections with a plain `SshClient.setUpDefaultClient()` against
a server that sends its KEXINIT at once.
Possible fix: release the queue after the message is passed on. With this,
the reproducer prints `[-1, 20, 30]` and 2000 handshakes and logins against an
`SshServer` work:
```diff
queue.addListener(f -> {
- lastWrite.compareAndSet(result, null);
if (f.isWritten()) {
...
}
+ lastWrite.compareAndSet(result, null);
});
```
Found and analysed with Claude (Anthropic's AI assistant, Claude Opus 5.5).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
