Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[improve][function] Introduced protections against deserialization attacks #22723

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

khac
Copy link

@khac khac commented May 16, 2024

Motivation

This change hardens Java deserialization operations against attack. Even a simple operation like an object deserialization is an opportunity to yield control of your system to an attacker. In fact, without specific, non-default protections, any object deserialization call can lead to arbitrary code execution.

Modifications

I have added pixee java security toolkit as a dependency, and in pulsar functions in the the Serialization/ Deserialization file I have added ObjectInputFilters.enableObjectFilterIfUnprotected to the object input stream.

Motivation

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

Copy link

@khac Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added doc-label-missing doc-required Your PR changes impact docs and you will update later. doc-not-needed Your PR changes do not impact docs and removed doc-label-missing doc-required Your PR changes impact docs and you will update later. labels May 16, 2024
@khac
Copy link
Author

khac commented May 16, 2024

hey lhotari, Technoboy-, codelipenghui, gaoran10, congbobo184 and liangyepianzhou

can you take a look?

@lhotari
Copy link
Member

lhotari commented May 16, 2024

hey lhotari, Technoboy-, codelipenghui, gaoran10, congbobo184 and liangyepianzhou

can you take a look?

Great contribution! Thanks.
Please setup Personal CI and run the build in your fork. You'll get feedback about fixing licenses and so on. Since this is a new library, the license will have to be added into the distribution, following the same way as others. The license file would go in distribution/licenses and that would be referenced in distribution/server/src/assemble/LICENSE.bin.txt (footer).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants