Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#18624 closed enhancement (implemented)

Dir auths should only give Guard if Stable

Reported by: arma Owned by: arma
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: TorCoreTeam201604, tor-guards-revamp
Cc: Actual Points:
Parent ID: Points: small
Reviewer: Sponsor: SponsorU-can

Description

$ grep "^s " cached-consensus |grep Guard|wc -l
2074
$ grep "^s " cached-consensus |grep Guard|grep -v Stable|wc -l
18
$ grep "^s " cached-consensus |grep Guard|grep -v Fast|wc -l
0

So right now, out of the 2074 Guards that we have, every one of them has the Fast flag, and all but 18 of them have the Stable flag.

At the same time, we have some complicated logic in choose_random_entry_impl() and populate_live_entry_guards() that looks at need_uptime (aka Stable) and need_capacity (aka Fast), including ugly code like

    if (!node && need_uptime) {
      need_uptime = 0; /* try without that requirement */
      goto retry;
    }
    if (!node && need_capacity) {
      /* still no? last attempt, try without requiring capacity */
      need_capacity = 0;
      goto retry;
    }

But worst of all, it produces this weird edge case for the small fraction of unlucky clients who picked a non-Stable Guard, since when they build circuits that require the Stable flag, they will always use their second Guard for those circuits.

In short, this is complexity that is cheap to get rid of. Let's do it.

Child Tickets

Change History (10)

comment:1 Changed 3 years ago by arma

Milestone: Tor: 0.2.9.x-final

I'm moving this into the 0.2.9 milestone, unilaterally, even though yesterday's meeting said we shouldn't just do that, because I don't know what the convention is instead. Please somebody do something smart with this ticket. :)

comment:2 Changed 3 years ago by arma

Summary: Dir auths should only give Guard if Stable and FastDir auths should only give Guard if Stable

Correction: it appears we already require the Fast flag. So this is now only about Stable.

comment:3 Changed 3 years ago by nickm

Points: small
Sponsor: SponsorU-can

comment:4 Changed 3 years ago by arma

Status: newneeds_review

My feature18624 branch implements this (one line) change.

comment:5 Changed 3 years ago by arma

For spelunking fun, it was back in commit 8fa70711 (October 2007) when we changed is_possible_guard to look at the WFU -- before that it looked at is_stable, which is what I'm adding back in with this patch.

And even back in 2007, is_possible_guard required is_fast.

comment:6 Changed 3 years ago by nickm

Keywords: TorCoreTeam201604 added

Every postponed needs_review ticket should get a review in April

comment:7 Changed 3 years ago by nickm

Owner: set to arma
Status: needs_reviewassigned

setting owner

comment:8 Changed 3 years ago by nickm

Status: assignedneeds_review

comment:9 Changed 3 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

merged to master!

comment:10 Changed 3 years ago by nickm

Keywords: tor-guards-revamp added
Note: See TracTickets for help on using tickets.