Skip to content

Conversation

@senthh
Copy link
Contributor

@senthh senthh commented Sep 18, 2024

Change Logs

We are seeing Critical level CVE CVE-2017-17485 in Hudi. And it is traced out from HTrace component(which uses jackson-databind version 2.4.0). So it is good to exclude jackson-databind in packaging hudi-spark-bundle module.

Uploading Screenshot 2024-09-18 at 6.29.04 PM.png…

Impact

No performance change, but fixing CRITICAL CVE CVE-2017-17485.

Risk level (write none, low medium or high below)

CRITICAL

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".

  • The config description must be updated if new configs are added or the default value of the configs are changed
  • Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
    ticket number here and follow the instruction to make
    changes to the website.

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@github-actions github-actions bot added the size:XS PR with lines of changes in <= 10 label Sep 18, 2024
@senthh senthh changed the title HUDI-8213 - Exclude jackson-databind from hudi-spark-bundle to fix CVE-2017-17485 [HUDI-8213] Exclude jackson-databind from hudi-spark-bundle to fix CVE-2017-17485 Sep 18, 2024
@hudi-bot
Copy link
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
<exclude>META-INF/services/javax.*</exclude>
<exclude>META-INF/maven/com.fasterxml.jackson.core/jackson-databind/*</exclude>
Copy link
Contributor

Choose a reason for hiding this comment

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

This change only removes the meta info from the bundle; the classes from jackson-databind are still included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yihua Yes correct, but this change is sufficient to get rid of critical CVE issue CVE-2017-17485
.

Copy link
Contributor

@yihua yihua left a comment

Choose a reason for hiding this comment

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

Thanks for your first contribution. Could you check if you can exclude the dependency in the bundle directly?

@senthh
Copy link
Contributor Author

senthh commented Sep 19, 2024

Thanks for your first contribution. Could you check if you can exclude the dependency in the bundle directly?

Yes @yihua I tried excluding directly, but it did not help and I wanted to do very minimal change so that actual functionality should not break. So found removing meta info is sufficient to get rid of critical CVE issue .

I welcome your alternate suggestion for fixing this issue.

@senthh senthh requested a review from yihua September 19, 2024 06:23
@senthh
Copy link
Contributor Author

senthh commented Sep 20, 2024

@yihua below is the screen-shot after fixing CVE

Screenshot 2024-09-18 at 6 40 05 PM

@yihua
Copy link
Contributor

yihua commented Sep 20, 2024

Thanks for your first contribution. Could you check if you can exclude the dependency in the bundle directly?

Yes @yihua I tried excluding directly, but it did not help and I wanted to do very minimal change so that actual functionality should not break. So found removing meta info is sufficient to get rid of critical CVE issue .

I welcome your alternate suggestion for fixing this issue.

@senthh Thanks for the clarification. I prefer to exclude the dependencies directly or use alternatives to get the same functionality. The reason is that removing the META-INF only tricks the scan to report no security issue (if the scan uses META-INF for checking vulnerabilities, correct me if I'm wrong); the actual security issue in the bundled classes may still exist. This can make security detection worse as the security risk is still there, though there is no report, hiding the actual vulnerabilities.

@yihua
Copy link
Contributor

yihua commented Sep 20, 2024

If HTrace is of concern, the community is making effort to remove HBase dependencies as the required ones. I've introduced our own HFile readers (see #10241, #10330) that do not depend on HBase, and we have a plan to introduce HFile writer implementation that is independent of HBase dependencies (HUDI-8222), so we can remove HBase dependencies in the future.

@senthh
Copy link
Contributor Author

senthh commented Sep 24, 2024

If HTrace is of concern, the community is making effort to remove HBase dependencies as the required ones. I've introduced our own HFile readers (see #10241, #10330) that do not depend on HBase, and we have a plan to introduce HFile writer implementation that is independent of HBase dependencies (HUDI-8222), so we can remove HBase dependencies in the future.

Good to hear you have already initiated to remove the HBase dependencies. So Shall I remove the jackson-databind classes also, by 'exclude' as below

                <exclude>META-INF/services/com.fasterxml.jackson.core/jackson-databind/*</exclude>

Or We can close this PR and wait for your PR to be completed? I happy to follow-up your feedback

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

Labels

size:XS PR with lines of changes in <= 10

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants