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