Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Netty HTTP codec module included in statsd JAR #2929

Closed
shakuzen opened this issue Dec 22, 2021 · 5 comments
Closed

Netty HTTP codec module included in statsd JAR #2929

shakuzen opened this issue Dec 22, 2021 · 5 comments
Labels
bug A general bug registry: statsd A StatsD Registry related issue regression A regression from a previous release
Milestone

Comments

@shakuzen
Copy link
Member

Did #2588 undo excluding netty http codec modules, and, if so, was that intentional?

I updated micrometer-registry-statsd from 1.7.6 to 1.8.1 and noticed that io/micrometer/shaded/io/netty/handler/codec/http is being included in the JAR file. Previously, I was able to write off CVEs like CVE-2021-43797 as a false positive because the http modules weren't being included, but that's not possible for me to justify anymore if I update to micrometer-registry-statsd >= 1.8.0.

Originally posted by @stevefranchak in #2531 (comment)

@shakuzen shakuzen added this to the 1.8.x milestone Dec 22, 2021
@shakuzen shakuzen added registry: statsd A StatsD Registry related issue regression A regression from a previous release labels Dec 22, 2021
@shakuzen
Copy link
Member Author

Thanks for the report @stevefranchak. It wasn't intentionally undone. From 1.0.x reactor-netty splits its modules into -http and -core where the core module isn't supposed to have HTTP-related dependencies. It looks like the netty-codec-http dependency is coming transitively through netty-handler-proxy, which reactor-netty-core depends on. I'm not sure if reactor-netty-core needs netty-codec-http or not. I'll see if we can explicitly exclude it in the meantime.

@violetagg do you know if it is intentional that reactor-netty-core has a transitive dependency on netty-codec-http?

@shakuzen
Copy link
Member Author

Previously, I was able to write off CVEs like CVE-2021-43797 as a false positive because the http modules weren't being included, but that's not possible for me to justify anymore if I update to micrometer-registry-statsd >= 1.8.0.

As for the CVE, it's still a false positive whether we fix this or not. The statsd module does not use HTTP, whether the classes are there or not.

@violetagg
Copy link
Contributor

Thanks for the report @stevefranchak. It wasn't intentionally undone. From 1.0.x reactor-netty splits its modules into -http and -core where the core module isn't supposed to have HTTP-related dependencies. It looks like the netty-codec-http dependency is coming transitively through netty-handler-proxy, which reactor-netty-core depends on. I'm not sure if reactor-netty-core needs netty-codec-http or not. I'll see if we can explicitly exclude it in the meantime.

@violetagg do you know if it is intentional that reactor-netty-core has a transitive dependency on netty-codec-http?

Please create an issue in Reactor Netty

@shakuzen
Copy link
Member Author

We have manually excluded the netty-codec-http module from inclusion in our shading, which restores the behavior we had before. In a future release of reactor-netty (2.0 it sounds like) we may not need to manually exclude this.

@stevefranchak Let us know if things look alright with 1.8.2 which we just released.

@stevefranchak
Copy link

@shakuzen, I pulled down micrometer-registry-statsd 1.8.2 and our vuln scanner is happy. I also verified that netty-codec-http is not contained in the shaded library. Thanks for working on this! It's very much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug registry: statsd A StatsD Registry related issue regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

3 participants