Fix signature verification for Ed25519, Ed448 by avoiding MPI encoding

pgpainless · Apr 24, 2024 · e8dd0a8 · e8dd0a8
1 parent 96dce35
commit e8dd0a8
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 42 deletions.
diff --git a/pg/src/main/java/org/bouncycastle/bcpg/SignaturePacket.java b/pg/src/main/java/org/bouncycastle/bcpg/SignaturePacket.java
@@ -258,36 +258,12 @@ private void parseSignature(BCPGInputStream in) throws IOException {
                 signature[2] = y;
                 break;
             case Ed448:
-                if (version == VERSION_6)
-                {
-                    signatureEncoding = new byte[114];
-                    in.readFully(signatureEncoding);
-                }
-                else // TODO: Tag Ed448 should not be used for non-v6 implementations, those use EDDSA_LEGACY
-                {
-                    MPInteger    ecR = new MPInteger(in);
-                    MPInteger    ecS = new MPInteger(in);
-
-                    signature = new MPInteger[2];
-                    signature[0] = ecR;
-                    signature[1] = ecS;
-                }
+                signatureEncoding = new byte[org.bouncycastle.math.ec.rfc8032.Ed448.SIGNATURE_SIZE];
+                in.readFully(signatureEncoding);
                 break;
             case Ed25519:
-                if (version == VERSION_6)
-                {
-                    signatureEncoding = new byte[64];
-                    in.readFully(signatureEncoding);
-                }
-                else // TODO: Tag Ed25519 should not be used for non-v6 implementations, those use EDDSA_LEGACY
-                {
-                    MPInteger    ecR = new MPInteger(in);
-                    MPInteger    ecS = new MPInteger(in);
-
-                    signature = new MPInteger[2];
-                    signature[0] = ecR;
-                    signature[1] = ecS;
-                }
+                signatureEncoding = new byte[org.bouncycastle.math.ec.rfc8032.Ed25519.SIGNATURE_SIZE];
+                in.readFully(signatureEncoding);
                 break;
             case ECDSA:
             case EDDSA_LEGACY:
@@ -389,6 +365,34 @@ public SignaturePacket(
         }
     }
 
+    public SignaturePacket(
+            int                     version,
+            int                     signatureType,
+            long                    keyID,
+            int                     keyAlgorithm,
+            int                     hashAlgorithm,
+            SignatureSubpacket[]    hashedData,
+            SignatureSubpacket[]    unhashedData,
+            byte[]                  fingerPrint,
+            byte[]                  signatureEncoding)
+    {
+        super(SIGNATURE);
+
+        this.version = version;
+        this.signatureType = signatureType;
+        this.keyID = keyID;
+        this.keyAlgorithm = keyAlgorithm;
+        this.hashAlgorithm = hashAlgorithm;
+        this.hashedData = hashedData;
+        this.unhashedData = unhashedData;
+        this.fingerPrint = fingerPrint;
+        this.signatureEncoding = Arrays.clone(signatureEncoding);
+        if (hashedData != null)
+        {
+            setCreationTime();
+        }
+    }
+
     /**
      * get the version number
      */

diff --git a/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java b/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java
@@ -451,23 +451,14 @@ public byte[] getSignature()
             {
                 signature = BigIntegers.asUnsignedByteArray(sigValues[0].getValue());
             }
-            else if (getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY ||
-                getKeyAlgorithm() == PublicKeyAlgorithmTags.Ed25519)
+            else if (getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY)
             {
                 byte[] a = BigIntegers.asUnsignedByteArray(sigValues[0].getValue());
                 byte[] b = BigIntegers.asUnsignedByteArray(sigValues[1].getValue());
                 signature = new byte[Ed25519.SIGNATURE_SIZE];
                 System.arraycopy(a, 0, signature, Ed25519.PUBLIC_KEY_SIZE - a.length, a.length);
                 System.arraycopy(b, 0, signature, Ed25519.SIGNATURE_SIZE - b.length, b.length);
             }
-            else if (getKeyAlgorithm() == PublicKeyAlgorithmTags.Ed448)
-            {
-                byte[] a = BigIntegers.asUnsignedByteArray(sigValues[0].getValue());
-                byte[] b = BigIntegers.asUnsignedByteArray(sigValues[1].getValue());
-                signature = new byte[Ed448.SIGNATURE_SIZE];
-                System.arraycopy(a, 0, signature, Ed448.PUBLIC_KEY_SIZE - a.length, a.length);
-                System.arraycopy(b, 0, signature, Ed448.SIGNATURE_SIZE - b.length, b.length);
-            }
             else
             {
                 try

diff --git a/pg/src/main/java/org/bouncycastle/openpgp/PGPSignatureGenerator.java b/pg/src/main/java/org/bouncycastle/openpgp/PGPSignatureGenerator.java
@@ -178,16 +178,20 @@ public PGPSignature generate()
             sigValues = new MPInteger[1];
             sigValues[0] = new MPInteger(new BigInteger(1, contentSigner.getSignature()));
         }
-        else if (contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY ||
-            contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.Ed25519 ||
-            contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.Ed448)
+        else if (contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY)
         {
             byte[] enc = contentSigner.getSignature();
             sigValues = new MPInteger[]{
                 new MPInteger(new BigInteger(1, Arrays.copyOfRange(enc, 0, enc.length / 2))),
                 new MPInteger(new BigInteger(1, Arrays.copyOfRange(enc, enc.length / 2, enc.length)))
             };
         }
+        else if (contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.Ed25519 ||
+            contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.Ed448)
+        {
+            // Contrary to EDDSA_LEGACY, the new PK algorithms Ed25519, Ed448 do not use MPI encoding
+            sigValues = null;
+        }
         else
         {
             sigValues = PGPUtil.dsaSigToMpi(contentSigner.getSignature());
@@ -199,7 +203,14 @@ else if (contentSigner.getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY
         fingerPrint[0] = digest[0];
         fingerPrint[1] = digest[1];
 
-        return new PGPSignature(new SignaturePacket(sigType, contentSigner.getKeyID(), contentSigner.getKeyAlgorithm(), contentSigner.getHashAlgorithm(), hPkts, unhPkts, fingerPrint, sigValues));
+        if (sigValues != null) {
+            return new PGPSignature(new SignaturePacket(sigType, contentSigner.getKeyID(), contentSigner.getKeyAlgorithm(),
+                    contentSigner.getHashAlgorithm(), hPkts, unhPkts, fingerPrint, sigValues));
+        } else {
+            // Ed25519, Ed448 use raw encoding instead of MPI
+            return new PGPSignature(new SignaturePacket(4, sigType, contentSigner.getKeyID(), contentSigner.getKeyAlgorithm(),
+                    contentSigner.getHashAlgorithm(), hPkts, unhPkts, fingerPrint, contentSigner.getSignature()));
+        }
     }
 
     /**