diff --git a/pom.xml b/pom.xml index 0a000ed2..d43d7e94 100644 --- a/pom.xml +++ b/pom.xml @@ -52,8 +52,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.7 - 1.7 + 8 + 8 ${project.build.sourceEncoding} 3.8.0 diff --git a/src/net/i2p/crypto/eddsa/EdDSAEngine.java b/src/net/i2p/crypto/eddsa/EdDSAEngine.java index 6b254102..2be450e8 100644 --- a/src/net/i2p/crypto/eddsa/EdDSAEngine.java +++ b/src/net/i2p/crypto/eddsa/EdDSAEngine.java @@ -305,6 +305,16 @@ private boolean x_engineVerify(byte[] sigBytes) throws SignatureException { h = key.getParams().getScalarOps().reduce(h); byte[] Sbyte = Arrays.copyOfRange(sigBytes, b/8, b/4); + + // Malleability, see + // https://datatracker.ietf.org/doc/html/rfc8032#section-3.4 + // https://github.com/novifinancial/ed25519-speccheck + // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-36843 + // + if (!key.getParams().getScalarOps().isValidFactor(Sbyte)) { + return false; + } + // R = SB - H(Rbar,Abar,M)A GroupElement R = key.getParams().getB().doubleScalarMultiplyVariableTime( ((EdDSAPublicKey) key).getNegativeA(), h, Sbyte); diff --git a/src/net/i2p/crypto/eddsa/math/ScalarOps.java b/src/net/i2p/crypto/eddsa/math/ScalarOps.java index 2876ecbd..a0662a77 100644 --- a/src/net/i2p/crypto/eddsa/math/ScalarOps.java +++ b/src/net/i2p/crypto/eddsa/math/ScalarOps.java @@ -31,4 +31,13 @@ public interface ScalarOps { * @return $(a*b + c) \bmod l$ */ public byte[] multiplyAndAdd(byte[] a, byte[] b, byte[] c); + + /** + * Check, whether the given factor is valid and smaller than + * the group order. + * + * @param s A little-endian encoded factor. + * @return Whether 0 ≤ s < $l$. + */ + public boolean isValidFactor(byte[] s); } diff --git a/src/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOps.java b/src/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOps.java index 68169dc8..9258a7a3 100644 --- a/src/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOps.java +++ b/src/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOps.java @@ -34,4 +34,12 @@ public byte[] multiplyAndAdd(byte[] a, byte[] b, byte[] c) { return enc.encode(enc.toBigInteger(a).multiply(enc.toBigInteger(b)).add(enc.toBigInteger(c)).mod(l)); } + @Override + public boolean isValidFactor(byte[] s) { + + BigInteger si = enc.toBigInteger(s); + + return si.signum() >= 0 && si.compareTo(l) < 0; + } + } diff --git a/src/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOps.java b/src/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOps.java index 55b51731..54796b40 100644 --- a/src/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOps.java +++ b/src/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOps.java @@ -25,6 +25,11 @@ */ public class Ed25519ScalarOps implements ScalarOps { + // Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000010"); + private static final byte[] GROUP_ORDER = + new byte[] { -19,-45,-11,92,26,99,18,88,-42,-100,-9,-94,-34,-7,-34,20, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,16 }; + /** * Reduction modulo the group order $q$. *

@@ -690,4 +695,28 @@ public byte[] multiplyAndAdd(byte[] a, byte[] b, byte[] c) { result[31] = (byte) (s11 >> 17); return result; } + + + @Override + public boolean isValidFactor(byte[] s) { + + if (s.length != GROUP_ORDER.length) { + return false; + } + + int sig = 0; + + for (int i = s.length-1;i>=0;--i) { + + int sigi = Integer.signum((s[i] & 0xff) - (GROUP_ORDER[i] & 0xff)); + + if (sig == 0 && sigi != 0) { + sig = sigi; + } + } + + return sig < 0; + } + + } diff --git a/test/net/i2p/crypto/eddsa/EdDSAEngineTest.java b/test/net/i2p/crypto/eddsa/EdDSAEngineTest.java index f9e16cb6..23ae7a10 100644 --- a/test/net/i2p/crypto/eddsa/EdDSAEngineTest.java +++ b/test/net/i2p/crypto/eddsa/EdDSAEngineTest.java @@ -16,11 +16,15 @@ import static org.junit.Assert.assertThat; import java.nio.charset.Charset; +import java.security.KeyFactory; import java.security.MessageDigest; import java.security.PrivateKey; import java.security.PublicKey; import java.security.Signature; import java.security.SignatureException; +import java.security.spec.X509EncodedKeySpec; +import java.text.NumberFormat; +import java.util.Locale; import net.i2p.crypto.eddsa.spec.EdDSANamedCurveTable; import net.i2p.crypto.eddsa.spec.EdDSAParameterSpec; @@ -43,6 +47,13 @@ public class EdDSAEngineTest { static final byte[] TEST_MSG = "This is a secret message".getBytes(Charset.forName("UTF-8")); static final byte[] TEST_MSG_SIG = Utils.hexToBytes("94825896c7075c31bcb81f06dba2bdcd9dcf16e79288d4b9f87c248215c8468d475f429f3de3b4a2cf67fe17077ae19686020364d6d4fa7a0174bab4a123ba0f"); + // case 6 and 7 from https://raw.githubusercontent.com/novifinancial/ed25519-speccheck/refs/heads/main/cases.json + static final byte[] TEST_MSG_MALL = Utils.hexToBytes("85e241a07d148b41e47d62c63f830dc7a6851a0b1f33ae4bb2f507fb6cffec40"); + static final byte[] TEST_PK_MALL = Utils.hexToBytes("442aad9f089ad9e14647b1ef9099a1ff4798d78589e66f28eca69c11f582a623"); + + static final byte[] TEST_MSG_MALL_SIG_OOB1 = Utils.hexToBytes("e96f66be976d82e60150baecff9906684aebb1ef181f67a7189ac78ea23b6c0e547f7690a0e2ddcd04d87dbc3490dc19b3b3052f7ff0538cb68afb369ba3a514"); + static final byte[] TEST_MSG_MALL_SIG_OOB2 = Utils.hexToBytes("8ce5b96c8f26d0ab6c47958c9e68b937104cd36e13c33566acd2fe8d38aa19427e71f98a473474f2f13f06f97c20d58cc3f54b8bd0d272f42b695dd7e89a8c22"); + @Rule public ExpectedException exception = ExpectedException.none(); @@ -216,6 +227,19 @@ public void testVerifyOneShot() throws Exception { assertThat("verifyOneShot() failed", sgr.verifyOneShot(TEST_MSG, TEST_MSG_SIG), is(true)); } + @Test + public void testVerifyMalleable() throws Exception { + EdDSAParameterSpec spec = EdDSANamedCurveTable.getByName(EdDSANamedCurveTable.ED_25519); + EdDSAPublicKeySpec pubKey = new EdDSAPublicKeySpec(TEST_PK_MALL, spec); + EdDSAEngine sgr = new EdDSAEngine(MessageDigest.getInstance(spec.getHashAlgorithm())); + PublicKey vKey = new EdDSAPublicKey(pubKey); + sgr.initVerify(vKey); + + assertThat("verifyOneShot() succeeded unexpectedly", sgr.verifyOneShot(TEST_MSG_MALL, TEST_MSG_MALL_SIG_OOB1), is(false)); + + assertThat("verifyOneShot() succeeded unexpectedly", sgr.verifyOneShot(TEST_MSG_MALL, TEST_MSG_MALL_SIG_OOB2), is(false)); + } + @Test public void testVerifyX509PublicKeyInfo() throws Exception { EdDSAParameterSpec spec = EdDSANamedCurveTable.getByName("Ed25519"); @@ -223,7 +247,16 @@ public void testVerifyX509PublicKeyInfo() throws Exception { for (Ed25519TestVectors.TestTuple testCase : Ed25519TestVectors.testCases) { EdDSAPublicKeySpec pubKey = new EdDSAPublicKeySpec(testCase.pk, spec); PublicKey vKey = new EdDSAPublicKey(pubKey); - PublicKey x509Key = X509Key.parse(new DerValue(vKey.getEncoded())); + + PublicKey x509Key; + if (NumberFormat.getIntegerInstance(Locale.ENGLISH).parse(System.getProperty("java.version")).intValue() <= 11) { + x509Key = X509Key.parse(new DerValue(vKey.getEncoded())); + } + else { + // with java-11+ support for Ed25519 has been added to the JVM. + KeyFactory kf = KeyFactory.getInstance("Ed25519"); + x509Key = kf.generatePublic(new X509EncodedKeySpec(vKey.getEncoded())); + } sgr.initVerify(x509Key); sgr.update(testCase.message); diff --git a/test/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOpsTest.java b/test/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOpsTest.java index 8e8040e4..e4dc8a98 100644 --- a/test/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOpsTest.java +++ b/test/net/i2p/crypto/eddsa/math/bigint/BigIntegerScalarOpsTest.java @@ -48,6 +48,13 @@ public void testReduce() { // Example from test case 1 byte[] r = Utils.hexToBytes("b6b19cd8e0426f5983fa112d89a143aa97dab8bc5deb8d5b6253c928b65272f4044098c2a990039cde5b6a4818df0bfb6e40dc5dee54248032962323e701352d"); assertThat(sc2.reduce(r), is(equalTo(Utils.hexToBytes("f38907308c893deaf244787db4af53682249107418afc2edc58f75ac58a07404")))); + + assertFalse(sc2.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000010"))); + assertFalse(sc2.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de140000000000000000000000000000007f"))); + assertFalse(sc2.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000080"))); + assertTrue(sc2.isValidFactor(Utils.hexToBytes("ecd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000010"))); + assertTrue(sc2.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1300000000000000000000000000000010"))); + assertTrue(sc2.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de140000000000000000000000000000000f"))); } /** diff --git a/test/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOpsTest.java b/test/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOpsTest.java index cc01942f..0233bdd1 100644 --- a/test/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOpsTest.java +++ b/test/net/i2p/crypto/eddsa/math/ed25519/Ed25519ScalarOpsTest.java @@ -20,7 +20,9 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author str4d @@ -41,6 +43,16 @@ public void testReduce() { assertThat(scalarOps.reduce(r), is(equalTo(Utils.hexToBytes("f38907308c893deaf244787db4af53682249107418afc2edc58f75ac58a07404")))); } + @Test + public void testIsValidFactor() { + assertFalse(scalarOps.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000010"))); + assertFalse(scalarOps.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de140000000000000000000000000000007f"))); + assertFalse(scalarOps.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000080"))); + assertTrue(scalarOps.isValidFactor(Utils.hexToBytes("ecd3f55c1a631258d69cf7a2def9de1400000000000000000000000000000010"))); + assertTrue(scalarOps.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de1300000000000000000000000000000010"))); + assertTrue(scalarOps.isValidFactor(Utils.hexToBytes("edd3f55c1a631258d69cf7a2def9de140000000000000000000000000000000f"))); + } + @Test public void reduceReturnsExpectedResult() { for (int i=0; i<1000; i++) {