From d7a2092f138693061e410a7d3ca19bca31d5a819 Mon Sep 17 00:00:00 2001 From: Mikhail Puzanov Date: Thu, 14 Dec 2023 15:06:43 +0100 Subject: [PATCH 1/5] Add withValidityPeriod to MFT and CRL builders --- .../cms/manifest/ManifestCmsBuilder.java | 8 +++++ .../commons/crypto/crl/X509CrlBuilder.java | 17 +++++++---- .../cms/manifest/ManifestCmsBuilderTest.java | 24 +++++++++++++++ .../crypto/crl/X509CrlBuilderTest.java | 29 +++++++++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java index ec696ffb3..d31fb7160 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java @@ -1,5 +1,6 @@ package net.ripe.rpki.commons.crypto.cms.manifest; +import net.ripe.rpki.commons.crypto.ValidityPeriod; import net.ripe.rpki.commons.crypto.cms.RpkiSignedObjectBuilder; import net.ripe.rpki.commons.crypto.util.Asn1Util; import net.ripe.rpki.commons.crypto.x509cert.X509CertificateBuilderHelper; @@ -57,6 +58,13 @@ public ManifestCmsBuilder withNextUpdateTime(DateTime instant) { return this; } + // This is preferred since ValidityPeriod will validate the dates + public ManifestCmsBuilder withValidityPeriod(ValidityPeriod validityPeriod) { + this.thisUpdateTime = validityPeriod.getNotValidBefore(); + this.nextUpdateTime = validityPeriod.getNotValidAfter(); + return this; + } + public ManifestCmsBuilder withSignatureProvider(String signatureProvider) { this.signatureProvider = signatureProvider; return this; diff --git a/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java b/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java index 8fcb83a9c..8ddb6d574 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java @@ -1,5 +1,8 @@ package net.ripe.rpki.commons.crypto.crl; +import lombok.Getter; +import net.ripe.rpki.commons.crypto.ValidityPeriod; +import net.ripe.rpki.commons.crypto.cms.manifest.ManifestCmsBuilder; import net.ripe.rpki.commons.crypto.crl.X509Crl.Entry; import net.ripe.rpki.commons.crypto.util.BouncyCastleUtil; import net.ripe.rpki.commons.crypto.x509cert.X509CertificateBuilderHelper; @@ -29,7 +32,9 @@ public class X509CrlBuilder { public static final int CRL_VERSION_2 = 2; private X500Principal issuerDN; + @Getter private DateTime thisUpdateTime; + @Getter private DateTime nextUpdateTime; private AuthorityKeyIdentifier authorityKeyIdentifier; private CRLNumber crlNumber; @@ -53,19 +58,19 @@ public X509CrlBuilder withThisUpdateTime(DateTime instant) { return this; } - public DateTime getThisUpdateTime() { - return thisUpdateTime; - } - public X509CrlBuilder withNextUpdateTime(DateTime instant) { this.nextUpdateTime = instant; return this; } - public DateTime getNextUpdateTime() { - return nextUpdateTime; + // This is preferred since ValidityPeriod will validate the dates + public X509CrlBuilder withValidityPeriod(ValidityPeriod validityPeriod) { + this.thisUpdateTime = validityPeriod.getNotValidBefore(); + this.nextUpdateTime = validityPeriod.getNotValidAfter(); + return this; } + /** * CRL number must be representable in 20 octets * https://tools.ietf.org/html/rfc5280#section-5.2.3 diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java index 0aaa0d5ce..b3a029140 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java @@ -1,6 +1,7 @@ package net.ripe.rpki.commons.crypto.cms.manifest; import net.ripe.rpki.commons.FixedDateRule; +import net.ripe.rpki.commons.crypto.ValidityPeriod; import org.bouncycastle.util.encoders.Hex; import org.junit.Before; import org.junit.Rule; @@ -96,4 +97,27 @@ public void shouldEncodeManifest() { subject.addFile("BaR", BAR_CONTENT); assertArrayEquals(ENCODED_MANIFEST, subject.encodeManifest()); } + + @Test + public void shouldBeEquivalentToSetDateInDifferentWays() { + var builder1 = new ManifestCmsBuilder(); + builder1.withManifestNumber(BigInteger.valueOf(68)); + builder1.withThisUpdateTime(THIS_UPDATE_TIME); + builder1.withNextUpdateTime(NEXT_UPDATE_TIME); + builder1.withCertificate(createValidManifestEECertificate()); + builder1.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); + + var builder2 = new ManifestCmsBuilder(); + builder2.withManifestNumber(BigInteger.valueOf(68)); + builder2.withValidityPeriod(new ValidityPeriod(THIS_UPDATE_TIME, NEXT_UPDATE_TIME)); + builder2.withNextUpdateTime(NEXT_UPDATE_TIME); + builder2.withCertificate(createValidManifestEECertificate()); + builder2.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); + + ManifestCms m1 = builder1.build(TEST_KEY_PAIR.getPrivate()); + ManifestCms m2 = builder1.build(TEST_KEY_PAIR.getPrivate()); + + assertEquals(m1.getThisUpdateTime(), m2.getThisUpdateTime()); + assertEquals(m1.getNextUpdateTime(), m2.getNextUpdateTime()); + } } diff --git a/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java b/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java index 54ace8f68..925279a41 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java @@ -1,5 +1,8 @@ package net.ripe.rpki.commons.crypto.crl; +import net.ripe.rpki.commons.crypto.ValidityPeriod; +import net.ripe.rpki.commons.crypto.cms.manifest.ManifestCms; +import net.ripe.rpki.commons.crypto.cms.manifest.ManifestCmsBuilder; import net.ripe.rpki.commons.crypto.util.KeyPairFactoryTest; import net.ripe.rpki.commons.crypto.util.KeyPairUtil; import net.ripe.rpki.commons.util.UTC; @@ -19,6 +22,8 @@ import java.security.SignatureException; import java.security.cert.CRLException; +import static net.ripe.rpki.commons.crypto.cms.manifest.ManifestCmsParserTest.*; +import static net.ripe.rpki.commons.crypto.cms.manifest.ManifestCmsParserTest.TEST_KEY_PAIR; import static net.ripe.rpki.commons.crypto.x509cert.X509CertificateBuilderHelper.*; import static org.junit.Assert.*; @@ -152,4 +157,28 @@ public void shouldRejectNegativeBigCrlNumber() { public void shouldRejectVeryBigCrlNumber() { subject.withNumber(BigInteger.valueOf(2).pow(160)); } + + @Test + public void shouldBeEquivalentToSetDateInDifferentWays() { + var builder1 = new X509CrlBuilder(); + builder1.withIssuerDN(new X500Principal("CN=ROOT")); + builder1.withThisUpdateTime(THIS_UPDATE_TIME); + builder1.withNextUpdateTime(NEXT_UPDATE_TIME); + builder1.withNumber(BigInteger.ONE); + builder1.withAuthorityKeyIdentifier(PUBLIC_KEY); + builder1.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); + + var builder2 = new X509CrlBuilder(); + builder2.withIssuerDN(new X500Principal("CN=ROOT")); + builder2.withValidityPeriod(new ValidityPeriod(THIS_UPDATE_TIME, NEXT_UPDATE_TIME)); + builder2.withNumber(BigInteger.ONE); + builder2.withAuthorityKeyIdentifier(PUBLIC_KEY); + builder2.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); + + var c1 = builder1.build(PRIVATE_KEY); + var c2 = builder2.build(PRIVATE_KEY); + + assertEquals(c1.getThisUpdateTime(), c2.getThisUpdateTime()); + assertEquals(c1.getNextUpdateTime(), c2.getNextUpdateTime()); + } } From 95723293b82fb930ebb7e8e6a3b39fd30c35b8a6 Mon Sep 17 00:00:00 2001 From: Mikhail Puzanov Date: Thu, 14 Dec 2023 15:09:02 +0100 Subject: [PATCH 2/5] Update readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 8a0faac9b..d02665692 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,7 @@ next (snapshot) release, e.g. `1.1-SNAPSHOT` after releasing `1.0`. ## 2024-xx-xx * Add support for router certificates to the time parsing in `SignedObjectUtil`. + * Add withValidityPeriod to manifest and CRL builders ## 2023-10-31 1.36 * Access the certificate for the generic signed object parser. From 0490fe8bcdc1c6702f1a7284e99409601c45c9e2 Mon Sep 17 00:00:00 2001 From: Mikhail Puzanov Date: Mon, 18 Dec 2023 12:53:11 +0100 Subject: [PATCH 3/5] Deprecate withNextUpdateTime and withNextUpdateTime --- .../commons/crypto/cms/manifest/ManifestCmsBuilder.java | 8 ++++++++ .../net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java index d31fb7160..98253fe77 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilder.java @@ -48,11 +48,19 @@ public ManifestCmsBuilder withManifestNumber(BigInteger number) { return this; } + /** + * @deprecated Use {@link #withValidityPeriod} instead + */ + @Deprecated public ManifestCmsBuilder withThisUpdateTime(DateTime instant) { this.thisUpdateTime = instant; return this; } + /** + * @deprecated Use {@link #withValidityPeriod} instead + */ + @Deprecated public ManifestCmsBuilder withNextUpdateTime(DateTime instant) { this.nextUpdateTime = instant; return this; diff --git a/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java b/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java index 8ddb6d574..08e643ec2 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilder.java @@ -53,11 +53,20 @@ public X509CrlBuilder withIssuerDN(X500Principal issuerDN) { return this; } + /** + * @deprecated Use {@link #withValidityPeriod} instead + */ + @Deprecated public X509CrlBuilder withThisUpdateTime(DateTime instant) { this.thisUpdateTime = instant; return this; } + + /** + * @deprecated Use {@link #withValidityPeriod} instead + */ + @Deprecated public X509CrlBuilder withNextUpdateTime(DateTime instant) { this.nextUpdateTime = instant; return this; From a5b9cee1dfaceec0282f76fb27ec75fdacb451b7 Mon Sep 17 00:00:00 2001 From: Mikhail Puzanov Date: Fri, 5 Jan 2024 16:47:50 +0100 Subject: [PATCH 4/5] Do not use deprecated method in tests unless necessary --- .../ManifestCMSBuilderPropertyTest.java | 35 ++++++++----------- .../cms/manifest/ManifestCmsBuilderTest.java | 3 +- .../cms/manifest/ManifestCmsParserTest.java | 4 +-- .../crypto/cms/manifest/ManifestCmsTest.java | 12 ++++--- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCMSBuilderPropertyTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCMSBuilderPropertyTest.java index a319006a6..0bf43e384 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCMSBuilderPropertyTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCMSBuilderPropertyTest.java @@ -2,6 +2,7 @@ import com.pholser.junit.quickcheck.Property; import com.pholser.junit.quickcheck.runner.JUnitQuickcheck; +import net.ripe.rpki.commons.crypto.ValidityPeriod; import org.joda.time.DateTime; import org.junit.runner.RunWith; @@ -14,27 +15,21 @@ @RunWith(JUnitQuickcheck.class) public class ManifestCMSBuilderPropertyTest { - - @Property public void buildEncodedParseCheck( - byte[] content, - BigInteger manifestNumber, - Integer validityHours - ){ - ManifestCmsBuilder builder = new ManifestCmsBuilder(); - builder.addFile("test.crl", content); - builder.withManifestNumber(manifestNumber); - builder.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); - builder.withCertificate(createValidManifestEECertificate(TEST_KEY_PAIR)); - DateTime start = DateTime.now(); - builder.withThisUpdateTime(start); - builder.withNextUpdateTime(start.plusHours(validityHours)); - ManifestCms manifestCms = builder.build(TEST_KEY_PAIR.getPrivate()); - - ManifestCmsParser mftParser = new ManifestCmsParser(); - mftParser.parse("test.mft", manifestCms.getEncoded()); - assertTrue(mftParser.getManifestCms().containsFile("test.crl")); + @Property + public void buildEncodedParseCheck(byte[] content, BigInteger manifestNumber, Integer validityHours) { + ManifestCmsBuilder builder = new ManifestCmsBuilder(); + builder.addFile("test.crl", content); + builder.withManifestNumber(manifestNumber); + builder.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); + builder.withCertificate(createValidManifestEECertificate(TEST_KEY_PAIR)); + DateTime start = DateTime.now(); + builder.withValidityPeriod(new ValidityPeriod(start, start.plusHours(Math.abs(validityHours)))); + ManifestCms manifestCms = builder.build(TEST_KEY_PAIR.getPrivate()); + + ManifestCmsParser mftParser = new ManifestCmsParser(); + mftParser.parse("test.mft", manifestCms.getEncoded()); + assertTrue(mftParser.getManifestCms().containsFile("test.crl")); } - } diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java index b3a029140..2c9fa1d67 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java @@ -27,8 +27,7 @@ public class ManifestCmsBuilderTest { @Before public void setUp() { subject.withManifestNumber(BigInteger.valueOf(68)); - subject.withThisUpdateTime(THIS_UPDATE_TIME); - subject.withNextUpdateTime(NEXT_UPDATE_TIME); + subject.withValidityPeriod(new ValidityPeriod(THIS_UPDATE_TIME, NEXT_UPDATE_TIME)); subject.withCertificate(createValidManifestEECertificate()); subject.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); } diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParserTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParserTest.java index 67f4ec572..5f63be9d6 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParserTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParserTest.java @@ -144,7 +144,7 @@ public void setUp() { DateTimeUtils.setCurrentMillisFixed(THIS_UPDATE_TIME.getMillis()); ManifestCmsBuilder builder = new ManifestCmsBuilder(); builder.withCertificate(createValidManifestEECertificate()).withManifestNumber(BigInteger.valueOf(68)); - builder.withThisUpdateTime(THIS_UPDATE_TIME).withNextUpdateTime(NEXT_UPDATE_TIME); + builder.withValidityPeriod(new ValidityPeriod(THIS_UPDATE_TIME, NEXT_UPDATE_TIME)); builder.addFile("foo1", FOO_CONTENT); builder.addFile("BaR", BAR_CONTENT); builder.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); @@ -201,7 +201,7 @@ public void shouldRejectManifestWithNonInheritEECert() { // Use 10/8 EE cert builder.withCertificate(createTenSlashEightResourceCertificate()).withManifestNumber(BigInteger.valueOf(68)); - builder.withThisUpdateTime(THIS_UPDATE_TIME).withNextUpdateTime(NEXT_UPDATE_TIME); + builder.withValidityPeriod(new ValidityPeriod(THIS_UPDATE_TIME, NEXT_UPDATE_TIME)); builder.addFile("foo1", FOO_CONTENT); builder.addFile("BaR", BAR_CONTENT); diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsTest.java index 4e8900137..9ae0c2c14 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsTest.java @@ -225,15 +225,19 @@ public void shouldRejectWhenThisUpdateTimeIsNotBeforeNextUpdateTime() { IpResourceSet resources = rootCertificate.getResources(); - CertificateRepositoryObjectValidationContext context = new CertificateRepositoryObjectValidationContext(ROOT_CERTIFICATE_LOCATION, rootCertificate, resources, Lists.newArrayList(rootCertificate.getSubject().getName())); + CertificateRepositoryObjectValidationContext context = new CertificateRepositoryObjectValidationContext( + ROOT_CERTIFICATE_LOCATION, rootCertificate, resources, Lists.newArrayList(rootCertificate.getSubject().getName())); ValidationResult result = ValidationResult.withLocation(ROOT_SIA_MANIFEST_RSYNC_LOCATION); subject.validateWithCrl(ROOT_SIA_MANIFEST_RSYNC_LOCATION.toASCIIString(), context, ValidationOptions.strictValidation(), result, crl); assertTrue(result.hasFailures()); assertEquals( - new ValidationCheck(ValidationStatus.ERROR, ValidationString.MANIFEST_THIS_UPDATE_TIME_BEFORE_NEXT_UPDATE_TIME, NEXT_UPDATE_TIME.plusSeconds(1).toString(), NEXT_UPDATE_TIME.toString()), - result.getResult(new ValidationLocation(ROOT_SIA_MANIFEST_RSYNC_LOCATION), ValidationString.MANIFEST_THIS_UPDATE_TIME_BEFORE_NEXT_UPDATE_TIME) + new ValidationCheck(ValidationStatus.ERROR, + ValidationString.MANIFEST_THIS_UPDATE_TIME_BEFORE_NEXT_UPDATE_TIME, + NEXT_UPDATE_TIME.plusSeconds(1).toString(), NEXT_UPDATE_TIME.toString()), + result.getResult(new ValidationLocation(ROOT_SIA_MANIFEST_RSYNC_LOCATION), + ValidationString.MANIFEST_THIS_UPDATE_TIME_BEFORE_NEXT_UPDATE_TIME) ); } @@ -455,7 +459,7 @@ public static ManifestCmsBuilder getRootManifestBuilder(ValidityPeriod validityP ManifestCmsBuilder builder = new ManifestCmsBuilder(); builder.withCertificate(getManifestEEResourceCertificateBuilder().build()); builder.withManifestNumber(BigInteger.valueOf(68)); - builder.withThisUpdateTime(validityPeriod.getNotValidBefore()).withNextUpdateTime(validityPeriod.getNotValidAfter()); + builder.withValidityPeriod(validityPeriod); builder.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER); return builder; } From 61db7470265d29e99c571776045a892037c59218 Mon Sep 17 00:00:00 2001 From: Mikhail Puzanov Date: Wed, 10 Jan 2024 16:14:06 +0100 Subject: [PATCH 5/5] Suppress deprecation wwarnings --- .../rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java | 1 + .../net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java index 2c9fa1d67..8e5f2199f 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java @@ -98,6 +98,7 @@ public void shouldEncodeManifest() { } @Test + @SuppressWarnings("deprecation") public void shouldBeEquivalentToSetDateInDifferentWays() { var builder1 = new ManifestCmsBuilder(); builder1.withManifestNumber(BigInteger.valueOf(68)); diff --git a/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java b/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java index 925279a41..aab58fb7a 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/crl/X509CrlBuilderTest.java @@ -159,6 +159,7 @@ public void shouldRejectVeryBigCrlNumber() { } @Test + @SuppressWarnings("deprecation") public void shouldBeEquivalentToSetDateInDifferentWays() { var builder1 = new X509CrlBuilder(); builder1.withIssuerDN(new X500Principal("CN=ROOT"));