HIVE-28866: Upgrade netty-codec-http to fix CVE-2024-29025 #5736
Conversation
asf-ci-hive
added
tests pending
tests failed
and removed
tests pending
tests failed
labels
| </dependency> | ||
| <dependency> | ||
| <groupId>io.netty</groupId> | ||
| <artifactId>netty-all</artifactId> |
There was a problem hiding this comment.
I think the goal is for the transitive dependencies to have higher version of netty?
Also, I see older version of netty (can be seen in your attached dependency tree) in packaging/target/apache-hive-4.1.0-SNAPSHOT-bin/apache-hive-4.1.0-SNAPSHOT-bin/lib coming from zookeeper 3.8.4 and has CVE's as well https://mvnrepository.com/artifact/io.netty/netty-handler/4.1.105.Final
netty-handler-4.1.105.Final.jar
netty-transport-native-epoll-4.1.105.Final.jar
There was a problem hiding this comment.
Have added these deps as well in dependencyManagement in bd96998 , so these transitive dependencies are of the same versions.
standalone-metastore/pom.xml
Outdated
| <libthrift.version>0.16.0</libthrift.version> | ||
| <log4j2.version>2.24.3</log4j2.version> | ||
| <mockito-core.version>3.4.4</mockito-core.version> | ||
| <netty.version>4.1.116.Final</netty.version> |
There was a problem hiding this comment.
can we move to 4.1.119.Final
https://mvnrepository.com/artifact/io.netty/netty-all/4.1.119.Final
There was a problem hiding this comment.
Checking it out.
There was a problem hiding this comment.
@ayushtkn PR is green for netty 4.1.119.Final. Could you please review?
|
| <artifactId>netty-all</artifactId> | ||
| <version>${netty.version}</version> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Why are we adding dependency is scope of upgrade? I don't see any exclusions either which could lead it this?
There was a problem hiding this comment.
We are adding these specifications in dependencyManagement, coz other deps were bringing in older versions of these transitively and with this addition we'll get only 4.1.119.Final.
We are addressing @Aggarwal-Raghav comment with this #5736 (comment)
There was a problem hiding this comment.
We are adding these specifications in dependencyManagement, coz other deps were bringing in older versions of these transitively and with this addition we'll get only 4.1.119.Final. We are addressing @Aggarwal-Raghav comment with this #5736 (comment)
@tanishq-chugh , can you please provide the sonatype report URL for this? I don't see netty-codec-http coming up in my nexus scan in my org.
Regarding this approach to add dependency of higher version is not a good idea, IMO, because if higher version of netty is not backward compatible with older version (required by transitive dependency) then we are prone to NoSuchMethod or NoClassFound errors at runtime. This is similar to the avro issue #5593 that we last discussed.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
4 participants
What changes were proposed in this pull request?
Upgrade a single instance of compile time netty-codec-http
Why are the changes needed?
To fix CVE-2024-29025
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
Yes
dpn_netty_119.txt
How was this patch tested?
Ran manual queries after local hive build with patch