Skip to content

Add process-wide ObjectInputFilter to reject Java deserialization by default - #22073

Merged
cwperks merged 4 commits into
opensearch-project:mainfrom
cwperks:object-input-filter
Jun 18, 2026
Merged

Add process-wide ObjectInputFilter to reject Java deserialization by default#22073
cwperks merged 4 commits into
opensearch-project:mainfrom
cwperks:object-input-filter

Conversation

@cwperks

@cwperks cwperks commented Jun 9, 2026

Copy link
Copy Markdown
Member

Description

This PR installs a process-wide ObjectInputFilter factory via ObjectInputFilter.Config.setSerialFilterFactory() during bootstrap. The factory rejects all Java deserialization by default. Code that legitimately requires deserialization can opt in by setting a filter on their ObjectInputStream, which the factory will delegate to.

This complements the existing forbidden-apis build-time check by providing runtime enforcement across the entire process, including plugins and third-party dependencies. Plugins that need deserialization (like the security plugin) can still override this filter with setObjectInputFilter with a separate filter. This filter will behave similar to JSM where any issues from plugins would arise at runtime instead of compile time.

This filter is gated behind the bootstrap.serial_filter setting (disabled by default, requires node restart):

bootstrap.serial_filter: true

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

cwperks added 3 commits June 3, 2026 18:03
…lso enforce at runtime

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit aca4592)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 Security concerns

Deserialization bypass:
The filter returns UNDECIDED for null serialClass, which may allow deserialization to proceed in certain scenarios. Additionally, using setSerialFilter() instead of setSerialFilterFactory() means the filter cannot be overridden per-stream, potentially blocking legitimate plugin use cases while failing to provide the intended protection model.

✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Incorrect Filter Logic

The REJECT_ALL_FILTER returns UNDECIDED when serialClass is null, but this allows deserialization to proceed. When a filter returns UNDECIDED, the JVM continues checking other filters or allows the operation. For a reject-all filter, this should return REJECTED for null serialClass checks (stream metadata) to ensure no deserialization succeeds without an explicit override. The current logic only blocks class resolution but permits the deserialization process itself.

static final ObjectInputFilter REJECT_ALL_FILTER = filterInfo -> filterInfo.serialClass() == null
    ? ObjectInputFilter.Status.UNDECIDED
    : ObjectInputFilter.Status.REJECTED;
Wrong API Used

The code calls setSerialFilter() which sets a static JVM-wide filter that cannot be overridden by individual streams. The PR description states plugins can override by calling setObjectInputFilter() on their stream, but this only works if setSerialFilterFactory() is used instead. With setSerialFilter(), the static filter applies to all streams and stream-level filters are ignored. This breaks the intended opt-in mechanism for plugins.

ObjectInputFilter.Config.setSerialFilter(REJECT_ALL_FILTER);

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to aca4592
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Enable security filter by default

The serial filter is disabled by default (false), which means the deserialization
protection is opt-in. This leaves systems vulnerable unless administrators
explicitly enable it. Consider making this true by default to provide security by
default, or document the security implications prominently.

server/src/main/java/org/opensearch/bootstrap/BootstrapSettings.java [62]

-public static final Setting<Boolean> SERIAL_FILTER_SETTING = Setting.boolSetting("bootstrap.serial_filter", false, Property.NodeScope);
+public static final Setting<Boolean> SERIAL_FILTER_SETTING = Setting.boolSetting("bootstrap.serial_filter", true, Property.NodeScope);
Suggestion importance[1-10]: 9

__

Why: This is a critical security suggestion. Changing the default from false to true would enable deserialization protection by default, following the security-by-default principle. The current opt-in approach leaves systems vulnerable unless administrators explicitly enable the setting, which is a significant security risk.

High
Verify existing filter security level

The method silently ignores when a serial filter is already set, which could leave
the system in an insecure state if an attacker or misconfiguration sets a permissive
filter before this code runs. Consider logging at WARN level and verifying the
existing filter is at least as restrictive as the intended one.

server/src/main/java/org/opensearch/bootstrap/Bootstrap.java [128-135]

 static void initializeSerialFilter() {
     try {
         ObjectInputFilter.Config.setSerialFilter(REJECT_ALL_FILTER);
     } catch (IllegalStateException e) {
         // Filter already set (e.g., via -Djdk.serialFilter system property or in tests)
-        LogManager.getLogger(Bootstrap.class).debug("Serial filter already initialized", e);
+        LogManager.getLogger(Bootstrap.class).warn("Serial filter already initialized - verify it provides adequate protection", e);
     }
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a security concern where an already-set filter might be less restrictive. However, changing from debug to warn is a moderate improvement since the code cannot verify or change an existing filter anyway. The suggestion is valid but has limited practical impact.

Medium
Add metadata limits validation

The filter returns UNDECIDED for null serialClass, which allows stream metadata
checks to pass. However, this could permit certain deserialization attacks that
exploit stream metadata manipulation. Consider explicitly validating depth, array
length, and byte limits even when serialClass is null.

server/src/main/java/org/opensearch/bootstrap/Bootstrap.java [137-139]

-static final ObjectInputFilter REJECT_ALL_FILTER = filterInfo -> filterInfo.serialClass() == null
-    ? ObjectInputFilter.Status.UNDECIDED
-    : ObjectInputFilter.Status.REJECTED;
+static final ObjectInputFilter REJECT_ALL_FILTER = filterInfo -> {
+    if (filterInfo.serialClass() == null) {
+        // Reject if metadata exceeds safe limits
+        if (filterInfo.depth() > 100 || filterInfo.arrayLength() > 10000 || filterInfo.streamBytes() > 1_000_000) {
+            return ObjectInputFilter.Status.REJECTED;
+        }
+        return ObjectInputFilter.Status.UNDECIDED;
+    }
+    return ObjectInputFilter.Status.REJECTED;
+};
Suggestion importance[1-10]: 6

__

Why: The suggestion adds defense-in-depth by validating stream metadata limits when serialClass is null. While this provides additional protection against metadata manipulation attacks, the primary security mechanism (rejecting all class deserialization) is already in place. The hardcoded limits (100, 10000, 1_000_000) are reasonable but somewhat arbitrary.

Low

Previous suggestions

Suggestions up to commit 876ec79
CategorySuggestion                                                                                                                                    Impact
Security
Log filter conflicts at WARN level

The method silently ignores when a filter is already set, which could mask security
issues if an unexpected filter was configured. Consider logging at WARN level
instead of DEBUG, and include information about the existing filter to help diagnose
potential security misconfigurations.

server/src/main/java/org/opensearch/bootstrap/Bootstrap.java [128-135]

 static void initializeSerialFilter() {
     try {
         ObjectInputFilter.Config.setSerialFilter(REJECT_ALL_FILTER);
     } catch (IllegalStateException e) {
         // Filter already set (e.g., via -Djdk.serialFilter system property or in tests)
-        LogManager.getLogger(Bootstrap.class).debug("Serial filter already initialized", e);
+        Logger logger = LogManager.getLogger(Bootstrap.class);
+        ObjectInputFilter existingFilter = ObjectInputFilter.Config.getSerialFilter();
+        logger.warn("Serial filter already initialized. Existing filter: {}", existingFilter, e);
     }
 }
Suggestion importance[1-10]: 5

__

Why: While logging at WARN level could help diagnose security misconfigurations, the current DEBUG level is appropriate for expected scenarios (tests, system properties). The suggestion adds value by including the existing filter information, but the severity change may cause unnecessary alarm in legitimate cases.

Low
General
Add null check for filterInfo

The filter should validate that filterInfo is not null before accessing its methods.
While unlikely in normal operation, defensive programming prevents potential
NullPointerException if the JVM passes unexpected input.

server/src/main/java/org/opensearch/bootstrap/Bootstrap.java [137-139]

-static final ObjectInputFilter REJECT_ALL_FILTER = filterInfo -> filterInfo.serialClass() == null
-    ? ObjectInputFilter.Status.UNDECIDED
-    : ObjectInputFilter.Status.REJECTED;
+static final ObjectInputFilter REJECT_ALL_FILTER = filterInfo -> {
+    if (filterInfo == null || filterInfo.serialClass() == null) {
+        return ObjectInputFilter.Status.UNDECIDED;
+    }
+    return ObjectInputFilter.Status.REJECTED;
+};
Suggestion importance[1-10]: 3

__

Why: This is defensive programming that adds a null check for filterInfo. However, the JVM's ObjectInputFilter contract guarantees non-null FilterInfo objects, making this check unnecessary in practice. The suggestion is technically safe but addresses an extremely unlikely scenario.

Low

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 876ec79: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 876ec79: SUCCESS

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.36364% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.41%. Comparing base (b53b8fe) to head (aca4592).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
.../main/java/org/opensearch/bootstrap/Bootstrap.java 30.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #22073      +/-   ##
============================================
- Coverage     73.43%   73.41%   -0.03%     
+ Complexity    75965    75894      -71     
============================================
  Files          6070     6070              
  Lines        344903   344913      +10     
  Branches      49625    49626       +1     
============================================
- Hits         253285   253207      -78     
+ Misses        71493    71467      -26     
- Partials      20125    20239     +114     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit aca4592

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for aca4592: SUCCESS

@cwperks
cwperks merged commit e2cdd7d into opensearch-project:main Jun 18, 2026
13 checks passed
OVyshnevskyi pushed a commit to OVyshnevskyi/OpenSearch that referenced this pull request Jun 22, 2026
…default (opensearch-project#22073)

* Extend forbidden api for java serialization from build-time only to also enforce at runtime

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Sandesh Kumar <sandeshkr419@gmail.com>
KhishorekumarBS pushed a commit to KhishorekumarBS/OpenSearch that referenced this pull request Jul 3, 2026
…default (opensearch-project#22073)

* Extend forbidden api for java serialization from build-time only to also enforce at runtime

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Sandesh Kumar <sandeshkr419@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants