Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add system properties to configure Jackson's stream read constraints #15720

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

edmocosta
Copy link
Contributor

@edmocosta edmocosta commented Dec 21, 2023

Release notes

Added jvm.options properties to configure the Jackson read constraints defaults (Maximum Number value length, Maximum String value length, and Maximum Nesting depth).

What does this PR do?

This PR adds 3 new custom system properties into the jvm.options. Those new properties are used to fine-tune the default Jackson stream read constraints values, which are used by Jackson to guard against malicious input by preventing processing of too big inputs. The added properties can be found here.

The intention of early set the Jackson defaults during the startup (runner.rb) was to ensure that all Logstash's functionalities that rely on Jackson are using those settings values.

Why is it important/What is the impact to the user?

Although the Jackson's defaults works for the majority of users, it might be too restrictive - and a blocker - for pipelines receiving big payload requests or with a deeper level of nested objects.

Having the possibility of tuning those properties is essential to unblock users with that use-case and for a more accurate guard against malicious inputs.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

  • Set any property value to a lower/higher value, e.g.: -Dlogstash.jackson.stream-read-constraints.max-string-length=2
  • Configure and run a pipeline with a beats input
  • Send any data to this pipeline that exceeds the configured property value
  • Check the logs for: com.fasterxml.jackson.core.exc.StreamConstraintsException: String length (24) exceeds the maximum length (2)

Related issues

@edmocosta edmocosta changed the title [WIP] Add system properties to configure Jackson's defaults [WIP] Add system properties to configure Jackson's StreamReadConstraints Dec 21, 2023
@edmocosta edmocosta changed the title [WIP] Add system properties to configure Jackson's StreamReadConstraints [WIP] Add system properties to configure Jackson's stream read constraints Dec 21, 2023
@edmocosta edmocosta changed the title [WIP] Add system properties to configure Jackson's stream read constraints Add system properties to configure Jackson's stream read constraints Dec 21, 2023
@edmocosta edmocosta marked this pull request as ready for review December 21, 2023 16:02
@jsvd jsvd self-requested a review January 8, 2024 11:57
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just tweaking the defaults here: raising the number and message size by 10x. especially message size 20mb will be too low

config/jvm.options Outdated Show resolved Hide resolved
config/jvm.options Outdated Show resolved Hide resolved
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jsvd
Copy link
Member

jsvd commented Jan 8, 2024

Please backport to 8.12 after merging.

Copy link

Quality Gate passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No Coverage information No data about Coverage
No Duplication information No data about Duplication

See analysis details on SonarQube

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@edmocosta edmocosta merged commit a21ced0 into elastic:main Jan 8, 2024
5 checks passed
@edmocosta edmocosta deleted the add-jackson-system-properties branch January 8, 2024 16:48
@edmocosta
Copy link
Contributor Author

@logstashmachine backport 8.12

github-actions bot pushed a commit that referenced this pull request Jan 8, 2024
…15720)

This commit added a few jvm.options properties to configure the Jackson read constraints defaults (Maximum Number value length, Maximum String value length, and Maximum Nesting depth).

(cherry picked from commit a21ced0)
jsvd pushed a commit that referenced this pull request Jan 8, 2024
…15720) (#15763)

This commit added a few jvm.options properties to configure the Jackson read constraints defaults (Maximum Number value length, Maximum String value length, and Maximum Nesting depth).

(cherry picked from commit a21ced0)

Co-authored-by: Edmo Vamerlatti Costa <11836452+edmocosta@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants