Skip to content

fix(net): gate peer traffic on hello validation and enforce timeout - #6993

Open
317787106 wants to merge 9 commits into
tronprotocol:release_v4.8.3from
317787106:fix/hello_check
Open

317787106 wants to merge 9 commits into
tronprotocol:release_v4.8.3from
317787106:fix/hello_check

Conversation

@317787106

@317787106 317787106 commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Require a validated application-layer HELLO before processing peer traffic, enforce a handshake deadline, and validate untrusted HELLO fields before formatting logs.

  • Before HELLO validation succeeds, allow only HELLO and DISCONNECT. Reject other message types with BAD_PROTOCOL before parsing or PBFT dispatch, and reject null or empty frames safely.
  • Add a 10-second HELLO timeout based on the local channel start time, independent of synchronization flags or recent peer activity. Use the existing status-check cycle and TIME_OUT disconnect, while retaining synchronization and request timeout checks. Make helloMessageReceive volatile for visibility to the status-check thread.
  • Extract validEndPoint() to validate the raw protobuf endpoint: port 1–65535, at least one IP address, a 200-byte limit per address field, and valid IPv4/IPv6 literals for every nonempty field. Run validation before toString() constructs a Node; preserve the log format for valid HELLO messages.
  • For invalid HELLO messages, log the three block-hash lengths and an endpointValid flag instead of full hashes. Read diagnostic lengths with ByteString.size() to avoid copying untrusted fields for logging.

Why are these changes required?

Peers could reach business handlers, including PBFT, before completing the application handshake, while handshake timeout depended indirectly on synchronization state. Malformed endpoint fields could also reach address formatting before validation, and oversized hashes could produce excessive warning output.

Handshake admission retains the existing version, genesis, and solid-block compatibility checks without adding a main-chain membership requirement for the peer's unfinalized head. This avoids rejecting otherwise compatible peers during a temporary fork, including reconnection after a disconnect or restart. Subsequent fork handling remains with the existing synchronization and broadcast logic.

This PR has been tested by:

  • Focused unit tests and regressions on ARM64 / JDK 17 covering message gating, endpoint validation, bounded logging, handshake compatibility, peer connections, and timeouts.
  • Logging scenarios consolidated into HandShakeServiceTest; HELLO timeout scenarios consolidated into PeerStatusCheckMockTest. The latest timeout consolidation passed 24 related tests.
  • HelloMessageTest.testValidAddressLogFormat verified on an actual Java 8 runtime after fixing its JDK-dependent IPv6 formatting expectation.
  • ./gradlew checkstyleMain checkstyleTest and git diff --check passed locally.

The full repository test suite and manual multi-node network testing were not run locally.

Follow up

The planned libp2p v2.3.0 fixes will be integrated into java-tron separately. This PR covers java-tron's application-layer HELLO handling and does not update the libp2p dependency.

Extra details

  • Ten seconds is the timeout threshold; disconnection occurs on the existing periodic status-check cycle.
  • No changes to the wire format, configuration, or fork-choice logic are introduced. The implementation remains Java 8 compatible.

@github-actions
github-actions Bot requested a review from xxo1shine September 22, 2026 08:57
Comment thread framework/src/main/java/org/tron/core/net/service/handshake/HandshakeService.java Outdated
Comment thread framework/src/main/java/org/tron/core/net/service/handshake/HandshakeService.java Outdated
Comment thread framework/src/main/java/org/tron/core/net/message/handshake/HelloMessage.java Outdated

@3for 3for left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM now.

@tronprotocol tronprotocol deleted a comment from nak23fr7 Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants