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
commit b4ee57647d1fa2fef9d6d98895d20515581e80fa
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Mon Jun 3 20:06:47 2024 +0200
Harden CBC cipher handling
Include the mitigations implemented in OpenSSH for a particular kind of
attack against CBC ciphers.[1][2]
The attack is thwarted by making disconnections due to a bad packet
length or due to a failing MAC check indistinguishable. When we detect
a bad packet length, we substitute a random packet length instead and
keep on requesting data, so an attacker cannot learn anything about the
initial 32bits of a packet. We then continue normally, until the MAC
check fails. In the very unlikely case that it doesn't fail, we
disconnect unconditionally after the MAC check. The disconnection
messages are identical in all these cases.
A note on the commit message of commit edb2b951: the attack described
in [2] is feasible only within one and the same SSH connection, where it
has a probability of 2**-14 or 2**-18 to learn the first 14 or 32 bits
of a captured 16-byte encrypted block that is re-injected later in that
same connection. (I had originally misunderstood their description.) If
that block is injected in a different SSH connection with different
keys, nothing is revealed. As such it makes sense to include a
mitigation for this even if CBC modes are not recommended in SSH, since
we do still provide the implementation to enable users to connect to
legacy systems.
The authors of [2] later published a follow-up paper [3] in which they
claimed the OpenSSH mitigation was still vulnerable via a timing side
channel. This timing weakness should not exist in the mitigation
included here.
[1] https://www.openssh.com/txt/cbc.adv
[2]
https://www.cs.umd.edu/~jkatz/security/downloads/PlaintextRecoverySSH.pdf
[3]
https://pure.royalholloway.ac.uk/ws/portalfiles/portal/27577238/surfeit.pdf
---
.../common/session/helpers/AbstractSession.java | 32 +++++++++++++++++++++-
.../sshd/common/session/helpers/SessionHelper.java | 19 +++++++++++--
2 files changed, 47 insertions(+), 4 deletions(-)
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 4dfb8e620..c62b28eaf 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
@@ -194,6 +194,7 @@ public abstract class AbstractSession extends SessionHelper
{
protected final SessionWorkBuffer decoderBuffer;
protected int decoderState;
protected int decoderLength;
+ protected SshException discarding;
protected final Object encodeLock = new Object();
protected final Object decodeLock = new Object();
protected final Object requestLock = new Object();
@@ -1597,13 +1598,37 @@ public abstract class AbstractSession extends
SessionHelper {
* Check packet length validity - we allow 8 times the
minimum required packet length support in
* order to be aligned with some OpenSSH versions that
allow up to 256k
*/
+ boolean lengthOK = true;
if ((decoderLength < SshConstants.SSH_PACKET_HEADER_LEN)
|| (decoderLength > (8 *
SshConstants.SSH_REQUIRED_PAYLOAD_PACKET_LENGTH_SUPPORT))) {
log.warn("decode({}) Error decoding packet(invalid
length): {}", this, decoderLength);
+ lengthOK = false;
+ } else if (inCipher != null
+ && ((decoderLength + ((authMode || etmMode) ? 0 :
Integer.BYTES)) % inCipherSize != 0)) {
+ log.warn("decode({}) Error decoding packet(padding;
not multiple of {}): {}", this, inCipherSize,
+ decoderLength);
+ lengthOK = false;
+ }
+ if (!lengthOK) {
decoderBuffer.dumpHex(getSimplifiedLogger(),
Level.FINEST,
"decode(" + this + ") invalid length packet",
this);
- throw new
SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR,
+ // Mitigation against CVE-2008-5161 AKA CPNI-957037:
make any disconnections due to decoding errors indistinguishable.
+ //
+ // If we disconnect here, a client may still deduce
(since it sent only one block) that the length check failed.
+ // So we keep on requesting more data and fail later.
OpenSSH actually discards the next 256kB of data, but in fact
+ // any number of bytes will do.
+ //
+ // Remember the exception, continue requiring an
arbitrary number of bytes, and throw the exception later.
+ discarding = new
SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR,
"Invalid packet length: " + decoderLength);
+ decoderLength = decoderBuffer.available() + (2 +
random.random(20)) * inCipherSize;
+ // Next larger multiple of the block size
+ decoderLength = ((decoderLength + (inCipherSize - 1))
/ inCipherSize) * inCipherSize;
+ if (!authMode && !etmMode) {
+ decoderLength -= Integer.BYTES;
+ }
+ log.warn("decode({}) Invalid packet length; requesting
{} bytes before disconnecting", this,
+ decoderLength - decoderBuffer.available());
}
// Ok, that's good, we can go to the next step
decoderState = 1;
@@ -1648,6 +1673,11 @@ public abstract class AbstractSession extends
SessionHelper {
validateIncomingMac(data, 0, decoderLength +
Integer.BYTES);
}
+ // Mitigation against CVE-2008-5161 AKA CPNI-957037. But
is is highly unlikely that we pass the AAD or MAC checks above.
+ if (discarding != null) {
+ throw new
SshException(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR, discarding);
+ }
+
// Increment incoming packet sequence number
seqi = (seqi + 1L) & 0x0ffffffffL;
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
index 29764c4ae..55ae75ea2 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
@@ -1186,9 +1186,22 @@ public abstract class SessionHelper extends
AbstractKexFactoryManager implements
signalDisconnect(reason, msg, languageTag, true);
Buffer buffer = createBuffer(SshConstants.SSH_MSG_DISCONNECT,
msg.length() + Short.SIZE);
- buffer.putInt(reason);
- buffer.putString(msg);
- buffer.putString("");
+ // Mitigation against CVE-2008-5161 AKA CPNI-957037: make any
disconnections due to decoding errors
+ // indistinguishable from failed MAC checks.
+ switch (reason) {
+ case SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR:
+ case SshConstants.SSH2_DISCONNECT_MAC_ERROR:
+ // OpenSSH *always* sends back DISCONNECT_PROTOCOL_ERROR
+ buffer.putInt(SshConstants.SSH2_DISCONNECT_PROTOCOL_ERROR);
+ // Yes, we don't tell the peer what exactly was wrong.
+ buffer.putString("Protocol error or corrupt packet");
+ break;
+ default:
+ buffer.putInt(reason);
+ buffer.putString(msg);
+ break;
+ }
+ buffer.putString(languageTag);
// Write the packet with a timeout to ensure a timely close of the
session
// in case the consumer does not read packets anymore.
