diff --git a/library/build.gradle b/library/build.gradle index 1b65d614..e1e148a0 100644 --- a/library/build.gradle +++ b/library/build.gradle @@ -19,7 +19,7 @@ repositories { } dependencies { - api "org.apache.avro:avro:1.12.1" + api "org.apache.avro:avro:1.12.2" implementation 'ch.qos.logback:logback-classic:1.5.32' testImplementation 'org.json:json:20260522' testImplementation 'com.fasterxml.jackson.core:jackson-databind:2.21.3' diff --git a/library/src/main/java/bio/terra/pfb/PfbClassSecurity.java b/library/src/main/java/bio/terra/pfb/PfbClassSecurity.java new file mode 100644 index 00000000..7e1584ae --- /dev/null +++ b/library/src/main/java/bio/terra/pfb/PfbClassSecurity.java @@ -0,0 +1,50 @@ +package bio.terra.pfb; + +import org.apache.avro.Schema; +import org.apache.avro.specific.SpecificData; +import org.apache.avro.util.ClassSecurityValidator; + +/** + * Marks the classes generated from {@code PfbSchema} as trusted for Avro deserialization. + * + *

As of Avro 1.12.2 (AVRO-4189), + * {@code SpecificDatumReader} refuses to instantiate any class that has not been explicitly + * allowlisted, failing with {@code SecurityException: Forbidden bio.terra.pfb.Entity! This class is + * not trusted to be included in Avro schemas.} The generated PFB types - {@code Entity}, {@code + * Metadata}, {@code Node} and friends - are the only classes this library asks Avro to instantiate, + * so we trust exactly those rather than a broad wildcard via {@code + * org.apache.avro.SERIALIZABLE_PACKAGES=*}. + * + *

{@link PfbReader} registers them automatically. Callers that read PFB files with their own + * {@code SpecificDatumReader} over these generated classes should call {@link #register()} first. + */ +public final class PfbClassSecurity { + + private static boolean registered = false; + + private PfbClassSecurity() {} + + /** + * Adds the generated PFB types to Avro's global allowlist. Idempotent, and safe to call from any + * thread. + */ + public static synchronized void register() { + if (registered) { + return; + } + + ClassSecurityValidator.Builder trustedPfbTypes = ClassSecurityValidator.builder(); + for (Schema type : PfbSchema.PROTOCOL.getTypes()) { + trustedPfbTypes.add(SpecificData.getClassName(type)); + } + + // Compose with the current validator instead of replacing it, so we don't overwrite + // Avro's own defaults or anything already allowed through the + // org.apache.avro.SERIALIZABLE_CLASSES / SERIALIZABLE_PACKAGES properties + ClassSecurityValidator.setGlobal( + ClassSecurityValidator.composite( + ClassSecurityValidator.getGlobal(), trustedPfbTypes.build())); + + registered = true; + } +} diff --git a/library/src/main/java/bio/terra/pfb/PfbReader.java b/library/src/main/java/bio/terra/pfb/PfbReader.java index 06968c1a..b6c427e7 100644 --- a/library/src/main/java/bio/terra/pfb/PfbReader.java +++ b/library/src/main/java/bio/terra/pfb/PfbReader.java @@ -23,6 +23,12 @@ public class PfbReader { // regex for decoding enums. See convertEnum(). private static final Pattern ENUM_PATTERN = Pattern.compile("_([A-Fa-f0-9]{2,3})_"); + static { + // Avro will not instantiate our generated types until they are allowlisted. + // See PfbClassSecurity. + PfbClassSecurity.register(); + } + public static String showSchema(String fileLocation) throws IOException { // TODO AJ-1288: the use of convertEnum here is incorrect. It performs decoding on the entire // string output of the schema. Instead, it should only perform decoding on the individual diff --git a/library/src/test/java/bio/terra/pfb/PfbClassSecurityTest.java b/library/src/test/java/bio/terra/pfb/PfbClassSecurityTest.java new file mode 100644 index 00000000..5f8c6e4e --- /dev/null +++ b/library/src/test/java/bio/terra/pfb/PfbClassSecurityTest.java @@ -0,0 +1,46 @@ +package bio.terra.pfb; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.stream.Stream; +import org.apache.avro.Schema; +import org.apache.avro.specific.SpecificData; +import org.apache.avro.util.ClassSecurityValidator; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +class PfbClassSecurityTest { + + @BeforeAll + static void register() { + PfbClassSecurity.register(); + } + + public static Stream> provideGeneratedTypes() { + return PfbSchema.PROTOCOL.getTypes().stream().map(PfbClassSecurityTest::loadGeneratedClass); + } + + /** Every type in pfbSchema.avdl must be instantiable by Avro's SpecificDatumReader. */ + @ParameterizedTest + @MethodSource("provideGeneratedTypes") + void generatedTypesAreTrusted(Class generatedType) { + assertDoesNotThrow(() -> ClassSecurityValidator.validate(generatedType)); + } + + /** Registering our types must not trust everything else along with them. */ + @Test + void otherClassesRemainForbidden() { + assertThrows(SecurityException.class, () -> ClassSecurityValidator.validate(PfbReader.class)); + } + + private static Class loadGeneratedClass(Schema type) { + try { + return Class.forName(SpecificData.getClassName(type)); + } catch (ClassNotFoundException e) { + throw new IllegalStateException("No generated class for schema " + type.getFullName(), e); + } + } +}