From 5a9b8a2c50cb5298b6a799729d1d8d642aaf683c Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Wed, 18 Aug 2021 12:55:24 +0200 Subject: [PATCH] Verify notBefore and notAfter on any message signatures --- .../DecryptionStream.java | 8 +- .../DecryptionStreamFactory.java | 2 +- .../SignatureVerifyingInputStream.java | 31 +-- .../signature/SignatureChainValidator.java | 28 +++ .../VerifyNotBeforeNotAfterTest.java | 204 ++++++++++++++++++ 5 files changed, 247 insertions(+), 26 deletions(-) create mode 100644 pgpainless-core/src/test/java/org/pgpainless/decryption_verification/VerifyNotBeforeNotAfterTest.java diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStream.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStream.java index 42c58252..c04229b2 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStream.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStream.java @@ -15,6 +15,8 @@ */ package org.pgpainless.decryption_verification; +import static org.pgpainless.signature.SignatureValidator.verifySignatureCreationTimeIsInBounds; + import java.io.IOException; import java.io.InputStream; import java.util.List; @@ -39,15 +41,18 @@ public class DecryptionStream extends InputStream { private static final Logger LOGGER = Logger.getLogger(DecryptionStream.class.getName()); private final InputStream inputStream; + private final ConsumerOptions options; private final OpenPgpMetadata.Builder resultBuilder; private boolean isClosed = false; private List integrityProtectedInputStreamList; private final InputStream armorStream; - DecryptionStream(@Nonnull InputStream wrapped, @Nonnull OpenPgpMetadata.Builder resultBuilder, + DecryptionStream(@Nonnull InputStream wrapped, @Nonnull ConsumerOptions options, + @Nonnull OpenPgpMetadata.Builder resultBuilder, List integrityProtectedInputStreamList, InputStream armorStream) { this.inputStream = wrapped; + this.options = options; this.resultBuilder = resultBuilder; this.integrityProtectedInputStreamList = integrityProtectedInputStreamList; this.armorStream = armorStream; @@ -99,6 +104,7 @@ public class DecryptionStream extends InputStream { private void maybeVerifyDetachedSignatures() { for (DetachedSignature s : resultBuilder.getDetachedSignatures()) { try { + verifySignatureCreationTimeIsInBounds(options.getVerifyNotBefore(), options.getVerifyNotAfter()).verify(s.getSignature()); boolean verified = SignatureChainValidator.validateSignature(s.getSignature(), (PGPPublicKeyRing) s.getSigningKeyRing(), PGPainless.getPolicy()); s.setVerified(verified); } catch (SignatureValidationException e) { diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java index d3044d42..73cb0a6d 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/DecryptionStreamFactory.java @@ -143,7 +143,7 @@ public final class DecryptionStreamFactory { } } - return new DecryptionStream(inputStream, factory.resultBuilder, factory.integrityProtectedStreams, + return new DecryptionStream(inputStream, options, factory.resultBuilder, factory.integrityProtectedStreams, (decoderStream instanceof ArmoredInputStream) ? decoderStream : null); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/SignatureVerifyingInputStream.java b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/SignatureVerifyingInputStream.java index d6e7e91b..902c80cd 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/SignatureVerifyingInputStream.java +++ b/pgpainless-core/src/main/java/org/pgpainless/decryption_verification/SignatureVerifyingInputStream.java @@ -15,8 +15,6 @@ */ package org.pgpainless.decryption_verification; -import static org.pgpainless.signature.SignatureValidator.signatureIsEffective; -import static org.pgpainless.signature.SignatureValidator.signatureStructureIsAcceptable; import static org.pgpainless.signature.SignatureValidator.verifySignatureCreationTimeIsInBounds; import java.io.FilterInputStream; @@ -30,7 +28,6 @@ import javax.annotation.Nonnull; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPObjectFactory; -import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPSignatureList; import org.pgpainless.PGPainless; @@ -38,7 +35,6 @@ import org.pgpainless.key.OpenPgpV4Fingerprint; import org.pgpainless.policy.Policy; import org.pgpainless.signature.OnePassSignature; import org.pgpainless.signature.SignatureChainValidator; -import org.pgpainless.exception.SignatureValidationException; public class SignatureVerifyingInputStream extends FilterInputStream { @@ -97,8 +93,8 @@ public class SignatureVerifyingInputStream extends FilterInputStream { private void validateOnePassSignatures() throws IOException { PGPSignatureList signatureList = findPgpSignatureList(); - try { - for (PGPSignature signature : signatureList) { + for (PGPSignature signature : signatureList) { + try { OpenPgpV4Fingerprint fingerprint = findFingerprintForSignature(signature); OnePassSignature onePassSignature = findOnePassSignature(fingerprint); if (onePassSignature == null) { @@ -107,31 +103,18 @@ public class SignatureVerifyingInputStream extends FilterInputStream { } verifySignatureOrThrowSignatureException(signature, onePassSignature); + } catch (PGPException | SignatureException e) { + LOGGER.log(LEVEL, "One-pass-signature verification failed for signature made by key " + + Long.toHexString(signature.getKeyID()) + ": " + e.getMessage(), e); } - } catch (PGPException | SignatureException e) { - throw new IOException(e.getMessage(), e); } } private void verifySignatureOrThrowSignatureException(PGPSignature signature, OnePassSignature onePassSignature) throws PGPException, SignatureException { Policy policy = PGPainless.getPolicy(); - try { - PGPPublicKey signingKey = onePassSignature.getVerificationKeys().getPublicKey(signature.getKeyID()); - signatureStructureIsAcceptable(signingKey, policy).verify(signature); - verifySignatureCreationTimeIsInBounds(options.getVerifyNotBefore(), options.getVerifyNotAfter()).verify(signature); - signatureIsEffective().verify(signature); - - SignatureChainValidator.validateSigningKey(signature, onePassSignature.getVerificationKeys(), PGPainless.getPolicy()); - - } catch (SignatureValidationException e) { - throw new SignatureException("Signature key is not valid.", e); - } - if (!onePassSignature.verify(signature)) { - throw new SignatureException("Bad Signature of key " + signature.getKeyID()); - } else { - LOGGER.log(LEVEL, "Verified signature of key {}", Long.toHexString(signature.getKeyID())); - } + verifySignatureCreationTimeIsInBounds(options.getVerifyNotBefore(), options.getVerifyNotAfter()).verify(signature); + SignatureChainValidator.validateOnePassSignature(signature, onePassSignature, policy); } private OnePassSignature findOnePassSignature(OpenPgpV4Fingerprint fingerprint) { diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureChainValidator.java b/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureChainValidator.java index 5267adea..ecf8b4d7 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureChainValidator.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/SignatureChainValidator.java @@ -15,7 +15,12 @@ */ package org.pgpainless.signature; +import static org.pgpainless.signature.SignatureValidator.signatureIsEffective; +import static org.pgpainless.signature.SignatureValidator.signatureStructureIsAcceptable; +import static org.pgpainless.signature.SignatureValidator.verifyWasPossiblyMadeByKey; + import java.io.InputStream; +import java.security.SignatureException; import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -28,6 +33,7 @@ import java.util.logging.Logger; import org.bouncycastle.bcpg.sig.KeyFlags; import org.bouncycastle.bcpg.sig.SignerUserID; +import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPSignature; @@ -263,4 +269,26 @@ public final class SignatureChainValidator { SignatureValidator.verifyInitializedSignature(signature, signingKey, policy, signature.getCreationTime()); return true; } + + public static boolean validateOnePassSignature(PGPSignature signature, OnePassSignature onePassSignature, Policy policy) throws PGPException, SignatureException { + PGPPublicKey signingKey = onePassSignature.getVerificationKeys().getPublicKey(signature.getKeyID()); + try { + verifyWasPossiblyMadeByKey(signingKey, signature); + signatureStructureIsAcceptable(signingKey, policy).verify(signature); + signatureIsEffective().verify(signature); + } catch (SignatureValidationException e) { + throw new SignatureException("Signature is not valid: " + e.getMessage(), e); + } + + try { + validateSigningKey(signature, onePassSignature.getVerificationKeys(), policy); + } catch (SignatureValidationException e) { + throw new SignatureException("Signing key " + Long.toHexString(signingKey.getKeyID()) + " is not valid: " + e.getMessage(), e); + } + if (!onePassSignature.verify(signature)) { + throw new SignatureException("Bad signature of key " + Long.toHexString(signingKey.getKeyID())); + } + + return true; + } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/VerifyNotBeforeNotAfterTest.java b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/VerifyNotBeforeNotAfterTest.java new file mode 100644 index 00000000..64033615 --- /dev/null +++ b/pgpainless-core/src/test/java/org/pgpainless/decryption_verification/VerifyNotBeforeNotAfterTest.java @@ -0,0 +1,204 @@ +/* + * Copyright 2021 Paul Schaub. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pgpainless.decryption_verification; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Date; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPublicKeyRing; +import org.bouncycastle.util.io.Streams; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.key.SubkeyIdentifier; +import org.pgpainless.key.TestKeys; +import org.pgpainless.util.DateUtil; + +public class VerifyNotBeforeNotAfterTest { + + private static final byte[] data = "Hello, World!\n".getBytes(StandardCharsets.UTF_8); + + private static final byte[] inlineSigned = ("" + + "-----BEGIN PGP MESSAGE-----\n" + + "Version: PGPainless\n" + + "\n" + + "kA0DAAoTVzbmkxrPNwwAyxRiAAAAAABIZWxsbywgV29ybGQhCoh1BAATCgAGBQJh\n" + + "G7iQACEJEFc25pMazzcMFiEET2ZcTcLEZgvGQl5BVzbmkxrPNwxH7AEAsEyOCQPG\n" + + "3F2JSWK/8AYyvsk17Gtb9TGn5SWqLo5Ac8YBAIlijYnSIHm0aatlMsK6t/rAB3bU\n" + + "eHyT9/mVlk9qrOWs\n" + + "=vkxO\n" + + "-----END PGP MESSAGE-----").getBytes(StandardCharsets.UTF_8); + + private static final byte[] detachedSignature = ("" + + "-----BEGIN PGP SIGNATURE-----\n" + + "Version: PGPainless\n" + + "\n" + + "iHUEABMKAAYFAmEbuJAAIQkQVzbmkxrPNwwWIQRPZlxNwsRmC8ZCXkFXNuaTGs83\n" + + "DEfsAQDNXmvhkh92aoUp9KNCpCqA6nvHmT5O0n1Lr0BmBccHtgD8CVR3VElemas+\n" + + "aH5l06cDUW1peQQs+xZ0FHltWmk5PJw=\n" + + "=RJDi\n" + + "-----END PGP SIGNATURE-----").getBytes(StandardCharsets.UTF_8); + + private static final Date signatureCreationDate = DateUtil.parseUTCDate("2021-08-17 13:24:32 UTC"); + private static final Date T0 = DateUtil.parseUTCDate("2021-08-17 12:30:00 UTC"); + private static final Date T1 = signatureCreationDate; + private static final Date T2 = DateUtil.parseUTCDate("2021-08-17 13:30:00 UTC"); + private static PGPPublicKeyRing certificate; + private static SubkeyIdentifier signingKey; + + @BeforeAll + public static void setup() throws IOException { + certificate = TestKeys.getEmilPublicKeyRing(); + signingKey = new SubkeyIdentifier(certificate); + } + + @Test + public void noConstraintsVerifyInlineSig() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .addVerificationCert(certificate); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(inlineSigned)) + .withOptions(options); + + OpenPgpMetadata metadata = processSignedData(verifier); + assertTrue(metadata.getVerifiedSignatures().containsKey(new SubkeyIdentifier(certificate))); + } + + @Test + public void noConstraintsVerifyDetachedSig() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .addVerificationCert(certificate) + .addVerificationOfDetachedSignatures(new ByteArrayInputStream(detachedSignature)); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(data)) + .withOptions(options); + + OpenPgpMetadata metadata = processSignedData(verifier); + assertTrue(metadata.getVerifiedSignatures().containsKey(new SubkeyIdentifier(certificate))); + } + + @Test + public void notBeforeT1DoesNotRejectInlineSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotBefore(T1) + .addVerificationCert(certificate); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(inlineSigned)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertTrue(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void notBeforeT1DoesNotRejectDetachedSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotBefore(T1) + .addVerificationCert(certificate) + .addVerificationOfDetachedSignatures(new ByteArrayInputStream(detachedSignature)); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(data)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertTrue(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void verifyNotBeforeT2DoesRejectInlineSignatureMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotBefore(T2) + .addVerificationCert(certificate); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(inlineSigned)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertFalse(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void verifyNotBeforeT2DoesRejectDetachedSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotBefore(T2) + .addVerificationCert(certificate) + .addVerificationOfDetachedSignatures(new ByteArrayInputStream(detachedSignature)); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(data)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertFalse(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void verifyNotAfterT1DoesNotRejectInlineSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotAfter(T1) + .addVerificationCert(certificate); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(inlineSigned)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertTrue(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void verifyNotAfterT1DoesRejectDetachedSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotAfter(T1) + .addVerificationCert(certificate) + .addVerificationOfDetachedSignatures(new ByteArrayInputStream(detachedSignature)); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(data)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertTrue(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void verifyNotAfterT0DoesRejectInlineSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotAfter(T0) + .addVerificationCert(certificate); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(inlineSigned)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertFalse(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + @Test + public void verifyNotAfterT0DoesRejectDetachedSigMadeAtT1() throws PGPException, IOException { + ConsumerOptions options = new ConsumerOptions() + .verifyNotAfter(T0) + .addVerificationCert(certificate) + .addVerificationOfDetachedSignatures(new ByteArrayInputStream(detachedSignature)); + DecryptionStream verifier = PGPainless.decryptAndOrVerify() + .onInputStream(new ByteArrayInputStream(data)) + .withOptions(options); + OpenPgpMetadata metadata = processSignedData(verifier); + assertFalse(metadata.getVerifiedSignatures().containsKey(signingKey)); + } + + private OpenPgpMetadata processSignedData(DecryptionStream verifier) throws IOException { + Streams.drain(verifier); + verifier.close(); + return verifier.getResult(); + } +}