From 69c95afaa5b37319b5f399b1aa9d03c5fcc00ca0 Mon Sep 17 00:00:00 2001 From: Gabor Szadovszky Date: Mon, 20 Oct 2025 20:04:08 +0200 Subject: [PATCH 1/4] AVRO-4189: [java] Simplify the setting of the serializable classes --- lang/java/avro/pom.xml | 3 +- lang/java/avro/src/it/pom.xml | 3 +- .../avro/specific/SpecificDatumReader.java | 95 ++----- .../avro/util/ClassSecurityValidator.java | 237 ++++++++++++++++++ .../java/org/apache/avro/util/ClassUtils.java | 11 +- .../avro/reflect/TestReflectDatumReader.java | 18 ++ .../specific/TestSpecificRecordWithUnion.java | 31 ++- .../avro/util/TestClassSecurityValidator.java | 115 +++++++++ lang/java/ipc/pom.xml | 3 +- 9 files changed, 432 insertions(+), 84 deletions(-) create mode 100644 lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java create mode 100644 lang/java/avro/src/test/java/org/apache/avro/util/TestClassSecurityValidator.java diff --git a/lang/java/avro/pom.xml b/lang/java/avro/pom.xml index 34d8758bd6c..3f0f0f10bc0 100644 --- a/lang/java/avro/pom.xml +++ b/lang/java/avro/pom.xml @@ -91,7 +91,8 @@ none - java.math.BigDecimal,java.math.BigInteger,java.net.URI,java.net.URL,java.io.File,java.lang.Integer,org.apache.avro.reflect.TestReflect$R10 + java.net.URI,java.net.URL,java.io.File,java.util.HashMap,java.util.List,java.util.Collection,java.util.Map,java.util.Set,java.util.concurrent.ConcurrentHashMap,java.util.LinkedHashMap,java.util.TreeMap + org.apache.avro diff --git a/lang/java/avro/src/it/pom.xml b/lang/java/avro/src/it/pom.xml index d6344fdca03..6911b573ab4 100644 --- a/lang/java/avro/src/it/pom.xml +++ b/lang/java/avro/src/it/pom.xml @@ -91,7 +91,8 @@ @maven-surefire-plugin.version@ - java.math.BigDecimal,java.math.BigInteger,java.net.URI,java.net.URL,java.io.File,java.lang.Integer,org.apache.avro.reflect.TestReflect$R10 + java.net.URI,java.net.URL,java.io.File,java.util.HashMap,java.util.List,java.util.Collection,java.util.Map,java.util.Set,java.util.concurrent.ConcurrentHashMap,java.util.LinkedHashMap,java.util.TreeMap + org.apache.avro false true diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index a6ba6550f4e..eb81046e812 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -22,15 +22,13 @@ import org.apache.avro.AvroRuntimeException; import org.apache.avro.generic.GenericDatumReader; import org.apache.avro.io.ResolvingDecoder; +import org.apache.avro.util.ClassSecurityValidator.SystemPropertiesPredicate; import org.apache.avro.util.ClassUtils; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.HashSet; -import java.util.Set; -import java.util.stream.Stream; +import org.apache.avro.util.ClassSecurityValidator; /** * {@link org.apache.avro.io.DatumReader DatumReader} for generated Java @@ -39,47 +37,20 @@ public class SpecificDatumReader extends GenericDatumReader { /** - * @deprecated prefer to use {@link #SERIALIZABLE_CLASSES} instead. + * @deprecated Use {@link SystemPropertiesPredicate} instead. + * @see ClassSecurityValidator */ @Deprecated - public static final String[] SERIALIZABLE_PACKAGES; - - public static final String[] SERIALIZABLE_CLASSES; - - static { - // no serializable classes by default - SERIALIZABLE_CLASSES = streamPropertyEntries(System.getProperty("org.apache.avro.SERIALIZABLE_CLASSES")) - .toArray(String[]::new); - - // no serializable packages by default - SERIALIZABLE_PACKAGES = streamPropertyEntries(System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES")) - // Add a '.' suffix to ensure we'll be matching package names instead of - // arbitrary prefixes, except for the wildcard "*", which allows all - // packages (this is only safe in fully controlled environments!). - .map(entry -> "*".equals(entry) ? entry : entry + ".").toArray(String[]::new); - } + public static final String[] SERIALIZABLE_PACKAGES = SystemPropertiesPredicate.SERIALIZABLE_PACKAGES + .toArray(new String[0]); /** - * Parse a comma separated list into non-empty entries. Leading and trailing - * whitespace is stripped. - * - * @param commaSeparatedEntries the comma separated list of entries - * @return a stream of the entries + * @deprecated Use {@link SystemPropertiesPredicate} instead. + * @see ClassSecurityValidator */ - private static Stream streamPropertyEntries(String commaSeparatedEntries) { - if (commaSeparatedEntries == null) { - return Stream.empty(); - } - return Stream.of(commaSeparatedEntries.split(",")).map(String::strip).filter(s -> !s.isEmpty()); - } - - // The primitive "class names" based on Class.isPrimitive() - private static final Set PRIMITIVES = new HashSet<>(Arrays.asList(Boolean.TYPE.getName(), - Character.TYPE.getName(), Byte.TYPE.getName(), Short.TYPE.getName(), Integer.TYPE.getName(), Long.TYPE.getName(), - Float.TYPE.getName(), Double.TYPE.getName(), Void.TYPE.getName())); - - private final List trustedPackages = new ArrayList<>(); - private final List trustedClasses = new ArrayList<>(); + @Deprecated + public static final String[] SERIALIZABLE_CLASSES = SystemPropertiesPredicate.SERIALIZABLE_PACKAGES + .toArray(new String[0]); public SpecificDatumReader() { this(null, null, SpecificData.get()); @@ -106,15 +77,11 @@ public SpecificDatumReader(Schema writer, Schema reader) { */ public SpecificDatumReader(Schema writer, Schema reader, SpecificData data) { super(writer, reader, data); - trustedPackages.addAll(Arrays.asList(SERIALIZABLE_PACKAGES)); - trustedClasses.addAll(Arrays.asList(SERIALIZABLE_CLASSES)); } /** Construct given a {@link SpecificData}. */ public SpecificDatumReader(SpecificData data) { super(data); - trustedPackages.addAll(Arrays.asList(SERIALIZABLE_PACKAGES)); - trustedClasses.addAll(Arrays.asList(SERIALIZABLE_CLASSES)); } /** Return the contained {@link SpecificData}. */ @@ -156,7 +123,6 @@ private Class getPropAsClass(Schema schema, String prop) { if (name == null) return null; try { - checkSecurity(name); Class clazz = ClassUtils.forName(getData().getClassLoader(), name); return clazz; } catch (ClassNotFoundException e) { @@ -164,43 +130,22 @@ private Class getPropAsClass(Schema schema, String prop) { } } - private boolean trustAllPackages() { - return (trustedPackages.size() == 1 && "*".equals(trustedPackages.get(0))); - } - - private void checkSecurity(String className) throws ClassNotFoundException { - if (trustAllPackages() || PRIMITIVES.contains(className)) { - return; - } - - for (String trustedClass : getTrustedClasses()) { - if (className.equals(trustedClass)) { - return; - } - } - - for (String trustedPackage : getTrustedPackages()) { - if (className.startsWith(trustedPackage)) { - return; - } - } - - throw new SecurityException("Forbidden " + className + "! This class is not trusted to be included in Avro " - + "schemas using java-class. Please set the system property org.apache.avro.SERIALIZABLE_CLASSES to the comma " - + "separated list of classes you trust. You can also set the system property " - + "org.apache.avro.SERIALIZABLE_PACKAGES to the comma separated list of the packages you trust."); - } - /** - * @deprecated Use getTrustedClasses() instead + * @deprecated Use {@link SystemPropertiesPredicate} instead. + * @see ClassSecurityValidator */ @Deprecated public final List getTrustedPackages() { - return trustedPackages; + return Arrays.asList(SERIALIZABLE_PACKAGES); } + /** + * @deprecated Use {@link SystemPropertiesPredicate} instead. + * @see ClassSecurityValidator + */ + @Deprecated public final List getTrustedClasses() { - return trustedClasses; + return Arrays.asList(SERIALIZABLE_CLASSES); } @Override diff --git a/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java b/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java new file mode 100644 index 00000000000..0f6792e1d9b --- /dev/null +++ b/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java @@ -0,0 +1,237 @@ +package org.apache.avro.util; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.NavigableSet; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Validates that a class is trusted to be included in Avro schemas. To be used + * by {@link ClassUtils} which therefore automatically guards not only the + * loading of the classes but, since the class names are translated into + * {@link Class} objects by using {@link ClassUtils}, also guards any other + * reflection-based mechanisms (e.g. instantiation, setting internal variables). + * + * @see #setGlobal(ClassSecurityPredicate) + * @see #getGlobal() + */ +public final class ClassSecurityValidator { + + /** + * Validates that the class is trusted to be included in Avro schemas. + * + *

+ * Note: this method shall be invoked with un-initialized classes only to + * prevent any potential security issues the initialization may trigger. + * + * @param clazz the class to validate + * @throws SecurityException if the class is not trusted + */ + public static void validate(Class clazz) { + while (clazz.isArray()) { + clazz = clazz.getComponentType(); + } + if (clazz.isPrimitive()) { + return; + } + if (!globalInstance.isTrusted(clazz)) { + globalInstance.forbiddenClass(clazz.getName()); + } + } + + /** + * Sets the global {@link ClassSecurityPredicate} that is used by + * {@link ClassUtils} to validate the trusted classes. + * + * @param validator the validator to use + */ + public static void setGlobal(ClassSecurityPredicate validator) { + globalInstance = Objects.requireNonNull(validator); + } + + /** + * Returns the global {@link ClassSecurityPredicate} that is used by + * {@link ClassUtils} to validate the trusted classes. + * + * @return the global validator + */ + public static ClassSecurityPredicate getGlobal() { + return globalInstance; + } + + private ClassSecurityValidator() { + } + + /** + * A predicate that checks if a class is trusted to be included in Avro schemas. + */ + public interface ClassSecurityPredicate { + /** + * Checks if the class is trusted to be included in Avro schemas. + * + * @param clazz the class to check + * @return true if the class is trusted, false otherwise + */ + boolean isTrusted(Class clazz); + + /** + * Throws a {@link SecurityException} with a message indicating that the class + * is not trusted to be included in Avro schemas. + * + * @param className the name of the class that is not trusted + */ + default void forbiddenClass(String className) { + throw new SecurityException("Forbidden " + className + "! This class is not trusted to be included in Avro " + + "schemas. You may either use the system properties org.apache.avro.SERIALIZABLE_CLASSES and " + + "org.apache.avro.SERIALIZABLE_PACKAGES to set the comma separated list of the classes or packages you trust, " + + "or you can set them via the API (see org.apache.avro.util.ReflectDataValidator)."); + } + } + + /** + * A couple of trusted classes that are safe to be loaded, instantiated with any + * constructors or alter any internals via reflection. + */ + public static final ClassSecurityPredicate DEFAULT_TRUSTED_CLASSES = builder().add("java.lang.Boolean") + .add("java.lang.Byte").add("java.lang.Character").add("java.lang.CharSequence").add("java.lang.Double") + .add("java.lang.Enum").add("java.lang.Float").add("java.lang.Integer").add("java.lang.Long") + .add("java.lang.Number").add("java.lang.Object").add("java.lang.Short").add("java.lang.String") + .add("java.lang.Void").add("java.math.BigDecimal").add("java.math.BigInteger").build(); + + /** + * The predicate that uses the system properties + * {@value SystemPropertiesPredicate#SYSPROP_SERIALIZABLE_CLASSES} and + * {@value SystemPropertiesPredicate#SYSPROP_SERIALIZABLE_PACKAGES}. + */ + public static final ClassSecurityPredicate SYSTEM_PROPERTIES = new SystemPropertiesPredicate(); + + /** + * The default predicate that uses both the system properties and the hard-coded + * trusted classes. + * + * @see #DEFAULT_TRUSTED_CLASSES + * @see #SYSTEM_PROPERTIES + */ + public static final ClassSecurityPredicate DEFAULT = composite(DEFAULT_TRUSTED_CLASSES, SYSTEM_PROPERTIES); + + private static ClassSecurityPredicate globalInstance = DEFAULT; + + /** + * Creates a builder for a {@link ClassSecurityValidator} that validates the + * trusted classes by whitelisting their names. Note: no parent validator is + * used. + * + * @return a new {@link ClassSecurityValidator} builder + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Creates a composite {@link ClassSecurityValidator} that delegates to the + * given validators. + * + * @param validators the validators to delegate to + * @return a new {@link ClassSecurityValidator} that delegates to the given + * validators + */ + public static ClassSecurityPredicate composite(ClassSecurityPredicate... validators) { + return clazz -> Arrays.stream(validators).anyMatch(v -> v.isTrusted(clazz)); + } + + public static class Builder { + private final Set allowedClassNames = new HashSet<>(); + + private Builder() { + } + + public Builder add(String className) { + allowedClassNames.add(className); + return this; + } + + public Builder add(Class clazz) { + return add(clazz.getName()); + } + + public ClassSecurityPredicate build() { + return clazz -> allowedClassNames.contains(clazz.getName()); + } + } + + public static class SystemPropertiesPredicate implements ClassSecurityPredicate { + + /** + * The set of trusted classes specified by the system property + * {@value #SYSPROP_SERIALIZABLE_CLASSES}. Empty by default. + */ + public static final Set SERIALIZABLE_CLASSES; + + /** + * The set of trusted packages specified by the system property + * {@value #SYSPROP_SERIALIZABLE_PACKAGES}. Empty by default. + */ + public static final NavigableSet SERIALIZABLE_PACKAGES; + + private static final boolean TRUST_ALL_PACKAGES; + + private static final String SYSPROP_SERIALIZABLE_CLASSES = "org.apache.avro.SERIALIZABLE_CLASSES"; + + private static final String SYSPROP_SERIALIZABLE_PACKAGES = "org.apache.avro.SERIALIZABLE_PACKAGES"; + + static { + // add the hard-coded trusted classes as well + SERIALIZABLE_CLASSES = Collections.unmodifiableSet( + streamPropertyEntries(System.getProperty(SYSPROP_SERIALIZABLE_CLASSES)).collect(Collectors.toSet())); + + // no default serializable packages are hard-coded + NavigableSet packages = streamPropertyEntries(System.getProperty(SYSPROP_SERIALIZABLE_PACKAGES)) + // Add a '.' suffix to ensure we'll be matching package names instead of + // arbitrary prefixes, except for the wildcard "*", which allows all + // packages (this is only safe in fully controlled environments!). + .map(entry -> "*".equals(entry) ? entry : entry + ".").collect(TreeSet::new, TreeSet::add, TreeSet::addAll); + TRUST_ALL_PACKAGES = packages.remove("*"); + + SERIALIZABLE_PACKAGES = Collections.unmodifiableNavigableSet(packages); + } + + /** + * Parse a comma separated list into non-empty entries. Leading and trailing + * whitespace is stripped. + * + * @param commaSeparatedEntries the comma separated list of entries + * @return a stream of the entries + */ + private static Stream streamPropertyEntries(String commaSeparatedEntries) { + if (commaSeparatedEntries == null) { + return Stream.empty(); + } + return Stream.of(commaSeparatedEntries.split(",")).map(s -> s.replaceAll("^\\s+|\\s+$", "")) + .filter(s -> !s.isEmpty()); + } + + private SystemPropertiesPredicate() { + } + + @Override + public boolean isTrusted(Class clazz) { + if (TRUST_ALL_PACKAGES) { + return true; + } + + String className = clazz.getName(); + + if (SERIALIZABLE_CLASSES.contains(className)) { + return true; + } + + String lower = SERIALIZABLE_PACKAGES.lower(className); + return lower != null && className.startsWith(lower); + } + } +} diff --git a/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java b/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java index dad59a551d6..360faf5ac67 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java +++ b/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java @@ -52,7 +52,7 @@ public static Class forName(Class contextClass, String className) throws C c = forName(className, Thread.currentThread().getContextClassLoader()); } if (c == null) { - throw new ClassNotFoundException("Failed to load class" + className); + throw new ClassNotFoundException("Failed to load class " + className); } return c; } @@ -75,14 +75,14 @@ public static Class forName(ClassLoader classLoader, String className) throws c = forName(className, Thread.currentThread().getContextClassLoader()); } if (c == null) { - throw new ClassNotFoundException("Failed to load class" + className); + throw new ClassNotFoundException("Failed to load class " + className); } return c; } /** * Loads a {@link Class} from the specified {@link ClassLoader} without throwing - * {@link ClassNotFoundException}. + * {@link ClassNotFoundException}. The class is loaded without initialization. * * @param className * @param classLoader @@ -92,7 +92,10 @@ private static Class forName(String className, ClassLoader classLoader) { Class c = null; if (classLoader != null && className != null) { try { - c = Class.forName(className, true, classLoader); + // Load the class without initializing it so we can distinguish between + // ClassNotFoundException and SecurityException may be thrown by the validator. + c = Class.forName(className, false, classLoader); + ClassSecurityValidator.validate(c); } catch (ClassNotFoundException e) { // Ignore and return null } diff --git a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java index 52b40b87b36..ecd2cecb677 100644 --- a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java +++ b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java @@ -19,6 +19,7 @@ package org.apache.avro.reflect; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -35,6 +36,8 @@ import org.apache.avro.io.DecoderFactory; import org.apache.avro.io.Encoder; import org.apache.avro.io.EncoderFactory; +import org.apache.avro.util.ClassSecurityValidator; +import org.apache.avro.util.ClassSecurityValidator.ClassSecurityPredicate; import org.junit.jupiter.api.Test; public class TestReflectDatumReader { @@ -49,6 +52,21 @@ private static byte[] serializeWithReflectDatumWriter(T toSerialize, Class new ReflectDatumReader<>(PojoWithArray.class)); + } finally { + ClassSecurityValidator.setGlobal(originalValidator); + } + } + @Test void read_PojoWithList() throws IOException { PojoWithList pojoWithList = new PojoWithList(); diff --git a/lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificRecordWithUnion.java b/lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificRecordWithUnion.java index e64b3f4c220..70f3e7ac90a 100644 --- a/lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificRecordWithUnion.java +++ b/lang/java/avro/src/test/java/org/apache/avro/specific/TestSpecificRecordWithUnion.java @@ -30,6 +30,8 @@ import org.apache.avro.io.BinaryEncoder; import org.apache.avro.io.Decoder; +import org.apache.avro.util.ClassSecurityValidator; +import org.apache.avro.util.ClassSecurityValidator.ClassSecurityPredicate; import org.junit.jupiter.api.Test; import java.io.ByteArrayInputStream; @@ -38,8 +40,30 @@ import java.math.BigDecimal; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; public class TestSpecificRecordWithUnion { + /** + * Test that the deserialization of a class that is not trusted throws a + * SecurityException. + */ + @Test + void testNotSerializableClasses() throws IOException { + final TestUnionRecord record = TestUnionRecord.newBuilder().setAmount(BigDecimal.ZERO).build(); + final Schema schema = SchemaBuilder.unionOf().nullType().and().type(record.getSchema()).endUnion(); + + byte[] recordBytes = serializeRecord( + "{ \"org.apache.avro.specific.TestUnionRecord\": { \"amount\": { \"bytes\": \"\\u0000\" } } }", schema); + + ClassSecurityPredicate originalValidator = ClassSecurityValidator.getGlobal(); + try { + ClassSecurityValidator.setGlobal(ClassSecurityValidator.builder().build()); + assertThrows(SecurityException.class, () -> deserializeRecord(schema, recordBytes)); + } finally { + ClassSecurityValidator.setGlobal(originalValidator); + } + } + @Test void unionLogicalDecimalConversion() throws IOException { final TestUnionRecord record = TestUnionRecord.newBuilder().setAmount(BigDecimal.ZERO).build(); @@ -48,11 +72,14 @@ void unionLogicalDecimalConversion() throws IOException { byte[] recordBytes = serializeRecord( "{ \"org.apache.avro.specific.TestUnionRecord\": { \"amount\": { \"bytes\": \"\\u0000\" } } }", schema); + assertEquals(record, deserializeRecord(schema, recordBytes)); + } + + private static SpecificRecord deserializeRecord(Schema schema, byte[] recordBytes) throws IOException { SpecificDatumReader specificDatumReader = new SpecificDatumReader<>(schema); ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(recordBytes); Decoder decoder = DecoderFactory.get().binaryDecoder(byteArrayInputStream, null); - final SpecificRecord deserialized = specificDatumReader.read(null, decoder); - assertEquals(record, deserialized); + return specificDatumReader.read(null, decoder); } public static byte[] serializeRecord(String value, Schema schema) throws IOException { diff --git a/lang/java/avro/src/test/java/org/apache/avro/util/TestClassSecurityValidator.java b/lang/java/avro/src/test/java/org/apache/avro/util/TestClassSecurityValidator.java new file mode 100644 index 00000000000..4dc1a55c54f --- /dev/null +++ b/lang/java/avro/src/test/java/org/apache/avro/util/TestClassSecurityValidator.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * https://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.apache.avro.util; + +import static org.junit.jupiter.api.Assertions.*; + +import java.math.BigInteger; +import org.apache.avro.util.ClassSecurityValidator.ClassSecurityPredicate; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class TestClassSecurityValidator { + + // To test inner classes + private static class TestInnerClass { + } + + private ClassSecurityPredicate originalValidator; + + @BeforeEach + public void saveOriginalValidator() { + originalValidator = ClassSecurityValidator.getGlobal(); + } + + @AfterEach + public void restoreOriginalValidator() { + ClassSecurityValidator.setGlobal(originalValidator); + } + + @Test + void testDefault() { + // Test a couple of default trusted classes via ClassUtils + assertDoesNotThrow(() -> ClassUtils.forName(boolean[][][][][][].class.getName())); + assertDoesNotThrow(() -> ClassUtils.forName("java.lang.String")); + assertDoesNotThrow(() -> ClassUtils.forName(java.math.BigDecimal[][][][].class.getName())); + + // The package "org.apache.avro" is allowed by default for the test environment + assertDoesNotThrow(() -> ClassUtils.forName("org.apache.avro.util.TestClassSecurityValidator$TestInnerClass")); + + // Test a couple of default untrusted classes via ClassUtils + assertThrows(SecurityException.class, () -> ClassUtils.forName("java.net.InetAddress")); + assertThrows(SecurityException.class, () -> ClassUtils.forName("java.io.FileInputStream")); + } + + @Test + void testBuilder() { + ClassSecurityValidator.setGlobal(ClassSecurityValidator.builder().add(TestClassSecurityValidator.class).build()); + + assertDoesNotThrow(() -> ClassUtils.forName("org.apache.avro.util.TestClassSecurityValidator")); + assertThrows(SecurityException.class, + () -> ClassUtils.forName("org.apache.avro.util.TestClassSecurityValidator$TestInnerClass")); + + // Test that arrays and primitives also work + assertDoesNotThrow(() -> ClassUtils.forName(short[][][][][].class.getName())); + assertDoesNotThrow(() -> ClassUtils.forName(TestClassSecurityValidator[][][][].class.getName())); + assertThrows(SecurityException.class, () -> ClassUtils.forName(TestInnerClass[][].class.getName())); + } + + @Test + void testOwnImplementation() { + ClassSecurityValidator.setGlobal(new ClassSecurityPredicate() { + @Override + public boolean isTrusted(Class clazz) { + return clazz.getSimpleName().contains("Inner"); + } + + @Override + public void forbiddenClass(String className) { + throw new SecurityException("Not inner"); + } + }); + assertDoesNotThrow(() -> ClassUtils.forName(TestInnerClass.class.getName())); + Exception e = assertThrows(SecurityException.class, + () -> ClassUtils.forName(TestClassSecurityValidator.class.getName())); + assertEquals("Not inner", e.getMessage()); + + // Test that arrays and primitives also work + assertDoesNotThrow(() -> ClassUtils.forName(char[][][][].class.getName())); + assertDoesNotThrow(() -> ClassUtils.forName(TestInnerClass[][][][].class.getName())); + e = assertThrows(SecurityException.class, () -> ClassUtils.forName(TestClassSecurityValidator[][].class.getName())); + assertEquals("Not inner", e.getMessage()); + } + + @Test + void testBuildComplexPredicate() { + ClassSecurityValidator.setGlobal(ClassSecurityValidator.composite( + ClassSecurityValidator.builder().add(TestInnerClass.class).add(TestClassSecurityValidator.class).build(), + ClassSecurityValidator.DEFAULT, c -> c.getPackageName().equals("java.lang"))); + + // Test that the defaults work since we included them + testDefault(); + + assertDoesNotThrow(() -> ClassUtils.forName(TestInnerClass.class.getName())); + assertDoesNotThrow(() -> ClassUtils.forName(TestClassSecurityValidator.class.getName())); + assertDoesNotThrow(() -> ClassUtils.forName(StringBuilder.class.getName())); + assertDoesNotThrow(() -> ClassUtils.forName("java.lang.StringBuffer")); + assertDoesNotThrow(() -> ClassUtils.forName(BigInteger.class.getName())); + } +} diff --git a/lang/java/ipc/pom.xml b/lang/java/ipc/pom.xml index 5d18931d8e0..d6e78afd053 100644 --- a/lang/java/ipc/pom.xml +++ b/lang/java/ipc/pom.xml @@ -63,7 +63,8 @@ false none - java.math.BigDecimal,java.math.BigInteger + java.net.URI,java.net.URL,java.io.File,java.util.HashMap,java.util.List,java.util.Collection,java.util.Map,java.util.Set,java.util.concurrent.ConcurrentHashMap,java.util.LinkedHashMap,java.util.TreeMap + org.apache.avro From e82341d691da0eb641e2e33977da8266f6c0fc75 Mon Sep 17 00:00:00 2001 From: Gabor Szadovszky Date: Mon, 20 Oct 2025 20:08:17 +0200 Subject: [PATCH 2/4] Fix missing license header --- .../avro/util/ClassSecurityValidator.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java b/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java index 0f6792e1d9b..cdb1c95db4b 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java +++ b/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java @@ -1,3 +1,20 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * https://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.apache.avro.util; import java.util.Arrays; From 3338b0af3638bd832890f0b001da7ad873beb4ac Mon Sep 17 00:00:00 2001 From: Gabor Szadovszky Date: Tue, 21 Oct 2025 11:52:26 +0200 Subject: [PATCH 3/4] Fix test failures + copilot findings --- lang/java/avro/pom.xml | 4 ---- lang/java/avro/src/it/pom.xml | 20 +++++++++++++---- .../avro/specific/SpecificDatumReader.java | 2 +- .../avro/util/ClassSecurityValidator.java | 2 +- .../java/org/apache/avro/util/ClassUtils.java | 3 ++- .../interop-data-test/src/it/check/pom.xml | 17 ++++++++++++++ lang/java/ipc/pom.xml | 6 +---- .../mapred/tether/TestWordCountTether.java | 1 + lang/java/pom.xml | 22 +++++++++++++++++++ .../org/apache/avro/tool/TestTetherTool.java | 1 + 10 files changed, 62 insertions(+), 16 deletions(-) diff --git a/lang/java/avro/pom.xml b/lang/java/avro/pom.xml index 3f0f0f10bc0..8cab8b75f5c 100644 --- a/lang/java/avro/pom.xml +++ b/lang/java/avro/pom.xml @@ -90,10 +90,6 @@ maven-surefire-plugin none - - java.net.URI,java.net.URL,java.io.File,java.util.HashMap,java.util.List,java.util.Collection,java.util.Map,java.util.Set,java.util.concurrent.ConcurrentHashMap,java.util.LinkedHashMap,java.util.TreeMap - org.apache.avro - diff --git a/lang/java/avro/src/it/pom.xml b/lang/java/avro/src/it/pom.xml index 6911b573ab4..03e903abbde 100644 --- a/lang/java/avro/src/it/pom.xml +++ b/lang/java/avro/src/it/pom.xml @@ -90,12 +90,24 @@ maven-surefire-plugin @maven-surefire-plugin.version@ - - java.net.URI,java.net.URL,java.io.File,java.util.HashMap,java.util.List,java.util.Collection,java.util.Map,java.util.Set,java.util.concurrent.ConcurrentHashMap,java.util.LinkedHashMap,java.util.TreeMap - org.apache.avro - false true + + + + java.net.URI,java.net.URL, + java.io.File, + java.util.HashMap, + java.util.List, + java.util.Collection, + java.util.Map, + java.util.Set, + java.util.concurrent.ConcurrentHashMap, + java.util.LinkedHashMap, + java.util.TreeMap + + org.apache.avro + diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java index eb81046e812..ca9da138c38 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java +++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java @@ -49,7 +49,7 @@ public class SpecificDatumReader extends GenericDatumReader { * @see ClassSecurityValidator */ @Deprecated - public static final String[] SERIALIZABLE_CLASSES = SystemPropertiesPredicate.SERIALIZABLE_PACKAGES + public static final String[] SERIALIZABLE_CLASSES = SystemPropertiesPredicate.SERIALIZABLE_CLASSES .toArray(new String[0]); public SpecificDatumReader() { diff --git a/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java b/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java index cdb1c95db4b..b50d0f0250a 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java +++ b/lang/java/avro/src/main/java/org/apache/avro/util/ClassSecurityValidator.java @@ -106,7 +106,7 @@ default void forbiddenClass(String className) { throw new SecurityException("Forbidden " + className + "! This class is not trusted to be included in Avro " + "schemas. You may either use the system properties org.apache.avro.SERIALIZABLE_CLASSES and " + "org.apache.avro.SERIALIZABLE_PACKAGES to set the comma separated list of the classes or packages you trust, " - + "or you can set them via the API (see org.apache.avro.util.ReflectDataValidator)."); + + "or you can set them via the API (see org.apache.avro.util.ClassSecurityValidator)."); } } diff --git a/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java b/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java index 360faf5ac67..c21f276d6d0 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java +++ b/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java @@ -93,7 +93,8 @@ private static Class forName(String className, ClassLoader classLoader) { if (classLoader != null && className != null) { try { // Load the class without initializing it so we can distinguish between - // ClassNotFoundException and SecurityException may be thrown by the validator. + // ClassNotFoundException and SecurityException (that may be thrown by the + // validator). c = Class.forName(className, false, classLoader); ClassSecurityValidator.validate(c); } catch (ClassNotFoundException e) { diff --git a/lang/java/interop-data-test/src/it/check/pom.xml b/lang/java/interop-data-test/src/it/check/pom.xml index 34159171689..0c8bf0d2406 100644 --- a/lang/java/interop-data-test/src/it/check/pom.xml +++ b/lang/java/interop-data-test/src/it/check/pom.xml @@ -99,6 +99,23 @@ false ${interop.datadir} + + + + java.net.URI, + java.net.URL, + java.io.File, + java.util.HashMap, + java.util.List, + java.util.Collection, + java.util.Map, + java.util.Set, + java.util.concurrent.ConcurrentHashMap, + java.util.LinkedHashMap, + java.util.TreeMap + + org.apache.avro + diff --git a/lang/java/ipc/pom.xml b/lang/java/ipc/pom.xml index d6e78afd053..6f5fb359df9 100644 --- a/lang/java/ipc/pom.xml +++ b/lang/java/ipc/pom.xml @@ -61,11 +61,7 @@ 1 false - none - - java.net.URI,java.net.URL,java.io.File,java.util.HashMap,java.util.List,java.util.Collection,java.util.Map,java.util.Set,java.util.concurrent.ConcurrentHashMap,java.util.LinkedHashMap,java.util.TreeMap - org.apache.avro - + none diff --git a/lang/java/mapred/src/test/java/org/apache/avro/mapred/tether/TestWordCountTether.java b/lang/java/mapred/src/test/java/org/apache/avro/mapred/tether/TestWordCountTether.java index e538895f758..5f621d28a84 100644 --- a/lang/java/mapred/src/test/java/org/apache/avro/mapred/tether/TestWordCountTether.java +++ b/lang/java/mapred/src/test/java/org/apache/avro/mapred/tether/TestWordCountTether.java @@ -77,6 +77,7 @@ private void _runjob(String proto) throws Exception { List execargs = new ArrayList<>(); execargs.add("-classpath"); execargs.add(System.getProperty("java.class.path")); + execargs.add("-Dorg.apache.avro.SERIALIZABLE_PACKAGES=org.apache.avro"); execargs.add("org.apache.avro.mapred.tether.WordCountTask"); FileInputFormat.addInputPaths(job, inputPath.toString()); diff --git a/lang/java/pom.xml b/lang/java/pom.xml index a80c177121d..7c12fa4433c 100644 --- a/lang/java/pom.xml +++ b/lang/java/pom.xml @@ -194,6 +194,28 @@ true false -Xmx1000m + + + + java.net.URI, + java.net.URL, + java.io.File, + java.util.HashMap, + java.util.List, + java.util.Collection, + java.util.Map, + java.util.Set, + java.util.concurrent.ConcurrentHashMap, + java.util.LinkedHashMap, + java.util.TreeMap, + example.avro.Bar, + com.google.protobuf.Timestamp + + + org.apache.avro, + test + + diff --git a/lang/java/tools/src/test/java/org/apache/avro/tool/TestTetherTool.java b/lang/java/tools/src/test/java/org/apache/avro/tool/TestTetherTool.java index c4e5639e2d1..d5fc1f35819 100644 --- a/lang/java/tools/src/test/java/org/apache/avro/tool/TestTetherTool.java +++ b/lang/java/tools/src/test/java/org/apache/avro/tool/TestTetherTool.java @@ -76,6 +76,7 @@ void test() throws Exception { // create a string of the arguments String execargs = "-classpath " + System.getProperty("java.class.path"); + execargs += " -Dorg.apache.avro.SERIALIZABLE_PACKAGES=org.apache.avro"; execargs += " org.apache.avro.mapred.tether.WordCountTask"; // Create a list of the arguments to pass to the tull run method From 7bdd7cdb57e55377a672df8b1fe1ef71d73a80b3 Mon Sep 17 00:00:00 2001 From: Gabor Szadovszky Date: Tue, 21 Oct 2025 16:10:43 +0200 Subject: [PATCH 4/4] Fix system property settings in pomx --- lang/java/avro/src/it/pom.xml | 4 ++- .../interop-data-test/src/it/check/pom.xml | 32 +++++++++---------- lang/java/pom.xml | 4 ++- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/lang/java/avro/src/it/pom.xml b/lang/java/avro/src/it/pom.xml index 03e903abbde..bd9bc523d7a 100644 --- a/lang/java/avro/src/it/pom.xml +++ b/lang/java/avro/src/it/pom.xml @@ -92,8 +92,9 @@ false true - + + java.net.URI,java.net.URL, java.io.File, @@ -107,6 +108,7 @@ java.util.TreeMap org.apache.avro + diff --git a/lang/java/interop-data-test/src/it/check/pom.xml b/lang/java/interop-data-test/src/it/check/pom.xml index 0c8bf0d2406..64eeaf4c850 100644 --- a/lang/java/interop-data-test/src/it/check/pom.xml +++ b/lang/java/interop-data-test/src/it/check/pom.xml @@ -99,23 +99,23 @@ false ${interop.datadir} + - - - java.net.URI, - java.net.URL, - java.io.File, - java.util.HashMap, - java.util.List, - java.util.Collection, - java.util.Map, - java.util.Set, - java.util.concurrent.ConcurrentHashMap, - java.util.LinkedHashMap, - java.util.TreeMap - - org.apache.avro - + + java.net.URI, + java.net.URL, + java.io.File, + java.util.HashMap, + java.util.List, + java.util.Collection, + java.util.Map, + java.util.Set, + java.util.concurrent.ConcurrentHashMap, + java.util.LinkedHashMap, + java.util.TreeMap + + org.apache.avro + diff --git a/lang/java/pom.xml b/lang/java/pom.xml index 7c12fa4433c..82f482963cc 100644 --- a/lang/java/pom.xml +++ b/lang/java/pom.xml @@ -194,8 +194,9 @@ true false -Xmx1000m - + + java.net.URI, java.net.URL, @@ -215,6 +216,7 @@ org.apache.avro, test +