Re: [tor-bugs] #32165 [Core Tor/Tor]: On first boot, Tor mistakenly tells me "The current consensus has no exit nodes"

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#32165: On first boot, Tor mistakenly tells me "The current consensus has no exit
nodes"
--------------------------+------------------------------------
 Reporter:  arma          |          Owner:  (none)
     Type:  defect        |         Status:  needs_revision
 Priority:  Medium        |      Milestone:  Tor: 0.4.2.x-final
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:  bootstrap     |  Actual Points:
Parent ID:                |         Points:
 Reviewer:  ahf           |        Sponsor:
--------------------------+------------------------------------

Comment (by teor):

 Replying to [comment:10 arma]:
 > Replying to [comment:9 teor]:
 > > We've switched this check a few times, and it breaks different things
 each time.
 > > So we need to be very careful about this change.
 > > I'm not sure if it is correct.
 > > Please check the history of changes to this line.
 >
 > Ok, there have been three changes to this line:
 >
 > The original commit, 9b2d106e4, which documented np and nu clearly (yay)
 but got their meanings reversed (oops).
 >
 > Nick's fix in #14918, which went in as commit 8eb3d81e6: it clarified
 the function comment in count_usable_descriptors (yay), fixed np to nu as
 it should, but left the (still wrong) comments explaining np and nu in
 place (oops).
 >
 > teor's update for #27236, which went in as commit 3ebbc1c84, which
 reintroduced the np vs nu bug.
 >
 > I just checked num_present vs num_usable again, and I am confident that
 the fix in 94cb4f8 (which was the same fix as 8eb3d81e6) is right:
 num_usable is how many are in the consensus that we would use, and
 num_present is how many of those we actually have descriptors for right
 now. So on first boot, when we have no descriptors present, we must check
 nu, not np, or we will wrongly conclude that we have a consensus with no
 exits.

 It's not just first boot, it's also "if tor is launched after most
 descriptors have expired".

 Whatever change you make, we need all the chutney tests in CI to pass. And
 we also need to avoid breaking Tor Browser.

 I think there were some weird complexities here last time. But let's have
 a go :-)

 > Now, #27236 speaks of internal-only onion service networks and chutney.
 But I don't see how commit 3ebbc1c8 does anything about those -- it
 essentially just reverts Nick's fix. I'm guessing it has something to do
 with the nearby commit 588c7767. It looks like that commit is trying to
 make sure that at least one relay in the consensus can exit to at least
 one port. Makes sense -- but it appears to accidentally also be checking
 that *we have a local descriptor* for such a relay. Can you help me
 understand what exactly we need to check for, in the "internal-only onion
 service networks and chutney" case? We shouldn't merge this new patch
 until we understand the chutney requirements better.

 Chutney runs some networks without exits, to make sure we are testing
 onion service circuits. (And to speed up the tests, by removing redundant
 relays.) Chutney doesn't care much about the difference between the exit
 flag and exit policy, because those networks don't have any of either.

 However, some tor code does care about the difference. Because it's trying
 to predict if any relays with exit policies will appear, once it downloads
 descriptors. Maybe it shouldn't care so much. Maybe it needs to care just
 enough to tell the user what is going on.

 > > We also need to change variable names, log messages, and
 documentation, see:
 > > https://trac.torproject.org/projects/tor/ticket/27448#comment:3
 >
 > Yes, I agree. I'll give this a go if you like my proposed name changes
 above: "present" -> "ready" and "usable" -> "listed". (I figure going
 through and doing it for the wrong names isn't a good use of anybody's
 time.)

 We want to avoid confusion. So let's call the variables what they *are*,
 not how we are using them. So we should end up using at least two names
 like:
 * n_exit_flag
 * n_exit_policy
 * n_exit_flag_and_policy

 (I can't easily work out what present, ready, usable, or listed mean. And
 that's part of the problem here.)

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

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs