diff --git a/pg/src/main/java/org/bouncycastle/openpgp/api/AbstractOpenPGPDocumentSignatureGenerator.java b/pg/src/main/java/org/bouncycastle/openpgp/api/AbstractOpenPGPDocumentSignatureGenerator.java index 4111697472..241c8e1b2b 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/api/AbstractOpenPGPDocumentSignatureGenerator.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/api/AbstractOpenPGPDocumentSignatureGenerator.java @@ -251,7 +251,7 @@ protected PGPSignatureGenerator initSignatureGenerator( } return Utils.getPgpSignatureGenerator(implementation, signingKey.getPGPPublicKey(), - unlockedKey.getPrivateKey(), parameters, null, null); + unlockedKey.getPrivateKey(), parameters, parameters.getSignatureCreationTime(), null); } private int getPreferredHashAlgorithm(OpenPGPCertificate.OpenPGPComponentKey key) diff --git a/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPCertificate.java b/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPCertificate.java index 19b954afa2..aeb8d4d434 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPCertificate.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPCertificate.java @@ -804,17 +804,17 @@ private void processSubkey(OpenPGPSubkey subkey) } OpenPGPSignatureChains issuerChains = getAllSignatureChainsFor(issuer); - if (!issuerChains.chains.isEmpty()) + if (issuerChains.chains.isEmpty()) + { + subkeyChains.add(OpenPGPSignatureChain.direct(sig)); + } + else { for (Iterator it2 = issuerChains.chains.iterator(); it2.hasNext(); ) { subkeyChains.add(it2.next().plus(sig)); } } - else - { - subkeyChains.add(new OpenPGPSignatureChain(OpenPGPSignatureChain.Link.create(sig))); - } } this.componentSignatureChains.put(subkey, subkeyChains); } @@ -2870,21 +2870,24 @@ public static class OpenPGPSignatureChain implements Comparable, Iterable { private final List chainLinks = new ArrayList(); + private final OpenPGPPolicy policy; - private OpenPGPSignatureChain(Link rootLink) + private OpenPGPSignatureChain(Link rootLink, OpenPGPPolicy policy) { this.chainLinks.add(rootLink); + this.policy = policy; } - private OpenPGPSignatureChain(List links) + private OpenPGPSignatureChain(List links, OpenPGPPolicy policy) { this.chainLinks.addAll(links); + this.policy = policy; } // copy constructor private OpenPGPSignatureChain(OpenPGPSignatureChain copy) { - this(copy.chainLinks); + this(copy.chainLinks, copy.policy); } /** @@ -2958,7 +2961,7 @@ public OpenPGPSignatureChain plus(OpenPGPComponentSignature sig) */ public static OpenPGPSignatureChain direct(OpenPGPComponentSignature sig) { - return new OpenPGPSignatureChain(Link.create(sig)); + return new OpenPGPSignatureChain(Link.create(sig), sig.target.certificate.policy); } /** @@ -3062,6 +3065,17 @@ public boolean isHardRevocation() * @return most recent signature creation time */ public Date getSince() + { + return policy.getComponentSignatureEffectivenessEvaluator() + .getSignatureChainValidityPeriodBeginning(this); + } + + /** + * Return the signature creation time of the most recent signature in the chain. + * + * @return most recent signature creation time in chain + */ + public Date getMostRecentLinkCreationTime() { Date latestDate = null; for (Iterator it = chainLinks.iterator(); it.hasNext(); ) @@ -3076,6 +3090,19 @@ public Date getSince() } return latestDate; } + + /** + * Return the key creation time of the component signatures target key. + * In certificates composed of a primary key and subkeys, this is sufficient, + * since subkeys MUST NOT predate the primary key. + * + * @return most recent + */ + public Date getTargetKeyCreationTime() + { + return getLeafLinkTargetKey().getCreationTime(); + } + // public Date getSince() // { // // Find most recent chain link @@ -3400,6 +3427,16 @@ public OpenPGPComponentSignature getSignature() { return signature; } + + /** + * Return the issuer key of this link. + * + * @return issuer + */ + public OpenPGPComponentKey getIssuer() + { + return getSignature().getIssuer(); + } } /** @@ -3646,4 +3683,83 @@ private void addSignaturesToChains(List signatures, O chains.add(OpenPGPSignatureChain.direct(it.next())); } } + + /** + * Delegate for component signature evaluation. + * The introduction of PQC in OpenPGP makes it desirable to allow for cleanup of historic binding signatures, + * since PQC signatures are rather large, so accumulating them can lead to very large certificates. + * The classic model of component signature evaluation evaluates the complete history of component binding + * signatures when evaluating the validity of a certificate component + * (see {@link #strictlyTemporallyConstrainedSignatureEvaluator()}). + * Removing old signatures with the classic evaluation model can lead to historic document- or certification + * signatures to suddenly become invalid. + * Therefore, we need a way to swap out the evaluation method by introducing this delegate, which can have + * different concrete implementations. + */ + public interface ComponentSignatureEvaluator { + Date getSignatureChainValidityPeriodBeginning(OpenPGPSignatureChain chain); + } + + /** + * This {@link ComponentSignatureEvaluator} strictly constraints the temporal validity of component signatures. + * This behavior is consistent with most OpenPGP implementations, but might lead to "temporal holes". + * When evaluating the validity of a component at evaluation time N, we ignore all binding signatures + * made after N and check if the latest binding before N is not yet expired at N. + * Hard revocations at any time invalidate the component. + * Soft revocations only invalidate the component if they are made before N, not yet expired at N and not yet + * overwritten by a valid binding newer than the revocation. + *

+ * The problem with this method of evaluation is, that it can lead to temporal holes when historic self signatures + * are removed from the certificate (e.g. in order to reduce its size). + * Removing all but the latest bindings will render the key invalid for document signatures made before the latest + * bindings. + * @return component signature evaluator consistent with legacy implementations + * + * @see + * OpenPGP Interoperability Test Suite - Temporary validity + + */ + public static ComponentSignatureEvaluator strictlyTemporallyConstrainedSignatureEvaluator() { + return new ComponentSignatureEvaluator() { + @Override + public Date getSignatureChainValidityPeriodBeginning(OpenPGPSignatureChain chain) { + return chain.getMostRecentLinkCreationTime(); + } + }; + } + + /** + * This {@link ComponentSignatureEvaluator} allows for retroactive validation of historic document- or + * third-party signatures via new component binding signatures. + * Compared to the implementation in {@link #strictlyTemporallyConstrainedSignatureEvaluator()}, + * this implementation prevents the issue of "temporal holes" and is therefore better suited for + * modern OpenPGP implementations where signatures are frequently cleaned up (e.g. PQC keys with + * large signatures). + *

+ * This evaluator considers a component valid at time N iff + *

    + *
  • the latest binding signature exists and does not predate the component key itself
    • + *
  • the latest binding signature is not yet expired at N
    • + *
  • the component key was created before or at N
    • + *
  • if there is a soft-revocation created after the latest binding; the revocation is + * expired at N
    • + *
  • the component is not hard-revoked
    • + *
+ * This implementation ensures that when superseded binding signatures are removed from a certificate, + * historic document signatures remain valid. + * Note though, that this method may render the certificate valid for historic periods where the certificate + * was purposefully temporarily invalidated by expiring self-signatures. + * + * @return component signature history evaluator which performs a simplified evaluation, fixing temporal holes + * @see + * OpenPGP Mailing List - PQC requires urgent semantic cleanup + */ + public static ComponentSignatureEvaluator retroactivelyTemporallyRevalidatingSignatureEvaluator() { + return new ComponentSignatureEvaluator() { + @Override + public Date getSignatureChainValidityPeriodBeginning(OpenPGPSignatureChain chain) { + return chain.getTargetKeyCreationTime(); + } + }; + } } diff --git a/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPDefaultPolicy.java b/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPDefaultPolicy.java index b751ce27ee..9821861cf5 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPDefaultPolicy.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPDefaultPolicy.java @@ -19,6 +19,7 @@ public class OpenPGPDefaultPolicy private int defaultDocumentSignatureHashAlgorithm = HashAlgorithmTags.SHA512; private int defaultCertificationSignatureHashAlgorithm = HashAlgorithmTags.SHA512; private int defaultSymmetricKeyAlgorithm = SymmetricKeyAlgorithmTags.AES_128; + private OpenPGPCertificate.ComponentSignatureEvaluator componentSignatureEvaluator; public OpenPGPDefaultPolicy() { @@ -80,6 +81,13 @@ public OpenPGPDefaultPolicy() acceptPublicKeyAlgorithm(PublicKeyAlgorithmTags.X448); acceptPublicKeyAlgorithm(PublicKeyAlgorithmTags.Ed25519); acceptPublicKeyAlgorithm(PublicKeyAlgorithmTags.Ed448); + + /* + * Certificate component signature evaluation + */ + // Strictly constrain the temporal validity of component signatures during signature evaluation. + // This is consistent with legacy OpenPGP implementations. + applyStrictTemporalComponentSignatureValidityConstraints(); } public OpenPGPDefaultPolicy rejectHashAlgorithm(int hashAlgorithmId) @@ -212,6 +220,57 @@ public boolean isAcceptablePublicKeyStrength(int publicKeyAlgorithmId, int bitSt return isAcceptable(publicKeyAlgorithmId, bitStrength, publicKeyMinimalBitStrengths); } + @Override + public OpenPGPCertificate.ComponentSignatureEvaluator getComponentSignatureEffectivenessEvaluator() { + return componentSignatureEvaluator; + } + + public OpenPGPDefaultPolicy setComponentSignatureEffectivenessEvaluator( + OpenPGPCertificate.ComponentSignatureEvaluator componentSignatureEvaluator) + { + this.componentSignatureEvaluator = componentSignatureEvaluator; + return this; + } + + /** + * When evaluating a document signature or third-party certification issued at time t1, + * only consider component binding signatures made at t1 or prior and reject component + * binding signatures made at t2+. + * This behavior is consistent with OpenPGP implementations, but might break historical + * document signatures and third-party certifications if old component signatures are + * cleaned from the certificate (temporal holes). + * You can prevent temporal holes with {@link #allowRetroactiveComponentSignatureValidation()}. + *

+ * This behavior is currently the default. + * + * @return policy + */ + public OpenPGPDefaultPolicy applyStrictTemporalComponentSignatureValidityConstraints() + { + return setComponentSignatureEffectivenessEvaluator( + OpenPGPCertificate.strictlyTemporallyConstrainedSignatureEvaluator()); + } + + /** + * When evaluating a document signature or third-party certification issued at time t1, + * also consider component binding signatures created at t2+. + * This behavior prevents historical document or certification signatures from breaking + * if older binding signatures are cleaned from the issuer certificate. + * Since PQC signatures may quickly blow up the certificate in size, it is desirable to + * clean up old signatures every once in a while and allowing retroactive validation of + * historic signatures via new component signatures prevents temporal holes. + *

+ * Calling this will overwrite the - currently default - behavior from + * {@link #applyStrictTemporalComponentSignatureValidityConstraints()}. + * + * @return policy + */ + public OpenPGPDefaultPolicy allowRetroactiveComponentSignatureValidation() + { + return setComponentSignatureEffectivenessEvaluator( + OpenPGPCertificate.retroactivelyTemporallyRevalidatingSignatureEvaluator()); + } + @Override public OpenPGPNotationRegistry getNotationRegistry() { diff --git a/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPPolicy.java b/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPPolicy.java index e8f870a479..d5015ea3d3 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPPolicy.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPPolicy.java @@ -321,4 +321,11 @@ public void addKnownNotation(String notationName) this.knownNotations.add(notationName); } } + + /** + * The {@link OpenPGPCertificate.ComponentSignatureEvaluator} delegate defines, how component signatures on + * {@link OpenPGPCertificate OpenPGPCertificates} are being evaluated. + * @return delegate for component signature evaluation + */ + OpenPGPCertificate.ComponentSignatureEvaluator getComponentSignatureEffectivenessEvaluator(); } diff --git a/pg/src/test/java/org/bouncycastle/openpgp/api/test/OpenPGPCertificateTest.java b/pg/src/test/java/org/bouncycastle/openpgp/api/test/OpenPGPCertificateTest.java index 7fbd93f5a2..66c399a611 100644 --- a/pg/src/test/java/org/bouncycastle/openpgp/api/test/OpenPGPCertificateTest.java +++ b/pg/src/test/java/org/bouncycastle/openpgp/api/test/OpenPGPCertificateTest.java @@ -1,26 +1,40 @@ package org.bouncycastle.openpgp.api.test; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Date; +import java.util.Iterator; import java.util.List; import org.bouncycastle.bcpg.ArmoredInputStream; import org.bouncycastle.bcpg.BCPGInputStream; +import org.bouncycastle.bcpg.HashAlgorithmTags; import org.bouncycastle.bcpg.KeyIdentifier; import org.bouncycastle.bcpg.SignatureSubpacketTags; import org.bouncycastle.bcpg.sig.Features; import org.bouncycastle.bcpg.sig.KeyFlags; import org.bouncycastle.openpgp.OpenPGPTestKeys; import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPKeyPair; import org.bouncycastle.openpgp.PGPObjectFactory; +import org.bouncycastle.openpgp.PGPPrivateKey; +import org.bouncycastle.openpgp.PGPPublicKey; +import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPSignature; +import org.bouncycastle.openpgp.PGPSignatureGenerator; import org.bouncycastle.openpgp.PGPSignatureList; import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator; import org.bouncycastle.openpgp.api.OpenPGPApi; import org.bouncycastle.openpgp.api.OpenPGPCertificate; +import org.bouncycastle.openpgp.api.OpenPGPDefaultPolicy; import org.bouncycastle.openpgp.api.OpenPGPKey; import org.bouncycastle.openpgp.api.OpenPGPKeyGenerator; +import org.bouncycastle.openpgp.api.OpenPGPMessageGenerator; +import org.bouncycastle.openpgp.api.OpenPGPMessageInputStream; +import org.bouncycastle.openpgp.api.OpenPGPMessageOutputStream; import org.bouncycastle.openpgp.api.SignatureParameters; import org.bouncycastle.openpgp.api.SignatureSubpacketsFunction; import org.bouncycastle.openpgp.api.util.UTCUtil; @@ -49,6 +63,8 @@ protected void performTestWith(OpenPGPApi api) testSKSignsPKRevokedNoSubpacket(api); testPKSignsPKRevocationSuperseded(api); testGetPrimaryUserId(api); + + testHistoricSignatureEvaluationWithCleanedCertificate(api); } private void testOpenPGPv6Key(OpenPGPApi api) @@ -826,6 +842,160 @@ public PGPSignatureSubpacketGenerator apply(PGPSignatureSubpacketGenerator subpa key.getPrimaryUserId(oneHourAgo)); } + private void testHistoricSignatureEvaluationWithCleanedCertificate(OpenPGPApi api) + throws PGPException, IOException { + Date t0 = UTCUtil.parse("2024-01-01 00:00:00 UTC"); + Date t1 = UTCUtil.parse("2024-01-02 00:00:00 UTC"); + Date t2 = UTCUtil.parse("2024-01-03 00:00:00 UTC"); + + String userId = "Alice "; + // Create key at t0 + OpenPGPKey initialKey = api.generateKey(4, t0) + .withPrimaryKey() + .addSigningSubkey() + .addUserId(userId) + .build(); + OpenPGPCertificate initialCert = initialKey.toCertificate(); + + // Generate message at t1 + OpenPGPMessageGenerator mGen = api.signAndOrEncryptMessage() + .addSigningKey(initialKey, new SignatureParameters.Callback() { + @Override + public SignatureParameters apply(SignatureParameters parameters) { + return parameters.setSignatureCreationTime(t1); + } + }); + ByteArrayOutputStream bOut = new ByteArrayOutputStream(); + OpenPGPMessageOutputStream mOut = mGen.open(bOut); + mOut.write("Hello, World!\n".getBytes(StandardCharsets.UTF_8)); + mOut.close(); + byte[] historicMessage = bOut.toByteArray(); + System.out.println(bOut.toString()); + + // Message is valid with initial certificate + ByteArrayInputStream bIn = new ByteArrayInputStream(historicMessage); + OpenPGPMessageInputStream mIn = api.decryptAndOrVerifyMessage() + .addVerificationCertificate(initialCert) + .process(bIn); + org.bouncycastle.util.io.Streams.drain(mIn); + mIn.close(); + isTrue(mIn.getResult().getSignatures().get(0).isValid()); + + // Create new key signatures at t2 and strip t0 signatures + List strippedKeys = new ArrayList<>(); + for (PGPPublicKey k : initialCert.getPGPPublicKeyRing()) + { + PGPPublicKey cleaned = PGPPublicKey.removeCertification(k, userId); + if (cleaned == null) + { + cleaned = k; + } + Iterator sigs = cleaned.getSignatures(); + while (sigs.hasNext()) { + PGPSignature sig = sigs.next(); + cleaned = PGPPublicKey.removeCertification(cleaned, sig); + } + strippedKeys.add(cleaned); + } + System.out.println(new OpenPGPCertificate(new PGPPublicKeyRing(strippedKeys)).toAsciiArmoredString()); + + List updateKeys = new ArrayList<>(); + PGPKeyPair primaryKey = initialKey.getPrimarySecretKey().unlock().getKeyPair(); + Iterator strippedIterator = strippedKeys.iterator(); + PGPPublicKey updatedPrimaryKey = strippedIterator.next(); + + // Reissue direct-key sig at t2 + PGPSignatureGenerator sGen = new PGPSignatureGenerator( + api.getImplementation().pgpContentSignerBuilder( + primaryKey.getPublicKey().getAlgorithm(), + HashAlgorithmTags.SHA3_512), + primaryKey.getPublicKey()); + PGPSignatureSubpacketGenerator spGen = new PGPSignatureSubpacketGenerator(); + spGen.setIssuerFingerprint(true, primaryKey.getPublicKey()); + spGen.setSignatureCreationTime(true, t2); + spGen.setKeyFlags(KeyFlags.CERTIFY_OTHER); + sGen.setHashedSubpackets(spGen.generate()); + sGen.init(PGPSignature.DIRECT_KEY, primaryKey.getPrivateKey()); + updatedPrimaryKey = PGPPublicKey.addCertification( + updatedPrimaryKey, + sGen.generateCertification(updatedPrimaryKey)); + + // reissue userid sig at t2 + sGen.init(PGPSignature.POSITIVE_CERTIFICATION, primaryKey.getPrivateKey()); + updatedPrimaryKey = PGPPublicKey.addCertification( + updatedPrimaryKey, + userId, + sGen.generateCertification(userId, updatedPrimaryKey)); + + updateKeys.add(updatedPrimaryKey); + + // reissue signature subkey binding at t2 + PGPPublicKey signingKey = strippedIterator.next(); + PGPPrivateKey privateSigningKey = initialKey.getSecretKey(signingKey.getKeyIdentifier()) + .unlock().getKeyPair().getPrivateKey(); + + PGPSignatureGenerator backSigGen = new PGPSignatureGenerator( + api.getImplementation().pgpContentSignerBuilder(signingKey.getAlgorithm(), HashAlgorithmTags.SHA3_512), + signingKey); + PGPSignatureSubpacketGenerator backSigSubPacketGen = new PGPSignatureSubpacketGenerator(); + backSigSubPacketGen.setSignatureCreationTime(t2); + backSigSubPacketGen.setIssuerFingerprint(true, signingKey); + backSigGen.setHashedSubpackets(backSigSubPacketGen.generate()); + backSigGen.init(PGPSignature.PRIMARYKEY_BINDING, privateSigningKey); + PGPSignature backSig = backSigGen.generateCertification(updatedPrimaryKey, signingKey); + + spGen = new PGPSignatureSubpacketGenerator(); + spGen.setIssuerFingerprint(true, updatedPrimaryKey); + spGen.setSignatureCreationTime(true, t2); + spGen.setKeyFlags(KeyFlags.SIGN_DATA); + spGen.addEmbeddedSignature(true, backSig); + sGen.setHashedSubpackets(spGen.generate()); + sGen.init(PGPSignature.SUBKEY_BINDING, primaryKey.getPrivateKey()); + + signingKey = PGPPublicKey.addCertification(signingKey, sGen.generateCertification(updatedPrimaryKey, signingKey)); + updateKeys.add(signingKey); + + // Reassemble update key + PGPPublicKeyRing updatedKeyRing = new PGPPublicKeyRing(updateKeys); + + // Check that with complete history evaluation, historic signature is now no longer valid + OpenPGPDefaultPolicy fullHistoryEvaluation = new OpenPGPDefaultPolicy() + .applyStrictTemporalComponentSignatureValidityConstraints(); + OpenPGPCertificate updatedCert = new OpenPGPCertificate( + updatedKeyRing, api.getImplementation(), fullHistoryEvaluation); + isFalse("With full history eval, primary key MUST NOT be bound at t1", + updatedCert.getPrimaryKey().isBoundAt(t1)); + isFalse("With full history eval, signing key MUST NOT be bound at t1", + updatedCert.getSigningKeys().get(0).isBoundAt(t1)); + + mIn = api.decryptAndOrVerifyMessage() + .addVerificationCertificate(updatedCert) + .process(new ByteArrayInputStream(historicMessage)); + org.bouncycastle.util.io.Streams.drain(mIn); + mIn.close(); + isFalse("With full history eval, historic message MUST NOT be validly signed by updated key", + mIn.getResult().getSignatures().get(0).isValid()); + + // Check that with simplified history evaluation, historic signatures remain valid + OpenPGPDefaultPolicy simplifiedHistoryEvaluation = new OpenPGPDefaultPolicy() + .allowRetroactiveComponentSignatureValidation(); + updatedCert = new OpenPGPCertificate( + updatedKeyRing, api.getImplementation(), simplifiedHistoryEvaluation); + isTrue("With simplified history eval, primary key MUST be bound at t1", + updatedCert.getPrimaryKey() + .isBoundAt(t1)); + isTrue("With simplified history eval, signing key MUST be bound at t1", + updatedCert.getSigningKeys().get(0).isBoundAt(t1)); + + mIn = api.decryptAndOrVerifyMessage() + .addVerificationCertificate(updatedCert) + .process(new ByteArrayInputStream(historicMessage)); + org.bouncycastle.util.io.Streams.drain(mIn); + mIn.close(); + isTrue("With simplified history eval, historic message MUST be validly signed by updated key", + mIn.getResult().getSignatures().get(0).isValid()); + } + public static class TestSignature { private final PGPSignature signature;