Skip to content

[fix][sec] Suppress already covered CVE-2023-2976 in clickhouse-jdbc-0.4.6-all.jar and canal.client-1.1.5.jar - #20792

Merged
tisonkun merged 7 commits into
apache:masterfrom
JooHyukKim:guava-false-alarm-clickhouse
Jul 20, 2023
Merged

[fix][sec] Suppress already covered CVE-2023-2976 in clickhouse-jdbc-0.4.6-all.jar and canal.client-1.1.5.jar#20792
tisonkun merged 7 commits into
apache:masterfrom
JooHyukKim:guava-false-alarm-clickhouse

Conversation

@JooHyukKim

@JooHyukKim JooHyukKim commented Jul 12, 2023

Copy link
Copy Markdown
Contributor

Fixes workflows that fails with

Motivation

The OWASP dependency check failed. Link: https://github.com/apache/pulsar/actions/runs/5521854569/jobs/10090413638?pr=20782

Error:  Failed to execute goal org.owasp:dependency-check-maven:8.2.1:aggregate (default) on project pulsar: 
Error:  
Error:  One or more dependencies were identified with vulnerabilities that have a CVSS score greater than or equal to '7.0': 
Error:  
Error:  canal.client-1.1.5.jar/META-INF/maven/com.google.guava/guava/pom.xml: CVE-2023-2976(7.1)
Error:  clickhouse-jdbc-0.4.6-all.jar/META-INF/maven/com.google.guava/guava/pom.xml: CVE-2023-2976(7.1)
Error:  
Error:  See the dependency-check report for more details.
Error:  -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Error:  
Error:  After correcting the problems, you can resume the build with the command
Error:    mvn <args> -rf :pulsar

The PR already addressed #20699 the CVE-2023-2976. And Pulsar currently uses Guava 32.1.1 version

Modifications

  • Suppress warning

Documentation

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

Matching PR in forked repository

PR in forked repository: https://github.com/JooHyukKim/pulsar/pull/21

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jul 12, 2023
@JooHyukKim

Copy link
Copy Markdown
Contributor Author

@tisonkun thank you for looking into this 🙏🏼

FYI, there is another one lined up for canal.client-1.1.5. jar (refer below photo).
I am getting this issue fixed one by one because depedency-related commits most often gets reverted and picked across versions.

image

@JooHyukKim
JooHyukKim requested a review from tisonkun July 18, 2023 08:38
@JooHyukKim

Copy link
Copy Markdown
Contributor Author

@tisonkun Sorry, merged upstream/master and now CI is blocked agaiin 🥲

@tisonkun

Copy link
Copy Markdown
Member

No sorry. You are doing things right :D

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comments inline. You may try to verified locally.

</suppress>
<suppress>
<notes><![CDATA[
file name: clickhouse-jdbc-0.4.6-all.jar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Error:  One or more dependencies were identified with vulnerabilities that have a CVSS score greater than or equal to '7.0': 
Error:  
Error:  canal.client-1.1.5.jar/META-INF/maven/com.google.guava/guava/pom.xml: CVE-2023-2976(7.1)
Error:  clickhouse-jdbc-0.4.6-all.jar/META-INF/maven/com.google.guava/guava/pom.xml: CVE-2023-2976(7.1)

It seems this patch doesn't suppress the false positive.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will check again, thanks ☺️🙏🏽

@JooHyukKim JooHyukKim Jul 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So currently canal client passes. but clickhouse doesn't.
What I can't seem to figure out why 🤔🤔🤔
Checksum I used.
https://repo1.maven.org/maven2/com/clickhouse/clickhouse-jdbc/0.4.6/clickhouse-jdbc-0.4.6-all.jar.sha1

@JooHyukKim JooHyukKim Jul 19, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tisonkun May I ask you for another Github workflow trigger? Because while verifying this PR locally, OWASP check that I ran locally failed on

  • clickhouse-jdbc-0.4.6-all.jar and
  • canal.client-1.1.5.jar

yesterday, but today we are failing a bunch.

FYI, command used,

 mvn -B -ntp verify -PskipDocker,skip-all,owasp-dependency-check -Dcheckstyle.skip=true -DskipTests 
    -pl '!pulsar-sql,!distribution/server,!distribution/io,!distribution/offloaders,!pulsar-sql/presto-distribution,!tiered-storage/file-system,!pulsar-io/flume,!pulsar-io/hbase,!pulsar-io/hdfs2,!pulsar-io/hdfs3,!pulsar-io/docs,!pulsar-io/jdbc/openmldb'
[ERROR] 
[ERROR] One or more dependencies were identified with vulnerabilities that have a CVSS score greater than or equal to '7.0': 
[ERROR] 
[ERROR] api-util-1.0.0-M20.jar: CVE-2018-1337(9.8)
[ERROR] avro-1.8.2.jar/META-INF/maven/com.google.guava/guava/pom.xml: CVE-2023-2976(7.1)
[ERROR] clickhouse-jdbc-0.4.6-all.jar/META-INF/maven/com.google.guava/guava/pom.xml: CVE-2023-2976(7.1)
.... omitted some (JooHyukKim)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll trigger one. But generally you can test in your personal fork and that should be the same.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tisonkun Right. I keep forgetting 🫣. Thank you still~

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Phew, took longer than I thought, to figure out why still not worked.
Verified in : https://github.com/JooHyukKim/pulsar/pull/21

@JooHyukKim
JooHyukKim marked this pull request as draft July 18, 2023 11:26
@JooHyukKim JooHyukKim changed the title [fix][sec] Suppress already covered CVE-2023-2976 in clickhouse-jdbc-0.4.6-all.jar [fix][sec] Suppress already covered CVE-2023-2976 in clickhouse-jdbc-0.4.6-all.jar and canal.client-1.1.5.jar Jul 18, 2023
@JooHyukKim

JooHyukKim commented Jul 19, 2023

Copy link
Copy Markdown
Contributor Author

@lhotari May I ask if you intentionally triggered this action? Ran into it while debugging on this PR.

I am asking this because I assumed there might, or will be bigger fix that covers this PR also. In which case this PR can just be omitted.

@JooHyukKim
JooHyukKim marked this pull request as ready for review July 19, 2023 02:23
@JooHyukKim

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@tisonkun

Copy link
Copy Markdown
Member

/pulsarbot rerun-failure-checks

@JooHyukKim
JooHyukKim requested a review from tisonkun July 19, 2023 12:24
@JooHyukKim

Copy link
Copy Markdown
Contributor Author

Note that current changes cover both clickhouse-jdbc-0.4.6-all.jar and canal.client-1.1.5.jar in one-go, for following reasons.

  • Both of the new suppressions handle the same CVE-2023-2976
  • If we do separate PRs, changes may be atomic. But downside is having to merge PRs with failed CI.

/cc @tisonkun 👍🏻

@JooHyukKim

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@JooHyukKim

Copy link
Copy Markdown
Contributor Author

Filed #20839 and it got fixed. No more blocker 👍🏻

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@tisonkun

Copy link
Copy Markdown
Member

Merging...

@tisonkun
tisonkun merged commit 514fa60 into apache:master Jul 20, 2023
@JooHyukKim
JooHyukKim deleted the guava-false-alarm-clickhouse branch July 20, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants