Skip to content

Commit 91226e4

Browse files
authored
[fix][sec] Upgrade avro to 1.12.2 (#24992)
1 parent ecf6dae commit 91226e4

16 files changed

Lines changed: 1484 additions & 13 deletions

File tree

build-logic/conventions/src/main/kotlin/pulsar.java-conventions.gradle.kts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,12 @@ tasks.withType<Test>().configureEach {
264264
// loopback would otherwise fail to start.
265265
"-Djava.net.preferIPv4Stack=true",
266266
)
267+
// Deliberately no org.apache.avro.SERIALIZABLE_* system properties here. Avro 1.12.2 (AVRO-4189)
268+
// only reflects over classes that are explicitly trusted, and Pulsar declares them where the
269+
// application hands over a class: building a schema from a class trusts it and everything the
270+
// derived schema references. Granting the whole Pulsar namespace here would give every test a
271+
// safety net that production does not have, so a path that fails to declare something would pass
272+
// in CI and fail for users.
267273
if (testJavaMajorVersion >= 24) {
268274
// Netty loads its native libraries (epoll, io_uring, tcnative) through
269275
// java.lang.System::loadLibrary, which is a restricted method as of Java 24. Without this

distribution/server/src/assemble/LICENSE.bin.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,8 +466,8 @@ The Apache Software License, Version 2.0
466466
* zt-zip
467467
- org.zeroturnaround-zt-zip-1.18.2.jar
468468
* Apache Avro
469-
- org.apache.avro-avro-1.12.0.jar
470-
- org.apache.avro-avro-protobuf-1.12.0.jar
469+
- org.apache.avro-avro-1.12.2.jar
470+
- org.apache.avro-avro-protobuf-1.12.2.jar
471471
* Apache Curator
472472
- org.apache.curator-curator-client-5.9.0.jar
473473
- org.apache.curator-curator-framework-5.9.0.jar

distribution/shell/src/assemble/LICENSE.bin.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,8 @@ The Apache Software License, Version 2.0
422422
* Google Error Prone Annotations - error_prone_annotations-2.50.0.jar
423423
* Javassist -- javassist-3.32.0-GA.jar
424424
* Apache Avro
425-
- avro-1.12.0.jar
426-
- avro-protobuf-1.12.0.jar
425+
- avro-1.12.2.jar
426+
- avro-protobuf-1.12.2.jar
427427
* RE2j -- re2j-1.8.jar
428428
* Spotify completable-futures -- completable-futures-0.3.6.jar
429429
* RoaringBitmap -- RoaringBitmap-1.6.20.jar

gradle/libs.versions.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ bouncycastle-bcpkix-fips = "2.1.12"
7676
bouncycastle-bcutil-fips = "2.1.7"
7777
bouncycastle-bc-fips = "2.1.3"
7878
# Serialization
79-
avro = "1.12.0"
79+
avro = "1.12.2"
8080
gson = "2.14.0"
8181
snakeyaml = "2.6"
8282
# Vert.x
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.pulsar.broker.service.schema;
20+
21+
import static org.assertj.core.api.Assertions.assertThat;
22+
import java.util.HashSet;
23+
import java.util.List;
24+
import java.util.Map;
25+
import org.apache.avro.util.ClassSecurityValidator;
26+
import org.apache.avro.util.ClassSecurityValidator.ClassSecurityPredicate;
27+
import org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitState;
28+
import org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateData;
29+
import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLoadData;
30+
import org.apache.pulsar.broker.loadbalance.extensions.data.TopBundlesLoadData;
31+
import org.apache.pulsar.broker.transaction.buffer.metadata.AbortTxnMetadata;
32+
import org.apache.pulsar.broker.transaction.buffer.metadata.TransactionBufferSnapshot;
33+
import org.apache.pulsar.broker.transaction.buffer.metadata.v2.TransactionBufferSnapshotIndexes;
34+
import org.apache.pulsar.client.api.Schema;
35+
import org.apache.pulsar.common.api.proto.CommandSubscribe.SubType;
36+
import org.apache.pulsar.common.events.ActionType;
37+
import org.apache.pulsar.common.events.EventType;
38+
import org.apache.pulsar.common.events.PulsarEvent;
39+
import org.apache.pulsar.common.events.TopicPoliciesEvent;
40+
import org.apache.pulsar.common.policies.data.InactiveTopicDeleteMode;
41+
import org.apache.pulsar.common.policies.data.InactiveTopicPolicies;
42+
import org.apache.pulsar.common.policies.data.PersistencePolicies;
43+
import org.apache.pulsar.common.policies.data.PublishRate;
44+
import org.apache.pulsar.common.policies.data.RetentionPolicies;
45+
import org.apache.pulsar.common.policies.data.SubscribeRate;
46+
import org.apache.pulsar.common.policies.data.TopicPolicies;
47+
import org.apache.pulsar.common.policies.data.impl.BacklogQuotaImpl;
48+
import org.apache.pulsar.common.policies.data.impl.DispatchRateImpl;
49+
import org.apache.pulsar.metadata.api.MetadataEvent;
50+
import org.apache.pulsar.metadata.api.NotificationType;
51+
import org.apache.pulsar.metadata.api.extended.CreateOption;
52+
import org.testng.annotations.AfterMethod;
53+
import org.testng.annotations.BeforeMethod;
54+
import org.testng.annotations.Test;
55+
56+
/**
57+
* Verifies that the broker can serialize every type it writes to its system topics with nothing declared
58+
* in advance — building the schema from the class is what trusts it, exactly as for an application POJO.
59+
*
60+
* <p>Each test pins the global validator to Avro's hardcoded {@code DEFAULT_TRUSTED_CLASSES}, so that
61+
* anything not covered by building the schema fails here rather than being carried by trust some other
62+
* test happened to declare first.
63+
*/
64+
@Test(groups = "broker")
65+
public class PulsarInternalAvroTypesTrustTest {
66+
67+
private ClassSecurityPredicate previousValidator;
68+
69+
@BeforeMethod
70+
public void useProductionBaselineValidator() {
71+
previousValidator = ClassSecurityValidator.getGlobal();
72+
// DEFAULT_TRUSTED_CLASSES excludes the system-properties predicate, unlike DEFAULT.
73+
ClassSecurityValidator.setGlobal(ClassSecurityValidator.DEFAULT_TRUSTED_CLASSES);
74+
}
75+
76+
@AfterMethod(alwaysRun = true)
77+
public void restoreValidator() {
78+
ClassSecurityValidator.setGlobal(previousValidator);
79+
}
80+
81+
@Test
82+
public void testMetadataEventRoundTrips() {
83+
// Published by PulsarMetadataEventSynchronizer. Exercises the HashSet "java-class" property and
84+
// the CreateOption / NotificationType enums.
85+
MetadataEvent event = new MetadataEvent("/path", "value".getBytes(), new HashSet<>(List.of(
86+
CreateOption.Ephemeral)), 1L, 2L, "cluster", NotificationType.Created);
87+
88+
Schema<MetadataEvent> schema = Schema.AVRO(MetadataEvent.class);
89+
90+
assertThat(schema.decode(schema.encode(event))).isEqualTo(event);
91+
}
92+
93+
@Test
94+
public void testPulsarEventRoundTrips() {
95+
// Published to the __change_events system topic, and reaches TopicPolicies through
96+
// TopicPoliciesEvent.
97+
PulsarEvent event = PulsarEvent.builder()
98+
.eventType(EventType.TOPIC_POLICY)
99+
.actionType(ActionType.UPDATE)
100+
.replicateTo(new HashSet<>(List.of("cluster-a")))
101+
.topicPoliciesEvent(TopicPoliciesEvent.builder()
102+
.domain("persistent")
103+
.tenant("public")
104+
.namespace("default")
105+
.topic("t1")
106+
// Populate the nested types too: Avro only resolves the class behind a schema
107+
// node when it actually writes it, so leaving these null would skip the enum
108+
// and the policy impl types entirely and hide a missing allow-list entry.
109+
.policies(TopicPolicies.builder()
110+
.subscriptionTypesEnabled(List.of(SubType.Shared, SubType.Key_Shared))
111+
.replicationClusters(List.of("cluster-a"))
112+
.backLogQuotaMap(Map.of("destination_storage",
113+
(BacklogQuotaImpl) BacklogQuotaImpl.builder().limitSize(1024L).build()))
114+
.dispatchRate(DispatchRateImpl.builder().dispatchThrottlingRateInMsg(10).build())
115+
.retentionPolicies(new RetentionPolicies(1, 2))
116+
.persistence(new PersistencePolicies(1, 1, 1, 1.0))
117+
.inactiveTopicPolicies(new InactiveTopicPolicies(
118+
InactiveTopicDeleteMode.delete_when_no_subscriptions, 60, true))
119+
.publishRate(new PublishRate(10, 1024))
120+
.subscribeRate(new SubscribeRate(10, 30))
121+
.build())
122+
.build())
123+
.build();
124+
125+
Schema<PulsarEvent> schema = Schema.AVRO(PulsarEvent.class);
126+
127+
assertThat(schema.decode(schema.encode(event))).isEqualTo(event);
128+
}
129+
130+
@Test
131+
public void testTransactionBufferSnapshotRoundTrips() {
132+
// The original snapshot format lives in ...buffer.metadata, not ...buffer.metadata.v2, so it is
133+
// only covered if the parent package is trusted.
134+
TransactionBufferSnapshot snapshot = new TransactionBufferSnapshot();
135+
snapshot.setTopicName("persistent://public/default/t1");
136+
snapshot.setMaxReadPositionLedgerId(1L);
137+
snapshot.setMaxReadPositionEntryId(2L);
138+
AbortTxnMetadata aborted = new AbortTxnMetadata();
139+
aborted.setTxnIdMostBits(3L);
140+
aborted.setTxnIdLeastBits(4L);
141+
snapshot.setAborts(List.of(aborted));
142+
143+
Schema<TransactionBufferSnapshot> schema = Schema.AVRO(TransactionBufferSnapshot.class);
144+
145+
// These snapshot classes do not define equals(), so compare field by field.
146+
assertThat(schema.decode(schema.encode(snapshot)))
147+
.usingRecursiveComparison().isEqualTo(snapshot);
148+
}
149+
150+
@Test
151+
public void testTransactionBufferSnapshotIndexesRoundTrip() {
152+
TransactionBufferSnapshotIndexes indexes = new TransactionBufferSnapshotIndexes();
153+
indexes.setTopicName("persistent://public/default/t1");
154+
indexes.setIndexList(List.of());
155+
156+
Schema<TransactionBufferSnapshotIndexes> schema = Schema.AVRO(TransactionBufferSnapshotIndexes.class);
157+
158+
assertThat(schema.decode(schema.encode(indexes)))
159+
.usingRecursiveComparison().isEqualTo(indexes);
160+
}
161+
162+
@Test
163+
public void testJsonSchemaOverBrokerInternalTypesRoundTrips() {
164+
// Schema.JSON also derives an Avro schema from the POJO, but reads and writes it with Jackson
165+
// rather than ReflectDatumReader/Writer, so it does not resolve classes reflectively and does not
166+
// need its types in the allow-list. These are the broker's own JSON-schema types, used by the
167+
// extensible load balancer; pin the behaviour so a future change to the JSON read/write path
168+
// cannot start requiring trust without a test noticing.
169+
Schema<ServiceUnitStateData> unitStateSchema = Schema.JSON(ServiceUnitStateData.class);
170+
ServiceUnitStateData unitState = new ServiceUnitStateData(
171+
ServiceUnitState.Owned, "dst-broker", "src-broker", 1L);
172+
assertThat(unitStateSchema.decode(unitStateSchema.encode(unitState)))
173+
.usingRecursiveComparison().isEqualTo(unitState);
174+
175+
Schema<BrokerLoadData> brokerLoadSchema = Schema.JSON(BrokerLoadData.class);
176+
assertThat(brokerLoadSchema.decode(brokerLoadSchema.encode(new BrokerLoadData()))).isNotNull();
177+
178+
Schema<TopBundlesLoadData> topBundlesSchema = Schema.JSON(TopBundlesLoadData.class);
179+
assertThat(topBundlesSchema.decode(topBundlesSchema.encode(new TopBundlesLoadData()))).isNotNull();
180+
}
181+
182+
@Test
183+
public void testTrustDoesNotLeakToUnrelatedApplicationClasses() {
184+
// Serializing the broker's own types must not widen the set for anything else. Build a schema
185+
// first so Pulsar's predicate is actually installed on top of the pinned baseline - otherwise
186+
// the assertion below would be answered by the baseline alone and could never fail, whatever
187+
// auto-registration did.
188+
Schema.AVRO(MetadataEvent.class);
189+
assertThat(ClassSecurityValidator.getGlobal().isTrusted(MetadataEvent.class)).isTrue();
190+
191+
assertThat(ClassSecurityValidator.getGlobal().isTrusted(UnusedPojo.class)).isFalse();
192+
}
193+
194+
/** Never passed to a schema, so nothing should have trusted it. */
195+
public static class UnusedPojo {
196+
private String field = "value";
197+
198+
public String getField() {
199+
return field;
200+
}
201+
202+
public void setField(String field) {
203+
this.field = field;
204+
}
205+
}
206+
}

pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/AvroSchema.java

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828
import org.apache.avro.LogicalTypes;
2929
import org.apache.avro.data.TimeConversions;
3030
import org.apache.avro.reflect.ReflectData;
31+
import org.apache.commons.lang3.StringUtils;
3132
import org.apache.pulsar.client.api.Schema;
3233
import org.apache.pulsar.client.api.schema.SchemaDefinition;
3334
import org.apache.pulsar.client.api.schema.SchemaReader;
3435
import org.apache.pulsar.client.api.schema.SchemaWriter;
3536
import org.apache.pulsar.client.impl.schema.reader.MultiVersionAvroReader;
3637
import org.apache.pulsar.client.impl.schema.writer.AvroWriter;
38+
import org.apache.pulsar.client.schema.AvroTrustedClasses;
3739
import org.apache.pulsar.common.schema.SchemaInfo;
3840
import org.apache.pulsar.common.schema.SchemaType;
3941
import org.joda.time.DateTime;
@@ -82,8 +84,10 @@ public Schema<T> clone() {
8284

8385
public static <T> AvroSchema<T> of(SchemaDefinition<T> schemaDefinition) {
8486
if (schemaDefinition.getSchemaReaderOpt().isPresent() && schemaDefinition.getSchemaWriterOpt().isPresent()) {
87+
SchemaInfo schemaInfo = parseSchemaInfo(schemaDefinition, SchemaType.AVRO);
88+
trustApplicationPojo(schemaDefinition, schemaInfo);
8589
return new AvroSchema<>(schemaDefinition.getSchemaReaderOpt().get(),
86-
schemaDefinition.getSchemaWriterOpt().get(), parseSchemaInfo(schemaDefinition, SchemaType.AVRO));
90+
schemaDefinition.getSchemaWriterOpt().get(), schemaInfo);
8791
}
8892
ClassLoader pojoClassLoader = null;
8993
if (schemaDefinition.getClassLoader() != null) {
@@ -92,7 +96,32 @@ public static <T> AvroSchema<T> of(SchemaDefinition<T> schemaDefinition) {
9296
pojoClassLoader = schemaDefinition.getPojo().getClassLoader();
9397
}
9498

95-
return new AvroSchema<>(parseSchemaInfo(schemaDefinition, SchemaType.AVRO), pojoClassLoader);
99+
SchemaInfo schemaInfo = parseSchemaInfo(schemaDefinition, SchemaType.AVRO);
100+
trustApplicationPojo(schemaDefinition, schemaInfo);
101+
return new AvroSchema<>(schemaInfo, pojoClassLoader);
102+
}
103+
104+
/**
105+
* Lets Avro reflect over the class the application supplied, and over the types the schema derived
106+
* from it references, so that using {@code Schema.AVRO(MyPojo.class)} needs no further declaration.
107+
*
108+
* <p>Guarded on the POJO being present, which is what keeps this on the right side of the trust
109+
* boundary. A definition built with {@code withJsonDef(...)} carries no POJO — that is how
110+
* {@link AutoConsumeSchema} passes a schema fetched from the registry — and a schema chosen by
111+
* whoever registered it must not be able to widen the allow-list.
112+
*/
113+
private static <T> void trustApplicationPojo(SchemaDefinition<T> schemaDefinition, SchemaInfo schemaInfo) {
114+
if (StringUtils.isNotBlank(schemaDefinition.getJsonDef())) {
115+
// SchemaUtil.createAvroSchema gives jsonDef precedence, so the schema was parsed from that
116+
// rather than derived from the POJO. Only the stock builder forbids setting both, and
117+
// SchemaDefinition is a public interface, so check the same condition here rather than
118+
// assuming: expanding trust from a schema Pulsar did not derive would let whoever supplied
119+
// that document choose the class names. The class the application named is still its own,
120+
// so trust that much; anything further in such a schema is for the application to declare.
121+
AvroTrustedClasses.trustExactly(schemaDefinition.getPojo());
122+
return;
123+
}
124+
AvroTrustedClasses.trustApplicationSchema(schemaDefinition.getPojo(), schemaInfo);
96125
}
97126

98127
public static <T> AvroSchema<T> of(Class<T> pojo) {

pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/ProtobufSchema.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.pulsar.client.api.schema.SchemaDefinition;
3535
import org.apache.pulsar.client.impl.schema.reader.ProtobufReader;
3636
import org.apache.pulsar.client.impl.schema.writer.ProtobufWriter;
37+
import org.apache.pulsar.client.schema.AvroTrustedClasses;
3738
import org.apache.pulsar.common.schema.SchemaInfo;
3839
import org.apache.pulsar.common.schema.SchemaType;
3940
import org.apache.pulsar.common.util.ObjectMapperFactory;
@@ -109,6 +110,13 @@ public static <T> ProtobufSchema of(SchemaDefinition<T> schemaDefinition) {
109110
+ " is not assignable from " + pojo.getName());
110111
}
111112

113+
// The application named this class, so let Avro reflect over it and over the protobuf runtime
114+
// types avro-protobuf resolves alongside it. This has to happen before the schema is derived,
115+
// not after: deriving it is itself a reflective resolution, which is also why there is no
116+
// derived schema to expand from here. Encoding and decoding go through protobuf's own reader
117+
// and writer, so the message's nested types are never resolved by Avro.
118+
AvroTrustedClasses.trustExactly(pojo);
119+
112120
SchemaInfo schemaInfo = SchemaInfoImpl.builder()
113121
.schema(createProtobufAvroSchema(schemaDefinition.getPojo()).toString().getBytes(UTF_8))
114122
.type(SchemaType.PROTOBUF)

pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/generic/GenericAvroReader.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Optional;
2626
import java.util.stream.Collectors;
2727
import lombok.CustomLog;
28+
import org.apache.avro.AvroRuntimeException;
2829
import org.apache.avro.Schema;
2930
import org.apache.avro.generic.GenericDatumReader;
3031
import org.apache.avro.io.BinaryEncoder;
@@ -87,7 +88,11 @@ public GenericAvroRecord read(byte[] bytes, int offset, int length) {
8788
null,
8889
decoder);
8990
return new GenericAvroRecord(schemaVersion, schema, fields, avroRecord);
90-
} catch (IOException | IndexOutOfBoundsException e) {
91+
} catch (IOException | IndexOutOfBoundsException | AvroRuntimeException e) {
92+
// Avro 1.12.1 enabled the fast reader by default (AVRO-3230). It reports malformed data as
93+
// AvroTypeException (e.g. "Enumeration out of range") where the classic path raised an
94+
// IndexOutOfBoundsException, so catch AvroRuntimeException too and keep reporting decoding
95+
// failures as SchemaSerializationException.
9196
throw new SchemaSerializationException(e);
9297
}
9398
}
@@ -101,7 +106,10 @@ public GenericRecord read(InputStream inputStream) {
101106
null,
102107
decoder);
103108
return new GenericAvroRecord(schemaVersion, schema, fields, avroRecord);
104-
} catch (IOException | IndexOutOfBoundsException e) {
109+
} catch (IOException | IndexOutOfBoundsException | AvroRuntimeException e) {
110+
// See the byte[] overload above: the fast reader, on by default since Avro 1.12.1, reports
111+
// malformed data as AvroTypeException. This is the overload consumers actually reach, via
112+
// MessageImpl.decodeBySchema -> AbstractStructSchema.decode -> AbstractMultiVersionReader.
105113
throw new SchemaSerializationException(e);
106114
} finally {
107115
try {

0 commit comments

Comments
 (0)