diff --git a/README.md b/README.md index e8adca9ff..bff0df8c6 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,8 @@ next (snapshot) release, e.g. `1.1-SNAPSHOT` after releasing `1.0`. ## Changelog ## 2024-xx-yy 1.37 + * Use bouncy castle 1.77 (and update API usage accordingly) + * Improved the parsing of resource extensions * **fix**: Do not encode redundant maxlength in ROAs * Use a more modern version of the jing (RelaxNG) library * Add support for router certificates to the time parsing in `SignedObjectUtil`. diff --git a/pom.xml b/pom.xml index ec172c8a7..e53d13c59 100644 --- a/pom.xml +++ b/pom.xml @@ -46,11 +46,10 @@ 11 1.52 - 1.74 - 32.1.2-jre - 2.10.13 + 1.77 + 33.0.0-jre + 2.12.7 1.4.20 - 2.11.0 1.18.30 @@ -100,12 +99,6 @@ ${net.ripe.ipresource.version} - - commons-io - commons-io - test - - com.google.guava guava diff --git a/src/main/java/net/ripe/rpki/commons/crypto/IllegalAsn1StructureException.java b/src/main/java/net/ripe/rpki/commons/crypto/IllegalAsn1StructureException.java new file mode 100644 index 000000000..641d39a8e --- /dev/null +++ b/src/main/java/net/ripe/rpki/commons/crypto/IllegalAsn1StructureException.java @@ -0,0 +1,14 @@ +package net.ripe.rpki.commons.crypto; + +/** + * Encountered an ASN.1 structure that was not expected (e.g. implicit instead of explicit tags). + */ +public class IllegalAsn1StructureException extends IllegalArgumentException { + public IllegalAsn1StructureException(String message) { + super(message); + } + + public IllegalAsn1StructureException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/net/ripe/rpki/commons/crypto/cms/RpkiSignedObjectParser.java b/src/main/java/net/ripe/rpki/commons/crypto/cms/RpkiSignedObjectParser.java index 151fc6a99..5eb7d3f14 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/cms/RpkiSignedObjectParser.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/cms/RpkiSignedObjectParser.java @@ -1,5 +1,6 @@ package net.ripe.rpki.commons.crypto.cms; +import net.ripe.rpki.commons.crypto.IllegalAsn1StructureException; import net.ripe.rpki.commons.crypto.util.BouncyCastleUtil; import net.ripe.rpki.commons.crypto.x509cert.AbstractX509CertificateWrapperException; import net.ripe.rpki.commons.crypto.x509cert.X509ResourceCertificate; @@ -373,18 +374,34 @@ private void verifyUnsignedAttributes(SignerInformation signer) { validationResult.rejectIfFalse(signer.getUnsignedAttributes() == null, UNSIGNED_ATTRS_OMITTED); } - protected static BigInteger getRpkiObjectVersion(ASN1Sequence seq) { - ASN1Primitive asn1Version = seq.getObjectAt(0).toASN1Primitive(); - BigInteger version = null; - if (asn1Version instanceof ASN1Integer) { - version = ((ASN1Integer) asn1Version).getValue(); - } else if (asn1Version instanceof DERTaggedObject){ - final ASN1Primitive o = ((DERTaggedObject) asn1Version).getObject(); - if (o instanceof ASN1Integer) { - version = ((ASN1Integer) o).getValue(); + /** + * Parse the version field in a signed object. + * By convention, the version is the first field in the sequence, and it is explicitly tagged. + * @param tagNo tag number + * @param seq sequence of content + * @return version number if present, empty otherwise. + */ + protected static Optional getTaggedVersion(int tagNo, ASN1Sequence seq) throws IllegalAsn1StructureException { + try { + ASN1Encodable maybeVersion = seq.getObjectAt(0).toASN1Primitive(); + if (maybeVersion instanceof DLTaggedObject) { + ASN1TaggedObject tagged = (ASN1TaggedObject) maybeVersion; + if (tagged.getTagNo() == tagNo) { + var integerVersion = tagged.getExplicitBaseObject(); + if (!tagged.isExplicit()) { + throw new IllegalAsn1StructureException("Expected explicit tag for version field"); + } + + if (integerVersion instanceof ASN1Integer) { + return Optional.of(((ASN1Integer) integerVersion).getValue()); + } + } } + + return Optional.empty(); + } catch (IllegalStateException e) { + // for example: version is implicitly tagged instead of explicitly + throw new IllegalAsn1StructureException("Error parsing version field", e); } - return version; } - } diff --git a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCms.java b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCms.java index 604d1976b..0f5509f97 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCms.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCms.java @@ -9,8 +9,6 @@ import net.ripe.rpki.commons.validation.ValidationResult; import net.ripe.rpki.commons.validation.ValidationString; import net.ripe.rpki.commons.validation.objectvalidators.CertificateRepositoryObjectValidationContext; -import net.ripe.rpki.commons.validation.objectvalidators.ResourceValidatorFactory; -import net.ripe.rpki.commons.validation.objectvalidators.X509ResourceCertificateParentChildValidator; import org.apache.commons.lang3.StringEscapeUtils; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.builder.ToStringBuilder; diff --git a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParser.java b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParser.java index 96c27d8f8..9b98aba2b 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParser.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsParser.java @@ -17,6 +17,7 @@ import java.math.BigInteger; import java.text.ParseException; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import static net.ripe.rpki.commons.crypto.util.Asn1Util.*; @@ -102,8 +103,13 @@ public void decodeAsn1Content(ASN1Encodable encoded) { final int itemCount = seq.size(); int offset = 0; if (itemCount == 6) { - BigInteger version = getRpkiObjectVersion(seq); - validationResult.rejectIfFalse(BigInteger.ZERO.equals(version), "mf.version", "manifest version must be 0, but is " + version); + Optional optionalVersion = getTaggedVersion(0, seq); + optionalVersion.ifPresentOrElse(foundVersion -> { + validationResult.rejectIfFalse(BigInteger.ZERO.equals(foundVersion), MANIFEST_VERSION, optionalVersion.get().toString()); + version = 0; + }, () -> { + validationResult.error(MANIFEST_VERSION, "missing/not explicitly tagged"); + }); offset++; } else if (itemCount == 5) { version = ManifestCms.DEFAULT_VERSION; diff --git a/src/main/java/net/ripe/rpki/commons/crypto/cms/roa/RoaCmsParser.java b/src/main/java/net/ripe/rpki/commons/crypto/cms/roa/RoaCmsParser.java index 944c62589..21bb83d97 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/cms/roa/RoaCmsParser.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/cms/roa/RoaCmsParser.java @@ -4,6 +4,7 @@ import net.ripe.ipresource.IpRange; import net.ripe.ipresource.IpResourceSet; import net.ripe.ipresource.IpResourceType; +import net.ripe.rpki.commons.crypto.IllegalAsn1StructureException; import net.ripe.rpki.commons.crypto.cms.RpkiSignedObjectInfo; import net.ripe.rpki.commons.crypto.cms.RpkiSignedObjectParser; import net.ripe.rpki.commons.crypto.rfc3779.AddressFamily; @@ -17,6 +18,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import static net.ripe.rpki.commons.crypto.util.Asn1Util.*; import static net.ripe.rpki.commons.validation.ValidationString.*; @@ -96,11 +98,15 @@ void parseRouteOriginAttestation(ASN1Encodable der) { final int itemCount = seq.size(); if (itemCount == 3) { - BigInteger version = getRpkiObjectVersion(seq); - if (validationResult.rejectIfFalse(BigInteger.ZERO.equals(version), ROA_ATTESTATION_VERSION, "attestation version must be 0, but is " + version)) { - asn = Asn1Util.parseAsId(seq.getObjectAt(1)); - prefixes = parseRoaIpAddressFamilySequence(seq.getObjectAt(2)); - } + Optional maybeVersion = getTaggedVersion(0, seq); + maybeVersion.ifPresentOrElse(version -> { + if (validationResult.rejectIfFalse(BigInteger.ZERO.equals(version), ROA_ATTESTATION_VERSION, "attestation version must be 0, but is " + version)) { + asn = Asn1Util.parseAsId(seq.getObjectAt(1)); + prefixes = parseRoaIpAddressFamilySequence(seq.getObjectAt(2)); + } + }, () -> { + validationResult.error(ROA_ATTESTATION_VERSION, "missing/not explicitly tagged"); + }); } else if (itemCount == 2) { asn = Asn1Util.parseAsId(seq.getObjectAt(0)); prefixes = parseRoaIpAddressFamilySequence(seq.getObjectAt(1)); diff --git a/src/main/java/net/ripe/rpki/commons/crypto/rfc3779/ResourceExtensionParser.java b/src/main/java/net/ripe/rpki/commons/crypto/rfc3779/ResourceExtensionParser.java index 33945154f..8523e3922 100644 --- a/src/main/java/net/ripe/rpki/commons/crypto/rfc3779/ResourceExtensionParser.java +++ b/src/main/java/net/ripe/rpki/commons/crypto/rfc3779/ResourceExtensionParser.java @@ -1,15 +1,9 @@ package net.ripe.rpki.commons.crypto.rfc3779; +import com.google.common.base.Preconditions; import net.ripe.ipresource.*; -import org.apache.commons.lang3.Validate; -import org.bouncycastle.asn1.ASN1Encodable; -import org.bouncycastle.asn1.ASN1Integer; -import org.bouncycastle.asn1.ASN1Null; -import org.bouncycastle.asn1.ASN1OctetString; -import org.bouncycastle.asn1.ASN1Primitive; -import org.bouncycastle.asn1.ASN1Sequence; -import org.bouncycastle.asn1.ASN1TaggedObject; -import org.bouncycastle.asn1.DERBitString; +import net.ripe.rpki.commons.crypto.IllegalAsn1StructureException; +import org.bouncycastle.asn1.*; import java.security.cert.X509Certificate; import java.util.EnumSet; @@ -36,6 +30,9 @@ public ResourceExtension parse(X509Certificate certificate) { ImmutableResourceSet.Builder builder = new ImmutableResourceSet.Builder(); byte[] ipAddressBlocksExtension = certificate.getExtensionValue(ResourceExtensionEncoder.OID_IP_ADDRESS_BLOCKS.getId()); if (ipAddressBlocksExtension != null) { + if (!certificate.getCriticalExtensionOIDs().contains(ResourceExtensionEncoder.OID_IP_ADDRESS_BLOCKS.getId())) { + throw new IllegalAsn1StructureException("id-pe-ipAddrBlocks must be marked as critical."); + } SortedMap ipResources = parseIpAddressBlocks(ipAddressBlocksExtension); for (Map.Entry resourcesByType : ipResources.entrySet()) { if (resourcesByType.getValue() == null) { @@ -48,6 +45,9 @@ public ResourceExtension parse(X509Certificate certificate) { byte[] asnExtension = certificate.getExtensionValue(ResourceExtensionEncoder.OID_AUTONOMOUS_SYS_IDS.getId()); if (asnExtension != null) { + if (!certificate.getCriticalExtensionOIDs().contains(ResourceExtensionEncoder.OID_AUTONOMOUS_SYS_IDS.getId())) { + throw new IllegalAsn1StructureException("id-pe-autonomousSysIds must be marked as critical."); + } IpResourceSet asResources = parseAsIdentifiers(asnExtension); if (asResources == null) { inheritedResourceTypes.add(IpResourceType.ASN); @@ -80,7 +80,7 @@ public SortedMap parseIpAddressBlocks(byte[] exten } for (AddressFamily addressFamily : map.keySet()) { - Validate.isTrue(!addressFamily.hasSubsequentAddressFamilyIdentifier(), "SAFI not supported"); + Preconditions.checkArgument(!addressFamily.hasSubsequentAddressFamilyIdentifier(), "SAFI not supported"); } return map; @@ -96,8 +96,8 @@ public IpResourceSet parseAsIdentifiers(byte[] extension) { expect(octetString, ASN1OctetString.class); ASN1OctetString o = (ASN1OctetString) octetString; IpResourceSet[] resources = derToAsIdentifiers(decode(o.getOctets())); - Validate.notNull(resources[1], "inheritance of resources has not been implemented yet"); - Validate.isTrue(resources[1].isEmpty(), "routing domain identifiers (RDI) not supported"); + Preconditions.checkNotNull(resources[1], "inheritance of resources has not been implemented yet"); + Preconditions.checkArgument(resources[1].isEmpty(), "routing domain identifiers (RDI) not supported"); return resources[0]; } @@ -108,6 +108,7 @@ SortedMap derToIpAddressBlocks(ASN1Encodable der) ASN1Sequence seq = expect(der, ASN1Sequence.class); SortedMap map = new TreeMap<>(); + Preconditions.checkArgument(seq.size() > 0, "IPAddrBlocks MUST NOT be empty"); for (int i = 0; i < seq.size(); i++) { derToIpAddressFamily(seq.getObjectAt(i), map); } @@ -120,7 +121,7 @@ SortedMap derToIpAddressBlocks(ASN1Encodable der) */ void derToIpAddressFamily(ASN1Encodable der, SortedMap map) { ASN1Sequence seq = expect(der, ASN1Sequence.class); - Validate.isTrue(seq.size() == 2, "IpAddressFamily must have exactly two entries: addressFamily and IpAddressChoice"); + Preconditions.checkArgument(seq.size() == 2, "IpAddressFamily must have exactly two entries: addressFamily and IpAddressChoice"); AddressFamily addressFamily = AddressFamily.fromDer(seq.getObjectAt(0)); IpResourceSet resources = derToIpAddressChoice(addressFamily.toIpResourceType(), seq.getObjectAt(1)); @@ -144,14 +145,14 @@ IpResourceSet derToIpAddressChoice(IpResourceType type, ASN1Encodable der) { IpResource current = derToIpAddressOrRange(type, seq.getObjectAt(i)); // Check if previous and next are (1) in order and (2) not continuous if (previous != null) { - Validate.isTrue(!previous.adjacent(current), "IP resources in extension MUST NOT be adjacent"); + Preconditions.checkArgument(!previous.adjacent(current), "IP resources in extension MUST NOT be adjacent"); // The addressesOrRanges element is a SEQUENCE OF IPAddressOrRange // types. The addressPrefix and addressRange elements MUST be sorted // using the binary representation of: // // | UniqueIpResource start = current.getStart(); - Validate.isTrue(previous.getEnd().compareTo(start) < 0, "addressOrRanges MUST be sorted"); + Preconditions.checkArgument(previous.getEnd().compareTo(start) < 0, "addressOrRanges MUST be sorted"); } result.add(current); @@ -182,7 +183,7 @@ IpResource derToIpAddressOrRange(IpResourceType type, ASN1Encodable der) { */ IpResource derToIpRange(IpResourceType type, ASN1Encodable der) { ASN1Sequence sequence = expect(der, ASN1Sequence.class); - Validate.isTrue(sequence.size() == 2, "IPRange MUST consist of two entries (start and end)"); + Preconditions.checkArgument(sequence.size() == 2, "IPRange MUST consist of two entries (start and end)"); IpAddress start = parseIpAddress(type, sequence.getObjectAt(0), false); IpAddress end = parseIpAddress(type, sequence.getObjectAt(1), true); @@ -195,7 +196,7 @@ IpResource derToIpRange(IpResourceType type, ASN1Encodable der) { */ IpResourceRange derToAsRange(ASN1Encodable der) { ASN1Sequence seq = expect(der, ASN1Sequence.class); - Validate.isTrue(seq.size() == 2, "ASN1Sequence with two elements expected"); + Preconditions.checkArgument(seq.size() == 2, "ASN1Sequence with two elements expected"); return parseAsId(seq.getObjectAt(0)).upTo(parseAsId(seq.getObjectAt(1))); } @@ -226,14 +227,15 @@ IpResourceSet derToAsIdsOrRanges(ASN1Encodable der) { // range whenever possible. The AS identifiers in the asIdsOrRanges // element MUST be sorted by increasing numeric value. IpResource previous = null; + Preconditions.checkArgument(seq.size() > 0, "asIdsOrRanges MUST NOT be empty"); for (int i = 0; i < seq.size(); ++i) { IpResource current = derToAsIdOrRange(seq.getObjectAt(i)); if (previous != null) { UniqueIpResource start = current.getStart(); - Validate.isTrue(!start.adjacent(previous.getEnd()), "ASIdOrRange entries MUST NOT be adjacent"); - Validate.isTrue(start.max(previous.getEnd()).equals(start), "ASIdOrRange entries MUST be sorted by increasing numeric value"); + Preconditions.checkArgument(!start.adjacent(previous.getEnd()), "ASIdOrRange entries MUST NOT be adjacent"); + Preconditions.checkArgument(start.max(previous.getEnd()).equals(start), "ASIdOrRange entries MUST be sorted by increasing numeric value"); } result.add(current); previous = current; @@ -267,17 +269,22 @@ IpResourceSet derToAsIdentifierChoice(ASN1Encodable der) { */ IpResourceSet[] derToAsIdentifiers(ASN1Encodable der) { expect(der, ASN1Sequence.class); - ASN1Sequence seq = (ASN1Sequence) der; - Validate.isTrue(seq.size() <= 2, "ASN1Sequence with 2 or fewer elements expected"); - - IpResourceSet[] result = {new IpResourceSet(), new IpResourceSet()}; - for (int i = 0; i < seq.size(); ++i) { - expect(seq.getObjectAt(i), ASN1TaggedObject.class); - ASN1TaggedObject tagged = (ASN1TaggedObject) seq.getObjectAt(i); - Validate.isTrue(tagged.getTagNo() == 0 || tagged.getTagNo() == 1, "unknown tag no: " + tagged.getTagNo()); - result[tagged.getTagNo()] = derToAsIdentifierChoice(tagged.getObject()); + try { + ASN1Sequence seq = (ASN1Sequence) der; + Preconditions.checkArgument(seq.size() <= 2, "ASN1Sequence with 2 or fewer elements expected"); + + IpResourceSet[] result = {new IpResourceSet(), new IpResourceSet()}; + for (int i = 0; i < seq.size(); ++i) { + expect(seq.getObjectAt(i), ASN1TaggedObject.class); + ASN1TaggedObject tagged = (ASN1TaggedObject) seq.getObjectAt(i); + Preconditions.checkArgument(tagged.getTagNo() == 0 || tagged.getTagNo() == 1, "unknown tag no: " + tagged.getTagNo()); + Preconditions.checkArgument((tagged.getTagClass() & BERTags.CONTEXT_SPECIFIC) != 0, "element tag is context specific."); + result[tagged.getTagNo()] = derToAsIdentifierChoice(tagged.getExplicitBaseObject()); + } + return result; + } catch (IllegalStateException e) { + throw new IllegalAsn1StructureException("Could not parse AsIdentifiers extension", e); } - return result; } } diff --git a/src/main/java/net/ripe/rpki/commons/provisioning/cms/ProvisioningCmsObjectParser.java b/src/main/java/net/ripe/rpki/commons/provisioning/cms/ProvisioningCmsObjectParser.java index 48cc4e40b..4094416e2 100644 --- a/src/main/java/net/ripe/rpki/commons/provisioning/cms/ProvisioningCmsObjectParser.java +++ b/src/main/java/net/ripe/rpki/commons/provisioning/cms/ProvisioningCmsObjectParser.java @@ -12,7 +12,6 @@ import net.ripe.rpki.commons.provisioning.x509.ProvisioningCmsCertificateParser; import net.ripe.rpki.commons.validation.ValidationLocation; import net.ripe.rpki.commons.validation.ValidationResult; -import org.apache.commons.lang3.StringUtils; import org.bouncycastle.asn1.ASN1Encodable; import org.bouncycastle.asn1.ASN1InputStream; import org.bouncycastle.asn1.ASN1ObjectIdentifier; @@ -123,7 +122,7 @@ private String extractMessages(CMSException e) { messages.add(t.getMessage()); t = t.getCause(); } - return StringUtils.join(messages, ", reason: "); + return String.join(", reason: ", messages); } public ProvisioningCmsObject getProvisioningCmsObject() { diff --git a/src/main/java/net/ripe/rpki/commons/provisioning/payload/PayloadParser.java b/src/main/java/net/ripe/rpki/commons/provisioning/payload/PayloadParser.java index 4a08c8423..dd0fd8267 100644 --- a/src/main/java/net/ripe/rpki/commons/provisioning/payload/PayloadParser.java +++ b/src/main/java/net/ripe/rpki/commons/provisioning/payload/PayloadParser.java @@ -17,7 +17,6 @@ import net.ripe.rpki.commons.validation.ValidationResult; import net.ripe.rpki.commons.validation.ValidationString; import net.ripe.rpki.commons.xml.XmlSerializer; -import org.apache.commons.lang3.NotImplementedException; import java.util.HashMap; import java.util.Map; @@ -96,7 +95,7 @@ public static String serialize(AbstractProvisioningPayload payload) { case error_response: return ERROR_RESPONSE_SERIALIZER.serialize((RequestNotPerformedResponsePayload) payload); default: - throw new NotImplementedException("Don't have serializer for PayloadMessageType: " + type); + throw new UnsupportedOperationException("Don't have serializer for PayloadMessageType: " + type); } } } diff --git a/src/main/java/net/ripe/rpki/commons/provisioning/x509/ProvisioningIdentityCertificateBuilder.java b/src/main/java/net/ripe/rpki/commons/provisioning/x509/ProvisioningIdentityCertificateBuilder.java index 015d51f90..c333eefaf 100644 --- a/src/main/java/net/ripe/rpki/commons/provisioning/x509/ProvisioningIdentityCertificateBuilder.java +++ b/src/main/java/net/ripe/rpki/commons/provisioning/x509/ProvisioningIdentityCertificateBuilder.java @@ -1,5 +1,7 @@ package net.ripe.rpki.commons.provisioning.x509; +import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import net.ripe.rpki.commons.crypto.ValidityPeriod; import net.ripe.rpki.commons.crypto.x509cert.X509CertificateBuilderHelper; import net.ripe.rpki.commons.util.UTC; @@ -48,9 +50,9 @@ public ProvisioningIdentityCertificateBuilder withSignatureProvider(String signa } public ProvisioningIdentityCertificate build() { - Validate.notNull(selfSigningKeyPair, "Self Signing KeyPair is required"); - Validate.notNull(selfSigningSubject, "Self Signing DN is required"); - Validate.notNull(signatureProvider, "Signature Provider is required"); + Preconditions.checkNotNull(selfSigningKeyPair, "Self Signing KeyPair is required"); + Preconditions.checkNotNull(selfSigningSubject, "Self Signing DN is required"); + Preconditions.checkNotNull(signatureProvider, "Signature Provider is required"); setUpImplicitRequirementsForBuilderHelper(); builderHelper.withPublicKey(selfSigningKeyPair.getPublic()); builderHelper.withSigningKeyPair(selfSigningKeyPair); diff --git a/src/main/java/net/ripe/rpki/commons/validation/ValidationString.java b/src/main/java/net/ripe/rpki/commons/validation/ValidationString.java index c67e5740d..6726f4116 100644 --- a/src/main/java/net/ripe/rpki/commons/validation/ValidationString.java +++ b/src/main/java/net/ripe/rpki/commons/validation/ValidationString.java @@ -171,6 +171,8 @@ private ValidationString() { public static final String MANIFEST_THIS_UPDATE_TIME_BEFORE_NEXT_UPDATE_TIME = "mf.this.update.before.next.update"; public static final String MANIFEST_ENTRY_FILE_NAME_IS_RELATIVE = "mf.entry.file.name.is.relative"; + public static final String MANIFEST_VERSION = "mf.version"; + //ghostbusters public static final String GHOSTBUSTERS_RECORD_CONTENT_TYPE = "ghostbusters.record.content.type"; public static final String GHOSTBUSTERS_RECORD_SINGLE_VCARD = "ghostbusters.record.single.vcard"; diff --git a/src/main/resources/validation_en.properties b/src/main/resources/validation_en.properties index c89d41470..7a4b89dc7 100644 --- a/src/main/resources/validation_en.properties +++ b/src/main/resources/validation_en.properties @@ -537,8 +537,8 @@ aspa.content.structure.passed=ASPA content structure is valid aspa.content.structure.warning=ASPA content structure is invalid aspa.content.structure.error=ASPA content structure is invalid aspa.version.passed=ASPA ASN.1 structure version is {0} -aspa.version.warning=ASPA ASN.1 structure seems to have wrong version, should be 1, was {0} -aspa.version.error=ASPA ASN.1 structure seems to have wrong version, should be 1, was {0} +aspa.version.warning=ASPA ASN.1 structure has the wrong version, should be 1, was {0} +aspa.version.error=ASPA ASN.1 structure has the wrong version, should be 1, was {0} aspa.provider.as.set.valid.passed=ASPA providers are valid aspa.provider.as.set.valid.warning=ASPA providers are invalid: {} aspa.provider.as.set.valid.error=ASPA provider set is invalid: {} @@ -565,6 +565,10 @@ mf.content.type.passed=Content type is Manifest mf.content.type.warning=Content type is not Manifest mf.content.type.error=Content type is not Manifest +mf.version.passed=Manifest ASN.1 structure version is {0} +mf.version.warning=Manifest ASN.1 structure has the have wrong version, should be 0, was {0} +mf.version.error=Manifest ASN.1 structure has the wrong version, should be 0, was {0} + mf.content.size.passed=Manifest content size is valid mf.content.size.warning=Manifest content size is invalid mf.content.size.error=Manifest content size is invalid diff --git a/src/test/java/net/ripe/rpki/commons/crypto/cms/ghostbuster/GhostbustersCmsParserTest.java b/src/test/java/net/ripe/rpki/commons/crypto/cms/ghostbuster/GhostbustersCmsParserTest.java index db3a88b36..431f3d19c 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/cms/ghostbuster/GhostbustersCmsParserTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/cms/ghostbuster/GhostbustersCmsParserTest.java @@ -8,7 +8,6 @@ import net.ripe.rpki.commons.crypto.x509cert.X509ResourceCertificateBuilder; import net.ripe.rpki.commons.util.UTC; import net.ripe.rpki.commons.validation.ValidationResult; -import org.apache.commons.io.FileUtils; import org.joda.time.DateTime; import org.junit.Ignore; import org.junit.Test; @@ -17,6 +16,7 @@ import java.io.File; import java.math.BigInteger; import java.net.URI; +import java.nio.file.Files; import java.security.KeyPair; import static net.ripe.rpki.commons.crypto.x509cert.X509CertificateBuilderHelper.DEFAULT_SIGNATURE_PROVIDER; @@ -37,7 +37,7 @@ public class GhostbustersCmsParserTest { @Test public void testShouldParseGoodGbr() throws Exception { String path = "src/test/resources/conformance/root/goodRealGbrNothingIsWrong.gbr"; - byte[] bytes = FileUtils.readFileToByteArray(new File(path)); + byte[] bytes = Files.readAllBytes(new File(path).toPath()); GhostbustersCmsParser parser = new GhostbustersCmsParser(); parser.parse(ValidationResult.withLocation("test1.gbr"), bytes); @@ -90,7 +90,7 @@ private ValidationResult validatePayload(String vCardPayload) { @Test(expected = IllegalArgumentException.class) public void testShouldParseBadGbr() throws Exception { String path = "src/test/resources/conformance/root/badGBRNotVCard.gbr"; - byte[] bytes = FileUtils.readFileToByteArray(new File(path)); + byte[] bytes = Files.readAllBytes(new File(path).toPath()); GhostbustersCmsParser parser = new GhostbustersCmsParser(); parser.parse(ValidationResult.withLocation("test2.gbr"), bytes); parser.getGhostbustersCms().getVCardContent(); diff --git a/src/test/java/net/ripe/rpki/commons/crypto/x509cert/X509CertificateUtilTest.java b/src/test/java/net/ripe/rpki/commons/crypto/x509cert/X509CertificateUtilTest.java index d58fc4708..974c97212 100644 --- a/src/test/java/net/ripe/rpki/commons/crypto/x509cert/X509CertificateUtilTest.java +++ b/src/test/java/net/ripe/rpki/commons/crypto/x509cert/X509CertificateUtilTest.java @@ -1,14 +1,13 @@ package net.ripe.rpki.commons.crypto.x509cert; import com.google.common.base.Charsets; +import com.google.common.io.CharSource; import net.ripe.ipresource.IpResourceSet; -import org.apache.commons.io.IOUtils; import org.junit.Ignore; import org.junit.Test; import java.io.IOException; import java.net.URI; -import java.nio.charset.Charset; import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; @@ -59,9 +58,9 @@ public void shouldGetEncodedSubjectPublicKeyInfo() throws CertificateEncodingExc @Test @Ignore - public void shouldParseRrdpRepositoryUrl() throws java.security.cert.CertificateException { + public void shouldParseRrdpRepositoryUrl() throws java.security.cert.CertificateException, IOException { final CertificateFactory factory = CertificateFactory.getInstance("X.509"); - X509Certificate certificate = (X509Certificate) factory.generateCertificate(IOUtils.toInputStream(CERT_WITH_RRDP_URL, Charsets.UTF_8)); + X509Certificate certificate = (X509Certificate) factory.generateCertificate(CharSource.wrap(CERT_WITH_RRDP_URL).asByteSource(Charsets.UTF_8).openStream()); URI rrdpNotifyUri = X509CertificateUtil.getRrdpNotifyUri(certificate); diff --git a/src/test/java/net/ripe/rpki/commons/interop/BBNCMSConformanceTest.java b/src/test/java/net/ripe/rpki/commons/interop/BBNCMSConformanceTest.java index 3e3c3438b..2e0b18482 100644 --- a/src/test/java/net/ripe/rpki/commons/interop/BBNCMSConformanceTest.java +++ b/src/test/java/net/ripe/rpki/commons/interop/BBNCMSConformanceTest.java @@ -10,7 +10,8 @@ import java.io.File; import java.io.IOException; -import static org.junit.Assert.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; + // CMS signed objects, generic (using ROAs) public class BBNCMSConformanceTest { @@ -60,7 +61,8 @@ public class BBNCMSConformanceTest { public void testGenericCMSSignedObject(String testNumber, String testCaseFile, String testCaseDescription) throws IOException { final String fileName = String.format("root/badCMS%s.roa", testCaseFile); - assertTrue("Should reject certificate with " + testCaseDescription + " from " + fileName, parseCertificate(fileName)); + assertThat(parseCertificate(fileName)).isTrue() + .withFailMessage("[" + testNumber + "] Should reject certificate with " + testCaseDescription + " from " + fileName); } @Disabled("These checks are not implemented yet.") @@ -74,7 +76,8 @@ public void testGenericCMSSignedObject(String testNumber, String testCaseFile, S public void testGenericCMSSignedObject_ignored(String testNumber, String testCaseFile, String testCaseDescription) throws IOException { final String fileName = String.format("root/badCMS%s.roa", testCaseFile); - assertTrue("Should reject certificate with " + testCaseDescription + " from " + fileName, parseCertificate(fileName)); + assertThat(parseCertificate(fileName)).isTrue() + .withFailMessage("[" + testNumber + "] Should reject certificate with " + testCaseDescription + " from " + fileName); } private boolean parseCertificate(String certificate) throws IOException { diff --git a/src/test/java/net/ripe/rpki/commons/interop/BBNCertificateConformanceTest.java b/src/test/java/net/ripe/rpki/commons/interop/BBNCertificateConformanceTest.java index 97a3be5f6..ac42e58f8 100644 --- a/src/test/java/net/ripe/rpki/commons/interop/BBNCertificateConformanceTest.java +++ b/src/test/java/net/ripe/rpki/commons/interop/BBNCertificateConformanceTest.java @@ -10,10 +10,9 @@ import java.io.File; import java.io.IOException; -import java.util.Arrays; -import java.util.List; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class BBNCertificateConformanceTest { @@ -24,33 +23,33 @@ public class BBNCertificateConformanceTest { public void shouldRejectSelfSignedCertificateWithCRLDP() throws IOException { // CRLDP is present in the trust anchor 6487#4.8.6 boolean hasFailure = parseCertificate("badRootBadCRLDP.cer"); - assertTrue(hasFailure); + assertThat(hasFailure).isTrue(); } @Test public void shouldRejectCertificateWithCRLDPWithReasonFieldNotOmitted() throws IOException { // CRL Dist Pt has reasons 6487#4.8.6 boolean hasFailure = parseCertificate("root/badCertCRLDPReasons.cer"); - assertTrue(hasFailure); + assertThat(hasFailure).isTrue(); } @Test public void shouldRejectCertificateWithCRLDPWithCrlIssuer() throws IOException { // CRL Dist Pt has CRL Issuer 6487#4.8.6 boolean hasFailure = parseCertificate("root/badCertCRLDPCrlIssuer.cer"); - assertTrue(hasFailure); + assertThat(hasFailure).isTrue(); } @Test public void shouldRejectCertificateWithoutKeyUsageBit() throws IOException { // 179 NoKeyUsage # no key usage extension 6487#4.8,4.8.4 - assertTrue(parseCertificate("root/badCertNoKeyUsage.cer")); + assertThat(parseCertificate("root/badCertNoKeyUsage.cer")).isTrue(); } @Test public void shouldRejectCertificateWithTwoKeyUsageBits() throws IOException { // 180 2KeyUsage # two key usage extensions 5280#4.2 - assertTrue(parseCertificate("root/badCert2KeyUsage.cer")); + assertThat(parseCertificate("root/badCert2KeyUsage.cer")).isTrue(); } @CsvSource({ @@ -64,14 +63,60 @@ public void shouldRejectCertificateWithTwoKeyUsageBits() throws IOException { public void shouldRejectCertificateWithIncorrectKeyUsageBits(String testCasenumber, String testCaseFile, String testCaseDescription) throws IOException { final String fileName = String.format("root/badCert%s.cer", testCaseFile); - assertTrue("Should reject certificate with " + testCaseDescription + " from " + fileName, parseCertificate(fileName)); + assertThat(parseCertificate(fileName)) + .isTrue() + .withFailMessage("Should reject certificate with " + testCaseDescription + " from " + fileName); + } + + @CsvSource({ + "218, ResourcesIP6Inherit, # (good) inherit IPv6 resources only, others explicit 6487#4.8.10", + "219, ResourcesIP4Inherit, # (good) inherit IPv4 resources only, others explicit 6487#4.8.10", + "220, ResourcesASInherit, # (good) inherit AS resources only, others explicit 6487#4.8.11", + "221, ResourcesAllInherit, # (good) inherit all resources 6487#4.8.10", + "222, ResourcesIP6InhOnly, # (good) inherit IPv6 resources only, others not present 6487#4.8.10", + "223, ResourcesIP4InhOnly, # (good) inherit IPv4 resources only, others not present 6487#4.8.10", + "224, ResourcesASInhOnly, # (good) inherit AS resources only, others not present 6487#4.8.11", + }) + @ParameterizedTest(name = "{displayName} - {0} {1} {2}") + public void shouldAcceptCertificateWithResourceExtension(String testCasenumber, String testCaseFile, String testCaseDescription) throws IOException { + final String fileName = String.format("root/goodCert%s.cer", testCaseFile); + + assertThat(parseCertificate(fileName)) + .isFalse() + .withFailMessage("Should accept certificate with " + testCaseDescription + " from " + fileName); + } + + @CsvSource({ + "138, ResourcesASNoCrit, # AS number extension not critical 6487#4.8.11", + "139, ResourcesBadAFI, # invalid IP address family 6487#4.8.10, IANA address-family-numbers", + "140, ResourcesBadASOrder, # AS numbers out of order 3779 (but full set is pending)", + "141, ResourcesBadV4Order, # IPv4 addresses out of order 3779 (but full set is pending)", + "142, ResourcesBadV6Order, # IPv6 addresses out of order 3779 (but full set is pending)", + "143, ResourcesIPNoCrit, # IP address extension not critical 6487#4.8.10", + "144, ResourcesNone, # neither AS nor IP 3779 extensions 6487#4.8.10", + "192, ResourcesIPEmpty, # empty set of IP addresses 6487#4.8.10", + "193, ResourcesASEmpty, # empty set of AS numbers 6487#4.8.11", + "145, ResourcesSAFI, # IP addresses has SAFI digit 6487#4.8.10", + }) + @ParameterizedTest(name = "{displayName} - {0} {1} {2}") + public void shouldRejectCertificateWithInvalidResourceExtension(String testCasenumber, String testCaseFile, String testCaseDescription) throws IOException { + final String fileName = String.format("root/badCert%s.cer", testCaseFile); + + assertThatThrownBy(() -> assertThat(parseCertificate(fileName)).isFalse()) + .isInstanceOfAny(IllegalArgumentException.class, IllegalStateException.class, AssertionError.class) + .withFailMessage("Should reject certificate with " + testCaseDescription + " from " + fileName); } private boolean certificateHasWarningOrFailure(String certificate) throws IOException { File file = new File(PATH_TO_BBN_OBJECTS, certificate); byte[] encoded = Files.toByteArray(file); ValidationResult result = ValidationResult.withLocation(file.getName()); - new X509ResourceCertificateParser().parse(result, encoded); + var parser = new X509ResourceCertificateParser(); + parser.parse(result, encoded); + // Trigger some lazy parsing + if (!result.hasFailures()) { + parser.getCertificate(); + } return result.hasFailures() || result.hasWarnings(); } @@ -79,7 +124,11 @@ private boolean parseCertificate(String certificate) throws IOException { File file = new File(PATH_TO_BBN_OBJECTS, certificate); byte[] encoded = Files.toByteArray(file); ValidationResult result = ValidationResult.withLocation(file.getName()); - new X509ResourceCertificateParser().parse(result, encoded); + var parser = new X509ResourceCertificateParser(); + parser.parse(result, encoded); + if (!result.hasFailures()) { + parser.getCertificate(); + } return result.hasFailures(); } } \ No newline at end of file diff --git a/src/test/java/net/ripe/rpki/commons/interop/BBNRoaConformanceTest.java b/src/test/java/net/ripe/rpki/commons/interop/BBNRoaConformanceTest.java index 60ee9c0b4..69db9a7e3 100644 --- a/src/test/java/net/ripe/rpki/commons/interop/BBNRoaConformanceTest.java +++ b/src/test/java/net/ripe/rpki/commons/interop/BBNRoaConformanceTest.java @@ -3,12 +3,14 @@ import com.google.common.io.Files; import net.ripe.rpki.commons.crypto.cms.roa.RoaCmsParser; import net.ripe.rpki.commons.validation.ValidationResult; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import java.io.File; import java.io.IOException; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.assertThat; public class BBNRoaConformanceTest { @@ -18,14 +20,32 @@ public class BBNRoaConformanceTest { public void shouldParseRoaWithMissingSignature() throws IOException { // no signature 6488#2.1.6.6 boolean hasFailure = parseRoa("root/badCMSSigInfoNoSig.roa"); - assertTrue(hasFailure); + assertThat(hasFailure).isTrue(); } @Test public void shouldParseRoaWithNoSignerInfo() throws IOException { // empty set of SignerInfos 6488#2.1 boolean hasFailure = parseRoa("root/badCMSNoSigInfo.roa"); - assertTrue(hasFailure); + assertThat(hasFailure).isTrue(); + } + + /** + * Apply a number of test cases for version. + * + * Note that in these objects version is implicit not explicit as required. + */ + @CsvSource({ + "557, VersionV1Explicit, # explicit V1 version (int 0) applied before signature 6482#3", + "558, VersionV1ExplicitBadSig, # explicit V1 version (int 0) applied after signature 6482#3", + "559, VersionV2, # Version V2 (int 1) 6482#3.1" + }) + @ParameterizedTest(name = "{displayName} - {0} {1} {2}") + public void shouldRejectBadRoaObject(String testNumber, String testCaseFile, String testCaseDescription) throws IOException { + final String fileName = String.format("root/badROA%s.roa", testCaseFile); + + assertThat(parseRoa(fileName)).isTrue() + .withFailMessage("[" + testNumber + "] Should reject certificate with " + testCaseDescription + " from " + fileName); } private boolean parseRoa(String roa) throws IOException { diff --git a/src/test/java/net/ripe/rpki/commons/provisioning/payload/list/response/ResourceClassListResponsePayloadSerializerTest.java b/src/test/java/net/ripe/rpki/commons/provisioning/payload/list/response/ResourceClassListResponsePayloadSerializerTest.java index bf06235da..6f40ed43f 100644 --- a/src/test/java/net/ripe/rpki/commons/provisioning/payload/list/response/ResourceClassListResponsePayloadSerializerTest.java +++ b/src/test/java/net/ripe/rpki/commons/provisioning/payload/list/response/ResourceClassListResponsePayloadSerializerTest.java @@ -6,14 +6,10 @@ import net.ripe.rpki.commons.provisioning.payload.common.CertificateElement; import net.ripe.rpki.commons.provisioning.payload.common.CertificateElementBuilder; import net.ripe.rpki.commons.provisioning.payload.common.GenericClassElementBuilder; -import net.ripe.rpki.commons.util.EqualsSupport; -import net.ripe.rpki.commons.xml.XStreamXmlSerializer; import net.ripe.rpki.commons.xml.XmlSerializer; -import org.apache.commons.lang3.builder.EqualsBuilder; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.junit.Test; -import org.mockito.internal.matchers.apachecommons.ReflectionEquals; import org.xml.sax.SAXException; import java.io.IOException; diff --git a/src/test/java/net/ripe/rpki/commons/validation/interop/BBNConformanceTest.java b/src/test/java/net/ripe/rpki/commons/validation/interop/BBNConformanceTest.java index ae1c94c5b..8f390ce73 100644 --- a/src/test/java/net/ripe/rpki/commons/validation/interop/BBNConformanceTest.java +++ b/src/test/java/net/ripe/rpki/commons/validation/interop/BBNConformanceTest.java @@ -1,21 +1,19 @@ package net.ripe.rpki.commons.validation.interop; -import com.google.common.io.Files; import net.ripe.rpki.commons.crypto.CertificateRepositoryObject; -import net.ripe.rpki.commons.crypto.cms.ghostbuster.GhostbustersCmsParser; -import net.ripe.rpki.commons.crypto.cms.manifest.ManifestCmsParser; -import net.ripe.rpki.commons.crypto.cms.roa.RoaCmsParser; -import net.ripe.rpki.commons.crypto.crl.X509Crl; import net.ripe.rpki.commons.crypto.util.CertificateRepositoryObjectFactory; -import net.ripe.rpki.commons.crypto.x509cert.X509ResourceCertificateParser; import net.ripe.rpki.commons.validation.ValidationResult; -import org.apache.commons.io.FileUtils; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.io.File; import java.io.IOException; -import java.util.Iterator; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Stream; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertTrue; public class BBNConformanceTest { @@ -24,35 +22,37 @@ public class BBNConformanceTest { @Test public void shouldParseAllObjects() throws IOException { - int objectCount = 0; - int errorCount = 0; - int exceptionCount = 0; - - Iterator fileIterator = FileUtils.iterateFiles(new File(PATH_TO_BBN_OBJECTS), new String[]{"cer", "crl", "mft", "roa"}, true); - while (fileIterator.hasNext()) { - objectCount++; - File file = fileIterator.next(); - byte[] encoded = Files.toByteArray(file); - ValidationResult result = ValidationResult.withLocation(file.getName()); - - try { - final CertificateRepositoryObject res = CertificateRepositoryObjectFactory.createCertificateRepositoryObject(encoded, result); - // Check that invariant that "parsing errors or result" holds. - assertTrue(result.hasFailures() || res != null); - - if (result.hasFailures() && file.getName().startsWith("good")) { - System.err.println("Supposed to be good: " + file.getName()); - errorCount++; - } else if (! result.hasFailures() && file.getName().startsWith("bad")) { - System.err.println("Supposed to be bad: " + file.getName()); - errorCount++; - } else { - System.out.println(file.getName() + " -> " + result.hasFailures()); + var objectCount = new AtomicInteger(); + var errorCount = new AtomicInteger(); + var exceptionCount = new AtomicInteger(); + + var extensionMatcher = FileSystems.getDefault().getPathMatcher("glob:**.{cer,crl,mft,roa}"); + + try (Stream paths = Files.find(new File(PATH_TO_BBN_OBJECTS).toPath(), Integer.MAX_VALUE, (p, attr) -> extensionMatcher.matches(p))) { + paths.forEach(path -> { + objectCount.incrementAndGet(); + try { + byte[] encoded = Files.readAllBytes(path); + ValidationResult result = ValidationResult.withLocation(path.getFileName().toString()); + + final CertificateRepositoryObject res = CertificateRepositoryObjectFactory.createCertificateRepositoryObject(encoded, result); + // Check that invariant that "parsing errors or result" holds. + assertThat(result.hasFailures() || res != null).isTrue(); + + if (result.hasFailures() && path.getFileName().startsWith("good")) { + System.err.println("Supposed to be good: " + path.getFileName()); + errorCount.incrementAndGet(); + } else if (!result.hasFailures() && path.getFileName().startsWith("bad")) { + System.err.println("Supposed to be bad: " + path.getFileName()); + errorCount.incrementAndGet(); + } else { + System.out.println(path.getFileName() + " -> " + result.hasFailures()); + } + } catch (IOException | RuntimeException ex) { + System.err.println("Exception while parsing " + path.getFileName()); + exceptionCount.incrementAndGet(); } - } catch (RuntimeException ex) { - System.err.println("Exception while parsing " + file.getName() ); - exceptionCount++; - } + }); } System.out.println(objectCount + " objects: " + errorCount + " errors, " + exceptionCount + " exceptions");