Opened 4 years ago

Closed 2 years ago

#17574 closed defect (wontfix)

Fallback mirrors should never fetch from fallback mirrors

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.4.7-alpha
Severity: Normal Keywords: tor-03-unspecified-201612
Cc: Actual Points:
Parent ID: #17709 Points: small
Reviewer: Sponsor:

Description

If we allow fallback mirrors to fallback to other fallback mirrors, we could get download loops or other nasty consequences. The bootstrap process should deliver a recent consensus and prevent this, but let's avoid the possibility - there's no need for the ~300 fallbacks to use mirrors.

While relays can check their own list of fallback mirrors, there's no way to predict which relays were/are fallbacks in past/future releases.

Therefore, any relays which could possibly become a fallback, must connect to an authority:

  • public servers (not a bridge)
  • with a dirport (not just an automatic dir cache with the V2Dir flag (#12538), but one with an actual, public, dirport that can be used for initial bootstrapping)

Currently, the authority connection code is advisory, we need to split it and make the above conditions mandatory.

This was introduced in 0.2.4.7-alpha as an unintended consequence of commits like 5c51b3f1f0d4c394392.

Child Tickets

Change History (18)

comment:1 Changed 4 years ago by teor

Status: newneeds_review

See my branch bug17574-no-fallback-loops on https://github.com/teor2345/tor.git

comment:2 Changed 4 years ago by teor

Status: needs_reviewneeds_revision

Revise to use the hard-coded fallback list rather than a heuristic (as discussed on IRC with nickm).

comment:3 Changed 4 years ago by teor

Status: needs_revisionneeds_review

Please see my branch bug17574-no-fallback-loops-v2.

This now depends on the fix to router_get_fallback_dirserver_by_digest in 0d5a439292a7.

comment:4 Changed 4 years ago by teor

(The router_get_fallback_dirserver_by_digest fix was in #17572.)

comment:5 Changed 4 years ago by teor

Parent ID: #15775#4483

I merged this into #4483 and will deal with it there, as both tickets modify the same code, and need a manual merge.

This ticket can close when #4483 closes.

comment:6 Changed 4 years ago by teor

This is implemented in my branch feature4483-v10 in commit "Prop210: Prevent fallback directory mirror loops".

Full details in #4483.

comment:7 Changed 4 years ago by teor

Please see feature4483-v11, the commit for this ticket is unchanged.

comment:8 Changed 4 years ago by teor

Parent ID: #4483
Status: needs_reviewneeds_information

Deferring this change - it's not clear it's necessary, and the current behaviour is acceptable:

  • fallbacks contact the authorities when bootstrapping, or other fallbacks if all authorities are inaccessible
  • fallbacks contact the authorities when updating their consensus, or other directory mirrors if all authorities are inaccessible

Let's make a decision on this before the first release with fallbacks, hopefully 0.2.8.

comment:9 Changed 4 years ago by teor

Cc: #17709 added

We might want to consider this is we proceed with #17709.

comment:10 Changed 4 years ago by teor

Cc: #17709 removed
Parent ID: #17709

comment:11 in reply to:  8 ; Changed 4 years ago by nickm

Replying to teor:

Deferring this change - it's not clear it's necessary, and the current behaviour is acceptable:

  • fallbacks contact the authorities when bootstrapping, or other fallbacks if all authorities are inaccessible
  • fallbacks contact the authorities when updating their consensus, or other directory mirrors if all authorities are inaccessible

(... or when looking for certs and (micro) descriptors that they don't currently have, right?)

comment:12 in reply to:  11 Changed 4 years ago by teor

Replying to nickm:

Replying to teor:

Deferring this change - it's not clear it's necessary, and the current behaviour is acceptable:

  • fallbacks contact the authorities when bootstrapping, or other fallbacks if all authorities are inaccessible
  • fallbacks contact the authorities when updating their consensus, or other directory mirrors if all authorities are inaccessible

(... or when looking for certs and (micro) descriptors that they don't currently have, right?)

It depends.

When fetching certs and microdescriptors, directory_get_from_dirserver() calls directory_fetches_from_authorities() to find out if it should contact an authority. (This happens regardless of whether tor has a valid consensus.)

directory_fetches_from_authorities() is a long list of special cases that would be easier to read if they had less ! and ||. But it leads to the following outcomes:

These tor instances will fetch from the authorities (or the mirrors in the consensus, or the fallbacks, in that order):

  • relays with recently uploaded descriptors that are:
    • directory mirrors, and/or
    • exits,
  • relays that don't know their address, and
  • clients with FetchDirInfoEarly set.

All other tor instances will fetch from the mirrors in the consensus (or the fallbacks, or the authorities, in that order):

  • clients,
  • bridges,
  • hidden services,
  • all relays during initial bootstrap, as long as they know their address,
  • non-directory, non-exit relays, and
  • bridge clients (or maybe they fetch from their bridge(s)).

comment:13 Changed 4 years ago by teor

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final

I don't think this is necessary.
In all the cases where it becomes an issue, the authorities are down.
So we'd focused on that, not this.

comment:14 Changed 3 years ago by nickm

Points: small

comment:15 Changed 3 years ago by isabela

Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

tickets market to be removed from milestone 029

comment:16 Changed 3 years ago by teor

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

Milestone renamed

comment:17 Changed 3 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:18 in reply to:  13 Changed 2 years ago by nickm

Resolution: wontfix
Status: needs_informationclosed

Replying to teor:

I don't think this is necessary.
In all the cases where it becomes an issue, the authorities are down.
So we'd focused on that, not this.

Okay; closing as wontfix.

Note: See TracTickets for help on using tickets.