PGPOnePassSignature: When verifying against signature: Compare salt v…

…alue
bcgit · Jul 2, 2024 · 95331b4 · 95331b4
1 parent 18152f9
commit 95331b4
Showing 4 changed files with 79 additions and 0 deletions.
diff --git a/pg/src/main/java/org/bouncycastle/openpgp/PGPOnePassSignature.java b/pg/src/main/java/org/bouncycastle/openpgp/PGPOnePassSignature.java
@@ -13,6 +13,7 @@
 import org.bouncycastle.openpgp.operator.PGPContentVerifier;
 import org.bouncycastle.openpgp.operator.PGPContentVerifierBuilder;
 import org.bouncycastle.openpgp.operator.PGPContentVerifierBuilderProvider;
+import org.bouncycastle.util.Arrays;
 
 /**
  * A one pass signature object.
@@ -113,6 +114,8 @@ public boolean verify(
         PGPSignature pgpSig)
         throws PGPException
     {
+        compareSalt(pgpSig);
+
         try
         {
             sigOut.write(pgpSig.getSignatureTrailer());
@@ -127,6 +130,19 @@ public boolean verify(
         return verifier.verify(pgpSig.getSignature());
     }
 
+    private void compareSalt(PGPSignature signature)
+            throws PGPException
+    {
+        if (version != SignaturePacket.VERSION_6)
+        {
+            return;
+        }
+        if (!Arrays.constantTimeAreEqual(getSalt(), signature.getSalt()))
+        {
+            throw new PGPException("Salt in OnePassSignaturePacket does not match salt in SignaturePacket.");
+        }
+    }
+
     /**
      * Return the packet version.
      *

diff --git a/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java b/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java
@@ -470,6 +470,11 @@ private PGPSignatureSubpacketVector createSubpacketVector(SignatureSubpacket[] p
         return null;
     }
 
+    byte[] getSalt()
+    {
+        return sigPck.getSalt();
+    }
+
     public byte[] getSignature()
         throws PGPException
     {

diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/PGPV6SignatureTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/PGPV6SignatureTest.java
@@ -71,6 +71,7 @@ public void performTest()
 
         verifyingSignatureWithMismatchedSaltSizeFails();
         verifyingOPSWithMismatchedSaltSizeFails();
+        verifyingInlineSignatureWithSignatureSaltValueMismatchFails();
     }
 
     private void verifyV6DirectKeySignatureTestVector()
@@ -292,6 +293,59 @@ private void verifyingOPSWithMismatchedSaltSizeFails()
         }
     }
 
+    private void verifyingInlineSignatureWithSignatureSaltValueMismatchFails()
+            throws IOException, PGPException
+    {
+        String ARMORED_MSG = "-----BEGIN PGP MESSAGE-----\n" +
+                "\n" +
+                "xEYGAQobIMcgFZRFzyKmYrqqNES9B0geVN5TZ6Wct6aUrITCuFyeyxhsTwYJppfk\n" +
+                "1S36bHIrDB8eJ8GKVnCPZSXsJ7rZrMkAyxR1AAAAAABIZWxsbywgV29ybGQhCsKY\n" +
+                "BgEbCgAAACkioQbLGGxPBgmml+TVLfpscisMHx4nwYpWcI9lJewnutmsyQWCZoJv\n" +
+                "WQAAAAAkFSAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAItltzKi2NN+\n" +
+                "XNJISXQ0X0f4TppBoHbpmwc5YCTIv2+vDZPI+tjzXL9m2e1jrqqaUMEwQ+Zy8B+K\n" +
+                "LC4rA6Gh2gY=\n" +
+                "=KRD3\n" +
+                "-----END PGP MESSAGE-----";
+
+        ByteArrayInputStream bIn = new ByteArrayInputStream(ARMORED_KEY.getBytes(StandardCharsets.UTF_8));
+        ArmoredInputStream aIn = new ArmoredInputStream(bIn);
+        BCPGInputStream pIn = new BCPGInputStream(aIn);
+        PGPObjectFactory objFac = new BcPGPObjectFactory(pIn);
+        PGPSecretKeyRing secretKeys = (PGPSecretKeyRing) objFac.nextObject();
+        PGPPublicKey signingPubKey = secretKeys.getPublicKey();
+
+        bIn = new ByteArrayInputStream(ARMORED_MSG.getBytes(StandardCharsets.UTF_8));
+        aIn = new ArmoredInputStream(bIn);
+        pIn = new BCPGInputStream(aIn);
+        objFac = new BcPGPObjectFactory(pIn);
+
+        PGPOnePassSignatureList opsList = (PGPOnePassSignatureList) objFac.nextObject();
+        PGPOnePassSignature ops = opsList.get(0);
+        isEncodingEqual("OPS salt MUST match our expectations.",
+                Hex.decode("C720159445CF22A662BAAA3444BD07481E54DE5367A59CB7A694AC84C2B85C9E"),
+                ops.getSalt());
+
+        ops.init(new BcPGPContentVerifierBuilderProvider(), signingPubKey);
+
+        PGPLiteralData lit = (PGPLiteralData) objFac.nextObject();
+        ByteArrayOutputStream plainOut = new ByteArrayOutputStream();
+        Streams.pipeAll(lit.getDataStream(), plainOut);
+
+        ops.update(plainOut.toByteArray());
+        PGPSignatureList sigList = (PGPSignatureList) objFac.nextObject();
+        PGPSignature sig = sigList.get(0);
+
+        try
+        {
+            ops.verify(sig);
+            fail("Verifying signature with mismatched salt MUST fail.");
+        }
+        catch (PGPException e)
+        {
+            // expected
+        }
+    }
+
     public static void main(String[] args)
     {
         runTest(new PGPV6SignatureTest());

diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/SQTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/SQTest.java
@@ -0,0 +1,4 @@
+package org.bouncycastle.openpgp.test;
+
+public class SQTest {
+}