You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by sa...@apache.org on 2026/10/22 13:12:43 UTC
(spark) branch branch-4.3 updated: [SPARK-58785][CORE][TEST] Add tests for FilteredObjectInputStream resolveClass allow-list
This is an automated email from the ASF dual-hosted git repository.
sarutak pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.3 by this push:
new d2a8dee58663 [SPARK-58785][CORE][TEST] Add tests for FilteredObjectInputStream resolveClass allow-list
d2a8dee58663 is described below
commit d2a8dee586637d45fb2c393e029f81092a8b93d7
Author: Rajesh Vakkalagadda <vr...@meta.com>
AuthorDate: Thu Oct 22 22:11:56 2026 +0900
[SPARK-58785][CORE][TEST] Add tests for FilteredObjectInputStream resolveClass allow-list
### What changes were proposed in this pull request?
Adds unit tests for `FilteredObjectInputStream.resolveClass`, the SPARK-20922 allow-list guarding the launcher's local socket protocol against arbitrary deserialization.
### Why are the changes needed?
This security-relevant allow-list (classes whose FQCN starts with `java.lang.` or `org.apache.spark.launcher.`) had zero direct test references. A silent regression here would reopen the deserialization hole it was added to close. The tests also document a real gap between the allow-list's stated intent ("just two packages") and its actual prefix-match implementation, which over-admits subpackages of both prefixes (`java.lang.reflect`/`invoke`/`ref`, and `org.apache.spark.launcher.*` [...]
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
New unit tests in `FilteredObjectInputStreamSuite` (10 total): round-trip allowed payloads (`java.lang.Long`, `java.lang.Integer`, `LauncherProtocol.Hello`/`SetAppId`) must deserialize; a disallowed payload (`java.io.File`, representative of any unrelated package - `resolveClass` takes the same rejection path regardless) must be rejected with `IllegalArgumentException`; a prefix-spoofing class (`org.apache.spark.launchermalicious.LauncherPrefixSpoof`) pins the trailing-dot allow-list [...]
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Meta AI
Closes #58016 from vrjdev/filtered-object-stream-test.
Lead-authored-by: Rajesh Vakkalagadda <vr...@meta.com>
Co-authored-by: Test <te...@test.com>
Signed-off-by: Kousuke Saruta <sa...@apache.org>
(cherry picked from commit fc0867a5fdd5440989fb196e171e746463c0b510)
Signed-off-by: Kousuke Saruta <sa...@apache.org>
---
.../launcher/FilteredObjectInputStreamSuite.java | 170 +++++++++++++++++++++
.../spark/launcher/testpkg/SubpackageAllowed.java | 31 ++++
.../launchermalicious/LauncherPrefixSpoof.java | 29 ++++
3 files changed, 230 insertions(+)
diff --git a/launcher/src/test/java/org/apache/spark/launcher/FilteredObjectInputStreamSuite.java b/launcher/src/test/java/org/apache/spark/launcher/FilteredObjectInputStreamSuite.java
new file mode 100644
index 000000000000..17a96b730fe3
--- /dev/null
+++ b/launcher/src/test/java/org/apache/spark/launcher/FilteredObjectInputStreamSuite.java
@@ -0,0 +1,170 @@
+/*
+ * 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.spark.launcher;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.ObjectOutputStream;
+
+import org.apache.spark.launcher.testpkg.SubpackageAllowed;
+import org.apache.spark.launchermalicious.LauncherPrefixSpoof;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Tests for {@link FilteredObjectInputStream} - the SPARK-20922 allow-list
+ * guarding the launcher's local socket protocol. Covers the security-relevant
+ * {@code resolveClass} path that previously had zero direct tests.
+ */
+public class FilteredObjectInputStreamSuite extends BaseSuite {
+
+ // Long rather than String: ObjectOutputStream writes a String as TC_STRING, with no class
+ // descriptor, so readObject() returns it without ever calling resolveClass.
+ @Test
+ public void testAllowedJavaLangLongIsAccepted() throws Exception {
+ Long original = 42L;
+ Object deserialized = roundTrip(original);
+ assertEquals(original, deserialized);
+ }
+
+ @Test
+ public void testAllowedJavaLangIntegerIsAccepted() throws Exception {
+ Integer original = 42;
+ Object deserialized = roundTrip(original);
+ assertEquals(original, deserialized);
+ }
+
+ @Test
+ public void testAllowedLauncherMessageIsAccepted() throws Exception {
+ LauncherProtocol.Hello original = new LauncherProtocol.Hello("secret", "3.5.0");
+ LauncherProtocol.Hello deserialized =
+ (LauncherProtocol.Hello) roundTrip(original);
+ assertEquals(original.secret, deserialized.secret);
+ assertEquals(original.sparkVersion, deserialized.sparkVersion);
+ }
+
+ @Test
+ public void testAllowedLauncherSetAppIdIsAccepted() throws Exception {
+ LauncherProtocol.SetAppId original = new LauncherProtocol.SetAppId("app-123");
+ LauncherProtocol.SetAppId deserialized =
+ (LauncherProtocol.SetAppId) roundTrip(original);
+ assertEquals(original.appId, deserialized.appId);
+ }
+
+ // resolveClass rejects purely on desc.getName() failing both allowed-prefix checks, so a
+ // class in any unrelated package - HashMap, ArrayList, File, or anything else - takes the
+ // same path; one representative case is enough. See
+ // testDisallowedLauncherPrefixSpoofIsRejected below for the one genuinely different rejection
+ // path: a user-defined class spoofing the allow-listed prefix rather than an unrelated one.
+ @Test
+ public void testDisallowedJavaIoFileIsRejected() throws Exception {
+ File payload = new File("/tmp/evil");
+ byte[] bytes = serialize(payload);
+ IllegalArgumentException thrown = assertThrows(
+ IllegalArgumentException.class,
+ () -> deserializeFiltered(bytes));
+ assertTrue(thrown.getMessage().contains("Unexpected class in stream"));
+ assertTrue(thrown.getMessage().contains("java.io.File"));
+ }
+
+ // ALLOWED_PACKAGES entries end in a literal dot, so a class merely sharing the
+ // "org.apache.spark.launcher" text without being in that package must still be
+ // rejected: LauncherPrefixSpoof (org.apache.spark.launchermalicious.LauncherPrefixSpoof)
+ // pins down that boundary, since startsWith("org.apache.spark.launcher.") is false
+ // once the character after the prefix is "m" rather than ".". A matching spoof of
+ // the "java.lang." prefix (e.g. java.langfoo.Bar) can't be tested the same way: the
+ // JVM refuses to define any class whose package starts with "java.", so no real
+ // Class backs that name.
+ @Test
+ public void testDisallowedLauncherPrefixSpoofIsRejected() throws Exception {
+ LauncherPrefixSpoof payload = new LauncherPrefixSpoof();
+ byte[] bytes = serialize(payload);
+ IllegalArgumentException thrown = assertThrows(
+ IllegalArgumentException.class,
+ () -> deserializeFiltered(bytes));
+ assertTrue(thrown.getMessage().contains("Unexpected class in stream"));
+ assertTrue(thrown.getMessage().contains(
+ "org.apache.spark.launchermalicious.LauncherPrefixSpoof"));
+ }
+
+ // The three tests below document CURRENT resolveClass behavior for classes in
+ // java.lang.* subpackages (java.lang.reflect, java.lang.invoke, java.lang.ref).
+ // desc.getName().startsWith("java.lang.") matches these too, since a subpackage's
+ // fully-qualified name still starts with the literal string "java.lang." - not just
+ // the java.lang package itself. The original SPARK-20922 PR's stated intent was "just
+ // two packages" (an exact-package match), so this is a real gap between intent and
+ // implementation, not a design choice made in this PR; fixing resolveClass itself is
+ // intentionally out of scope here and tracked as a follow-up in SPARK-59544.
+ //
+ // Field, MethodHandle, and WeakReference are not Serializable, so each test round-trips
+ // the Class object instead: ObjectOutputStream writes a Class as TC_CLASS followed by its
+ // class descriptor, which readObject() still passes to resolveClass.
+
+ @Test
+ public void testJavaLangReflectFieldIsCurrentlyAllowed() throws Exception {
+ Class<?> original = java.lang.reflect.Field.class;
+ Object deserialized = roundTrip(original);
+ assertEquals(original, deserialized);
+ }
+
+ @Test
+ public void testJavaLangInvokeMethodHandleIsCurrentlyAllowed() throws Exception {
+ Class<?> original = java.lang.invoke.MethodHandle.class;
+ Object deserialized = roundTrip(original);
+ assertEquals(original, deserialized);
+ }
+
+ @Test
+ public void testJavaLangRefWeakReferenceIsCurrentlyAllowed() throws Exception {
+ Class<?> original = java.lang.ref.WeakReference.class;
+ Object deserialized = roundTrip(original);
+ assertEquals(original, deserialized);
+ }
+
+ // The "org.apache.spark.launcher." prefix has the same subpackage over-admission as
+ // "java.lang." above: SubpackageAllowed lives in org.apache.spark.launcher.testpkg, not in
+ // org.apache.spark.launcher itself, but its FQCN still starts with the allow-listed prefix.
+ @Test
+ public void testLauncherSubpackageIsCurrentlyAllowed() throws Exception {
+ SubpackageAllowed original = new SubpackageAllowed();
+ Object deserialized = roundTrip(original);
+ assertEquals(SubpackageAllowed.class, deserialized.getClass());
+ }
+
+ // Helpers
+
+ private static byte[] serialize(Object obj) throws Exception {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ try (ObjectOutputStream oos = new ObjectOutputStream(bos)) {
+ oos.writeObject(obj);
+ }
+ return bos.toByteArray();
+ }
+
+ private static Object deserializeFiltered(byte[] bytes) throws Exception {
+ ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+ try (FilteredObjectInputStream in = new FilteredObjectInputStream(bis)) {
+ return in.readObject();
+ }
+ }
+
+ private static Object roundTrip(Object obj) throws Exception {
+ return deserializeFiltered(serialize(obj));
+ }
+}
diff --git a/launcher/src/test/java/org/apache/spark/launcher/testpkg/SubpackageAllowed.java b/launcher/src/test/java/org/apache/spark/launcher/testpkg/SubpackageAllowed.java
new file mode 100644
index 000000000000..664ae373665d
--- /dev/null
+++ b/launcher/src/test/java/org/apache/spark/launcher/testpkg/SubpackageAllowed.java
@@ -0,0 +1,31 @@
+/*
+ * 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.spark.launcher.testpkg;
+
+import java.io.Serializable;
+
+/**
+ * Test-only fixture for {@code FilteredObjectInputStreamSuite}. Its FQCN starts with the
+ * "org.apache.spark.launcher." prefix from ALLOWED_PACKAGES in {@code
+ * FilteredObjectInputStream.resolveClass} without being in the {@code org.apache.spark.launcher}
+ * package itself, documenting that the prefix match over-admits subpackages here just as it does
+ * for "java.lang." - unlike the java.lang. case, the JVM does not block user-defined classes from
+ * this package name, so this fixture can be exercised with a normal round trip.
+ */
+public class SubpackageAllowed implements Serializable {
+}
diff --git a/launcher/src/test/java/org/apache/spark/launchermalicious/LauncherPrefixSpoof.java b/launcher/src/test/java/org/apache/spark/launchermalicious/LauncherPrefixSpoof.java
new file mode 100644
index 000000000000..18798781f9ed
--- /dev/null
+++ b/launcher/src/test/java/org/apache/spark/launchermalicious/LauncherPrefixSpoof.java
@@ -0,0 +1,29 @@
+/*
+ * 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.spark.launchermalicious;
+
+import java.io.Serializable;
+
+/**
+ * Test-only fixture for {@code FilteredObjectInputStreamSuite}. Its FQCN shares the
+ * "org.apache.spark.launcher" text with the SPARK-20922 allow-list without being in
+ * that package, pinning down the trailing-dot boundary of ALLOWED_PACKAGES in
+ * {@code FilteredObjectInputStream.resolveClass}.
+ */
+public class LauncherPrefixSpoof implements Serializable {
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org