You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by "Nexory (via GitHub)" <gi...@apache.org> on 2026/07/13 07:41:57 UTC
[PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Nexory opened a new pull request, #2832:
URL: https://github.com/apache/shiro/pull/2832
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
- This originated from a private thread on security@shiro.apache.org, where the PMC suggested opening it publicly. Tracking issue: #2831.
- The production diff is about 100 lines, past the template's ~20-line ICLA-free threshold. I am happy to file an ICLA if you would like one for this.
- `@since 3.1` is a placeholder for the next release; the current release is 3.0.0 and I did not see a next-minor milestone, so please correct the tag to the actual next version at merge if it differs.
## Checklist (per PR template)
- [x] GitHub issue filed and linked: #2831
- [x] PR title formatted per convention ([#2831])
- [x] `mvn verify`-equivalent checks run locally (build, full test suites, checkstyle)
- [x] Contribution licensed under Apache License 2.0
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on code in PR #2832:
URL: https://github.com/apache/shiro/pull/2832#discussion_r3575665358
##########
lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java:
##########
@@ -94,4 +110,38 @@ public T deserialize(byte[] serialized) throws SerializationException {
protected ObjectInputStream createObjectInputStream(InputStream inputStream) throws IOException {
return new ClassResolvingObjectInputStream(inputStream);
}
+
+ /**
+ * Returns the <a href="https://openjdk.org/jeps/290">JEP-290</a> {@link ObjectInputFilter} applied to the
+ * {@link ObjectInputStream} used by {@link #deserialize(byte[])}, or {@code null} if none is configured.
+ *
+ * @return the configured {@code ObjectInputFilter}, or {@code null} if none is configured.
+ * @since 3.1
+ */
+ public ObjectInputFilter getObjectInputFilter() {
Review Comment:
This should be pulled up
##########
lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java:
##########
@@ -94,4 +110,38 @@ public T deserialize(byte[] serialized) throws SerializationException {
protected ObjectInputStream createObjectInputStream(InputStream inputStream) throws IOException {
return new ClassResolvingObjectInputStream(inputStream);
}
+
+ /**
+ * Returns the <a href="https://openjdk.org/jeps/290">JEP-290</a> {@link ObjectInputFilter} applied to the
+ * {@link ObjectInputStream} used by {@link #deserialize(byte[])}, or {@code null} if none is configured.
+ *
+ * @return the configured {@code ObjectInputFilter}, or {@code null} if none is configured.
+ * @since 3.1
+ */
+ public ObjectInputFilter getObjectInputFilter() {
+ return objectInputFilter;
+ }
+
+ /**
+ * Sets a <a href="https://openjdk.org/jeps/290">JEP-290</a> {@link ObjectInputFilter} to apply to the
+ * {@link ObjectInputStream} used by {@link #deserialize(byte[])}, providing defense-in-depth against
+ * malicious serialized payloads (for example, a class or resource-limit allow-list) in addition to any
+ * validation the caller performs on the deserialized result.
+ * <p/>
+ * The filter is consulted by the JVM for every class resolved while reading the stream, before that
+ * class is instantiated - a rejecting filter causes {@code deserialize} to fail (wrapped in a
+ * {@link SerializationException}) instead of constructing the disallowed object. See
+ * {@link ObjectInputFilter.Config#createFilter(String)} for a convenient way to build a pattern-based
+ * filter combining class allow/deny lists with depth, reference, and byte-count limits.
+ * <p/>
+ * The default is {@code null} (no filter), matching this class's behavior prior to this option being
+ * introduced. Callers handling untrusted input, such as {@link org.apache.shiro.mgt.AbstractRememberMeManager
+ * AbstractRememberMeManager}'s RememberMe cookie deserialization, are encouraged to configure one.
+ *
+ * @param objectInputFilter the filter to apply, or {@code null} to disable filtering (the default).
+ * @since 3.1
+ */
+ public void setObjectInputFilter(ObjectInputFilter objectInputFilter) {
Review Comment:
This should be pulled up
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #2832:
URL: https://github.com/apache/shiro/pull/2832#issuecomment-4969147488
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.
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on code in PR #2832:
URL: https://github.com/apache/shiro/pull/2832#discussion_r3575845267
##########
lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java:
##########
@@ -34,6 +35,18 @@
* @since 0.9
*/
public class DefaultSerializer<T> implements Serializer<T> {
+
+ /**
+ * Optional <a href="https://openjdk.org/jeps/290">JEP-290</a> filter applied to the
+ * {@link ObjectInputStream} used by {@link #deserialize(byte[])}.
+ * <p/>
+ * {@code null} by default, meaning no filter is applied and behavior is unchanged from prior releases -
+ * existing callers of this class are not affected unless they opt in via {@link #setObjectInputFilter}.
+ *
+ * @since 3.0.1
+ */
+ private ObjectInputFilter objectInputFilter;
Review Comment:
nope
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "Nexory (via GitHub)" <gi...@apache.org>.
Nexory commented on PR #2832:
URL: https://github.com/apache/shiro/pull/2832#issuecomment-4965280975
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.
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on code in PR #2832:
URL: https://github.com/apache/shiro/pull/2832#discussion_r3575609247
##########
core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java:
##########
@@ -119,9 +152,17 @@ protected ObjectInputStream createObjectInputStream(InputStream inputStream) thr
/**
* Default constructor that initializes a {@link DefaultSerializer} as the {@link #getSerializer() serializer} and
* an {@link AesCipherService} as the {@link #getCipherService() cipherService}.
+ * <p/>
+ * As defense-in-depth against the {@link #getRememberedPrincipals(SubjectContext)} deserialization path
+ * operating on untrusted, client-supplied input, the default serializer is also pre-configured with the
+ * {@link #DEFAULT_OBJECT_INPUT_FILTER_PATTERN conservative resource-limit ObjectInputFilter} described above.
*/
+ @SuppressWarnings("unchecked")
public AbstractRememberMeManager() {
setCipherKey(((AesCipherService) cipherService).generateNewKey().getEncoded());
+ if (serializer instanceof DefaultSerializer) {
Review Comment:
Pull up to `Serializer` API, there should be no need for cast
##########
core/src/main/java/org/apache/shiro/mgt/AbstractRememberMeManager.java:
##########
@@ -91,6 +92,38 @@ protected ClassLoader doGetClassLoader() {
}
};
+ /**
+ * Default <a href="https://openjdk.org/jeps/290">JEP-290</a> filter pattern applied to the
+ * {@link #getSerializer() serializer}'s {@code ObjectInputStream} when deserializing the RememberMe cookie
+ * payload, an untrusted, client-supplied value (see {@link #getRememberedPrincipals(SubjectContext)}).
+ * <p/>
+ * This default is deliberately a <em>resource-limit-only</em> filter: it bounds the object graph depth,
+ * array size, back-reference count, and total stream size that {@code readObject()} will process, but it
+ * does not restrict <em>which</em> classes may be deserialized. It is intended as defense-in-depth against
+ * oversized or deeply nested (denial-of-service shaped) payloads reaching this deserialization sink. It is
+ * <em>not</em> protection against remote-code-execution gadget chains: typical serialization gadget chains
+ * (for example the Apache Commons Collections family) are shallow and small, so they stay well within these
+ * limits and are <em>not</em> rejected by them. Stopping such chains requires a class-based allow-list,
+ * which cannot be a safe default here because principal types are entirely application-defined (custom
+ * {@code Serializable} principal classes are common) and a default allow-list would break existing
+ * deployments. Applications that need that stronger, class-based defense (for example to reduce the blast
+ * radius of a leaked or static cipher key, the classic Shiro-550 / CVE-2016-4437 scenario) can configure
+ * one in a single line; see {@link #getSerializer()}. The depth limit is set generously (well above the
+ * depth of realistic principal object graphs) so that well-formed principal data is not rejected.
+ *
+ * @since 3.1
Review Comment:
Please replace all occurrences with `@since 3.0.1`
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "Nexory (via GitHub)" <gi...@apache.org>.
Nexory commented on PR #2832:
URL: https://github.com/apache/shiro/pull/2832#issuecomment-4970167228
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.
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "steinarb (via GitHub)" <gi...@apache.org>.
steinarb merged PR #2832:
URL: https://github.com/apache/shiro/pull/2832
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2832:
URL: https://github.com/apache/shiro/pull/2832#discussion_r3575829560
##########
lang/src/main/java/org/apache/shiro/lang/io/DefaultSerializer.java:
##########
@@ -34,6 +35,18 @@
* @since 0.9
*/
public class DefaultSerializer<T> implements Serializer<T> {
+
+ /**
+ * Optional <a href="https://openjdk.org/jeps/290">JEP-290</a> filter applied to the
+ * {@link ObjectInputStream} used by {@link #deserialize(byte[])}.
+ * <p/>
+ * {@code null} by default, meaning no filter is applied and behavior is unchanged from prior releases -
+ * existing callers of this class are not affected unless they opt in via {@link #setObjectInputFilter}.
+ *
+ * @since 3.0.1
+ */
+ private ObjectInputFilter objectInputFilter;
Review Comment:
`DefaultSerializer` can be shared and used concurrently (e.g., as the `AbstractRememberMeManager` serializer). Since `objectInputFilter` is mutable and read during `deserialize()`, it should be `volatile` (or otherwise safely published) to guarantee visibility if the filter is configured after construction.
##########
core/src/test/java/org/apache/shiro/mgt/AbstractRememberMeManagerObjectInputFilterTest.java:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.shiro.mgt;
+
+import org.apache.shiro.lang.io.Serializer;
+import org.apache.shiro.subject.PrincipalCollection;
+import org.apache.shiro.subject.SimplePrincipalCollection;
+import org.apache.shiro.subject.Subject;
+import org.apache.shiro.subject.SubjectContext;
+import org.apache.shiro.subject.support.DefaultSubjectContext;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InvalidClassException;
+import java.io.ObjectInputFilter;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Test cases proving {@link AbstractRememberMeManager}'s RememberMe cookie deserialization path is
+ * protected by a <a href="https://openjdk.org/jeps/290">JEP-290</a> {@link ObjectInputFilter} by default,
+ * without breaking legitimate {@link PrincipalCollection} round-tripping.
+ */
+class AbstractRememberMeManagerObjectInputFilterTest {
+
+ /** Deeper than the default filter's {@code maxdepth=30}, to trigger a resource-limit rejection. */
+ private static final int DEEP_CHAIN_LENGTH = 60;
+
+ @Test
+ void testLegitimatePrincipalsRoundTripUnderDefaultFilter() {
+ InMemoryRememberMeManager rmm = new InMemoryRememberMeManager();
+ PrincipalCollection principals = new SimplePrincipalCollection("joecool", "myRealm");
+
+ rmm.rememberIdentity(null, principals);
+ PrincipalCollection remembered = rmm.getRememberedPrincipals(new DefaultSubjectContext());
+
+ assertThat(remembered).isNotNull();
+ assertThat(remembered.getPrimaryPrincipal()).isEqualTo("joecool");
+ }
+
+ @Test
+ void testDefaultFilterRejectsOversizedPayloadBeforeFullConstruction() {
+ // The default filter (see AbstractRememberMeManager.DEFAULT_OBJECT_INPUT_FILTER_PATTERN) bounds the
+ // object graph depth/array size/reference count/byte count of the deserialized payload. This is
+ // denial-of-service-shaped-payload hardening: it does not restrict classes and does not stop RCE
+ // gadget chains (which are shallow and small). Feed an oversized/deeply nested payload that exceeds
+ // maxdepth, encrypted the way a real RememberMe cookie is, and confirm getRememberedPrincipals() fails
+ // closed with a JEP-290 filter rejection (InvalidClassException) before the graph is materialized. The
+ // InvalidClassException cause is the discriminating assertion: without the filter this same payload
+ // deserializes fully and fails only later with an unrelated ClassCastException, so asserting the cause
+ // is what proves the filter itself fired.
+ InMemoryRememberMeManager rmm = new InMemoryRememberMeManager();
+
+ List<Object> deepChain = new ArrayList<>();
+ List<Object> cursor = deepChain;
+ for (int i = 0; i < DEEP_CHAIN_LENGTH; i++) {
+ List<Object> next = new ArrayList<>();
+ cursor.add(next);
+ cursor = next;
+ }
+
+ byte[] serialized = plainJdkSerialize(deepChain);
+ byte[] encrypted = rmm.encryptForTest(serialized);
+ rmm.injectRawSerializedIdentity(encrypted);
+
+ assertThatThrownBy(() -> rmm.getRememberedPrincipals(new DefaultSubjectContext()))
+ .isInstanceOf(RuntimeException.class)
+ .hasCauseInstanceOf(InvalidClassException.class);
+ // onRememberedPrincipalFailure must have run its "forget" cleanup path.
+ assertThat(rmm.forgetCount).isEqualTo(1);
+ }
+
+ @Test
+ void testCustomStricterAllowListFilterCanBeConfigured() {
+ // Documented override path (see AbstractRememberMeManager#getSerializer javadoc): replace the default
+ // serializer's filter with a strict class allow-list. Only SimplePrincipalCollection,
+ // AbstractRememberMeManager.RememberedIdentity, and JDK collection/primitive/java.time plumbing are let
+ // through. Note: java.time types (e.g. Instant) don't serialize themselves directly - they writeReplace()
+ // to an internal java.time serialization proxy class, which is what actually appears in the stream.
+ InMemoryRememberMeManager rmm = new InMemoryRememberMeManager();
+ rmm.getSerializer()
+ .setObjectInputFilter(ObjectInputFilter.Config.createFilter(
+ "org.apache.shiro.subject.SimplePrincipalCollection;"
+ + "org.apache.shiro.mgt.AbstractRememberMeManager$RememberedIdentity;"
+ + "java.time.*;java.util.*;java.lang.*;!*"));
+
+ PrincipalCollection principals = new SimplePrincipalCollection("joecool", "myRealm");
+ rmm.rememberIdentity(null, principals);
+ PrincipalCollection remembered = rmm.getRememberedPrincipals(new DefaultSubjectContext());
+ assertThat(remembered.getPrimaryPrincipal()).isEqualTo("joecool");
+
+ // A disallowed class must now be rejected outright (not merely resource-limited).
+ byte[] disallowed = plainJdkSerialize(new NotAllowlisted());
+ rmm.injectRawSerializedIdentity(rmm.encryptForTest(disallowed));
+
+ assertThatThrownBy(() -> rmm.getRememberedPrincipals(new DefaultSubjectContext()))
+ .isInstanceOf(RuntimeException.class);
+ }
Review Comment:
The assertion for the disallowed-class case is too weak: it only checks for a `RuntimeException`, which would also be thrown if the payload deserializes successfully and then fails later (e.g., due to a cast). To prove the strict allow-list filter is actually enforced, assert the JEP-290 rejection (`InvalidClassException`) as the cause, and also assert the forget/cleanup path ran (as you already do in the default-filter test).
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [#2831] - Add optional JEP-290 ObjectInputFilter support to DefaultSerializer, with a conservative resource-limit default on RememberMe deserialization [shiro]
Posted by "Nexory (via GitHub)" <gi...@apache.org>.
Nexory commented on PR #2832:
URL: https://github.com/apache/shiro/pull/2832#issuecomment-4964985020
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` `@Override`s 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.
--
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: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org