#27402 closed enhancement (implemented)

stop reporting "internal paths" during bootstrap

Reported by: catalyst Owned by: catalyst
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: s8-bootstrap, tor-spec, 035-deferred-20180930
Cc: Actual Points: 0.1
Parent ID: #28018 Points: 0.1
Reviewer: nickm Sponsor: Sponsor8-can

Description

bootstrap_status_to_string() can report "internal paths" when the consensus actually contains exits, because it's conditional on router_have_consensus_path() which checks descriptors. In the cold cache case, there are likely no existing descriptors, so tor will give the misleading impression that the consensus lacks exits (when it's actually more likely that it simply hasn't gotten around to downloading enough descriptors yet).

Removing the code that handles these conditions also simplifies bootstrap_status_to_string().

This probably needs a tor-spec update.

Child Tickets

Change History (16)

comment:1 Changed 15 months ago by teor

Keywords: 034-backport 034-must added
Owner: set to teor
Status: newassigned

This issue is caused (or made much worse) by #27236, which makes the chutney bridge and onion service networks work again after changes in 0.3.4.

If we remove the relevant code, then most of the chutney networks will stop working.

Instead, let's fix the log message so it correctly reports that:

  • there are exits in the consensus, but they don't have descriptors, or
  • there are no exits in the consensus

comment:2 in reply to:  1 Changed 15 months ago by catalyst

Replying to teor:

This issue is caused (or made much worse) by #27236, which makes the chutney bridge and onion service networks work again after changes in 0.3.4.

If we remove the relevant code, then most of the chutney networks will stop working.

Instead, let's fix the log message so it correctly reports that:

  • there are exits in the consensus, but they don't have descriptors, or
  • there are no exits in the consensus

I'm not sure I understand. Does chutney require that the bootstrap log messages distinguish between having exits and not having exits? Is there some place other than the bootstrap status messages where it makes sense to report this information?

I think it might make sense to report this somewhere else. In practice, barring catastrophe, don't we expect the live Tor network to always have exits? Maybe the distinction is only important for test networks.

comment:3 Changed 15 months ago by teor

Owner: teor deleted

I just talked with catalyst on IRC. We can change the bootstrap_status_to_string() messages.

comment:4 Changed 15 months ago by teor

Keywords: 034-backport 034-must removed

comment:5 Changed 15 months ago by nickm

Keywords: 035-deferred-20180930 added
Milestone: Tor: 0.3.5.x-finalTor: 0.3.6.x-final

Deferring several items from 0.3.5. I think these are features, not bugfixes, and therefore no longer great for 0.3.5. Please let me know if I'm wrong.

comment:6 Changed 13 months ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:7 Changed 13 months ago by catalyst

Owner: set to catalyst

comment:8 Changed 13 months ago by catalyst

Parent ID: #28018
Points: 0.1

Turning this into a table lookup and removing the dependency on nodelist.h. It often gave the wrong answer about whether tor was limited to internal paths only.

comment:9 Changed 13 months ago by catalyst

WIP patch is on branch https://github.com/tlyu/tor/tree/orconn-tracker which is mainly for #27167 work.

comment:10 Changed 13 months ago by catalyst

Actual Points: 0.1
Status: assignedneeds_review

Pull request at https://github.com/torproject/tor/pull/555

This contains prerequisite refactoring for #27167.

comment:11 Changed 12 months ago by dgoulet

Reviewer: nickm

comment:12 Changed 12 months ago by teor

Does this branch fix #27448 as well?

comment:13 Changed 12 months ago by nickm

Status: needs_reviewmerge_ready

This branch LGTM; please let me know if I should merge it now or as part of a larger thing.

comment:14 in reply to:  12 Changed 12 months ago by catalyst

Replying to teor:

Does this branch fix #27448 as well?

I think it doesn't. It seems like #27448 is mostly independent, other than the bootstrap code was getting incorrect information from router_have_consensus_path().

Therefore, I think #27448 shouldn't be a child ticket, so we can close this independently.

comment:15 in reply to:  13 Changed 12 months ago by catalyst

Replying to nickm:

This branch LGTM; please let me know if I should merge it now or as part of a larger thing.

Please merge it when convenient. It would help me keep divergence from master down on orconn-tracker.

comment:16 Changed 12 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.