Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#2189 closed defect (fixed)

Control Protocol Spec Error, Grammars Use Outdated Production ServiceID

Reported by: Poet Owned by:
Priority: Low Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version: Tor: 0.2.2.17-alpha
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Problem

The grammar for circuit-status (https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l473) is not correct. Note that the grammar of Path (https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l1003) specifies a list of ServerIDs (https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l100), which are either a Nickname (https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l102) or Fingerprint (https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l104). However, as you can see below, I got a list of LongNames (https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l124) instead.

How to Reproduce

Start Tor with open control port

mil:~ tim$ tor --ControlPort 9051

Nov 15 !08:27:11.110 [notice] Tor v0.2.2.17-alpha (git-dadd9608d2720368)

Query for circuit-status

mil:~ tim$ nc 127.0.0.1 9051
AUTHENTICATE ""
250 OK
getinfo circuit-status
250+circuit-status=
5 BUILT $7EA6EAD6FD83083C538F44038BBFA077587DD755=dizum,$4765D3E659EED7368CE3E3320E6F19287BDBAACE=AIVD,$30CE3219D2B62474F94C299BB2D508DC856EBC1D=TorRox PURPOSE=GENERAL
4 BUILT $DF4DBB7C093E640F2C8DA91007A5BFB166DA2D04=Nihilopticon,$4765D3E659EED7368CE3E3320E6F19287BDBAACE=AIVD,$92ADC1C92B0CDFCAD18538EA59C28D02CA1899AD=feydakins PURPOSE=GENERAL
3 BUILT $BA0BE940337C8FD2AD0B97D74BB6EF3B4E992BED=WeAreAHedge,$52D8562CA21D40C0F8396F0952DBDA26C67339BB=PDQVPN1,$4D1B7FF1E49E249042451F91EF996DC34EDFCDEE~bazinga PURPOSE=GENERAL
.
250 OK

How To Fix

To fix this specific problem the grammer of circuit-status or Path needs to be updated. I haven't explored the specification in depth to see what effect this will have in other areas.

Child Tickets

Change History (21)

comment:1 Changed 10 years ago by Sebastian

See also the VERBOSE NAMES section https://gitweb.torproject.org/tor.git/blob/HEAD:/doc/spec/control-spec.txt#l801 which will also need updating. We should probably add a reference to that paragraph from the ServerID specification (and update the paragraph in question)

comment:2 Changed 10 years ago by nickm

Component: - Select a componentTor Client
Milestone: Tor: 0.2.2.x-final

Changed 10 years ago by Poet

Attachment: feature_section.diff added

Clarifications in USEFEATURE.

Changed 10 years ago by Poet

Attachment: general_use_tokens.diff added

Clarifications in General Use Tokens.

Changed 10 years ago by Poet

Corrections in GETINFO responses and events.

comment:3 Changed 10 years ago by Poet

Hi, I have just added 3 patches related to this issue.

(1) The a patch to the description of USEFEATURE. I noticed while fixing the original problem that the descriptions of features is not as clear as it could be. nickm and I discussed this briefly on IRC.

19:34 < poet> also I am wondering, once a feature is enabled by default is it impossible to turn off

19:35 < poet> through a configuration option that is (obviously you can recompile)

19:35 < nickm> I believe that is so. There is no DONT_USEFEATURE

!19:35 < nickm> Best practice is probably just to USEFEATURE both the known features always.

!19:35 < poet> Ok

!19:36 < nickm> Versions of Tor that do not have them are absolete and nonworking, and we haven't introduced a new one since 0.1.2

!19:36 < poet> the feature section of the control spec uses both "always-on" and "on by default", so I figured it could be clearer 

!19:36 < poet> ah

!19:37 < nickm> "always on" is probably the better phrase

!19:37 < poet> ok

(2) A patch to general use tokens to make it more clear that ServerID is no longer in use.

(3) The actual fixes discussed in the original bug report. Changes to the grammars of events and GETINFO responses in order to reflect the fact that VERBOSE_NAMING is a part of the protocol now.

I appreciate any comments and criticisms!

comment:4 Changed 10 years ago by Poet

Summary: Control Protocol Specification Error -- circuit-status grammar incorrect.Control Protocol Spec Error, Grammars Use Outdated Production ServiceID

comment:5 Changed 10 years ago by Poet

Status: newneeds_review

I have uploaded patches to incorporate suggestions given by Sebastian and I set this to needs_review. Please consult the most recent patches, as we seem to be unable to remove the old files.

comment:6 Changed 10 years ago by Sebastian

Poet made a branch available at github for this so we can stop attaching patches here. Repository lives at https://github.com/poet/tor.git, branch master. Looks good to me (reviewed up until 2d309663249598b)

comment:7 Changed 10 years ago by nickm

Comments:

  • It's not correct to say "Before 0.1.2.2-alpha each line is..."; instead you probably mean "In versions before 0.2.2.1-alpha, if VERBOSE_NAMES is not enabled, each line is..." In other words, make it clear that you are describing not only versions before 0.1.2.2-alpha, but also versoins between 0.1.2.2-alpha and 0.2.2.1-alpha when VERBOSE_NAMES is not set.


Otherwise, it looks fine to me. One thing, though: Your branch is against master. Did we want this to go into 0.2.2?

comment:8 Changed 10 years ago by Sebastian

Either is fine imo. People who read the spec should read the latest version always anyways, I think. In either case, rebasing onto maint works without a problem.

comment:9 Changed 10 years ago by Poet

Committed nickm's corrections here: https://github.com/poet/tor/commit/9e2410d025d628d5c886eec876315e13b140b50c.

comment:10 Changed 10 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Rebased onto maint-0.2.2, merged there and onto master. Thanks!

comment:11 Changed 8 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.