[tor-bugs] #20509 [Core Tor/Tor]: Directory authorities should take away Guard flag from relays with #20499 bug

Tor Bug Tracker & Wiki blackhole at torproject.org
Wed Nov 9 12:01:57 UTC 2016


#20509: Directory authorities should take away Guard flag from relays with #20499
bug
-------------------------------------------------+-------------------------
 Reporter:  arma                                 |          Owner:
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.2.9.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.2.9.1-alpha
 Severity:  Normal                               |     Resolution:
 Keywords:  028-backport, easy,                  |  Actual Points:
  TorCoreTeam201611                              |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by teor):

 Replying to [comment:15 arma]:
 > (For those who are curious, check out git commit {{{cd678ae790}}} from
 #13152 for the last time we had code like this.)
 >
 > Hm -- {{{tor_version_compare}}} ends in comparing the git_tag_len entry
 for each tor_version_t. Does that mean that in reality, relays running
 0.3.0.0-alpha-dev won't match the version string from 0.3.0.0-alpha-dev,
 because they will have a git tag? If so, maybe we want some hack like "not
 as new as 0.3.0.1-alpha, but in the same series as it"? After more
 digging, I think the answer is "no, this current code is fine", since
 relay descriptors don't have the git tag in their platform string. But I
 figured I would leave this paragraph here for the next person who wonders
 it.

 The unit tests also verify your logic here.

 > Here's a tiny detail that could be improved:
 > {{{
 > +  if (!tor_version_as_new_as(platform, "Tor 0.2.9.1-alpha-dev") ||
 > +      (tor_version_as_new_as(platform, "Tor 0.2.9.5-alpha") &&
 > +      tor_version_compare(&parsed_platform, &parsed_0300_alpha_dev) !=
 0))
 > }}}
 >
 > That third line should be indented one column over -- I read it the
 first several times as being "if x or y, and z", and then I saw the open-
 paren on line two, and had to puzzle through that actually it means "if x,
 or y and z". Having the z line up with the y will help with readability.
 Assuming I am in fact reading it correctly now?

 I agree this compound statement is confusing, with the combination of `!,
 &&, ||, (, )`.
 How about we split it into multiple statements:
 {{{

 if (exactly 0.3.0-alpha-dev) {
   return 1;
 }

 if (older than 0.2.9.1-alpha-dev) {
   return 0;
 }

 if (as new as 0.2.9.5-alpha-dev) {
   return 0;
 }

 return 1;
 }}}

 >
 > Overall, I think it looks good! Is the plan that we merge it into
 0.2.9.x, and merge forward to 0.3.0.0,

 Are we sure 0.2.9.5-alpha and 0.3.0.1-alpha fix the stale consensus issue?
 I guess we can always change the version range in a later commit.
 (Maybe we should do it like cd678ae790, and use variables for all the
 versions?)

 > and moria1 tries it out,

 That's one way to make sure it works

 > and maybe also somebody tries it in a chutney network

 This should happen before merge, at least to the point we know the code
 runs and doesn't blow up.

 The mixed target would be ideal for testing the actual flag assignments -
 make tor-stable a bad version, and it should fail. Make it a good version,
 and it should succeed. But perhaps the failures would only show up if we
 turned off `TestingDirAuthVoteGuard *`.

 > and/or in teor and dgoulet's separate Tor network

 We can do this, but I think we'd end up with two guards in the network,
 because it's almost all on master. I guess that's when we tell people to
 (up)grade to 0.2.9.5-alpha to keep the guard flag. (Since there's no
 0.3.0.1-alpha yet.)

 > and hopefully we find and fix bugs in it by the time we want to backport
 it to a new 0.2.8 release?

 We want to do this backport to make sure the authorities get the patch
 before 0.2.9 goes stable?
 I think that's important enough for a backport.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20509#comment:17>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list