This is an automated email from the ASF dual-hosted git repository.
lgoldstein 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 37ffa46 [SSHD-1004] Using a more constant time MAC validation to
minimize timing side channel information leak
37ffa46 is described below
commit 37ffa46e5bcf6ded8be3dddfd66fc7a815c1c40e
Author: Lyor Goldstein <[email protected]>
AuthorDate: Mon Aug 10 18:09:04 2020 +0300
[SSHD-1004] Using a more constant time MAC validation to minimize timing
side channel information leak
---
CHANGES.md | 1 +
.../main/java/org/apache/sshd/common/mac/Mac.java | 28 ++++++++++++++++++++++
.../sshd/common/util/buffer/BufferUtils.java | 20 ++++++++++++++++
.../common/session/helpers/AbstractSession.java | 2 +-
4 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/CHANGES.md b/CHANGES.md
index 0dbba59..bd4bae8 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -20,6 +20,7 @@ or `-key-file` command line option.
## Minor code helpers
+* [SSHD-1004](https://issues.apache.org/jira/browse/SSHD-1004) Using a more
constant time MAC validation to minimize timing side channel information leak.
* [SSHD-1030](https://issues.apache.org/jira/browse/SSHD-1030) Added a
NoneFileSystemFactory implementation
* [SSHD-1042](https://issues.apache.org/jira/browse/SSHD-1042) Added more
callbacks to SftpEventListener
* [SSHD-1040](https://issues.apache.org/jira/browse/SSHD-1040) Make server key
available after KEX completed.
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/mac/Mac.java
b/sshd-common/src/main/java/org/apache/sshd/common/mac/Mac.java
index 3af44a7..f5ff7c4 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/mac/Mac.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/mac/Mac.java
@@ -48,4 +48,32 @@ public interface Mac extends MacInformation {
}
void doFinal(byte[] buf, int offset) throws Exception;
+
+ /*
+ * Executes a more-or-less constant time verification in order
+ * to avoid timing side-channel information leak
+ */
+ static boolean equals(byte[] a1, int a1Offset, byte[] a2, int a2Offset,
int length) {
+ int len1 = NumberUtils.length(a1);
+ int len2 = NumberUtils.length(a2);
+ int result = 0;
+
+ if (len1 < (a1Offset + length)) {
+ length = Math.min(length, len1 - a1Offset);
+ length = Math.max(length, 0);
+ result |= 0x00FF;
+ }
+
+ if (len2 < (a2Offset + length)) {
+ length = Math.min(length, len2 - a2Offset);
+ length = Math.max(length, 0);
+ result |= 0xFF00;
+ }
+
+ for (int cmpLen = length; cmpLen > 0; a1Offset++, a2Offset++,
cmpLen--) {
+ result |= a1[a1Offset] ^ a2[a2Offset];
+ }
+
+ return result == 0;
+ }
}
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java
b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java
index a7e3a59..be67be0 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/BufferUtils.java
@@ -522,6 +522,14 @@ public final class BufferUtils {
return Long.BYTES;
}
+ /**
+ * Compares the contents of 2 arrays of bytes - <B>Note:</B> do not use it
to execute security related comparisons
+ * (e.g. MACs) since the method leaks timing information. Use {@code
Mac#equals} method instead.
+ *
+ * @param a1 1st array
+ * @param a2 2nd array
+ * @return {@code true} if all bytes in the compared arrays are equal
+ */
public static boolean equals(byte[] a1, byte[] a2) {
int len1 = NumberUtils.length(a1);
int len2 = NumberUtils.length(a2);
@@ -532,6 +540,18 @@ public final class BufferUtils {
}
}
+ /**
+ * Compares the contents of 2 arrays of bytes - <B>Note:</B> do not use it
to execute security related comparisons
+ * (e.g. MACs) since the method leaks timing information. Use {@code
Mac#equals} method instead.
+ *
+ * @param a1 1st array
+ * @param a1Offset Offset to start comparing in 1st array
+ * @param a2 2nd array
+ * @param a2Offset Offset to start comparing in 2nd array
+ * @param length Number of bytes to compare
+ * @return {@code true} if all bytes in the compared arrays are
equal when compared from the specified
+ * offsets and up to specified length
+ */
@SuppressWarnings("PMD.AssignmentInOperand")
public static boolean equals(byte[] a1, int a1Offset, byte[] a2, int
a2Offset, int length) {
int len1 = NumberUtils.length(a1);
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 dcc9f40..9509d09 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
@@ -1456,7 +1456,7 @@ public abstract class AbstractSession extends
SessionHelper {
inMac.doFinal(inMacResult, 0);
// Check the computed result with the received mac (just after the
packet data)
- if (!BufferUtils.equals(inMacResult, 0, data, offset + len,
inMacSize)) {
+ if (!Mac.equals(inMacResult, 0, data, offset + len, inMacSize)) {
throw new SshException(SshConstants.SSH2_DISCONNECT_MAC_ERROR,
"MAC Error");
}
}
