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 f6f50a6653ceb709fbf6880efaddbcf89788f8cb
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sun Apr 6 12:53:40 2025 +0200
Use long for packet sequence numbers
Sequence numbers in SSH are unsigned 32 values. But using int as storage
leads to difficult code, since version 2.X used long and now it's too
easy to miss a place where the value would have to be extended manually
using & 0xFFFF_FFFFL. So use int only internally in CryptFilter
(facilitates updating atomically with automatic wrap-around), but use
long everywhere else. Provide an explicit getLastInputSequenceNumber()
method.
---
.../apache/sshd/common/session/filters/CryptFilter.java | 15 ++++++++++-----
.../common/session/filters/CryptStatisticsProvider.java | 17 +++++++++++++----
.../common/session/filters/PacketLoggingFilter.java | 5 ++---
.../sshd/common/session/filters/SshTransportFilter.java | 8 ++++++--
.../sshd/common/session/filters/kex/KexFilter.java | 14 +++++++-------
.../sshd/common/session/helpers/AbstractSession.java | 6 ++----
6 files changed, 40 insertions(+), 25 deletions(-)
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptFilter.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptFilter.java
index 3dbc1c305..b196d0e40 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptFilter.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptFilter.java
@@ -159,13 +159,18 @@ public class CryptFilter extends IoFilter implements
CryptStatisticsProvider {
}
@Override
- public int getInputSequenceNumber() {
- return input.sequenceNumber.get();
+ public long getLastInputSequenceNumber() {
+ return (input.sequenceNumber.get() - 1) & 0xFFFF_FFFFL;
}
@Override
- public int getOutputSequenceNumber() {
- return output.sequenceNumber.get();
+ public long getInputSequenceNumber() {
+ return input.sequenceNumber.get() & 0xFFFF_FFFFL;
+ }
+
+ @Override
+ public long getOutputSequenceNumber() {
+ return output.sequenceNumber.get() & 0xFFFF_FFFFL;
}
@Override
@@ -371,7 +376,7 @@ public class CryptFilter extends IoFilter implements
CryptStatisticsProvider {
Buffer encrypted = message;
if (encrypted != null) {
try {
- listeners.forEach(listener ->
listener.aboutToEncrypt(message, sequenceNumber.get()));
+ listeners.forEach(listener ->
listener.aboutToEncrypt(message, getOutputSequenceNumber()));
encrypted = encode(cmd, message);
} catch (IOException e) {
throw e;
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptStatisticsProvider.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptStatisticsProvider.java
index 5fd9a78b0..d3728bc64 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptStatisticsProvider.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/CryptStatisticsProvider.java
@@ -20,16 +20,25 @@ package org.apache.sshd.common.session.filters;
public interface CryptStatisticsProvider {
+ /**
+ * Retrieves the previous input sequence number.
+ * <p>
+ * This is the sequence number of the last received packet.
+ * </p>
+ *
+ * @return the sequence number as an unsigned 32bit value.
+ */
+ long getLastInputSequenceNumber();
+
/**
* Retrieves the current input sequence number.
* <p>
- * This is the sequence number expected for the next packet. To get the
sequence number of the <em>current</em>
- * packet, subtract one.
+ * This is the sequence number expected for the next packet.
* </p>
*
* @return the sequence number as an unsigned 32bit value.
*/
- int getInputSequenceNumber();
+ long getInputSequenceNumber();
/**
* Retrieves the current output sequence number.
@@ -39,7 +48,7 @@ public interface CryptStatisticsProvider {
*
* @return the sequence number as an unsigned 32bit value.
*/
- int getOutputSequenceNumber();
+ long getOutputSequenceNumber();
/**
* Retrieves the input counters.
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/PacketLoggingFilter.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/PacketLoggingFilter.java
index a57fad789..4272693c2 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/filters/PacketLoggingFilter.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/filters/PacketLoggingFilter.java
@@ -50,17 +50,16 @@ public class PacketLoggingFilter extends IoFilter {
private final OutputHandler output;
- public PacketLoggingFilter(Session session, CryptFilter crypt) {
+ public PacketLoggingFilter(Session session, CryptStatisticsProvider crypt)
{
Objects.requireNonNull(session);
Objects.requireNonNull(crypt);
this.input = new BufferInputHandler() {
@Override
public void handleMessage(Buffer message) throws Exception {
- int sequenceNumber = crypt.getInputSequenceNumber() - 1;
if (LOG.isTraceEnabled()) {
message.dumpHex(SIMPLE, Level.FINEST,
- "receivePacket(" + session + ") packet #" +
sequenceNumber, session);
+ "receivePacket(" + session + ") packet #" +
crypt.getLastInputSequenceNumber(), session);
}
owner().passOn(message);
}
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 e22dec21c..b6c742298 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
@@ -163,11 +163,15 @@ public class SshTransportFilter extends IoFilter {
return cryptFilter.isSecure();
}
- public int getInputSequenceNumber() {
+ public long getLastInputSequenceNumber() {
+ return cryptFilter.getLastInputSequenceNumber();
+ }
+
+ public long getInputSequenceNumber() {
return cryptFilter.getInputSequenceNumber();
}
- public int getOutputSequenceNumber() {
+ public long getOutputSequenceNumber() {
return cryptFilter.getOutputSequenceNumber();
}
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 36c15e04b..09a19719e 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
@@ -234,7 +234,7 @@ public class KexFilter extends IoFilter {
private volatile boolean strictKex;
- private volatile int initialKexInitSequenceNumber;
+ private volatile long initialKexInitSequenceNumber;
public KexFilter(AbstractSession session, Random random, CryptFilter
crypt, CompressionFilter compression,
SessionListener listener, Proposer proposer,
HostKeyChecker checker) {
@@ -373,7 +373,7 @@ public class KexFilter extends IoFilter {
"KEX: received SSH_MSG_KEXINIT while already in KEX");
}
if (!initialKexDone) {
- initialKexInitSequenceNumber = crypt.getInputSequenceNumber();
+ initialKexInitSequenceNumber = crypt.getLastInputSequenceNumber();
}
parsePeerProposal(message);
if (starting == KexStart.PEER) {
@@ -605,11 +605,11 @@ public class KexFilter extends IoFilter {
LOG.debug("negotiate({}) strict KEX={} client={}
server={}", session, strictKex, strictKexClient,
strictKexServer);
}
- if (strictKex && initialKexInitSequenceNumber != 1) {
+ if (strictKex && initialKexInitSequenceNumber != 0) {
throw new
SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
MessageFormat.format(
"KEX: strict KEX negotiated but there were
{0} messages before the first SSH_MSG_KEXINIT",
- initialKexInitSequenceNumber - 1));
+ initialKexInitSequenceNumber));
}
}
KexExtensionHandler extHandler = session.getKexExtensionHandler();
@@ -1108,7 +1108,7 @@ public class KexFilter extends IoFilter {
private abstract class WithSequenceNumber {
- private int initialSequenceNumber;
+ private long initialSequenceNumber;
private boolean first = true;
@@ -1122,8 +1122,8 @@ public class KexFilter extends IoFilter {
}
if (first) {
first = false;
- initialSequenceNumber = crypt.getInputSequenceNumber();
- } else if (initialSequenceNumber ==
crypt.getInputSequenceNumber()) {
+ initialSequenceNumber = crypt.getLastInputSequenceNumber();
+ } else if (initialSequenceNumber ==
crypt.getLastInputSequenceNumber()) {
throw new
SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
"Incoming sequence number wraps around during initial
KEX");
}
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index a38a5edab..1213458c1 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -456,7 +456,7 @@ public abstract class AbstractSession extends SessionHelper
{
protected void handleMessage(Buffer buffer) throws Exception {
int cmd = buffer.getUByte();
if (log.isDebugEnabled()) {
- log.debug("doHandleMessage({}) process #{} {}", this,
sshTransport.getInputSequenceNumber() - 1,
+ log.debug("doHandleMessage({}) process #{} {}", this,
sshTransport.getLastInputSequenceNumber(),
SshConstants.getCommandMessageName(cmd));
}
@@ -926,9 +926,7 @@ public abstract class AbstractSession extends SessionHelper
{
if (doInvokeUnimplementedMessageHandler(cmd, buffer)) {
return null;
}
-
- int seq = sshTransport.getInputSequenceNumber() - 1;
- return sendNotImplemented(seq & 0xFFFF_FFFFL);
+ return sendNotImplemented(sshTransport.getLastInputSequenceNumber());
}
/**
