SignatureEd25519: fix vulnerability in net.i2p.crypto:eddsa:0.3.0

net.i2p.crypto:eddsa:0.3.0 has a bug and omits a crucial check in
signature verification. This was reported as CVE-2020-36843[1]
(signature malleability).

The problem affects signature verification, but luckily the missing
check can also be performed outside of the library. With this commit
we do so and verify that the second 32 bytes of the signature is
actually in the range [0..L),[2] with L the order as given in RFC
7748.[3]

This means that the use of net.i2p.crypto:eddsa:0.3.0, if used for
ed25519 signatures via Apache MINA SSHD, is safe and **not** subject of
CVE-2020-36843. Of course, vulnerability scanners will still report the
vulnerability.

Note that Apache MINA SSHD has only a completely optional dependency on
net.i2p.crypto:eddsa:0.3.0. If that artifact is not present in the
application using Apache MINA SSHD, Apache MINA SSHD will still work. In
that case, ed25519 is supported via Bouncy Castle.

[1] https://www.cve.org/CVERecord?id=CVE-2020-36843
[2] https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.7
[3] https://www.rfc-editor.org/rfc/rfc7748.html#section-4.1

Author: tomaswolf

CHANGES.md

@@ -34,6 +34,9 @@
 
 ## New Features
 
+* [GH-814](https://github.com/apache/mina-sshd/pull/814) Include a fix for CVE-2020-36843 in optional dependency net.i2p.crypto:eddsa:0.3.0: perform the missing range check in Apache MINA SSHD before delegating to the signature verification in net.i2p.crypto:eddsa:0.3.0. This means that using net.i2p.crypto:eddsa:0.3.0 in Apache MINA SSHD is
+safe despite that CVE in the dependency.
+
 ## Potential Compatibility Issues
 
 ## Major Code Re-factoring

sshd-common/src/main/java/org/apache/sshd/common/util/security/eddsa/SignatureEd25519.java

@@ -18,6 +18,8 @@
  */
 package org.apache.sshd.common.util.security.eddsa;
 
+import java.security.SignatureException;
+
 import net.i2p.crypto.eddsa.EdDSAEngine;
 import org.apache.sshd.common.util.security.eddsa.generic.GenericSignatureEd25519;
 
@@ -27,7 +29,44 @@
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public class SignatureEd25519 extends GenericSignatureEd25519 {
+
+    // See https://www.rfc-editor.org/rfc/rfc7748.html#section-4.1
+    // 2^252 + 0x14def9dea2f79cd65812631a5cf5d3ed; little-endian
+    private static final int[] ED25519_ORDER = { //
+            0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58, //
+            0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14, //
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, //
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10 //
+    };
+
     public SignatureEd25519() {
         super(EdDSAEngine.SIGNATURE_ALGORITHM);
     }
+
+    @Override
+    protected boolean doVerify(byte[] data) throws SignatureException {
+        // Fix CVE 2020-36843 in net.i2p.crypto.eddsa 0.3.0: check that s is in the range [0 .. L), where
+        // L is the order.
+        //
+        // Note: Wikipedia says 0 < S < L. https://en.wikipedia.org/w/index.php?title=EdDSA&oldid=1304068429
+        // RFC 8032 says 0 <= S < L. https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.7
+        //
+        // We stick to RFC 8032 here.
+        if (data.length != 64 || !isValidFactor(data)) {
+            return false;
+        }
+        return super.doVerify(data);
+    }
+
+    private static boolean isValidFactor(byte[] sig) {
+        // Must be strictly smaller than the field order (little-endian).
+        for (int i = 31; i >= 0; i--) {
+            int y = (sig[i + 32] & 0xFF) - ED25519_ORDER[i];
+            if (y != 0) {
+                return y < 0;
+            }
+        }
+        return false;
+    }
+
 }