Update zookeeper version to fix CVEs by bibith4 · Pull Request #24403 · prestodb/presto

bibith4 · 2025-01-21T09:50:01Z

Description

Changes to upgrade zookeeper versions to 3.9.3 to remove vulnerabilities

Motivation and Context

The presto-accumulo, presto-delta,presto-hive,presto-kafka and presto-hudi have interdependencies with zookeeper version 3.4.14, which contain vulnerabilities. These vulnerabilities can be removed by upgrading the zookeeper dependency to 3.9.3

Impact

Test Plan

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Security Changes
* Upgrade zookeeper to 3.9.3 to fix security vulnerability in presto-accumulo, presto-delta,presto-hive,presto-kafka and presto-hudi  in response to `CVE-2023-44981 <https://nvd.nist.gov/vuln/detail/cve-2023-44981>`_.

BryanCutler

LGTM pending tests

presto-accumulo/src/test/java/com/facebook/presto/accumulo/TestingAccumuloServer.java

aaneja · 2025-01-22T07:35:47Z

pom.xml

                <groupId>org.apache.zookeeper</groupId>
                <artifactId>zookeeper</artifactId>
-                <version>3.4.14</version>
+                <version>3.9.3</version>


JFYI - We do have an upgrade to Kafka dependencies as a WIP too -#24382
It should not impact this PR (tests seem to pass) cc : @ZacBlanco

@aaneja The test case failures in this PR will be resolved once the Kafka upgrade PR is merged. The failures occur because the ZooKeeper client used in our Kafka connector doesn’t support newer versions of ZooKeeper. However, since newer versions of Kafka have fully removed support for ZooKeeper, these failures will disappear after the Kafka upgrade
cc : @ZacBlanco @imjalpreet

steveburnett · 2025-01-22T15:17:42Z

Thanks for the release note! Suggest adding a little description of the work done in the PR ("Upgrade zookeeper to 3.9.3") that results in fixing the security vulnerabilities.

== RELEASE NOTES ==

Security Changes
* Upgrade zookeeper to 3.9.3 to fix security vulnerability in presto-accumulo, presto-delta,presto-hive,presto-kafka and presto-hudi  in response to `CVE-2023-44981 <https://nvd.nist.gov/vuln/detail/cve-2023-44981>`_. :pr:`24403`

bibith4 · 2025-01-29T05:29:16Z

Thanks for the release note! Suggest adding a little description of the work done in the PR ("Upgrade zookeeper to 3.9.3") that results in fixing the security vulnerabilities.
== RELEASE NOTES ==

Security Changes
* Upgrade zookeeper to 3.9.3 to fix security vulnerability in presto-accumulo, presto-delta,presto-hive,presto-kafka and presto-hudi  in response to `CVE-2023-44981 <https://nvd.nist.gov/vuln/detail/cve-2023-44981>`_. :pr:`24403`

@steveburnett Corrected. Please check

aaneja

Please edit your commit message as per our guidelines https://github.com/prestodb/presto/wiki/Review-and-Commit-guidelines#example-commit-message. You can add the CVE id this addresses to the commit message

bibith4 · 2025-01-31T10:57:50Z

Please edit your commit message as per our guidelines https://github.com/prestodb/presto/wiki/Review-and-Commit-guidelines#example-commit-message. You can add the CVE id this addresses to the commit message

@aaneja modified the commit message by adding CVE id

BryanCutler

Couple of small issues

presto-accumulo/src/test/java/com/facebook/presto/accumulo/TestingAccumuloServer.java

steveburnett · 2025-02-03T15:02:58Z

New release note guidelines as of last week: PR #24354 automatically adds links to this PR to the release notes. Please remove the manual PR link in the following format from the release note entries for this PR.

:pr:`12345`

I have updated the Release Notes Guidelines to remove the examples of manually adding the PR link.

bibith4 · 2025-02-04T09:31:49Z

New release note guidelines as of last week: PR #24354 automatically adds links to this PR to the release notes. Please remove the manual PR link in the following format from the release note entries for this PR.
:pr:`12345`
I have updated the Release Notes Guidelines to remove the examples of manually adding the PR link.

@steveburnett Corrected . Please check

steveburnett · 2025-02-04T14:40:47Z

@steveburnett Corrected . Please check

Looks good, thanks!

BryanCutler

LGTM, pending tests

ethanyzhang · 2025-05-01T22:57:06Z

Hi @bibith4, what's the plan for this PR?

presto-accumulo/src/test/java/com/facebook/presto/accumulo/TestingAccumuloServer.java

ZacBlanco · 2025-05-02T16:01:22Z

presto-accumulo/src/test/java/com/facebook/presto/accumulo/TestingAccumuloServer.java

+import java.time.Duration;
+
+import static java.lang.String.format;
+public class TestingAccumuloServer


Can you explain why we're going this route vs updating the MiniAccumuloCluster? This seems like a lot of change just to support zookeeper. Is there an advantage to using this over mini accumulo cluster?

@ZacBlanco We tried to address the vulnerability by upgrading the ZooKeeper version to 3.9.3. However, after the upgrade, the Accumulo test cases were failing because the ZooKeeper instance couldn't start. During our investigation, we found this pull request from Trino:
trinodb/trino#5598,
which uses Accumulo in Docker for unit testing. With this changes we tried running the tests using the presto-accumulo JAR, and they completed successfully.

Cherry-pick of trinodb/trino@f7a8471 Co-Authored-By: "Adam J. Shook" <shook@datacatessen.com>

prestodb-ci added the from:IBM PR from IBM label Jan 21, 2025

prestodb-ci requested review from a team, BryanCutler and aaneja and removed request for a team January 21, 2025 09:50

bibith4 force-pushed the zookeeper_update_v1.1.6 branch 2 times, most recently from b819bf6 to dc1cc7b Compare January 21, 2025 09:59

BryanCutler previously approved these changes Jan 22, 2025

View reviewed changes

bibith4 dismissed BryanCutler’s stale review via ecd5c5c January 22, 2025 04:29

aaneja reviewed Jan 22, 2025

View reviewed changes

bibith4 changed the title ~~Update zookeeper version to fix common vulnerabilities and exposures~~ Update zookeeper version to fix common vulnerabilities and exposures(Do not review) Jan 22, 2025

bibith4 force-pushed the zookeeper_update_v1.1.6 branch from c11877c to ab4f9e5 Compare January 28, 2025 06:13

bibith4 changed the title ~~Update zookeeper version to fix common vulnerabilities and exposures(Do not review)~~ Update zookeeper version to fix common vulnerabilities and exposures(Do not merge) Jan 29, 2025

bibith4 marked this pull request as ready for review January 29, 2025 05:30

bibith4 requested a review from a team as a code owner January 29, 2025 05:30

bibith4 requested review from BryanCutler, aaneja and presto-oss January 29, 2025 05:30

aaneja reviewed Jan 31, 2025

View reviewed changes

bibith4 force-pushed the zookeeper_update_v1.1.6 branch 2 times, most recently from d72ddd1 to 78f9857 Compare January 31, 2025 10:00

bibith4 requested a review from aaneja January 31, 2025 11:04

aaneja previously approved these changes Jan 31, 2025

View reviewed changes

BryanCutler requested changes Feb 1, 2025

View reviewed changes

presto-accumulo/src/test/java/com/facebook/presto/accumulo/TestingAccumuloServer.java Outdated Show resolved Hide resolved

presto-accumulo/src/test/java/com/facebook/presto/accumulo/TestingAccumuloServer.java Outdated Show resolved Hide resolved

bibith4 dismissed aaneja’s stale review via d87373c February 3, 2025 05:02

bibith4 force-pushed the zookeeper_update_v1.1.6 branch from d87373c to 71714c9 Compare February 3, 2025 05:04

bibith4 requested a review from BryanCutler February 3, 2025 10:29

bibith4 requested a review from aaneja February 5, 2025 06:03

BryanCutler previously approved these changes Feb 6, 2025

View reviewed changes

aaneja previously approved these changes Feb 7, 2025

View reviewed changes

bibith4 dismissed stale reviews from aaneja and BryanCutler via 0c25890 May 2, 2025 10:22

ZacBlanco reviewed May 2, 2025

View reviewed changes

bibith4 force-pushed the zookeeper_update_v1.1.6 branch from 0c25890 to 957d25c Compare May 3, 2025 07:38

bibith4 requested a review from a team as a code owner May 5, 2025 06:53

bibith4 force-pushed the zookeeper_update_v1.1.6 branch from 9989846 to aae0886 Compare May 5, 2025 07:06

bibith4 requested review from ZacBlanco and aaneja May 5, 2025 09:56

ethanyzhang changed the title ~~Update zookeeper version to fix common vulnerabilities and exposures(Do not merge)~~ Update zookeeper version to fix common vulnerabilities and exposures May 5, 2025

ZacBlanco changed the title ~~Update zookeeper version to fix common vulnerabilities and exposures~~ Update zookeeper version to fix CVEs May 6, 2025

bibith4 force-pushed the zookeeper_update_v1.1.6 branch from aae0886 to e844c9b Compare May 6, 2025 16:01

Update zookeeper version to fix CVE-2023-44981

4b03b38

Cherry-pick of trinodb/trino@f7a8471 Co-Authored-By: "Adam J. Shook" <shook@datacatessen.com>

bibith4 force-pushed the zookeeper_update_v1.1.6 branch from e844c9b to 4b03b38 Compare May 6, 2025 16:08

ZacBlanco approved these changes May 6, 2025

View reviewed changes

bibith4 requested a review from BryanCutler May 7, 2025 03:37

aaneja approved these changes May 7, 2025

View reviewed changes

aaneja merged commit 0d3a2ed into prestodb:master May 7, 2025
97 checks passed

ZacBlanco mentioned this pull request May 29, 2025

Add release notes for 0.293 #25222

Merged

21 tasks

This was referenced May 30, 2025

Add release notes for 0.293 unix280/presto#31

Closed

Add release notes for 0.293 unix280/presto#32

Closed

Conversation

bibith4 commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

BryanCutler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aaneja Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

bibith4 Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steveburnett commented Jan 22, 2025

Uh oh!

bibith4 commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaneja left a comment

Choose a reason for hiding this comment

Uh oh!

bibith4 commented Jan 31, 2025

Uh oh!

BryanCutler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

steveburnett commented Feb 3, 2025

Uh oh!

bibith4 commented Feb 4, 2025

Uh oh!

steveburnett commented Feb 4, 2025

Uh oh!

BryanCutler left a comment

Choose a reason for hiding this comment

Uh oh!

ethanyzhang commented May 1, 2025

Uh oh!

Uh oh!

Uh oh!

ZacBlanco May 2, 2025

Choose a reason for hiding this comment

Uh oh!

bibith4 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bibith4 commented Jan 21, 2025 •

edited

Loading

bibith4 Jan 29, 2025 •

edited

Loading

bibith4 commented Jan 29, 2025 •

edited

Loading