L1nq0 opened a new pull request, #9075:
URL: https://github.com/apache/storm/pull/9075

   Closes #9073
   
   **Upgrade note**: clusters running 
`topology.fall.back.on.java.serialization=true` get a new default serial filter 
with this change: known gadget packages are rejected and streams are capped at 
10 MB (`maxbytes=10485760`). Payloads that legitimately cross the fallback 
bridge with denied classes or larger sizes will start failing; set 
`topology.fall.back.on.java.serialization.filter` to an empty value to restore 
the previous unfiltered behavior.
   
   ## What this adds
   
   A new config key, `topology.fall.back.on.java.serialization.filter`, holding 
a JEP-290 filter pattern for the java serialization fallback bridge. 
`DefaultKryoFactory` parses the pattern once at kryo construction, so an 
invalid pattern fails worker setup with the config key in the error. 
`SerializableSerializer` installs it via `setObjectInputFilter` whenever it 
deserializes. The filter is topology-scoped and also covers programmatic 
construction such as local mode, which a JVM-wide `jdk.serialFilter` in 
`worker.childopts` cannot reach.
   
   `conf/defaults.yaml` ships a default pattern: a deny-list of well-known 
gadget namespaces (commons-collections 3/4 functors and comparators, beanutils, 
xalan external and JDK-internal, rowset, c3p0, groovy closures) plus 
`maxbytes=10485760`. Sites that legitimately exchange denied classes or bigger 
payloads can override or clear the key per topology.
   
   This differs from the option (2) lean in the issue thread: I ended up 
shipping the deny-list as the default rather than an empty value. The fallback 
is documented as something to keep disabled in production, but clusters still 
run it, and the cost of the default is a narrow, overridable deny-list while 
the cost of an empty default is that the dangerous path stays unconstrained for 
every deployment that never discovers the knob. One line in 
`conf/defaults.yaml` flips this back to opt-in; the code is identical either 
way. Happy to drop the default if you'd rather keep (2) pure.
   
   A couple of details:
   
   - Wildcard depth follows JEP-290: `pkg.*` covers direct package members, 
`pkg.**` also covers subpackages. The tests exercise both depths with a fixture 
in a subpackage that the single-level form would allow.
   - `!com.sun.org.apache.rowset.internal.*` is a defensive entry: current JDKs 
don't carry that namespace, the real rowset gadget is covered by 
`!com.sun.rowset.*`.
   
   ## Tests
   
   8 cases in `SerializableSerializerFilterTest`, all running end to end 
through `KryoValuesSerializer`/`KryoValuesDeserializer`: reject/allow 
round-trips, subpackage coverage, the maxbytes limit (many small arrays, so the 
filter re-invokes mid-stream), unset-key no-op, fail-fast on invalid patterns, 
and a defaults.yaml consistency check.
   
   Docs: the mitigation is described in `docs/SECURITY.md` (Serialization 
Security) and the new key in `docs/Serialization.md`.
   


-- 
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]

Reply via email to