[
https://issues.apache.org/jira/browse/KAFKA-20822?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101098#comment-18101098
]
Yunseop Eom commented on KAFKA-20822:
-------------------------------------
Follow-up on PR #22936: https://github.com/apache/kafka/pull/22936
The PR hardens Trogdor TaskSpec deserialization with a
BasicPolymorphicTypeValidator allowlist for the built-in task, fault, and
workload packages while preserving the existing JSON shape. Regression coverage
rejects unsupported subtypes and accepts built-in types.
The TDD regression, full trogdor tests, static checks, and git diff --check
pass locally. The PR is open and awaiting review.
> Trogdor uses insecure Jackson Polymorphic Id.CLASS setting
> ----------------------------------------------------------
>
> Key: KAFKA-20822
> URL: https://issues.apache.org/jira/browse/KAFKA-20822
> Project: Kafka
> Issue Type: Task
> Reporter: PJ Fanning
> Priority: Major
>
> Kafka team have previously rejected security reports about this with the
> reasoning that Trogdor is a testing feature and that you do not recommend to
> users to use it in production environments.
> It would still be useful for security hardening to update the Jackson related
> code in Trogdor to add a PolymorphicTypeValidator that polices that you only
> databind to known classes when deserializing. A couple of lines of code maybe
> 1 or 2 tests on top.
> *
> [https://github.com/FasterXML/jackson-databind/blob/9b2adbf5db974947220b8c208735f51b9fdec831/src/main/java/tools/jackson/databind/jsontype/BasicPolymorphicTypeValidator.java#L36]
> * You can create an ObjectMapper with a PolymorphicTypeValidator ptv with
> `JsonMapper.builder.polymorphicTypeValidator(ptv).build()`
> *
> [https://github.com/apache/kafka/blob/ebac341b28d4224c296ada31eb45122176e8b27b/trogdor/src/main/java/org/apache/kafka/trogdor/common/JsonUtil.java#L24]
> *
> [https://github.com/apache/kafka/blob/ebac341b28d4224c296ada31eb45122176e8b27b/trogdor/src/main/java/org/apache/kafka/trogdor/task/TaskSpec.java#L32]
> * Without registering a polymorphicTypeValidator, Jackson 2.x will load any
> class that the JSON names and try to create instances of it (when the
> dangerous `@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, property = "class")` is
> used)
> * It may be feasible to switch to the safer `use = JsonTypeInfo.Id.NAME` and
> add JsonSubType info to the annotation listing the valid subtypes. This would
> be instead of adding a polymorphicTypeValidator. This would only work if the
> code that serializes and deserializes the data is in synch. The JSON will be
> different from the existing `use = JsonTypeInfo.Id.CLASS` result.
> * There are a few other ways to approach this too and I can enumerate them
> if the already listed (and easier) solutions are not acceptable.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
