fix(remote-debug): gate registration messages on completed handshake - #800
Open
SashaMIT wants to merge 1 commit into
Open
fix(remote-debug): gate registration messages on completed handshake#800SashaMIT wants to merge 1 commit into
SashaMIT wants to merge 1 commit into
Conversation
The remote-debugging TCP server authenticates clients with a handshake message carrying a per-tenant key, but onMessage never enforced it: ASSET_CHUNK, the declaration messages and END were all processed while runtime.handshake was still false. The only enforcement was a 10 second timer closing connections that never handshake, and a full registration completes well inside that window, so an unauthenticated client could write assets to media storage, consume connection slots and register arbitrary plugin identities in the node registry. Reject every registration message other than HAND_SHAKE until the handshake completes, close the connection with a handshake failure and latch handshakeFailed so later messages are ignored. The 10 second timer stays as-is.
dosubot
Bot
added
size:XS
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
1 participant
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.
The remote-debugging TCP server no longer processes plugin registration messages before the handshake completes.
What
The debugging server authenticates clients with a handshake message carrying a per-tenant key (122-bit UUID, 2h TTL), but
onMessagenever enforced it:ASSET_CHUNK, the declaration messages, andENDwere all processed whileruntime.handshakewas still false. The only enforcement was a 10-second timer inOnOpenthat closes connections which never handshake, and a full registration completes well inside that window.Why
An unauthenticated client with network reachability to the remote-installing port could, per connection: write up to 50MB of attacker-chosen asset data into media storage (repeatable across connections, a storage-exhaustion vector), occupy a connection slot against
maxConn(default 256, locking out legitimate developers), and register arbitrary attacker-chosen plugin identities in the node's runtime registry for the life of the connection.Honest scope limits, verified while testing: invocation hijack or shadowing an installed plugin is not possible through this path, because runtime identity includes a checksum over the declaration plus tenant id, and a handshake-less connection has no tenant id to collide with. The handshake key itself is not guessable. Port exposure is deployment-dependent; the feature is on by default.
How
One gate in
onMessagebefore the registration switch: any registration message other thanHAND_SHAKEarriving beforeruntime.handshakeis true is rejected, the connection is closed with ahandshake failederror (consistent with the existing invalid-handshake path), and the runtime is latchedhandshakeFailedso later messages are ignored. The 10-second timer stays as-is.How verified
internal/core/debugging_runtime/hooks_test.go:TestRegistrationRejectedBeforeHandshakeconnects over TCP and sends a manifest declaration, an endpoint declaration, an asset chunk, and END with no handshake. Each is rejected with ahandshake failedreply and a closed connection, and no runtime is ever registered.TestAcceptConnection(requires Redis and Postgres, runs in CI).go buildpasses for the touched package, and allinternal/core/debugging_runtimetests that do not require a local Postgres/Redis pass, including the new one.Same disclosure class as our merged #796: a missing check visible in the public source, fixed directly via public PR.
Made with Cursor