fix: limit incoming gRPC connections - #19
Open
GuipaiQigong111 wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maxConnections = 512,maxConnectionsPerIp = 32)Security rationale
The existing HTTP/2 stream cap limits concurrent streams on one connection, but it does not bound an attacker opening many connections and a small number of streams on each. That pattern can consume the process file-descriptor budget when
ulimit -nis low.This change adds application-level admission control as defense in depth:
Configuration
Both values must be non-negative, and the per-IP limit must not exceed the global limit. A value of
0selects the secure default rather than disabling the protection. Operators behind a reverse proxy should enforce the client-IP limit at the proxy, because java-tron sees the proxy source address unless the original source address is preserved at the network layer.Validation
Passed:
:common:compileJava,:framework:compileJava, and:framework:compileTestJavaNodeConfigTest, including defaults, zero fallback, invalid values, and explicit overridesGrpcConnectionLimiterTest, including global/per-IP limits, idempotent release, replacement admission, and 64-way concurrent admission against a cap of 8GrpcNettyMaxConcurrentStreamsLimiterTestRpcServiceHttp2SecurityTest, including real TCP/HTTP2 connections, immediate rejection, permit release, replacement admission, and one shared limit across two live RPC serversArgsTestJsonFormatTest, including strict-JSON round trips for chain-controlled name/URL control characters, backslash/quote escaping, and invalid UTF-8 fallback:framework:checkstyleMainand:framework:checkstyleTestgit diff --checkA full
:common:test :framework:testsweep was also started. It reached the large RocksDB/Spring integration-test section, but the host temporary volume became full and RocksDB began failing withNo space left on device, so the sweep was stopped. Before disk exhaustion, the failures reported were in unchanged Jetty/VM tests (SizeLimitHandlerTest,AllowTvmLondonTest, andValidateMultiSignContractTest); none of the changed gRPC/configuration tests failed.An expanded
org.tron.core.services.http.*sweep was attempted after the JSON review fix. It passed the serializer tests and many servlet tests, but the same nearly-full host volume eventually prevented Gradle and RocksDB from writing test results. The focusedJsonFormatTestsuite was then rerun independently and passed in full.Review follow-up: HTTP JSON byte-field escaping
The self-type HTTP formatter previously escaped only double quotes for name-like bytes fields and then validated the result with a deliberately permissive JSON parser. Raw control characters could therefore reach HTTP JSON responses, while invalid UTF-8 was decoded with the platform default replacement behavior.
The follow-up commit now:
Scope and operational note
This is application-level connection admission, not a replacement for OS and edge controls. A TCP socket is necessarily accepted before the child channel can be rejected, so connection-rate floods should still be handled with an appropriate
ulimit, firewall/load-balancer connection limits, backlog tuning, and monitoring.