Skip to content

Commit

Permalink
Merge pull request #149 from RIPE-NCC/fix/compiler-warnings
Browse files Browse the repository at this point in the history
Fix most compiler warnings
  • Loading branch information
ties authored Feb 28, 2024
2 parents cda6034 + 13d89fb commit af8fc00
Show file tree
Hide file tree
Showing 26 changed files with 71 additions and 238 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ next (snapshot) release, e.g. `1.1-SNAPSHOT` after releasing `1.0`.

## 2024-xx-yy 1.37
* Use bouncy castle 1.77 (and update API usage accordingly)
* **removed** deprecated RemoteCertificateFetcher
* **removed** deprected ProvisioningCmsObject constructor
* Improved the parsing of resource extensions
* **fix**: Do not encode redundant maxlength in ROAs
* Use a more modern version of the jing (RelaxNG) library
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
package net.ripe.rpki.commons.crypto.cms;

import org.bouncycastle.asn1.ASN1EncodableVector;
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
import org.bouncycastle.asn1.ASN1OctetString;
import org.bouncycastle.asn1.ASN1Set;
import org.bouncycastle.asn1.DEROctetString;
import org.bouncycastle.asn1.DERSet;
import org.bouncycastle.asn1.*;
import org.bouncycastle.asn1.cms.CMSObjectIdentifiers;
import org.bouncycastle.asn1.cms.ContentInfo;
import org.bouncycastle.asn1.cms.SignerInfo;
Expand All @@ -19,6 +14,8 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.util.Collection;
import java.util.List;

import static net.ripe.rpki.commons.crypto.cms.CMSUtils.attachSignersToOutputStream;
import static net.ripe.rpki.commons.crypto.cms.CMSUtils.createDerSetFromList;
Expand All @@ -44,6 +41,7 @@ public RPKISignedDataGenerator() {
* @param content the content to be signed.
* @param encapsulate true if the content should be encapsulated in the signature, false otherwise.
*/
@SuppressWarnings("unchecked")
@Override
public CMSSignedData generate(
// FIXME Avoid accessing more than once to support CMSProcessableInputStream
Expand Down Expand Up @@ -85,7 +83,7 @@ public CMSSignedData generate(
bOut = new ByteArrayOutputStream();
}

OutputStream cOut = attachSignersToOutputStream(signerGens, bOut);
OutputStream cOut = attachSignersToOutputStream((Collection<SignerInfoGenerator>)signerGens, bOut);

// Just in case it's unencapsulated and there are no signers!
cOut = getSafeOutputStream(cOut);
Expand Down Expand Up @@ -125,14 +123,14 @@ public CMSSignedData generate(

if (!certs.isEmpty())
{
certificates = createDerSetFromList(certs);
certificates = createDerSetFromList((List<? extends ASN1Encodable>) certs);
}

ASN1Set certrevlist = null;

if (!crls.isEmpty())
{
certrevlist = createDerSetFromList(crls);
certrevlist = createDerSetFromList((List<? extends ASN1Encodable>)crls);
}

ContentInfo encInfo = new ContentInfo(contentTypeOID, octs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ private void addSignerInfo(RPKISignedDataGenerator generator, PrivateKey private
DigestCalculatorProvider digestProvider = BouncyCastleUtil.DIGEST_CALCULATOR_PROVIDER;
SignerInfoGenerator gen = new JcaSignerInfoGeneratorBuilder(digestProvider).setSignedAttributeGenerator(
new DefaultSignedAttributeTableGenerator(createSignedAttributes(signingCertificate.getNotBefore())) {
@SuppressWarnings("rawtypes")
@Override
public AttributeTable getAttributes(Map parameters) {
return super.getAttributes(parameters).remove(CMSAttributes.cmsAlgorithmProtect);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package net.ripe.rpki.commons.crypto.cms.manifest;

import com.google.common.net.UrlEscapers;
import net.ripe.rpki.commons.crypto.cms.RpkiSignedObject;
import net.ripe.rpki.commons.crypto.cms.RpkiSignedObjectInfo;
import net.ripe.rpki.commons.crypto.crl.X509Crl;
Expand All @@ -9,7 +10,6 @@
import net.ripe.rpki.commons.validation.ValidationResult;
import net.ripe.rpki.commons.validation.ValidationString;
import net.ripe.rpki.commons.validation.objectvalidators.CertificateRepositoryObjectValidationContext;
import org.apache.commons.lang3.StringEscapeUtils;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
Expand Down Expand Up @@ -142,7 +142,7 @@ private void checkEntries(ValidationResult result) {
result.rejectIfFalse(
failedEntries.isEmpty(),
ValidationString.MANIFEST_ENTRY_FILE_NAME_IS_RELATIVE,
failedEntries.stream().map(StringEscapeUtils::escapeJava).collect(Collectors.joining(", "))
failedEntries.stream().map(UrlEscapers.urlFormParameterEscaper().asFunction()).collect(Collectors.joining(", "))
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ private CertificateRepositoryObjectFactory() {
* @return a parsed {@link CertificateRepositoryObject} or {@code null} in case the encoded object has a valid location
* but its contents can not be parsed.
*/
@SuppressWarnings("fallthrough")
public static CertificateRepositoryObject createCertificateRepositoryObject(byte[] encoded, ValidationResult validationResult) {

RepositoryObjectType objectType = RepositoryObjectType.parse(validationResult.getCurrentLocation().getName());
Expand All @@ -52,6 +53,7 @@ public static CertificateRepositoryObject createCertificateRepositoryObject(byte
return parseGbr(encoded, validationResult);
case Aspa:
return parseAspa(encoded, validationResult);
// intentional usage of fall-through: All three cases should result in UnknownCertificateRepositoryObject, yet only two are unsupported.
case SignedChecklist:
case TrustAnchorKey:
log.info("Encountered unsupported object type: {} uri={}", objectType, validationResult.getCurrentLocation().getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,6 @@ public class ProvisioningCmsObject {
/** Signing time MUST be present https://datatracker.ietf.org/doc/html/rfc6492#section-3.1.1.6.4.3 */
private final DateTime signingTime;

// No support for signingTime, which is a required attribute for valid objects.
@Deprecated
public ProvisioningCmsObject(byte[] encodedContent, X509Certificate cmsCertificate, Collection<X509Certificate> caCertificates, X509CRL crl, AbstractProvisioningPayload payload) {
// -
// ArrayIsStoredDirectly
this.encodedContent = encodedContent;
this.cmsCertificate = cmsCertificate;
this.caCertificates = caCertificates;
this.crl = crl;
this.payload = payload;
this.signingTime = null;
}

public ProvisioningCmsObject(byte[] encodedContent, X509Certificate cmsCertificate, Collection<X509Certificate> caCertificates, X509CRL crl, AbstractProvisioningPayload payload, @NonNull DateTime signingTime) {
// -
// ArrayIsStoredDirectly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ private void addSignerInfo(RPKISignedDataGenerator generator, PrivateKey private
generator.addSignerInfoGenerator(
new JcaSignerInfoGeneratorBuilder(digestProvider)
.setSignedAttributeGenerator(new DefaultSignedAttributeTableGenerator(createSignedAttributes()) {

@SuppressWarnings("rawtypes")
@Override
public AttributeTable getAttributes(Map parameters) {
return super.getAttributes(parameters).remove(CMSAttributes.cmsAlgorithmProtect);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@
* This is a self-signed X.509 BPKI certificate that will be the issuer of the BPKI EE certificates that the child uses
* when sending provisioning protocol messages to the parent.
*/
public class ProvisioningIdentityCertificate extends ProvisioningCertificate implements Serializable {
public final class ProvisioningIdentityCertificate extends ProvisioningCertificate implements Serializable {

private static final long serialVersionUID = 1L;


public ProvisioningIdentityCertificate(X509Certificate certificate) {
super(certificate);

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/ripe/rpki/commons/rsync/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private int runExternalCommand() {
outputReader.start();

//allows the readers to start
yield();
Thread.yield();

outputReader.join();
errorReader.join();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public AliasedNetRipeTypePermission(XStream xStream) {
* @param type type to check
* @return whether an alias has been applied to the type
*/
@SuppressWarnings("rawtypes")
@Override
public boolean allows(Class type) {
return type.getName().startsWith("net.ripe.") && !type.getName().equals(xStream.getMapper().serializedClass(type));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

import javax.security.auth.x500.X500Principal;

public class XStreamXmlSerializerBuilder<T> {
public final class XStreamXmlSerializerBuilder<T> {

private static final boolean STRICT = true;
private static final boolean NOT_STRICT = false;
Expand All @@ -47,27 +47,28 @@ public class XStreamXmlSerializerBuilder<T> {


public static <C> XStreamXmlSerializerBuilder<C> newStrictXmlSerializerBuilder(Class<C> objectType) {
return new XStreamXmlSerializerBuilder<C>(objectType, STRICT);
return new XStreamXmlSerializerBuilder<>(objectType, STRICT);
}

public static <C> XStreamXmlSerializerBuilder<C> newForgivingXmlSerializerBuilder(Class<C> objectType) {
return new XStreamXmlSerializerBuilder<C>(objectType, NOT_STRICT);
return new XStreamXmlSerializerBuilder<>(objectType, NOT_STRICT);
}

protected XStreamXmlSerializerBuilder(Class<T> objectType, boolean strict) {
XStreamXmlSerializerBuilder(Class<T> objectType, boolean strict) {
super();
this.objectType = objectType;
createDefaultXStream(strict);
}

/**
* Instantiate XStream and set-up the security framework to prevent injection and remote code execution.
* Instantiate XStream and set up the security framework to prevent injection and remote code execution.
*
* Types that are allowed are:
* * A list of default types included in XStream.
* * The type the serializer is built for.
* * Types that have been aliased (i.e. the mapped name of the class is not it's qualified name).
*
* Note that the whitelist is <emph>only</emph> checked on deserialization.
* Note that the allowlist is <emph>only</emph> checked on deserialization.
*/
private void createDefaultXStream(boolean strict) {
if(strict) {
Expand All @@ -87,7 +88,7 @@ private void createDefaultXStream(boolean strict) {

// Allow type this serializer is instantiated for as well as its descendant types
xStream.allowTypeHierarchy(this.objectType);
xStream.allowTypes(new Class[]{ this.objectType });
xStream.allowTypes(new Class<?>[]{ this.objectType });
// Not all registered types are part of this module.
// A wildcard could pull in classes that are not safe to deserialize -> allow types from net.ripe
// for which there exists an alias.
Expand All @@ -98,14 +99,10 @@ private void createDefaultXStream(boolean strict) {
registerRpkiRelated();
}

protected HierarchicalStreamDriver getStreamDriver() {
private HierarchicalStreamDriver getStreamDriver() {
return new XppDriver();
}

protected final Class<T> getObjectType() {
return objectType;
}

private void registerIpResourceRelated() {
withAliasType("resource", IpResource.class);
withConverter(new IpResourceConverter());
Expand Down Expand Up @@ -148,12 +145,12 @@ private void registerRpkiRelated() {
withAllowedType(RoaCms.class);
}

public final XStreamXmlSerializerBuilder<T> withConverter(Converter converter) {
public XStreamXmlSerializerBuilder<T> withConverter(Converter converter) {
xStream.registerConverter(converter);
return this;
}

public final XStreamXmlSerializerBuilder<T> withConverter(SingleValueConverter converter) {
public XStreamXmlSerializerBuilder<T> withConverter(SingleValueConverter converter) {
xStream.registerConverter(converter);
return this;
}
Expand All @@ -178,7 +175,7 @@ public final XStreamXmlSerializerBuilder<T> withAttribute(String childNode, Clas
* @param classType type to allow.
*/
public final XStreamXmlSerializerBuilder<T> withAllowedType(Class<?> classType) {
xStream.allowTypes(new Class[]{classType});
xStream.allowTypes(new Class<?>[]{classType});
return this;
}

Expand All @@ -199,14 +196,10 @@ public final XStreamXmlSerializerBuilder<T> withAliasField(String alias, Class<?
}

public XStreamXmlSerializer<T> build() {
return new XStreamXmlSerializer<T>(xStream, objectType);
}

protected XStream getXStream() {
return xStream;
return new XStreamXmlSerializer<>(xStream, objectType);
}

private final static class MyXStream extends XStream {
private static final class MyXStream extends XStream {

private MyXStream(HierarchicalStreamDriver hierarchicalStreamDriver) {
super(new SunUnsafeReflectionProvider(), hierarchicalStreamDriver);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,13 @@ public void shouldRejectWhenManifestIsTooStaleDueToNegativeGracePeriod() {
);
}

@SuppressWarnings("deprecation")
@Test
public void shouldRejectWhenThisUpdateTimeIsNotBeforeNextUpdateTime() {
X509Crl crl = getRootCrl();
DateTimeUtils.setCurrentMillisFixed(NEXT_UPDATE_TIME.plusDays(1).getMillis());

// validity period checks the ordering of the dates, so use withThisUpdate explicitly
subject = getRootManifestBuilder().withThisUpdateTime(NEXT_UPDATE_TIME.plusSeconds(1)).build(MANIFEST_KEY_PAIR.getPrivate());

IpResourceSet resources = rootCertificate.getResources();
Expand Down Expand Up @@ -362,7 +364,7 @@ public void shouldRejectFileNamesThatEscapeRepository() {
new ValidationCheck(
ValidationStatus.ERROR,
ValidationString.MANIFEST_ENTRY_FILE_NAME_IS_RELATIVE,
", \\u0000, , ., .., cannot-contain-a/slash.cer, extension-must-be-lowercase.CER, multiple.dots.not.allowed, non-empty-extension-required., only-letters-allowed-for-extension.123"
", %00, +++, ., .., cannot-contain-a%2Fslash.cer, extension-must-be-lowercase.CER, multiple.dots.not.allowed, non-empty-extension-required., only-letters-allowed-for-extension.123"
),
result.getResult(new ValidationLocation(ROOT_SIA_MANIFEST_RSYNC_LOCATION), ValidationString.MANIFEST_ENTRY_FILE_NAME_IS_RELATIVE)
);
Expand Down Expand Up @@ -443,8 +445,7 @@ private X509ResourceCertificate getRootResourceCertificate() {
private X509CrlBuilder getRootCrlBuilder() {
X509CrlBuilder builder = new X509CrlBuilder();
builder.withIssuerDN(X509ResourceCertificateTest.TEST_SELF_SIGNED_CERTIFICATE_NAME);
builder.withThisUpdateTime(NEXT_UPDATE_TIME.minusHours(24));
builder.withNextUpdateTime(NEXT_UPDATE_TIME.plusHours(24));
builder.withValidityPeriod(new ValidityPeriod(NEXT_UPDATE_TIME.minusHours(24), NEXT_UPDATE_TIME.plusHours(24)));
builder.withNumber(BigInteger.TEN);
builder.withAuthorityKeyIdentifier(ROOT_KEY_PAIR.getPublic());
builder.withSignatureProvider(DEFAULT_SIGNATURE_PROVIDER);
Expand Down
Loading

0 comments on commit af8fc00

Please sign in to comment.