expand getinfo circuit-status to show build_state flags

We've got a usability bug where users are misinterpreting the circuit listing on Vidalia's network map. In #2114 (closed) I suggest that Vidalia should start giving the user more information.

But Tor really doesn't export enough information for Vidalia to know what to say. Right now we export a PURPOSE= argument, but the string we send isn't very useful. Also, we don't export some flags, like onehup_tunnel, which Vidalia needs to know to tell the user that the circuit in question is probably for fetching directory information.

We should come up with a spec change and implement it. Shouldn't be too hard.

changed milestone to %Tor: 0.2.3.x-final

added component::core tor/tor milestone::Tor: 0.2.3.x-final owner::rransom parent::2114 priority::high resolution::implemented status::closed tor-client type::enhancement labels

Trac:
Milestone: N/A to Tor: 0.2.3.x-final
Points: N/A to N/A
Actualpoints: N/A to N/A

To solve #1090 (moved) more fully, we'll want Vidalia to be able to tell the user what types of circuits she's looking at. We can't do that until we close this bug.

Trac:
Priority: normal to major

Trac:
Owner: N/A to rransom
Type: defect to enhancement
Status: new to accepted

We should also expose the onehop_tunnel flag, the is_internal flag, and maybe expand the array of states that we export. Anything else? We should be specific.

(err, by "expand the array of states", I mean, "list more useful purposes if we are squashing purposes")

See bug2411a ( git://git.torproject.org/rransom/torspec.git bug2411a ) for documentation for the existing PURPOSE field of CIRC events, and some other minor fixups.

For both our hidden service performance-measurement tools and an improved circuit list in Vidalia, we will need to expose more information about circuit states. For the HS performance-measurement tools, we will also need to expose some new event types; I asked on the tor-dev mailing list whether we could add new event types to the CIRC event, but no one answered. I could assume that that silence means that no non-toy controllers would break, but it's more likely that the controller developers/maintainers aren't paying attention to the tor-dev list (or will claim to have not been paying attention if I make that change).

Trac:
Status: accepted to needs_review

Replying to rransom:

See bug2411a ( git://git.torproject.org/rransom/torspec.git bug2411a ) for documentation for the existing PURPOSE field of CIRC events, and some other minor fixups.

I just force-updated this branch to remove commit 5a4db0706ccd0211e6567368af1d5817c7792dec (Warn that CIRC, STREAM, ORCONN, BW, and STATUS_* ‘extended’ events aren't), because the specification of extended events in section 4.1 (a) is not actually a valid specification of anything, although it looked like one until I tried to figure out what it actually meant, and (b) isn't actually used anywhere (or rather, the extra-lines part isn't used; someone tried (and mostly failed) to specify extra arguments on single-line events there as well).

See feature2411-v2 ( git://git.torproject.org/rransom/tor.git feature2411-v2 ) for a BUILDFLAGS field in CIRC events and GETINFO circuit-status. This branch is not a complete solution for this ticket; it still needs documentation in a torspec patch, and I need several other pieces of information in CIRC events. The ticket is in ‘needs_review’ status for the bug2411a torspec branch [comment:6 above].

Replying to rransom:

See feature2411-v2 ( git://git.torproject.org/rransom/tor.git feature2411-v2 ) for a BUILDFLAGS field in CIRC events and GETINFO circuit-status. This branch is not a complete solution for this ticket; it still needs documentation in a torspec patch, and I need several other pieces of information in CIRC events. The ticket is in ‘needs_review’ status for the bug2411a torspec branch [comment:6 above].

I'm done with the code changes for this ticket now. I need to add new circuit-related control-port events to Tor, but they don't belong here.

See feature2411-v3 ( git://git.torproject.org/rransom/tor.git feature2411-v3 ) for a pre-squashed version of feature2411-v2; -v3 will be easier to review than -v2 if you haven't read previous commits on -v2.

My bug2411a branch now contains documentation for the new fields provided by my feature2411-v2 and feature2411-v3 branches.

Noting here so that it doesn't get lost:

The patch looks good. Before we merge it, though, there's a problem with controllers: at least pytorctl has the problem that if we insert new fields in CIRC events before the reason fields, it won't understand REASON fields any more.

Our options are:

• Fix pytorctl, give everybody plenty time to upgrade, then merge this.
• Change this patch so that the new CIRC fields come last, then merge it. In parallel, fix pytorctl so we don't have to do this nonsense in the future.

I favor the second; rransom (I believe) favors the first.

Replying to nickm:

Noting here so that it doesn't get lost:

The patch looks good. Before we merge it, though, there's a problem with controllers: at least pytorctl has the problem that if we insert new fields in CIRC events before the reason fields, it won't understand REASON fields any more.

Our options are:

• Fix pytorctl, give everybody plenty time to upgrade, then merge this.
• Change this patch so that the new CIRC fields come last, then merge it. In parallel, fix pytorctl so we don't have to do this nonsense in the future.

I favor the second; rransom (I believe) favors the first.

Some of the reasons I don't want to change this patch are:

• Rearranging the code to put event-specific flags for FAILED and CLOSED events in the middle of the keywords describing a circuit's current state would significantly uglify Tor's code.
• The PURPOSE event extension was added after the REASON and REMOTE_REASON fields were, so the spec has previously been interpreted as allowing new event extensions to be placed anywhere after the non-extended event. PyTorCtl is thus quite broken.

See bug2411a-v2 ( git://git.torproject.org/rransom/torspec.git bug2411a-v2 ) for a pre-squashed version of my bug2411a torspec branch.

Replying to rransom:

• Rearranging the code to put event-specific flags for FAILED and CLOSED events in the middle of the keywords describing a circuit's current state would significantly uglify Tor's code.

Not greatly IMO. I'd just have the function that generates a state description take an optional reason_flags argument, and make an XXX note that those events should get moved to the end later.

• The PURPOSE event extension was added after the REASON and REMOTE_REASON fields were, so the spec has previously been interpreted as allowing new event extensions to be placed anywhere after the non-extended event. PyTorCtl is thus quite broken.

Like I said on IRC, I don't believe that the spec was "interpreted" on that point when I added PURPOSE back in 2008: I think I just ignored the possibility that stuff would break based on a tighter/looser reading of the spec, and nobody caught it in time. In fact, I think that stuff might have broken when I added it, which wasted people's time.

Regardless of whether we think that that pytorctl's breakage is conformant or nonconformant, the fact remains that this change will break pytorctl as-is, which means that we need one of the two options suggested above. I'd rather take the "fix in parallel" approach so we don't need to delay this feature, but if you're okay with holding off on merging it until pytorctl and its users won't break, I can live with that.

Change to address this for pytorctl: https://trac.torproject.org/projects/tor/ticket/3679

FYI: The TorCtl changes seem to be breaking the bw auths mysteriously (#4015 (moved))

Note: I'm planning to merge this some time in December; 6 months is long enough for tools to be expected to become compliant.

See feature2411-v4 ( https://git.torproject.org/rransom/tor.git feature2411-v4 ) for a branch rebased onto current master.

Merging as threatened^Wpromised. Let's hope everything works, and fix whatever doesn't.

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

Trac:
Keywords: N/A deleted, tor-client added

Trac:
Component: Tor Client to Tor

closed

mentioned in issue #3457 (moved)

mentioned in issue #3457 (moved)

mentioned in issue #3457 (moved)

mentioned in issue #3457 (moved)

mentioned in issue #3627 (moved)

mentioned in issue #3632 (moved)

mentioned in issue #3679 (moved)

mentioned in issue #3680 (moved)

mentioned in issue #4003 (moved)

mentioned in issue #4892 (moved)

moved to tpo/core/tor#2411 (closed)

mentioned in issue tpo/core/tor#4892 (closed)