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

GH-36209: [Java] Upgrade Netty due to security vulnerability #36211

Merged
merged 2 commits into from Jun 21, 2023

Conversation

BryanCutler
Copy link
Member

@BryanCutler BryanCutler commented Jun 21, 2023

Rationale for this change

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

What changes are included in this PR?

Upgrading Netty, gRPC and Protobuf dependencies

Are these changes tested?

Existing tests

Are there any user-facing changes?

No

This PR contains a "Critical Fix".

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

@BryanCutler
Copy link
Member Author

@lidavidm this is a bit of a problem because Arrow memory needed to be patched to use the fixed version. Is there any discussion of doing a 12.0.2 release that could include this?

@lidavidm
Copy link
Member

I think 12.0.2 is unlikely, given the 13.0.0 code freeze is a few weeks away, but if you raise it on the ML we can see.

I think this can affect arrow flight, given gRPC uses Netty.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Jun 21, 2023
@lidavidm lidavidm merged commit ea4f03a into apache:main Jun 21, 2023
15 checks passed
@BryanCutler BryanCutler deleted the upgrade-netty-36209 branch June 21, 2023 19:59
@BryanCutler
Copy link
Member Author

Thanks @lidavidm !

@conbench-apache-arrow
Copy link

Conbench analyzed the 6 benchmark runs on commit ea4f03ac.

There were 3 benchmark results indicating a performance regression:

The full Conbench report has more details.

lriggs pushed a commit to lriggs/arrow that referenced this pull request Jun 28, 2023
…pache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jun 28, 2023
…pache#36211) (#27)

* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jun 28, 2023
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
@idelpivnitskiy
Copy link

What is ETA for 13.0.0 release?

@lidavidm
Copy link
Member

lidavidm commented Jul 6, 2023

The code freeze is proposed to be July 10th (https://lists.apache.org/thread/f9r0dsd65ohdtcvc7fnnlfs23n3z0n7f). It would then be one to several weeks to chase down release blockers, prepare binaries, and vote on the release, depending on if any last-minute issues are found.

Generally releases are conducted quarterly.

@idelpivnitskiy
Copy link

Thanks for response @lidavidm,
I understand your general release cadence, but would like to highlight that this incompatibility does not allow upgrading Netty version for anything else in the same classpath. It will be great for the project if Arrow can find a way to either backport it to older versions (maybe MethodHandlers with runtime Netty version check can help), or find a way to stop using internal Netty API completely (preferred approach).

In the meanwhile, are there any known downsides or issues for users switching from arrow-memory-netty to arrow-memory-unsafe? I couldn't find much information about side-effects. Assuming that they are interchangeable.

@lidavidm
Copy link
Member

lidavidm commented Jul 7, 2023

We would love to be more agile with releases. However, it takes quite a bit of maintainer effort. Help is welcome.

@danepitkin was exploring if reflection could help. If you have ideas on how exactly that could work, that may be useful. I'm not familiar with why the code is structured like this, other than performance (but I'm not aware of benchmarks for this).

arrow-memory-unsafe has not received as much usage, in my estimation, because netty was for a long time the only implementation and is still the 'default' in effect. Also, if your application also uses Netty, the unsafe implementation is not aware of memory allocated by Netty and vice versa (may have to tweak the JVM native memory limit).

@lidavidm
Copy link
Member

lidavidm commented Jul 7, 2023

I filed #36562 to investigate this in the future.

(Though you gave me an idea; maybe we could make InnerAllocator here an interface and dispatch between a naive version and an internals-using version at runtime; presumably the JIT could devirtualize the call over time.)

lriggs added a commit to dremio/arrow that referenced this pull request Jul 13, 2023
…pache#36211) (#27)

* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jul 13, 2023
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jul 21, 2023
…pache#36211) (#27)

* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jul 21, 2023
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jul 28, 2023
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to dremio/arrow that referenced this pull request Jul 28, 2023
…pache#36211) (#27)

* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
xxlaykxx pushed a commit to dremio/arrow that referenced this pull request Oct 29, 2023
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
DenisTarasyuk pushed a commit to dremio/arrow that referenced this pull request Dec 19, 2023
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
DenisTarasyuk pushed a commit to dremio/arrow that referenced this pull request Dec 19, 2023
…pache#36211) (#27)

* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
DenisTarasyuk pushed a commit to dremio/arrow that referenced this pull request Jan 16, 2024
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
DenisTarasyuk pushed a commit to dremio/arrow that referenced this pull request Mar 6, 2024
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to lriggs/arrow that referenced this pull request Mar 12, 2024
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
lriggs added a commit to lriggs/arrow that referenced this pull request Apr 25, 2024
* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211)

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

Upgrading Netty, gRPC and Protobuf dependencies

Existing tests

No

**This PR contains a "Critical Fix".**

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

* Closes: apache#36209

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Restore jackson version.

* Use local based ccache for Mac build instead of sscache.

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: Bryan Cutler <cutlerb@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Java] Upgrade Netty due to CVE
3 participants