Remove or extract default typing

[jroper]

As a downstream project that depends on Jackson, Jackson's default typing feature is a PITA. The feature, by design, enables remote code execution. Personally I don't understand how all these CVE vulnerabilities that essentially say "Jacksons remote code execution feature allows remote code execution" come to exist, they aren't vulnerabilities, they are Jackson working exactly as documented and intended. But it causes a pain for us because every time a new way to use Jacksons remote code execution feature to execute remote code is found, we get multiple notifications from people using and auditing our dependencies saying "OMG! PLAY FRAMEWORK HAS A SECURITY VULNERABILITY". And surely given the incredibly wide use of Jackson, we're not the only project that feels this.

I think the feature should be removed from the core of Jackson databind. Perhaps the necessary plug in points to make it possible to implement the feature by a separate module could be left behind, this would ensure that the people that really need the ability to shoot themselves in the foot still can - is there a Darwin awards for IT security? But this wouldn't just make things easier for us (and Jackson itself), it will also protect users that don't realise just how dangerous the feature is, because it will introduce friction, they won't just be able to discover and enable the method, they'll have to actually add a new dependency. Preferably the module would not even be related to FasterXML, so that automated scans and other reports wouldn't associate the CVEs reported against it with sensible use of Jackson. I suggest a group id of: io.danger.will.robinson.

[cowtowncoder]

There is no real way to do that for 2.x, unfortunately, so we'll be stuck with this one for 2.10:

https://github.com/FasterXML/jackson-databind/issues/2195

but approach for 3.x is open. My thinking has been that yes, one MUST EXPLICITLY specify validator that is allowing everything, and that if such permissive validator is provided, its name contains "Unsafe" or something similar to indicate.
I am open to discussion on specific details.

So I think we are thinking along same lines here?

At tactical level I think that with issue #2195 to included in 2.10 it will be easier for frameworks to provide reasonable defaults, as well as for users to provide Allow-list approaches.
And what we could and should do, I think, is to add new set of methods for allowing default typing with ClassNameValidator (or whatever name is used), deprecate one that does not take it, and at least start process that will be completed in 3.0.

Does this make sense?

[jroper]

The main thing that I would want to see is that it is clear, beyond any shadow of doubt, that any configuration that doesn't use an explicit whitelist, is running in a mode that is vulnerable to exploitation, such that when security researchers try to raise vulnerabilities, the response can be "no, this vulnerability only exists when a user has explicitly and consciously opted into a mode that enables this vulnerability". I would rather remove all black lists from Jackson (perhaps they can be supplied in documentation), since the existence of a black list may cause people to trust it, and it's when people trust it that a new class that is vulnerable and not in the black list might be considered a vulnerability. Push it on the user to maintain the lists, and then that pushes the vulnerabilities to the user code, rather than in Jackson.

[cowtowncoder]

@jroper
Ok. I am not sure I 100% understand, but maybe I can try to summarize challenges to see where we agree.

So, first, it would be important to be able to know for sure that a user is not using unsafe settings.
This would help deal with security tools, vendors, to point out there is a way to tell that usage of specific version is safe. This is important for security-conscious users as well.
I agree there of course.

But the problem in getting there is due to 2 dimensions:

1. There are 2 vectors to enable vuln: "default typing" and "@JsonTypeInfo" with basetype of java.lang.Object (and small number of alternatives)
2. Backwards compatibility requirements: 2.x vs 3.0

Starting with (2), simple immediate removal of functionality (or methods to enable it) should not be done in a minor release, ideally. But given that migration from 2.x to 3.0 will take literally years, doing nothing is not a good option either.
At practical level I would be ok with something like "mark methods we do not want anyone to [be able to] use Deprecated in 2.10, pointing to alternatives, remove in 2.11".
This because I do think that there are sometimes cases where following SemVer is less valuable than "doing the right thing for users". Without doing this there would not be easy upgrade path that improves security.
With 3.0 we can start with clean slate regardless, iff we can find such combination.

But the tricky part, I think, is the (1).

Specifically I do not object to changing "ObjectMapper.enableDefaultTyping()" (and variations) to require passing of something that indicates allowed class names to use. I do have minor disagreement on whether Jackson ought to maintain blacklist (to me it seems that at least on short term, it should, although separated out in modular way). Perhaps answer there is a separate module that just contains implementation for Blacklist-backed verifier.

So far so good. But the problem is the annotation part. It is not so much that of configuring validator: I think mechanism used for "default typing" could suffice as the baseline. But what I worry about are users using @JsonTypeInfo on java.lang.Object property, where there is no simple way to break things in a way that is immediately obvious -- there is no way to induce a compilation failure, and since type lookups are dynamic, failure (with version 2.11, or maybe even earlier) would come at runtime, at unpredictable time.
Ideally users would have unit tests that would catch this, but knowing what I know I think there are enough users who do not have this to make it very painful for them, and by extension, Jackson maintainers (mostly me).

So. I would like to brainstorm the specific way to deal with:

• 2.x users who use @JsonTypeInfo on java.lang.Object properties

What would be mechanisms to

1. Configure new validator (per-mapper default, new annotation for validator to use for case of java.lang.Object base? Extending @JsonTypeInfo more difficult as it can't refer to types defined in jackson-databind
2. Specify exactly how things fail upon detecting missing configuration.

[cowtowncoder]

@jroper WDYT about #2195 -- I have initial implementation ready and I think this would be a reasonable way forward in near term.

[daniel-beck]

Personally I don't understand how all these CVE vulnerabilities that essentially say "Jacksons remote code execution feature allows remote code execution" come to exist, they aren't vulnerabilities, they are Jackson working exactly as documented and intended.

FWIW to the best of my understanding, these issues should not be assigned CVEs at all. Quoting https://cve.mitre.org/cve/cna/rules.html#Appendix_C

If there is a way to use the library, protocol, or standard without being vulnerable: Assign a CVE ID to each affected codebase or product and continue to INC1

If the using the library, protocol, or standard requires the product to be vulnerable: Assign a single CVE ID and continue to INC1

Since users of the library need to opt into the feature enabling these vulnerabilities in the first place (something that many of us don't), I'd argue these CVEs are incorrectly assigned to jackson-databind. I recommend someone from this project reach out to the CNAs assigning these CVEs (MITRE, mostly, AFAICT) to get clarity here.

[cowtowncoder]

Update for concurrent progress:

1. Jackson 2.10.0 included Add abstraction PolymorphicTypeValidator, for limiting subtypes allowed by default typing, @JsonTypeInfo #2195, which adds safe alternatives, deprecates unsafe
2. Jackson 2.11.0 (not yet released) will add Add MapperFeature.BLOCK_UNSAFE_POLYMORPHIC_BASE_TYPES to allow blocking use of unsafe base type for polymorphic deserialization #2587 -- addition of MapperFeature.BLOCK_UNSAFE_POLYMORPHIC_BASE_TYPES that allows blocking use even via unsafe methods -- to complete functionality to lock down unsafe usage

This is somewhat orthogonal to question of 3.x, although with these settings 2.11 can be locked down to same level as what 3.0 has by default (i.e. not allowing use of potentially unsafe base types at all without explicit configuration to allow that).