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 [email protected], 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
