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.

Sign up for GitHub

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

Jump to bottom

remove unnecessary elasticsearch dependencies to fix CVE regressions #15443

Merged
merged 4 commits into from
Dec 3, 2023
Merged

remove unnecessary elasticsearch dependencies to fix CVE regressions #15443

merged 4 commits into from
Dec 3, 2023

Conversation

janjwerner-confluent
Copy link
Contributor

@janjwerner-confluent janjwerner-confluent commented Nov 28, 2023 •
edited
Loading

Description

Recent upgrade of ranger introduced CVE regressions due to outdated elasticsearch components.
Druid-ranger-plugin does not elasticsearch components , and they have been explicitly removed.

Update woodstox-core to 6.4.0 to address GHSA-3f7h-mf4q-vrm4

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@janjwerner-confluent
Copy link
Contributor Author

@xvrl
Another request for a quick review - trimming the dependencies introduced by the ranger that are not necessary.

abhishekagarwal87
abhishekagarwal87 approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@abhishekagarwal87 abhishekagarwal87 left a comment

Choose a reason for hiding this comment

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

LGTM. Did you also build the extension to verify that dependency in extension folder doesn't change significantly? Not that it should but verifying the dependencies still seems useful.

@janjwerner-confluent
Copy link
Contributor Author

LGTM. Did you also build the extension to verify that dependency in extension folder doesn't change significantly? Not that it should but verifying the dependencies still seems useful.

I built and tested it, I don't think there are integration tests for ranger :(

@abhishekagarwal87
Copy link
Contributor

can you fix conflicts? I will merge once that is done.

@janjwerner-confluent janjwerner-confluent mentioned this pull request Nov 30, 2023
Merged
10 tasks
@abhishekagarwal87 abhishekagarwal87 merged commit b854058 into apache:master Dec 3, 2023
83 checks passed
@janjwerner-confluent janjwerner-confluent deleted the janjwerner-fix-ranger-cve-regressions branch December 4, 2023 00:56
janjwerner-confluent added a commit to janjwerner-confluent/druid that referenced this pull request Dec 4, 2023
@janjwerner-confluent
remove elasticsearch CVEs resolved in apache#15443
1dd5b23
janjwerner-confluent added a commit to janjwerner-confluent/druid that referenced this pull request Dec 4, 2023
@janjwerner-confluent
remove sdk-bundle fixed in apache#15443
66dc117
Pankaj260100 added a commit to confluentinc/druid that referenced this pull request Dec 13, 2023
@Pankaj260100
remove unnecessary elasticsearch dependencies to fix CVE regressions (a…
3b2f1a3 
…pache#15443)
Pankaj260100 pushed a commit to confluentinc/druid that referenced this pull request Dec 13, 2023
@janjwerner-confluent @Pankaj260100
remove unnecessary elasticsearch dependencies to fix CVE regressions (a…
6cd61f1 
…pache#15443)

Recent upgrade of ranger introduced CVE regressions due to outdated elasticsearch components.
Druid-ranger-plugin does not elasticsearch components , and they have been explicitly removed.

Update woodstox-core to 6.4.0 to address GHSA-3f7h-mf4q-vrm4
@Pankaj260100 Pankaj260100 mentioned this pull request Dec 19, 2023
Merged
Pankaj260100 pushed a commit to confluentinc/druid that referenced this pull request Dec 19, 2023
@janjwerner-confluent @Pankaj260100
remove unnecessary elasticsearch dependencies to fix CVE regressions (a…
eefbad1 
…pache#15443)

Recent upgrade of ranger introduced CVE regressions due to outdated elasticsearch components.
Druid-ranger-plugin does not elasticsearch components , and they have been explicitly removed.

Update woodstox-core to 6.4.0 to address GHSA-3f7h-mf4q-vrm4
Pankaj260100 pushed a commit to confluentinc/druid that referenced this pull request Dec 19, 2023
@janjwerner-confluent @Pankaj260100
remove unnecessary elasticsearch dependencies to fix CVE regressions (a…
35bae5f 
…pache#15443)

Recent upgrade of ranger introduced CVE regressions due to outdated elasticsearch components.
Druid-ranger-plugin does not elasticsearch components , and they have been explicitly removed.

Update woodstox-core to 6.4.0 to address GHSA-3f7h-mf4q-vrm4
@LakshSingla LakshSingla added this to the 29.0.0 milestone Jan 29, 2024
LakshSingla
LakshSingla reviewed Feb 9, 2024
View reviewed changes
extensions-core/druid-ranger-security/pom.xml
Comment on lines +45 to +48
<groupId>com.amazonaws</groupId>
<artifactId>aws-java-sdk-bundle</artifactId>
<version>${aws.sdk.version}</version>
</dependency>
Copy link
Contributor

@LakshSingla LakshSingla Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This upgrade blows up the size of the extension from 94MB to 440MB. Is there a way to import only the required JARs from the bundle?

Copy link
Member

Choose a reason for hiding this comment

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

@janjwerner-confluent is this something you could look into?

LakshSingla added a commit to LakshSingla/druid that referenced this pull request Feb 12, 2024
@LakshSingla
Revert "remove unnecessary elasticsearch dependencies to fix CVE regr…
bed59ac 
…essions (apache#15443)"

This reverts commit b854058.
@abhishekagarwal87
Copy link
Contributor

@janjwerner-confluent - Will you be looking into this? This would become a problem again in the next major release.

@janjwerner-confluent
Copy link
Contributor Author

janjwerner-confluent commented Feb 29, 2024 •
edited
Loading

@abhishekagarwal87
Yes, I will try to pick up only the only the needed items. It is compile dependency of https://mvnrepository.com/artifact/org.apache.ranger/ranger-plugins-audit/2.4.0
I will look into excluding the azure-sdk altogether.

@janjwerner-confluent janjwerner-confluent mentioned this pull request Feb 29, 2024
Merged
10 tasks
xvrl pushed a commit that referenced this pull request Mar 8, 2024
@janjwerner-confluent
remove aws-sdk from ranger-extension (#16011)
a7b2747 
Fixes # size blowup regression introduced in #15443

This PR removes the transitive dependency of ranger-plugins-audit to reduce the size of the compiled artifacts

* add aws-logs-sdk to ensure that all the transitive dependencies are satisfied
* replace aws-bundle-sdk with aws-logs-sdk
* add additional guidance on ranger update, add dependency ignore to satisfy dependency analyzer
* add aws-sdk-logs to list of ignored dependencies to satisfy the maven plugin
* align aws-sdk versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xvrl xvrl xvrl left review comments

@LakshSingla LakshSingla LakshSingla left review comments

@abhishekagarwal87 abhishekagarwal87 abhishekagarwal87 approved these changes

Assignees
No one assigned
Labels
Area - Dependencies
Projects
None yet
Milestone
29.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@janjwerner-confluent @abhishekagarwal87 @xvrl @LakshSingla