#20667 closed defect (fixed)

Make FetchUselessDescriptors fetch all consensus flavours

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version: Tor: 0.3.0.0-alpha-dev
Severity: Normal Keywords: easy, intro, review-group-13
Cc: Actual Points: 0.5
Parent ID: Points: 0.5
Reviewer: nickm Sponsor:

Description (last modified by teor)

FetchUselessDescriptors 1 used to imply UseMicrodescriptors 0, but due to #6769, it doesn't in 0.3.0 and later.

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

We should document this in the man page and probably the changelog summary, as it is a breaking change for many configs that used to obtain a full consensus.

(Alternately, we could fix this bug by FetchUselessDescriptors download a full consensus, even on clients, and document that behaviour.)

Discovered when running a test bandwidth authority - see #20621.

Child Tickets

TicketStatusOwnerSummaryComponent
#20839closedStop rejecting NS descriptors when microdescs are the defaultCore Tor/Tor

Change History (18)

comment:1 Changed 12 months ago by arma

I think we should make FetchUselessDescriptors fetch all the consensus flavors and all the descriptor flavors. That's what most people seem to use it for at least.

comment:2 Changed 12 months ago by arma

(The comments and man page entry talk about how it will make you fetch descriptors for non-running relays too. But that concept is now obsolete, since non-runnings relays are never even mentioned anymore.)

comment:3 in reply to:  1 Changed 12 months ago by teor

Description: modified (diff)
Keywords: easy intro added; doc removed
Points: 0.10.5
Summary: Clarify FetchUselessDescriptors and UseMicrodescriptors in 0.3.0Make FetchUselessDescriptors actually fetch all flavours on clients

Replying to arma:

I think we should make FetchUselessDescriptors fetch all the consensus flavors and all the descriptor flavors. That's what most people seem to use it for at least.

I don't think it has done this for quite some time, at least on clients - unless the download schedule for the useless descriptors is minutes long.

I tested with 0.2.7, 0.2.8, and 0.2.9, and FetchUselessDescriptors only fetched the full consensus and descriptors, not microdescriptors

And I tested with 0.3.0, and FetchUselessDescriptors only fetched the micro consensus and microdescriptors.

Since this is not a regression in 0.2.9, I'm keeping it in 0.3.0, and I've retitled the bug and edited the description.

comment:4 in reply to:  description ; Changed 12 months ago by arma

Replying to teor:

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

Another possibility is that we treat #20621 as a bug in the bwauthority documentation / configuration ("if you wanted the microdesc-flavored consensus, you should have asked for it explicitly"), and then we close out the fetchuselessdescriptors config option as deprecated and eventually obsolete, since its original reason for existing ("fetch info about relays that you know you won't use") no longer makes sense since we took those relays out of the consensus.

comment:5 in reply to:  4 ; Changed 12 months ago by teor

Replying to arma:

Replying to teor:

Therefore, clients that want to download a full consensus have to explictly set UseMicrodescriptors 0.

Another possibility is that we treat #20621 as a bug in the bwauthority documentation / configuration ("if you wanted the microdesc-flavored consensus, you should have asked for it explicitly"), and then we close out the fetchuselessdescriptors config option as deprecated and eventually obsolete, since its original reason for existing ("fetch info about relays that you know you won't use") no longer makes sense since we took those relays out of the consensus.

Ok, that sounds sensible. We could even deprecate it with a message that says: "Set UseMicrodescriptors 0 to download the full consensus on a client", or something perhaps worded a bit better.

Do we need an option that makes a client fetch both full descriptors and microdescriptors?
If not, this sounds like a sensible solution.

comment:6 Changed 12 months ago by teor

Summary: Make FetchUselessDescriptors actually fetch all flavours on clientsDeprecate FetchUselessDescriptors, mentioning UseMicrodescriptors 0 in the message

comment:7 in reply to:  5 ; Changed 12 months ago by arma

Replying to teor:

Do we need an option that makes a client fetch both full descriptors and microdescriptors?
If not, this sounds like a sensible solution.

Once upon a time, that option was called DirPort.

Now I kind of assumed it would be what DirCache does, but looking at the code, maybe it isn't.

What is the thing that makes relays fetch all the things? We should let people turn that on.

comment:8 Changed 12 months ago by arma

directory_caches_dir_info() looks promising, which in turn calls dir_server_mode(), which in turn looks at options->DirPort_set.

comment:9 in reply to:  7 Changed 12 months ago by teor

Replying to arma:

Replying to teor:

Do we need an option that makes a client fetch both full descriptors and microdescriptors?
If not, this sounds like a sensible solution.

Once upon a time, that option was called DirPort.

Now I kind of assumed it would be what DirCache does, but looking at the code, maybe it isn't.

What is the thing that makes relays fetch all the things? We should let people turn that on.

Nothing, currently - they are all conditional.
(Except for perhaps DirPort 127.0.0.1:9999, which is a sleazy hack.)

I think we need a new option, which makes the following functions return true:
directory_caches_unknown_auth_certs
directory_caches_dir_info

But doesn't change:
dir_server_mode
Because DirPort, BridgeRelay, and (usually) DirCache do that.

Nor:
directory_fetches_from_authorities
Because FetchDirInfoEarly does that.

And then we might want to review every use of dir_server_mode to see if we really meant to say one or the other of:
directory_caches_unknown_auth_certs
directory_caches_dir_info

comment:10 Changed 12 months ago by teor

Summary: Deprecate FetchUselessDescriptors, mentioning UseMicrodescriptors 0 in the messageMake FetchUselessDescriptors fetch all consensus flavours

(OK, fixing the title for the last time. Sorry!)

comment:11 Changed 12 months ago by teor

getinfo_helper_dir checks directory_caches_dir_info before providing the NS consensus. Maybe this is ok if we fix directory_caches_dir_info. But maybe we want to use we_want_to_fetch_flavor instead.

comment:12 Changed 12 months ago by teor

networkstatus_set_current_consensus drops the flavor we're not using, unless directory_caches_dir_info is true. Maybe we want to use we_want_to_fetch_flavor here instead. Or fix directory_caches_dir_info.

comment:13 Changed 12 months ago by teor

update_consensus_router_descriptor_downloads does the same thing.

comment:14 Changed 12 months ago by teor

authority_certs_fetch_missing and trusted_dirs_load_certs_from_string do a similar thing with directory_caches_unknown_auth_certs.

comment:15 Changed 12 months ago by teor

Actual Points: 0.5
Status: newneeds_review

This is fixed in my github branch bug20667, this branch includes #20839.

comment:16 Changed 12 months ago by nickm

Keywords: review-group-13 added

comment:17 Changed 12 months ago by nickm

Reviewer: nickm

comment:18 Changed 12 months ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Looks lovely to me. Merging!

Note: See TracTickets for help on using tickets.