[tor-bugs] #21425 [Core Tor/Tor]: entry_list_is_constrained() should look at the guard_selection_t object

[Tor Bug Tracker & Wiki]

#21425: entry_list_is_constrained() should look at the guard_selection_t object
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  (none)
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  guards, 031-deferred-20170425,       |  Actual Points:
  review-group-18                                |
Parent ID:  #20822                               |         Points:  .5
 Reviewer:  asn                                  |        Sponsor:
                                                 |  SponsorV-can
-------------------------------------------------+-------------------------

Comment (by teor):

 Replying to [comment:23 asn]:
 > Hello people,
 >
 > two comments on the patch so that I understand a bit better what's going
 on because it's been a while and I'm a bit confuse:
 >
 > 1) What are we trying to achieve with this new
 `entry_list_is_constrained()` logic? Can someone explain to me why we
 consider the entry list constrained if it contains more than 50 guards,
 and why this logic makes sense given the way `entry_list_is_constrained()`
 is used in the rest of the code? I think this is a very important question
 and ideally we should have a clear and concise answer :)

 We check guards more aggressively when entry list is constrained,
 There's no point checking guards more aggressively if we have a lot of
 potential guards.
 We chose 50, because we want to make sure Tor Browser's obfs4 guard set is
 considered "constrained".

 Neel, please explain why we chose 50 in the comments in the patch.

 Replying to [ticket:21425 nickm]:
 > We use entry_list_is_constrained() in a few places to find out if our
 list of entry points is highly limited (e.g., to a few bridges or a few
 EntryNodes).  But it doesn't do that very well:  instead, it looks to see
 if EntryNodes is set or UseBridges is set.
 >
 > We have better ways: we should be looking at the size of the guard
 sample, or something.

 > > 2. If #1 is okay, which capacity size should
 {{{sampled_entry_guards}}} be to return 1? Right now, I am guessing 3
 guards. Should it be more? Less?
 >
 > If we want to consider Tor Browser bridge users constrained, the answer
 is around 10 or 20.
 > If not, the answer is around 3.

 Replying to [comment:16 teor]:
 > There are 27 obfs4 bridges in Tor Browser at the moment:
 > https://gitweb.torproject.org/builders/tor-browser-
 build.git/tree/projects/tor-browser/Bundle-Data/PTConfigs/bridge_prefs.js
 > The other bridge types have 1-5 bridges.
 >
 > So let's set the limit to 50 or 100?

 > 2) Why are we using the `capacity` field of a smartlist? We should
 (almost) never dig into the guts of smartlists. If we want to check the
 current number of sampled guards we should use `smartlist_len()`. Is that
 we are trying to do here?

 I asked for this change already, but I forgot to check that it had
 happened:

 Replying to [comment:15 teor]:
 > Replying to [comment:14 neel]:
 > > 1. Is checking the capacity of {{{sampled_entry_guards}}} from the
 {{{guard_selection_t}}} object okay for this patch?
 >
 > Checking the current size of sampled entry guards is ok.
 > (The capacity of a smartlist is the size it can grow to without
 allocating additional memory.)

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