EdDSA verifiers now reject overly long signatures

- see https://github.com/bcgit/bc-java/issues/508

8 files changed, 85 insertions, 23 deletions

diff --git a/crypto/Readme.html b/crypto/Readme.html
index 40edf0c53..01eddda7c 100644
--- a/crypto/Readme.html
+++ b/crypto/Readme.html
@@ -31,6 +31,8 @@
 				<a href="#mozTocId3413">Notes:</a>
 		<ol>
             <li>
+                <a href="#mozTocId85320">Release 1.8.6</a>
+            <li>
                 <a href="#mozTocId85319">Release 1.8.5</a>
             <li>
                 <a href="#mozTocId85318">Release 1.8.4</a>
@@ -298,11 +300,18 @@ We state, where EC MQV has not otherwise been disabled or removed:
 		<hr style="WIDTH: 100%; HEIGHT: 2px">
 		<h3><a class="mozTocH3" name="mozTocId3413"></a>Notes:</h3>
 
+        <h4><a class="mozTocH4" name="mozTocId85320"></a>Release 1.8.6, TBD</h4>
+
+        <h5>Defects Fixed</h5>
+        <ul>
+            <li>EdDSA verifiers now reject overly long signatures.</li>
+        </ul>
+
         <h4><a class="mozTocH4" name="mozTocId85319"></a>Release 1.8.5, Thursday January 31, 2019</h4>
 
         <h5>Additional Features and Functionality</h5>
         <ul>
-            <li>Supported added for encoding and decoding of GOST3410-2012 keys</li>
+            <li>Support added for encoding and decoding of GOST3410-2012 keys</li>
             <li>Basic support added for CMP (RFC 4210) and CRMF (RFC 4211), including the PKI archive control.</li>
         </ul>
 
diff --git a/crypto/src/crypto/signers/Ed25519Signer.cs b/crypto/src/crypto/signers/Ed25519Signer.cs
index ef8714188..a916601e6 100644
--- a/crypto/src/crypto/signers/Ed25519Signer.cs
+++ b/crypto/src/crypto/signers/Ed25519Signer.cs
@@ -99,6 +99,9 @@ namespace Org.BouncyCastle.Crypto.Signers
 
             internal bool VerifySignature(Ed25519PublicKeyParameters publicKey, byte[] signature)
             {
+                if (Ed25519.SignatureSize != signature.Length)
+                    return false;
+
                 lock (this)
                 {
 #if PORTABLE
diff --git a/crypto/src/crypto/signers/Ed25519ctxSigner.cs b/crypto/src/crypto/signers/Ed25519ctxSigner.cs
index 60c708019..ab7201b62 100644
--- a/crypto/src/crypto/signers/Ed25519ctxSigner.cs
+++ b/crypto/src/crypto/signers/Ed25519ctxSigner.cs
@@ -101,6 +101,9 @@ namespace Org.BouncyCastle.Crypto.Signers
 
             internal bool VerifySignature(Ed25519PublicKeyParameters publicKey, byte[] ctx, byte[] signature)
             {
+                if (Ed25519.SignatureSize != signature.Length)
+                    return false;
+
                 lock (this)
                 {
 #if PORTABLE
diff --git a/crypto/src/crypto/signers/Ed25519phSigner.cs b/crypto/src/crypto/signers/Ed25519phSigner.cs
index 548ca1f29..2538b16f5 100644
--- a/crypto/src/crypto/signers/Ed25519phSigner.cs
+++ b/crypto/src/crypto/signers/Ed25519phSigner.cs
@@ -75,6 +75,8 @@ namespace Org.BouncyCastle.Crypto.Signers
         {
             if (forSigning || null == publicKey)
                 throw new InvalidOperationException("Ed25519phSigner not initialised for verification");
+            if (Ed25519.SignatureSize != signature.Length)
+                return false;
 
             byte[] pk = publicKey.GetEncoded();
             return Ed25519.VerifyPrehash(signature, 0, pk, 0, context, prehash);
diff --git a/crypto/src/crypto/signers/Ed448Signer.cs b/crypto/src/crypto/signers/Ed448Signer.cs
index 0863e5dd1..b0563d544 100644
--- a/crypto/src/crypto/signers/Ed448Signer.cs
+++ b/crypto/src/crypto/signers/Ed448Signer.cs
@@ -101,6 +101,9 @@ namespace Org.BouncyCastle.Crypto.Signers
 
             internal bool VerifySignature(Ed448PublicKeyParameters publicKey, byte[] ctx, byte[] signature)
             {
+                if (Ed448.SignatureSize != signature.Length)
+                    return false;
+
                 lock (this)
                 {
 #if PORTABLE
diff --git a/crypto/src/crypto/signers/Ed448phSigner.cs b/crypto/src/crypto/signers/Ed448phSigner.cs
index 8f451f9e8..d656c1392 100644
--- a/crypto/src/crypto/signers/Ed448phSigner.cs
+++ b/crypto/src/crypto/signers/Ed448phSigner.cs
@@ -75,6 +75,8 @@ namespace Org.BouncyCastle.Crypto.Signers
         {
             if (forSigning || null == publicKey)
                 throw new InvalidOperationException("Ed448phSigner not initialised for verification");
+            if (Ed448.SignatureSize != signature.Length)
+                return false;
 
             byte[] pk = publicKey.GetEncoded();
             return Ed448.VerifyPrehash(signature, 0, pk, 0, context, prehash);
diff --git a/crypto/test/src/crypto/test/Ed25519Test.cs b/crypto/test/src/crypto/test/Ed25519Test.cs
index 82e36d991..c520eac2b 100644
--- a/crypto/test/src/crypto/test/Ed25519Test.cs
+++ b/crypto/test/src/crypto/test/Ed25519Test.cs
@@ -7,6 +7,7 @@ using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Crypto.Signers;
 using Org.BouncyCastle.Math.EC.Rfc8032;
 using Org.BouncyCastle.Security;
+using Org.BouncyCastle.Utilities;
 using Org.BouncyCastle.Utilities.Test;
 
 namespace Org.BouncyCastle.Crypto.Tests
@@ -87,24 +88,43 @@ namespace Org.BouncyCastle.Crypto.Tests
             byte[] signature = signer.GenerateSignature();
 
             ISigner verifier = CreateSigner(algorithm, context);
-            verifier.Init(false, publicKey);
-            verifier.BlockUpdate(msg, 0, msg.Length);
-            bool shouldVerify = verifier.VerifySignature(signature);
 
-            if (!shouldVerify)
             {
-                Fail("Ed25519(" + algorithm + ") signature failed to verify");
+                verifier.Init(false, publicKey);
+                verifier.BlockUpdate(msg, 0, msg.Length);
+                bool shouldVerify = verifier.VerifySignature(signature);
+
+                if (!shouldVerify)
+                {
+                    Fail("Ed25519(" + algorithm + ") signature failed to verify");
+                }
             }
 
-            signature[Random.Next() % signature.Length] ^= (byte)(1 << (Random.NextInt() & 7));
+            {
+                byte[] wrongLengthSignature = Arrays.Append(signature, 0x00);
+
+                verifier.Init(false, publicKey);
+                verifier.BlockUpdate(msg, 0, msg.Length);
+                bool shouldNotVerify = verifier.VerifySignature(wrongLengthSignature);
 
-            verifier.Init(false, publicKey);
-            verifier.BlockUpdate(msg, 0, msg.Length);
-            bool shouldNotVerify = verifier.VerifySignature(signature);
+                if (shouldNotVerify)
+                {
+                    Fail("Ed25519(" + algorithm + ") wrong length signature incorrectly verified");
+                }
+            }
 
-            if (shouldNotVerify)
             {
-                Fail("Ed25519(" + algorithm + ") bad signature incorrectly verified");
+                byte[] badSignature = Arrays.Clone(signature);
+                badSignature[Random.Next() % badSignature.Length] ^= (byte)(1 << (Random.NextInt() & 7));
+
+                verifier.Init(false, publicKey);
+                verifier.BlockUpdate(msg, 0, msg.Length);
+                bool shouldNotVerify = verifier.VerifySignature(badSignature);
+
+                if (shouldNotVerify)
+                {
+                    Fail("Ed25519(" + algorithm + ") bad signature incorrectly verified");
+                }
             }
         }
     }
diff --git a/crypto/test/src/crypto/test/Ed448Test.cs b/crypto/test/src/crypto/test/Ed448Test.cs
index b035f554e..a73292430 100644
--- a/crypto/test/src/crypto/test/Ed448Test.cs
+++ b/crypto/test/src/crypto/test/Ed448Test.cs
@@ -7,6 +7,7 @@ using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Crypto.Signers;
 using Org.BouncyCastle.Math.EC.Rfc8032;
 using Org.BouncyCastle.Security;
+using Org.BouncyCastle.Utilities;
 using Org.BouncyCastle.Utilities.Test;
 
 namespace Org.BouncyCastle.Crypto.Tests
@@ -83,24 +84,43 @@ namespace Org.BouncyCastle.Crypto.Tests
             byte[] signature = signer.GenerateSignature();
 
             ISigner verifier = CreateSigner(algorithm, context);
-            verifier.Init(false, publicKey);
-            verifier.BlockUpdate(msg, 0, msg.Length);
-            bool shouldVerify = verifier.VerifySignature(signature);
 
-            if (!shouldVerify)
             {
-                Fail("Ed448(" + algorithm + ") signature failed to verify");
+                verifier.Init(false, publicKey);
+                verifier.BlockUpdate(msg, 0, msg.Length);
+                bool shouldVerify = verifier.VerifySignature(signature);
+
+                if (!shouldVerify)
+                {
+                    Fail("Ed448(" + algorithm + ") signature failed to verify");
+                }
             }
 
-            signature[Random.Next() % signature.Length] ^= (byte)(1 << (Random.NextInt() & 7));
+            {
+                byte[] wrongLengthSignature = Arrays.Append(signature, 0x00);
+
+                verifier.Init(false, publicKey);
+                verifier.BlockUpdate(msg, 0, msg.Length);
+                bool shouldNotVerify = verifier.VerifySignature(wrongLengthSignature);
 
-            verifier.Init(false, publicKey);
-            verifier.BlockUpdate(msg, 0, msg.Length);
-            bool shouldNotVerify = verifier.VerifySignature(signature);
+                if (shouldNotVerify)
+                {
+                    Fail("Ed448(" + algorithm + ") wrong length signature incorrectly verified");
+                }
+            }
 
-            if (shouldNotVerify)
             {
-                Fail("Ed448(" + algorithm + ") bad signature incorrectly verified");
+                byte[] badSignature = Arrays.Clone(signature);
+                badSignature[Random.Next() % badSignature.Length] ^= (byte)(1 << (Random.NextInt() & 7));
+
+                verifier.Init(false, publicKey);
+                verifier.BlockUpdate(msg, 0, msg.Length);
+                bool shouldNotVerify = verifier.VerifySignature(badSignature);
+
+                if (shouldNotVerify)
+                {
+                    Fail("Ed448(" + algorithm + ") bad signature incorrectly verified");
+                }
             }
         }
     }