Croway commented on code in PR #26026:
URL: https://github.com/apache/camel/pull/26026#discussion_r3912239456
##########
components/camel-avro-rpc/camel-avro-rpc-component/src/main/java/org/apache/camel/component/avro/AvroEndpoint.java:
##########
@@ -114,4 +118,21 @@ private void validateConfiguration(AvroConfiguration
config) throws Exception {
}
}
}
+
+ private void configureClassSecurity(AvroConfiguration config) {
+ AvroClassSecuritySupport.ensureAvroIpcPackagesTrusted();
+
AvroClassSecuritySupport.trustPackages(config.getSerializablePackages());
+ AvroClassSecuritySupport.trustClassName(config.getProtocolClassName());
+ if (config.getProtocol() != null) {
+
AvroClassSecuritySupport.trustPackages(config.getProtocol().getNamespace());
+ for (Schema type : config.getProtocol().getTypes()) {
+ if (type.getNamespace() != null) {
Review Comment:
**JDK packages leak into the JVM-global allowlist for reflection protocols.**
For `protocolClassName` pointing at a plain interface,
`ReflectData.getProtocol()` registers a named error RECORD for every declared
exception type, with the namespace set to the exception's package. So
`Protocol.getTypes()` contains e.g. `java.io.IOException` and
`java.lang.Exception`, and this loop adds `java.io.` / `java.lang.` to the
trusted prefixes.
Probe against avro-1.12.2:
```java
interface Svc {
void put(String s) throws IOException;
Pojo get(Pojo p) throws Exception;
}
// ReflectData.get().getProtocol(Svc.class).getTypes()
// -> java.io.IOException (ns java.io), java.lang.Exception (ns java.lang)
```
After `avro:netty:0.0.0.0:9090?protocolClassName=com.acme.Svc` initialises,
`ClassSecurityValidator.validate(java.io.ObjectInputStream.class)` and
`validate(java.lang.ProcessBuilder.class)` pass for every Avro reader in the
process, including non-Camel `ReflectDatumReader`s resolving attacker-supplied
`java-class` props. That's the gadget-loading path Avro's validator exists to
block, and it is granted silently with no option set.
Suggest: skip `type.isError()` schemas, filter system prefixes (`java.`,
`javax.`, `jdk.`, `sun.`), or only `trustClassName(type.getFullName())` for
exact names rather than trusting the whole namespace.
_Claude Code on behalf of Croway_
##########
components/camel-avro/src/main/java/org/apache/camel/avro/support/AvroClassSecuritySupport.java:
##########
@@ -0,0 +1,172 @@
+/*
+ * 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
+ *
+ * http://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.camel.avro.support;
+
+import java.util.Arrays;
+import java.util.NavigableSet;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.avro.util.ClassSecurityValidator;
+import org.apache.avro.util.ClassSecurityValidator.ClassSecurityPredicate;
+
+/**
+ * Configures Apache Avro {@link ClassSecurityValidator} with Camel trusted
packages.
+ * <p>
+ * Avro 1.12+ validates classes resolved from schemas. Camel automatically
trusts packages derived from configured
+ * protocol or schema classes. Additional packages can be configured through
the {@code serializablePackages} endpoint
+ * option.
+ * <p>
+ * Trusted packages are stored in a JVM-wide registry shared by all Camel
contexts in the process. Trust is cumulative
+ * and cannot be revoked in production.
+ */
+public final class AvroClassSecuritySupport {
+
+ private static final Set<String> TRUSTED_PACKAGES =
ConcurrentHashMap.newKeySet();
+
+ private static final Set<String> TRUSTED_CLASSES =
ConcurrentHashMap.newKeySet();
+
+ private static final Object LOCK = new Object();
+
+ private static final ClassSecurityPredicate CAMEL_TRUSTED =
AvroClassSecuritySupport::isCamelTrusted;
+
+ private static volatile ClassSecurityPredicate baseValidator =
ClassSecurityValidator.DEFAULT;
+
+ private static volatile ClassSecurityPredicate installedGlobal;
+
+ private static volatile NavigableSet<String> normalizedPackagePrefixes =
new TreeSet<>();
+
+ private AvroClassSecuritySupport() {
+ }
+
+ /**
+ * Trusts Avro IPC classes required for camel-avro-rpc handshake.
+ */
+ public static void ensureAvroIpcPackagesTrusted() {
+ trustPackages("org.apache.avro.ipc");
+ }
+
+ /**
+ * Trusts the exact class name and its package for schema resolution.
+ */
+ public static void trustClassName(String className) {
+ if (className == null || className.isBlank()) {
+ return;
+ }
+ synchronized (LOCK) {
+ TRUSTED_CLASSES.add(className);
+ int lastDot = className.lastIndexOf('.');
+ if (lastDot > 0) {
+ TRUSTED_PACKAGES.add(normalizePackage(className.substring(0,
lastDot)));
+ }
+ rebuildNormalizedPackagePrefixes();
+ refreshGlobal();
+ }
+ }
+
+ /**
+ * Trusts the comma-separated list of packages.
+ */
+ public static void trustPackages(String packages) {
+ if (packages == null || packages.isBlank()) {
+ return;
+ }
+ trustPackages(parsePackages(packages).toArray(String[]::new));
+ }
+
+ /**
+ * Trusts the given packages.
+ */
+ public static void trustPackages(String... packages) {
+ if (packages == null || packages.length == 0) {
+ return;
+ }
+ synchronized (LOCK) {
+ for (String pkg : packages) {
+ if (pkg != null && !pkg.isBlank()) {
+ TRUSTED_PACKAGES.add(normalizePackage(pkg));
+ }
+ }
+ rebuildNormalizedPackagePrefixes();
+ refreshGlobal();
+ }
+ }
+
+ /**
+ * Clears Camel-managed trusted classes and packages. Intended for tests.
+ */
+ public static void resetForTesting() {
+ synchronized (LOCK) {
+ TRUSTED_PACKAGES.clear();
+ TRUSTED_CLASSES.clear();
+ normalizedPackagePrefixes = new TreeSet<>();
+ baseValidator = ClassSecurityValidator.DEFAULT;
+ installedGlobal = null;
+ ClassSecurityValidator.setGlobal(ClassSecurityValidator.DEFAULT);
+ }
+ }
+
+ private static void refreshGlobal() {
+ if (installedGlobal == null) {
Review Comment:
**A validator installed by other code after Camel's first trust call is
silently discarded.**
The pre-existing global is captured only when `installedGlobal == null`.
Every later `trustPackages` / `trustClassName` call unconditionally does
`setGlobal(composite(baseValidator, CAMEL_TRUSTED))`, overwriting whatever was
installed in between.
Runtime repro against avro-1.12.2 with this class:
```java
AvroClassSecuritySupport.trustPackages("a.b");
ClassSecurityValidator.setGlobal(
ClassSecurityValidator.composite(ClassSecurityValidator.getGlobal(), c
-> c == UUID.class));
ClassSecurityValidator.validate(UUID.class); // ok
AvroClassSecuritySupport.trustPackages("c.d"); // any new endpoint /
data format init, second CamelContext, or schema-less marshal message
ClassSecurityValidator.validate(UUID.class); // SecurityException:
Forbidden java.util.UUID
```
Concretely, a Kafka Avro serde or an operator-installed stricter predicate
layered after Camel start is dropped without a log line, and two classloader
copies of this class clobber each other.
Suggest: in `refreshGlobal`, re-read `getGlobal()` and, if it is not
identity-equal to `installedGlobal`, adopt it as the new `baseValidator` before
wrapping.
_Claude Code on behalf of Croway_
##########
components/camel-avro/src/main/java/org/apache/camel/dataformat/avro/AvroDataFormat.java:
##########
@@ -135,6 +158,9 @@ protected Schema loadSchema(String className) throws
CamelException, ClassNotFou
@Override
public void marshal(Exchange exchange, Object graph, OutputStream
outputStream) throws Exception {
+ if (actualSchema == null) {
+
AvroClassSecuritySupport.trustClassName(graph.getClass().getName());
Review Comment:
**Schema-less marshal trusts the body's package before validating the body.**
`trustClassName` also trusts the class's whole package, JVM-wide and with no
revocation API outside `resetForTesting`. Here it runs before `loadSchema`
checks that the body is a `GenericContainer`. So with
`from("direct:in").marshal().avro(AvroLibrary.ApacheAvro)` (the schema-less
usage exercised in `AvroMarshalAndUnmarshalTest`), a String / HashMap / any
wrong body class still throws `CamelException("must be instanceof
GenericContainer")` as before, but as a side effect `java.lang.` / `java.util.`
/ whatever package the body came from is now permanently on Avro's global
allowlist.
Secondary: on the happy path this runs on every exchange, taking
`AvroClassSecuritySupport.LOCK`, rebuilding the `TreeSet` and calling
`ClassSecurityValidator.setGlobal()` each time, which serialises all marshal
threads.
Suggest: trust only after the `GenericContainer` check succeeds (e.g. inside
`loadSchema` after the `isAssignableFrom` test), trust the exact class rather
than its package, and short-circuit in `trustClassName` when the name is
already in `TRUSTED_CLASSES`.
_Claude Code on behalf of Croway_
##########
components/camel-avro/src/main/java/org/apache/camel/dataformat/avro/AvroDataFormat.java:
##########
@@ -91,6 +97,11 @@ protected void doInit() throws Exception {
} else if (instanceClassName != null) {
actualSchema = loadSchema(instanceClassName);
}
+
+ if (actualSchema != null) {
+
AvroClassSecuritySupport.trustPackages(actualSchema.getNamespace());
+
AvroClassSecuritySupport.trustClassName(actualSchema.getFullName());
Review Comment:
**Regression: nested named types are not trusted, so unmarshal fails on the
first message.**
Only the root schema's namespace and full name are inferred here. But
`SpecificDatumReader` resolves every nested record / enum / fixed via
`SpecificData.getClass -> ClassUtils.forName ->
ClassSecurityValidator.validate`, and the `SecurityException` is not caught
(`SpecificDataNoCache.newRecord` calls `getClass` first).
Scenario: generated `com.acme.orders.Order` with a field of generated type
`com.acme.common.Money`. `unmarshal().avro(AvroLibrary.ApacheAvro,
Order.SCHEMA$)` starts fine (only `com.acme.orders` trusted) and the first
message fails with `SecurityException: Forbidden com.acme.common.Money`. On
4.22 with the documented `-Dorg.apache.avro.SERIALIZABLE_PACKAGES` this worked,
and the new upgrade guide entry tells users Camel now derives trust from the
schema automatically, so people will drop the property and hit this.
Suggest: walk fields, array items, map values and union branches (with a
visited set) collecting named types, then trust each full name. The same helper
would replace the `Protocol.getTypes()` loop in `AvroEndpoint` and could apply
the JDK-package filter from the other comment in one place.
_Claude Code on behalf of Croway_
##########
components/camel-avro/src/main/java/org/apache/camel/avro/support/AvroClassSecuritySupport.java:
##########
@@ -0,0 +1,172 @@
+/*
+ * 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
+ *
+ * http://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.camel.avro.support;
+
+import java.util.Arrays;
+import java.util.NavigableSet;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.avro.util.ClassSecurityValidator;
+import org.apache.avro.util.ClassSecurityValidator.ClassSecurityPredicate;
+
+/**
+ * Configures Apache Avro {@link ClassSecurityValidator} with Camel trusted
packages.
+ * <p>
+ * Avro 1.12+ validates classes resolved from schemas. Camel automatically
trusts packages derived from configured
+ * protocol or schema classes. Additional packages can be configured through
the {@code serializablePackages} endpoint
+ * option.
+ * <p>
+ * Trusted packages are stored in a JVM-wide registry shared by all Camel
contexts in the process. Trust is cumulative
+ * and cannot be revoked in production.
+ */
+public final class AvroClassSecuritySupport {
+
+ private static final Set<String> TRUSTED_PACKAGES =
ConcurrentHashMap.newKeySet();
+
+ private static final Set<String> TRUSTED_CLASSES =
ConcurrentHashMap.newKeySet();
+
+ private static final Object LOCK = new Object();
+
+ private static final ClassSecurityPredicate CAMEL_TRUSTED =
AvroClassSecuritySupport::isCamelTrusted;
+
+ private static volatile ClassSecurityPredicate baseValidator =
ClassSecurityValidator.DEFAULT;
+
+ private static volatile ClassSecurityPredicate installedGlobal;
+
+ private static volatile NavigableSet<String> normalizedPackagePrefixes =
new TreeSet<>();
+
+ private AvroClassSecuritySupport() {
+ }
+
+ /**
+ * Trusts Avro IPC classes required for camel-avro-rpc handshake.
+ */
+ public static void ensureAvroIpcPackagesTrusted() {
+ trustPackages("org.apache.avro.ipc");
+ }
+
+ /**
+ * Trusts the exact class name and its package for schema resolution.
+ */
+ public static void trustClassName(String className) {
+ if (className == null || className.isBlank()) {
+ return;
+ }
+ synchronized (LOCK) {
+ TRUSTED_CLASSES.add(className);
+ int lastDot = className.lastIndexOf('.');
+ if (lastDot > 0) {
+ TRUSTED_PACKAGES.add(normalizePackage(className.substring(0,
lastDot)));
+ }
+ rebuildNormalizedPackagePrefixes();
+ refreshGlobal();
+ }
+ }
+
+ /**
+ * Trusts the comma-separated list of packages.
+ */
+ public static void trustPackages(String packages) {
+ if (packages == null || packages.isBlank()) {
+ return;
+ }
+ trustPackages(parsePackages(packages).toArray(String[]::new));
+ }
+
+ /**
+ * Trusts the given packages.
+ */
+ public static void trustPackages(String... packages) {
+ if (packages == null || packages.length == 0) {
+ return;
+ }
+ synchronized (LOCK) {
+ for (String pkg : packages) {
+ if (pkg != null && !pkg.isBlank()) {
+ TRUSTED_PACKAGES.add(normalizePackage(pkg));
+ }
+ }
+ rebuildNormalizedPackagePrefixes();
+ refreshGlobal();
+ }
+ }
+
+ /**
+ * Clears Camel-managed trusted classes and packages. Intended for tests.
+ */
+ public static void resetForTesting() {
+ synchronized (LOCK) {
+ TRUSTED_PACKAGES.clear();
+ TRUSTED_CLASSES.clear();
+ normalizedPackagePrefixes = new TreeSet<>();
+ baseValidator = ClassSecurityValidator.DEFAULT;
+ installedGlobal = null;
+ ClassSecurityValidator.setGlobal(ClassSecurityValidator.DEFAULT);
+ }
+ }
+
+ private static void refreshGlobal() {
+ if (installedGlobal == null) {
+ ClassSecurityPredicate current =
ClassSecurityValidator.getGlobal();
+ if (current != null && current != ClassSecurityValidator.DEFAULT) {
+ baseValidator = current;
+ }
+ }
+ installedGlobal = ClassSecurityValidator.composite(baseValidator,
CAMEL_TRUSTED);
+ ClassSecurityValidator.setGlobal(installedGlobal);
+ }
+
+ private static boolean isCamelTrusted(Class<?> clazz) {
+ String className = clazz.getName();
+ if (TRUSTED_CLASSES.contains(className)) {
+ return true;
+ }
+ NavigableSet<String> packages = normalizedPackagePrefixes;
+ String lower = packages.lower(className);
Review Comment:
**Prefix matching only tests the single closest entry, so a nested trusted
package shadows its trusted parent.**
This copies Avro's own `lower()` + `startsWith` trick, but that only works
when no trusted prefix is itself a prefix of another. Camel's automatic
inference (protocol namespace + every type namespace + every class's package)
makes nested prefixes the normal case.
Reproduced in a scratch program:
```
trusted = { "com.example.", "com.example.orders.avro." }
className = "com.example.zzz.Foo"
lower() = "com.example.orders.avro." -> startsWith fails, "com.example."
is never consulted -> rejected
```
Same with the PR's own fixtures:
`avro:netty:...?protocolClassName=org.apache.camel.avro.generated.KeyValueProtocol&serializablePackages=org.apache.camel.avro`
still rejects `org.apache.camel.avro.test.TestPojo` because `lower()` returns
`org.apache.camel.avro.generated.` (`'g' < 't'`).
`shouldMergePackagesAcrossCalls` only passes because `'V'` sorts before `'e'`.
Suggest: iterate the prefix set (it is small), or loop on `lower()` until a
real `startsWith` match or exhaustion.
_Claude Code on behalf of Croway_
##########
components/camel-avro/src/main/java/org/apache/camel/dataformat/avro/AvroDataFormat.java:
##########
@@ -91,6 +97,11 @@ protected void doInit() throws Exception {
} else if (instanceClassName != null) {
actualSchema = loadSchema(instanceClassName);
}
+
+ if (actualSchema != null) {
+
AvroClassSecuritySupport.trustPackages(actualSchema.getNamespace());
Review Comment:
**Regression: non-named root schemas now fail at context start.**
`Schema.getNamespace()` throws `AvroRuntimeException("Not a named type")`
for ARRAY / MAP / UNION / primitive roots. Pre-PR this class only ever called
`getFullName()`, which returns `"array"` / `"union[...]"` and is safe, and
`SpecificDatumWriter` / `SpecificDatumReader` round-trip such roots fine.
Probe against avro-1.12.2:
```java
Schema.createArray(Value.SCHEMA$).getNamespace(); //
AvroRuntimeException: Not a named type
Schema.createUnion(Schema.create(NULL), Value.SCHEMA$).getNamespace(); //
same
```
So `new AvroDataFormat(Schema.createArray(Value.SCHEMA$))` or a `["null",
Record]` top-level schema for unmarshal, which work on `main`, now abort route
startup.
Suggest: guard on `actualSchema.getType()` being RECORD / ENUM / FIXED, or
better, walk the schema graph as suggested on the next line.
_Claude Code on behalf of Croway_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
