Skip to content

[#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization#2832

Open
Nexory wants to merge 3 commits into
apache:mainfrom
Nexory:hardening/rememberme-objectinputfilter
Open

[#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization#2832
Nexory wants to merge 3 commits into
apache:mainfrom
Nexory:hardening/rememberme-objectinputfilter

Conversation

@Nexory

@Nexory Nexory commented Jul 13, 2026

Copy link
Copy Markdown

fixes #2831

What

Adds a first-class, opt-in way to constrain the ObjectInputStream that Shiro uses when it deserializes a RememberMe cookie, and wires a conservative default in:

  1. DefaultSerializer gains an optional ObjectInputFilter (getObjectInputFilter() / setObjectInputFilter(ObjectInputFilter)), applied in deserialize() if set. Default is null, so behavior for every existing caller is unchanged unless they opt in.
  2. AbstractRememberMeManager's default serializer is pre-configured with a conservative, resource-limit-only filter: maxdepth=30;maxarray=100000;maxrefs=10000;maxbytes=10000000. It does not restrict which classes may be deserialized.
  3. The getSerializer() javadoc documents how an application that knows its principal class shape can opt into a stricter class-based allow-list in one line.

The primary value here is the new setObjectInputFilter API and the documented allow-list override: it gives security-conscious operators a supported, one-line way to lock down the RememberMe deserialization sink (DefaultSerializer#deserialize(byte[]) -> AbstractRememberMeManager#getRememberedPrincipals, which reads the client-supplied cookie before authentication). Today that requires subclassing.

Why, and an honest scope of what the default does and does not do

This is the same deserialization sink as the historical Shiro-550 / CVE-2016-4437 issue. The hardcoded default key was fixed there; no per-stream deserialization filter was ever added, and there is no supported way to configure one short of subclassing DefaultSerializer. SHIRO-824 (2021) shows a user asking for exactly this and being pointed at manual JEP-290 self-subclassing.

Being precise about the resource-limit default, because JEP-290 resource limits and class filtering solve different problems:

  • The default (maxdepth/maxarray/maxrefs/maxbytes) is defense-in-depth against oversized or deeply nested, denial-of-service-shaped payloads reaching readObject().
  • It does NOT stop remote-code-execution gadget chains. Typical chains (Commons Collections, etc.) are shallow and small, so they stay within these limits. Stopping those requires a class-based allow-list, which cannot be a safe default because principal types are application-defined and a default allow-list would break existing logins on upgrade.
  • So the classic Shiro-550 scenario (leaked/static cipher key + a gadget library on the classpath) remains exactly as exploitable through this default as before. The new API is what lets an operator actually close it, via the documented one-line class allow-list.

Key management is an operator responsibility per Shiro's own security model, so this is not claiming to close a stated guarantee; it is optional hardening plus a supported configuration point that did not exist before.

Design choice: resource-limit default, not a class allow-list

  • No filter (status quo): safest for compat, does nothing, and SHIRO-824 shows it does not get discovered/fixed in practice.
  • Strict class allow-list by default: strongest defense, but would need to guess every application's principal classes and would break logins silently on upgrade.
  • Resource-limit-only default (chosen): cannot break any existing deployment (a PrincipalCollection + timestamp is orders of magnitude below every limit; maxdepth=30 sits well above the depth of realistic principal graphs), while bounding pathological payload sizes. The stronger class-based allow-list is a documented one-liner for applications that want it.

Testing

  • New tests: DefaultSerializerTest (4 cases, shiro-lang) and AbstractRememberMeManagerObjectInputFilterTest (4 cases, shiro-core), exercising the raw serializer and the full encrypt/decrypt/deserialize AbstractRememberMeManager round trip. The oversized-payload rejection test asserts on the InvalidClassException cause (the JEP-290 rejection), which is what actually distinguishes the filtered path from the unfiltered one, plus a real SimplePrincipalCollection round trip under the new default and the documented allow-list override path.
  • Full existing suites: shiro-lang 8/8, shiro-core 344/344, all green.
  • mvn verify clean (build + tests + checkstyle) on both modules.
  • Target Java level is 17 (root POM); java.io.ObjectInputFilter is stable JDK 9+ API, used directly, no reflection.

Note on API surface

I initially added a setObjectInputFilter convenience method directly on AbstractRememberMeManager, but mvn checkstyle:check reported the class was already at the configured MethodCountCheck ceiling of 30 methods. Rather than bundle a checkstyle config change into this change, I removed that method: the default is wired inline in the existing no-arg constructor, and the override path uses the setObjectInputFilter method now available on DefaultSerializer (via a documented cast on getSerializer()). Net new public API on AbstractRememberMeManager: zero. Happy to revisit if you would rather raise the method-count limit and have a first-class setter.

A few process notes

Checklist (per PR template)

@github-actions github-actions Bot added java Pull requests that update Java code tests labels Jul 13, 2026
@lprimak lprimak added this to the 3.0.1 milestone Jul 13, 2026
…aultSerializer

Add an optional ObjectInputFilter (getObjectInputFilter/setObjectInputFilter,
null by default) to DefaultSerializer, applied in deserialize() when set, and
pre-configure AbstractRememberMeManager's default serializer with a conservative
resource-limit-only filter (maxdepth=30;maxarray=100000;maxrefs=10000;maxbytes=10000000).
Document the one-line class-based allow-list override on getSerializer(). The null
default means no behavior change for existing callers.
@Nexory
Nexory force-pushed the hardening/rememberme-objectinputfilter branch from 098b81c to 7c6a78d Compare July 14, 2026 02:12

@lprimak lprimak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for your contribution!

Please don't force-push. We do squash-and-merge at the end so multiple commits are OK

Looks good for my end, with minor changes.

  • This could go to 3.0.1 (milestone is already there and open) so you can change all the @since occurences there
  • There should not be any casting needed, and if there is, please use JDK 17 pattern matching instanceof syntax instead of casting.
  • Pull up the API into Serializer class so all instanceof and casting can be avoided.
  • You can add checkstyle suppressions where necessary, if that's an impediment

Comment thread core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java Outdated
Comment thread core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java Outdated
Comment thread lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java
Comment thread lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java
…3.0.1

Move getObjectInputFilter/setObjectInputFilter onto the Serializer interface
as default methods (getter returns null, setter is a no-op); DefaultSerializer
overrides them. AbstractRememberMeManager now calls
serializer.setObjectInputFilter(...) directly, removing the instanceof check,
the cast, and @SuppressWarnings. The documented override example and the test
drop their casts too. Change all @SInCE 3.1 occurrences to 3.0.1.
@Nexory

Nexory commented Jul 14, 2026

Copy link
Copy Markdown
Author

Thanks @lprimak, all addressed in a new commit (no force-push):

  • @since -> 3.0.1 everywhere (Serializer, DefaultSerializer, AbstractRememberMeManager).
  • Pulled the filter API up into Serializer: getObjectInputFilter() / setObjectInputFilter(...) are now default methods on the interface (getter returns null, setter is a no-op), and DefaultSerializer @Overrides them. Since an interface can't hold the filter field, the state stays in DefaultSerializer; non-Java-serialization Serializer implementations just inherit the no-op default and are unaffected. Happy to make them abstract instead if you'd prefer, though that would be a breaking change for existing Serializer implementors.
  • With the API on Serializer, AbstractRememberMeManager now calls serializer.setObjectInputFilter(DEFAULT_OBJECT_INPUT_FILTER) directly, so the instanceof, the cast, and the @SuppressWarnings are all gone. The documented override example and the test dropped their casts too, no checkstyle suppression needed.

Rebuilt locally: shiro-lang and shiro-core suites green, checkstyle and pre-commit clean.

@lprimak
lprimak self-requested a review July 14, 2026 03:10

@lprimak lprimak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. I think it gives users an easier way to provide defence-in-depth to RememberMe users.

I really don't see any downsides, but others should take a look at this to see if there is anything I missed.

One question did come up from @rmannibucau - Why do we need all this code if you can just add a system property at runtime?
Because:

  • The property (-Djdk.serialFilter=<filter-pattern>) needs to be added to each application deployment (this way, it's added by default)
  • The property is global only. The Shiro filtering can be applied only to RememberMe deserialization, not globally

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces opt-in JEP-290 (java.io.ObjectInputFilter) support for Shiro’s Java-serialization-based DefaultSerializer, and preconfigures AbstractRememberMeManager to apply a conservative, resource-limit-only filter when deserializing RememberMe cookie payloads (while documenting how applications can override with a stricter class allow-list).

Changes:

  • Add default getObjectInputFilter() / setObjectInputFilter(ObjectInputFilter) hooks to the Serializer API (no-op by default).
  • Implement filter storage + application in DefaultSerializer#deserialize(byte[]), and wire a default resource-limit filter into AbstractRememberMeManager’s default serializer.
  • Add unit/integration tests covering unfiltered behavior, allow-list behavior, and RememberMe round-trips under the default filter.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lang/src/main/java/org/apache/shiro/lang/io/Serializer.java Adds default JEP-290 filter accessor/mutator methods to the serializer interface.
lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java Stores an optional ObjectInputFilter and applies it during deserialization.
core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java Preconfigures the default serializer with a conservative resource-limit filter and documents override guidance.
lang/src/test/java/org/apache/shiro/lang/io/DefaultSerializerTest.java Adds focused tests for filtered/unfiltered DefaultSerializer behavior.
core/src/test/java/org/apache/shiro/mgt/AbstractRememberMeManagerObjectInputFilterTest.java Adds RememberMe-path tests validating default filter behavior and customization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@Nexory

Nexory commented Jul 14, 2026

Copy link
Copy Markdown
Author

Good question, @rmannibucau. I'd frame these as solving different problems, complementary to the global property rather than a replacement.

Scope is the main one. -Djdk.serialFilter is process-global: it governs every ObjectInputStream in the app that doesn't set its own filter (caches, session/cluster replication, RMI/JMX, third-party libraries, ...). To make it strict enough to meaningfully constrain the RememberMe path, you'd have to enumerate every class the whole application legitimately deserializes anywhere, which isn't practical and risks breaking unrelated subsystems. A per-stream filter on the RememberMe sink only ever sees principal data, so it can be tightened to a small class allow-list the application owner actually knows (com.example.MyPrincipal;!*) without affecting anything else. Per-stream filtering is exactly the JEP-290 mechanism for this sink-specific case.

On-by-default, and config not flags. The property has to be set on every deployment's JVM launch (container entrypoint / systemd / app-server args); it's easy to omit and lives in ops/infra config, invisible to the application. A conservative default in Shiro gives RememberMe baseline protection with no operator action, and the stricter allow-list is one line of Shiro config that travels with the application (version-controlled, reviewable) rather than a launch flag.

To be upfront about scope: the default this PR ships is resource-limits-only (maxdepth/maxrefs/...). It bounds oversized or deeply-nested payloads but does not by itself stop known RCE gadget chains; the opt-in class allow-list is what does that. The value here is making that scoped, tight filter trivial to enable, plus a safe default, which the global property can't do as cleanly.

@rmannibucau

Copy link
Copy Markdown
Contributor

The rational of my comment is that you already need to set it on any JVM due to all CVE we got by the past in that area, this doesn't prevent to have a saner default if not set (easy to check in default impl). On a security standpoint it is saner to use a globally immutable config than a mutable (setter) config I think, this is why most of the solutions come as system properties and static init (thinking to AMQ for ex).

Now to make it clear, there is no strong disagreement on the PR, just not a match on the security practise I do promote and rely upon but this is not a big deal if it helps somebody.

@Nexory

Nexory commented Jul 14, 2026

Copy link
Copy Markdown
Author

Thanks @rmannibucau, that's a fair distinction and I don't think we actually disagree on much.

You're right that a security-conscious operator will already have a global jdk.serialFilter set, and that immutable JVM-level config (system property + static init) is harder to tamper with at runtime than a setter. ActiveMQ's serializable-packages allow-list is a good precedent for that model.

Two reasons it landed as a setter here rather than a property:

  • It matches Shiro's config model. Everything on AbstractRememberMeManager (cipher key, cipher service, serializer) is already configured via setters/beans (INI or Spring), set once at startup and not mutated afterwards. A jdk.serialFilter-style property would be the only piece of Shiro security config living outside that model; in practice the filter is as immutable-after-startup as the cipher key next to it, and anything able to call setters on your SecurityManager beans at runtime is already past what this would protect.
  • Scope needs per-instance config. The value over the global property is that the filter is scoped to the RememberMe sink, so it can be a tight class allow-list of just the principal types, which requires per-RememberMeManager config a single static can't express.

That said, your comment points at a real interaction worth handling. Because Shiro sets a per-stream filter, in the default configuration (no custom filter factory) it replaces the global jdk.serialFilter for that stream rather than composing with it (confirmed on JDK 21: a loose per-stream filter set via setObjectInputFilter lets through a payload a stricter global filter would reject). So an operator who already set a strict global filter would have it silently downgraded to our looser resource-limit default on the RememberMe path. I'd rather not clobber an operator's global policy: happy to guard the default so it only applies when the JVM has no filter configured, deferring to the global one otherwise. Keeps secure-by-default when nothing is set, while respecting a stricter global filter when the operator opted into one.

Either way, no objection to it landing as-is. Thanks for the review.

@fpapon fpapon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice contribution, thanks @Nexory !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java Pull requests that update Java code tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative default on RememberMe deserialization

5 participants