Repository navigation
tor: fix circuit/stream ID misparsed as a control-port status code - #5731
Open
corporategoth wants to merge 1 commit into
Open
corporategoth wants to merge 1 commit into
corporategoth wants to merge 1 commit into
Conversation
send_query() strips a leading 3-digit token from every line it reads, assuming it is always a Tor control-protocol status code (e.g. "250 OK", "250-foo", "250+bar"). Inside a "250+key=" multiline reply, the raw data lines are not prefixed with a status code at all, but circuit-status and stream-status data lines legitimately start with a numeric circuit or stream ID. Once a bridge/relay has been up long enough for its Tor process to allocate a 3-digit circuit ID (100-999 -- routine after a few hours of uptime), that ID is misidentified as a status code and stripped, and the now-missing ID makes get_circuit()'s `data.shift.to_i` shift off the literal word "BUILT" instead, which .to_i's to 0. Every circuit then collides on hash key 0, so the Diagnostics > Circuits/Streams page in the GUI shows only the single most-recently-processed circuit, no matter how many are actually open. Fix: track whether we are inside a "250+"-introduced multiline data block and skip the status-code detection entirely for lines read while inside it, matching the control-spec's actual framing (data lines in a multiline reply carry no status-code prefix). Verified against a live bridge showing 2 concurrent circuits that were previously collapsing to 1.
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.
Summary
The Diagnostics > Circuits / Streams page for the Tor plugin only ever shows one row, no matter how many circuits are actually open, once the bridge/relay has been running long enough to allocate a 3-digit circuit ID.
Root cause
TorCTL#send_queryintor_diagstrips a leading 3-digit token from every line it reads, on the assumption it's always a control-protocol status code (250 OK,250-foo,250+bar). But inside a250+key=multiline reply, the data lines are not prefixed with a status code at all — per Tor's control-spec, only the introducer and terminator carry codes.circuit-status/stream-statusdata lines legitimately start with a numeric circuit/stream ID, and once that ID reaches 100-999 (routine after a few hours of uptime) it is misidentified as a status code and stripped.Downstream,
TorCTL#get_circuit'sdata.shift.to_ithen shifts off the literal word"BUILT"instead of the (now-missing) ID, and"BUILT".to_iis0. Every circuit collides on hash key0, so only the last one processed survives in the JSON the GUI renders.Reproduced live on a bridge relay:
GETINFO circuit-statusgenuinely returning 2 circuits,tor_diag -c(and the GUI's/api/tor/service/circuits) reporting only 1.Fix
Track whether
send_queryis inside a250+-introduced data block and skip status-code detection entirely for lines read while inside it, matching the protocol's actual line framing. No change to the public data format returned to callers.Testing
ruby -csyntax checkconfigctl tor circuit(the actual code path the GUI's API controller uses) — both now correctly return both circuit IDs with accurate fields.configctl tor streamsstill returns valid (empty) JSON, unaffected by the change.