Skip to content

Upgrade io.grpc to 1.60.1#21668

Closed
Akanksha-kedia wants to merge 1 commit intoprestodb:masterfrom
Akanksha-kedia:fi2
Closed

Upgrade io.grpc to 1.60.1#21668
Akanksha-kedia wants to merge 1 commit intoprestodb:masterfrom
Akanksha-kedia:fi2

Conversation

@Akanksha-kedia
Copy link
Contributor

@Akanksha-kedia Akanksha-kedia commented Jan 10, 2024

Description

outlines the direct vulnerabilities and vulnerabilities from dependencies for io.grpc and com.google.protobuf.

Motivation and Context

The io.grpc package, specifically the grpc-protobuf artifact version 1.35.0, has several identified vulnerabilities:

Direct vulnerabilities:

CVE-2023-32732
CVE-2023-32731
CVE-2023-1428
Vulnerabilities from dependencies:

CVE-2023-2976
CVE-2022-3510
CVE-2022-3509
CVE-2022-3171
CVE-2021-22570
CVE-2021-22569
CVE-2020-8908
The com.google.protobuf package, specifically the protobuf-java artifact version 3.12.0, has several identified vulnerabilities:

Direct vulnerabilities:

CVE-2022-3510
CVE-2022-3509
CVE-2022-3171
CVE-2021-22570
CVE-2021-22569
Vulnerabilities from dependencies:

CVE-2023-2976
CVE-2020-8908
CVE-2020-15250

Impact

At this time, these vulnerabilities are not expected to have a major impact.

Test Plan

Unit tests for presto-bigquery and other relevant components will be conducted to ensure system stability.

[INFO]
[INFO] <<< spotbugs:3.1.10:check (default) < :spotbugs @ presto-bigquery <<<
[INFO]
[INFO]
[INFO] --- spotbugs:3.1.10:check (default) @ presto-bigquery ---
[INFO]
[INFO] >>> pmd:3.11.0:check (default) > :pmd @ presto-bigquery >>>
[INFO]
[INFO] --- pmd:3.11.0:pmd (pmd) @ presto-bigquery ---
[INFO]
[INFO] <<< pmd:3.11.0:check (default) < :pmd @ presto-bigquery <<<
[INFO]
[INFO]
[INFO] --- pmd:3.11.0:check (default) @ presto-bigquery ---
[INFO]
[INFO] --- install:2.5.2:install (default-install) @ presto-bigquery ---
[INFO] Installing /Users/akedia/dec2024/prestodb/presto-bigquery/target/presto-bigquery-0.286-SNAPSHOT.zip to /Users/akedia/.m2/prestodb_28/com/facebook/presto/presto-bigquery/0.286-SNAPSHOT/presto-bigquery-0.286-SNAPSHOT.zip
[INFO] Installing /Users/akedia/dec2024/prestodb/presto-bigquery/pom.xml to /Users/akedia/.m2/prestodb_28/com/facebook/presto/presto-bigquery/0.286-SNAPSHOT/presto-bigquery-0.286-SNAPSHOT.pom
[INFO] Installing /Users/akedia/dec2024/prestodb/presto-bigquery/target/presto-bigquery-0.286-SNAPSHOT.jar to /Users/akedia/.m2/prestodb_28/com/facebook/presto/presto-bigquery/0.286-SNAPSHOT/presto-bigquery-0.286-SNAPSHOT.jar
[INFO] Installing /Users/akedia/dec2024/prestodb/presto-bigquery/target/presto-bigquery-0.286-SNAPSHOT-tests.jar to /Users/akedia/.m2/prestodb_28/com/facebook/presto/presto-bigquery/0.286-SNAPSHOT/presto-bigquery-0.286-SNAPSHOT-tests.jar
[INFO] Installing /Users/akedia/dec2024/prestodb/presto-bigquery/target/presto-bigquery-0.286-SNAPSHOT-sources.jar to /Users/akedia/.m2/prestodb_28/com/facebook/presto/presto-bigquery/0.286-SNAPSHOT/presto-bigquery-0.286-SNAPSHOT-sources.jar
[INFO] Installing /Users/akedia/dec2024/prestodb/presto-bigquery/target/presto-bigquery-0.286-SNAPSHOT-test-sources.jar to /Users/akedia/.m2/prestodb_28/com/facebook/presto/presto-bigquery/0.286-SNAPSHOT/presto-bigquery-0.286-SNAPSHOT-test-sources.jar

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • 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 ==
Security Changes
* Upgrade io.grpc to 1.60.1
* Upgrade com.google.protobuf to 3.25.1

@Akanksha-kedia Akanksha-kedia requested a review from a team as a code owner January 10, 2024 07:09
@Akanksha-kedia Akanksha-kedia force-pushed the fi2 branch 4 times, most recently from fcd1a82 to 62a2118 Compare January 10, 2024 12:01
@Akanksha-kedia
Copy link
Contributor Author

@tdcmeehan please help to review

@steveburnett
Copy link
Contributor

steveburnett commented Jan 10, 2024

Would it help to mention these changes in the release notes? I wasn't sure what the title "Direct vulnerabilities for io.grpc" meant until I read the files being changed, and also not only io.grpc appears to be changed by this PR.

Based on the Release Note Guidelines, perhaps release note entries for these changes similar to the following suggestion might be appropriate:

== RELEASE NOTES ==
Security Changes
* Upgrade io.grpc to 1.60.1
* Upgrade com.google.protobuf to 3.25.1

I suggest Security Changes as a potential better heading than General Changes for these entries, given the CVEs affecting the older versions of these two items appear to be the motivation for this PR.

@Akanksha-kedia Akanksha-kedia changed the title Direct vulnerabilities for io.grpc Upgrade io.grpc to 1.60.1 Jan 11, 2024
@Akanksha-kedia Akanksha-kedia force-pushed the fi2 branch 3 times, most recently from 1a63ec8 to 43ea714 Compare January 11, 2024 17:12
@tdcmeehan tdcmeehan self-assigned this Jan 12, 2024
@skairali skairali self-requested a review January 16, 2024 08:35
@skairali skairali self-assigned this Jan 16, 2024
Copy link
Member

@skairali skairali left a comment

Choose a reason for hiding this comment

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

@Akanksha-kedia This is a needed change for many use cases, Thank you

But I see that grpc version is not changed at all places

Example : https://github.com/Akanksha-kedia/prestodb/blob/fi2/presto-pinot/pom.xml

Could you please make sure that upgrade is comprehensive?

@Akanksha-kedia
Copy link
Contributor Author

Akanksha-kedia commented Jan 16, 2024 via email

@Akanksha-kedia Akanksha-kedia force-pushed the fi2 branch 3 times, most recently from 6b224d4 to 5cf00ab Compare January 16, 2024 16:28
@Akanksha-kedia
Copy link
Contributor Author

@skairali please review, i see presto-docs is failing but i see no changes related to mine.

@Akanksha-kedia Akanksha-kedia force-pushed the fi2 branch 3 times, most recently from 40f45a2 to 1f61f1c Compare January 17, 2024 03:04
@Akanksha-kedia
Copy link
Contributor Author

@skairali please review

@steveburnett
Copy link
Contributor

Please format the release note entry with a row of three ` above and below. If my description here isn't clear, you can see an example by selecting Edit on my earlier comment.

@skairali
Copy link
Member

@Akanksha-kedia Changes looks good.

Please complete the release notes as @steveburnett requested in this case.

Also can you please explain why do we have packages in exclusion list?

@Akanksha-kedia
Copy link
Contributor Author

@steveburnett i have corrected please review or guide how to correctly do, @skairali can you reframe the question?

@steveburnett
Copy link
Contributor

@steveburnett i have corrected please review or guide how to correctly do, @skairali can you reframe the question?

Your release note entry is not properly formatted, and contains lines it does not need to.

  1. Delete the following lines in your release note entry
General Changes

No change
Hive Changes

No change
  1. Above the line Security Changes, add a new line containing three `, like this:

```

  1. Below the line Upgrade com.google.protobuf to 3.25.1, add a new line containing three `, like this:

```

Your final result should look like the example that I provided in my comment last week.

@skairali
Copy link
Member

@Akanksha-kedia I see below

image

Just need an explanation of why this is required?

@Akanksha-kedia
Copy link
Contributor Author

@Akanksha-kedia I see below

image Just need an explanation of why this is required?

@skairali i was getting this error :
[WARNING] Found duplicate (but equal) classes in [io.grpc:grpc-api:1.60.1, io.grpc:grpc-context:1.35.0]: [WARNING] io.grpc.PersistentHashArrayMappedTrie [WARNING] io.grpc.ThreadLocalContextStorage [WARNING] Found duplicate and different classes in [io.grpc:grpc-api:1.60.1, io.grpc:grpc-context:1.35.0]: [WARNING] io.grpc.Context [WARNING] io.grpc.Deadline [WARNING] Found duplicate classes/resources in compile classpath. [WARNING] Found duplicate (but equal) classes in [io.grpc:grpc-api:1.60.1, io.grpc:grpc-context:1.35.0]: [WARNING] io.grpc.PersistentHashArrayMappedTrie [WARNING] io.grpc.ThreadLocalContextStorage [WARNING] Found duplicate and different classes in [io.grpc:grpc-api:1.60.1, io.grpc:grpc-context:1.35.0]: [WARNING] io.grpc.Context [WARNING] io.grpc.Deadline [WARNING] Found duplicate classes/resources in runtime classpath. [WARNING] Found duplicate (but equal) classes in [io.grpc:grpc-api:1.60.1, io.grpc:grpc-context:1.35.0]: [WARNING] io.grpc.PersistentHashArrayMappedTrie [WARNING] io.grpc.ThreadLocalContextStorage [WARNING] Found duplicate and different classes in [io.grpc:grpc-api:1.60.1, io.grpc:grpc-context:1.35.0]: [WARNING] io.grpc.Context [WARNING] io.grpc.Deadline [WARNING] Found duplicate classes/resources in test classpath.

@steveburnett please review @skairali and help to merge.

@steveburnett
Copy link
Contributor

@steveburnett please review @skairali and help to merge.

Your final result should look like the example that I provided in #21668 (comment).

@Akanksha-kedia
Copy link
Contributor Author

@steveburnett please review @skairali and help to merge.

Your final result should look like the example that I provided in #21668 (comment).

@steveburnett done

@Akanksha-kedia
Copy link
Contributor Author

@steveburnett @skairali @tdcmeehan please review.

@Akanksha-kedia
Copy link
Contributor Author

@steveburnett @skairali @tdcmeehan please review.

please review @skairali

@Akanksha-kedia
Copy link
Contributor Author

@tdcmeehan please review

@skairali
Copy link
Member

@Akanksha-kedia Please squash the commits and let me know

@Akanksha-kedia
Copy link
Contributor Author

@Akanksha-kedia Please squash the commits and let me know

i have done @skairali

Copy link
Member

@skairali skairali left a comment

Choose a reason for hiding this comment

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

Approved

Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Reviewed at @skairali's request - the release note looks good and I have no other concerns.

@tdcmeehan
Copy link
Contributor

@Akanksha-kedia how have you tested these changs?

@Akanksha-kedia
Copy link
Contributor Author

Akanksha-kedia commented Jan 30, 2024 via email

@Akanksha-kedia
Copy link
Contributor Author

@skairali @tdcmeehan review please

@Akanksha-kedia
Copy link
Contributor Author

@tdcmeehan review please

@tdcmeehan
Copy link
Contributor

@Akanksha-kedia I am not confident in just the UT coverage. Please try to run against actual deployments of BigQuery and Pinot and report your findings.

@tdcmeehan tdcmeehan removed their assignment Mar 18, 2024
@elharo
Copy link
Contributor

elharo commented Sep 24, 2024

moot, we're now at 1.64.0

@elharo elharo closed this Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

5 participants