Merge branch '1844-openpgp-signature-join' into 'main'

1844 Improve comparison logic during signature joining

See merge request root/bc-java!37

Author: dghgit

pg/src/main/java/org/bouncycastle/openpgp/PGPPublicKey.java

@@ -47,7 +47,7 @@ public class PGPPublicKey
     List<List<PGPSignature>> idSigs = new ArrayList<List<PGPSignature>>();
 
     List<PGPSignature> subSigs = null;
-    
+
     private KeyIdentifier keyIdentifier;
     private int keyStrength;
 
@@ -150,7 +150,7 @@ public PGPPublicKey(PublicKeyPacket publicKeyPacket, KeyFingerPrintCalculator fi
         this.publicPk = key.publicPk;
         this.trustPk = trust;
         this.subSigs = subSigs;
-        
+
         this.keyStrength = key.keyStrength;
         this.keyIdentifier = key.keyIdentifier;
     }
@@ -1216,7 +1216,8 @@ private static void joinPgpSignatureList(List<PGPSignature> source,
             for (int i = 0; isNotNull && i < rlt.size(); i++)
             {
                 PGPSignature existingSubSig = (PGPSignature)rlt.get(i);
-                if (PGPSignature.isSignatureEncodingEqual(existingSubSig, copySubSig))
+                if (existingSubSig.getVersion() == copySubSig.getVersion() &&
+                    PGPSignature.isSignatureEncodingEqual(existingSubSig, copySubSig))
                 {
                     found = true;
                     // join existing sig with copy to apply modifications in unhashed subpackets

pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java

@@ -45,7 +45,7 @@ public class PGPSignature
      * This signature type is used to create data signatures.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-binary-signature-type-id-0x">
-     *     RFC9580 - Binary Signature of a Document</a>
+     * RFC9580 - Binary Signature of a Document</a>
      */
     public static final int BINARY_DOCUMENT = 0x00;
 
@@ -56,15 +56,15 @@ public class PGPSignature
      * This signature type is used to create data signatures.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-text-signature-type-id-0x01">
-     *     RFC9580 - Text Signature of a Canonical Document</a>
+     * RFC9580 - Text Signature of a Canonical Document</a>
      */
     public static final int CANONICAL_TEXT_DOCUMENT = 0x01;
 
     /**
      * The signature is made only over its own signature subpackets.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-standalone-signature-type-i">
-     *     RFC9580 - Standalone Signature</a>
+     * RFC9580 - Standalone Signature</a>
      */
     public static final int STAND_ALONE = 0x02;
 
@@ -77,7 +77,7 @@ public class PGPSignature
      * of a third party.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-generic-certification-signa">
-     *     RFC9580 - Generic Certification Signature of a User ID and Public Key Packet</a>
+     * RFC9580 - Generic Certification Signature of a User ID and Public Key Packet</a>
      */
     public static final int DEFAULT_CERTIFICATION = 0x10;
 
@@ -89,7 +89,7 @@ public class PGPSignature
      * of a third party.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-persona-certification-signa">
-     *     RFC9580 - Persona Certification Signature of a User ID and Public Key Packet</a>
+     * RFC9580 - Persona Certification Signature of a User ID and Public Key Packet</a>
      */
     public static final int NO_CERTIFICATION = 0x11;
 
@@ -102,7 +102,7 @@ public class PGPSignature
      * of a third party.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-casual-certification-signat">
-     *     RFC9580 - Casual Certification of a User ID an Public Key Packet</a>
+     * RFC9580 - Casual Certification of a User ID an Public Key Packet</a>
      */
     public static final int CASUAL_CERTIFICATION = 0x12;
 
@@ -114,7 +114,7 @@ public class PGPSignature
      * of a third party.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-positive-certification-sign">
-     *     RFC9580 - Positive Certification Signature of a User ID and Public Key Packet</a>
+     * RFC9580 - Positive Certification Signature of a User ID and Public Key Packet</a>
      */
     public static final int POSITIVE_CERTIFICATION = 0x13;
 
@@ -123,7 +123,7 @@ public class PGPSignature
      * This signature type is used to bind a subkey to the primary key of a certificate.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-subkey-binding-signature-ty">
-     *     RFC9580 - Subkey Binding Signature</a>
+     * RFC9580 - Subkey Binding Signature</a>
      */
     public static final int SUBKEY_BINDING = 0x18;
 
@@ -133,7 +133,7 @@ public class PGPSignature
      * a claim by the subkey, stating that it is in fact a subkey of the primary key.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-primary-key-binding-signatu">
-     *     RFC9580 - Primary Key Binding Signature</a>
+     * RFC9580 - Primary Key Binding Signature</a>
      */
     public static final int PRIMARYKEY_BINDING = 0x19;
 
@@ -144,23 +144,23 @@ public class PGPSignature
      * Issued as a signature over a third-party certificate, it can be used to mark said certificate as a CA.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-direct-key-signature-type-i">
-     *     RFC9580 - Direct Key Signature</a>
+     * RFC9580 - Direct Key Signature</a>
      */
     public static final int DIRECT_KEY = 0x1f;
 
     /**
      * The signature is used to revoke a primary key (and in turn the whole certificate with all its subkeys).
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-key-revocation-signature-ty">
-     *     RFC9580 - Key Revocation Signature</a>
+     * RFC9580 - Key Revocation Signature</a>
      */
     public static final int KEY_REVOCATION = 0x20;
 
     /**
      * The signature is used to revoke the binding of a particular subkey.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-subkey-revocation-signature">
-     *     RFC9580 - Subkey Revocation Signature</a>
+     * RFC9580 - Subkey Revocation Signature</a>
      */
     public static final int SUBKEY_REVOCATION = 0x28;
 
@@ -172,15 +172,15 @@ public class PGPSignature
      * Issued over a third-party certificate, it revokes the attestation of the third-party's claim.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-certification-revocation-si">
-     *     RFC9580 - Certification Revocation Signature</a>
+     * RFC9580 - Certification Revocation Signature</a>
      */
     public static final int CERTIFICATION_REVOCATION = 0x30;
 
     /**
      * The signature is only meaningful for the timestamp contained in it.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-timestamp-signature-type-id">
-     *     RFC9580 - Timestamp Signature</a>
+     * RFC9580 - Timestamp Signature</a>
      */
     public static final int TIMESTAMP = 0x40;
 
@@ -191,7 +191,7 @@ public class PGPSignature
      * to prevent certificate flooding.
      *
      * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-third-party-confirmation-si">
-     *     RFC9580 - Third-Party Confirmation Signature/a>
+     * RFC9580 - Third-Party Confirmation Signature/a>
      */
     public static final int THIRD_PARTY_CONFIRMATION = 0x50;
 
@@ -212,6 +212,7 @@ private static SignaturePacket cast(Packet packet)
 
     /**
      * Parse a {@link PGPSignature} from an OpenPGP packet input stream.
+     *
      * @param pIn packet input stream
      * @throws IOException
      * @throws PGPException
@@ -302,7 +303,7 @@ public boolean isCertification()
      * Initialize the signature for verification.
      *
      * @param verifierBuilderProvider provide the implementation for signature verification
-     * @param pubKey issuer public key
+     * @param pubKey                  issuer public key
      * @throws PGPException
      */
     public void init(PGPContentVerifierBuilderProvider verifierBuilderProvider, PGPPublicKey pubKey)
@@ -334,7 +335,7 @@ PGPContentVerifierBuilder createVerifierProvider(PGPContentVerifierBuilderProvid
         return verifierBuilderProvider.get(sigPck.getKeyAlgorithm(), sigPck.getHashAlgorithm());
     }
 
-    void init(PGPContentVerifier verifier) 
+    void init(PGPContentVerifier verifier)
         throws PGPException
     {
         this.verifier = verifier;
@@ -362,7 +363,7 @@ private void checkSaltSize()
     }
 
     private void updateWithSalt()
-            throws PGPException
+        throws PGPException
     {
         if (getVersion() == SignaturePacket.VERSION_6)
         {
@@ -382,6 +383,7 @@ private void updateWithSalt()
      * Note: The fact that this method returned <pre>true</pre> does not yet mean that the signature is valid.
      * A correct signature may very well be expired, the issuer key may be revoked, etc.
      * All these constraints are not checked by this method.
+     *
      * @return true if the signature is correct
      * @throws PGPException
      */
@@ -599,10 +601,10 @@ boolean doVerifyCertification(
 
     /**
      * Return the type id of the signature.
-     * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-signature-types">
-     *     RFC9580 - Signature Types</a>
      *
      * @return type id
+     * @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#name-signature-types">
+     * RFC9580 - Signature Types</a>
      */
     public int getSignatureType()
     {
@@ -670,13 +672,13 @@ private List<KeyIdentifier> extractKeyIdentifiers(SignatureSubpacket[] subpacket
             SignatureSubpacket s = subpackets[idx];
             if (s instanceof IssuerFingerprint)
             {
-                IssuerFingerprint issuer = (IssuerFingerprint) s;
+                IssuerFingerprint issuer = (IssuerFingerprint)s;
                 identifiers.add(new KeyIdentifier(issuer.getFingerprint()));
             }
 
             if (s instanceof IssuerKeyID)
             {
-                IssuerKeyID issuer = (IssuerKeyID) s;
+                IssuerKeyID issuer = (IssuerKeyID)s;
                 identifiers.add(new KeyIdentifier(issuer.getKeyID()));
             }
         }
@@ -747,6 +749,7 @@ private PGPSignatureSubpacketVector createSubpacketVector(SignatureSubpacket[] p
 
     /**
      * Return the salt of a v6 signature.
+     *
      * @return salt
      */
     byte[] getSalt()
@@ -757,6 +760,7 @@ byte[] getSalt()
     /**
      * Return the cryptographic raw signature contained in the OpenPGP signature packet.
      * The value is dependent on the signing algorithm.
+     *
      * @return cryptographic signature
      * @throws PGPException
      */
@@ -815,6 +819,7 @@ else if (getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY)
 
     /**
      * Return the OpenPGP packet encoding of the signature.
+     *
      * @return OpenPGP packet encoding
      * @throws IOException
      */
@@ -848,6 +853,7 @@ public byte[] getEncoded(boolean forTransfer)
     /**
      * Encode the signature to an OpenPGP packet stream.
      * This method does not strip out any trust packets.
+     *
      * @param outStream packet stream
      * @throws IOException
      */
@@ -901,6 +907,7 @@ public static boolean isCertification(int signatureType)
 
     /**
      * Return true, if the cryptographic signature encoding of the two signatures match.
+     *
      * @param sig1 first signature
      * @param sig2 second signature
      * @return true if both signatures contain the same cryptographic signature
@@ -916,6 +923,7 @@ public static boolean isSignatureEncodingEqual(PGPSignature sig1, PGPSignature s
      * (e.g. an embedded {@link #THIRD_PARTY_CONFIRMATION} signature), an implementation might want to
      * join an existing instance of a signature with an updated copy, e.g. retrieved from a key server.
      * This method merges both signature instances by joining unhashed subpackets.
+     *
      * @param sig1 first signature
      * @param sig2 second signature
      * @return merged signature
@@ -924,7 +932,14 @@ public static boolean isSignatureEncodingEqual(PGPSignature sig1, PGPSignature s
     public static PGPSignature join(PGPSignature sig1, PGPSignature sig2)
         throws PGPException
     {
-        if (!isSignatureEncodingEqual(sig1, sig2))
+        if (sig1.getVersion() < SignaturePacket.VERSION_4)
+        {
+            // Version 2/3 signatures have no subpackets, so don't need to get merged.
+            return sig1;
+        }
+
+        if (sig1.getVersion() != sig2.getVersion() ||
+            !isSignatureEncodingEqual(sig1, sig2))
         {
             throw new IllegalArgumentException("These are different signatures.");
         }