Opened 3 years ago

Last modified 6 weeks ago

#19009 assigned defect

bandwidth testing circuits should be allowed to use our guards

Reported by: arma Owned by: juga
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-bandwidth, torflow, measurement, metrics, tor-bwauth, needs-proposal
Cc: Actual Points:
Parent ID: #22453 Points:
Reviewer: Sponsor:

Description

In git commit 267e61d0, we fixed bug #654, where relays were putting all their bandwidth testing circuits over the same small set of guards, thus having an accidental bottleneck, meaning we got an inaccurate bandwidth estimate.

But we fixed it by having them *never* use a guard for their first hop of a testing circuit, which in turn produced surprising behavior in tiny test networks, because relays can't make testing circuits if all the available relays are in their guard list.

teor fixed that in commit 22a1e9cac by making us not avoid our guards if testingtornetwork, and not avoid our guards if all the nodes in the consensus are on our guard list. It turns out that latter check isn't quite good enough, because we're picking two hops, so having at least one relay in the network that isn't in our guard list isn't enough to complete a circuit.

The underlying problem is that when UseEntryGuards is true, the rest of choose_good_entry_server() is entirely about picking a new entry guard. Except we reuse it for some edge cases where we want to just pick some entry point and not use our guard list. Some refactoring or something seems wise.

Child Tickets

Change History (12)

comment:1 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:2 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:3 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:4 Changed 2 years ago by nickm

Keywords: tor-bandwidth torflow measurement metrics added

I hope this is eventually obsoleted by torflow or some successor thereof.

comment:5 Changed 13 months ago by juga

Cc: juga added

comment:6 Changed 13 months ago by juga

Parent ID: 25925

comment:7 Changed 13 months ago by juga

Parent ID: 25925#25925

comment:8 Changed 12 months ago by juga

teor fixed that in commit 22a1e9cac by making us not avoid our guards if testingtornetwork, and not avoid our guards if all the nodes in the consensus are on our guard list. It turns out that latter check isn't quite good enough, because we're picking two hops, so having at least one relay in the network that isn't in our guard list isn't enough to complete a circuit.

It is not anymore the case where the code "avoid our guards if all the nodes in the consensus are on our guard list" [0]

The 2nd hop can be in the guard list right?

The 2nd hop seems to be picked by choose_good_middle_server, called by choose_good_entry_server

I can find the code where 2 hops are being picked.

Running chutney, circuits build 3 hops. There's also no test for this.

I hope this is eventually obsoleted by torflow or some successor thereof.

hmm, i think this does not have to do with a bandwidth scanner, but with Tor self-test.

[0] https://gitweb.torproject.org/tor.git/tree/src/or/circuitbuild.c#n2692

Last edited 12 months ago by juga (previous) (diff)

comment:9 Changed 12 months ago by juga

Cc: juga removed
Keywords: tor-bwauth added
Owner: set to juga
Status: newassigned

comment:10 Changed 12 months ago by pastly

This seems to be all about bandwidth self tests (see router_perform_bandwidth_test in src/or/router.c for where the self test is performed).

Regarding sbws: it currently uses consensus weights (unfortunately) and will soon use relays' RelayBandwidthRate/MaxAdvertisedBandwidth (AKA: whatever ends up as the first number on the bandwidth line in server descriptors). It doesn't care about self test results.

comment:11 Changed 6 months ago by teor

Parent ID: #25925#22453

We might remove the bandwidth self-test entirely.

comment:12 Changed 6 weeks ago by teor

Keywords: needs-proposal added

These tickets need a proposal before we write any code.

The two competing proposals in #22453 are:

  1. Remove the bandwidth self-test
  2. Increase the bandwidth self-test so it measures a reasonable amount of bandwidth
Note: See TracTickets for help on using tickets.