Opened 5 years ago

Last modified 5 months ago

#11966 needs_revision defect

"Bootstrapped 20%: Asking for networkstatus consensus" is a lie for bridge users

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-bridge, sponsor8-maybe, bootstrap, 033-triage-20180320, 033-removed-20180320, 040-deferred-20190220, ex-28018-child, ex-sponsor19
Cc: catalyst, brade, mcs, ahf, dgoulet Actual Points:
Parent ID: Points: 1
Reviewer: arma Sponsor:

Description

When a Tor client that's configured to use a bridge sees

[notice] Bootstrapped 20%: Asking for networkstatus consensus

its next plan is actually to send a DIR_PURPOSE_FETCH_SERVERDESC request for the bridge's descriptor. This is surprising.

Child Tickets

Change History (49)

comment:1 Changed 5 years ago by arma

One option is to add another bootstrap phase for people who need to fetch bridge descriptors. Somewhere in the 18% range I guess.

Another option is to fold this bridge descriptor fetching into the 15% phase -- that is, treat it as not having a circuit open until after we've got our bridge descriptor. Kind of klunky.

A third option is to leave it alone, since it's not really a big problem in most cases. I only noticed because #11965 sure seemed (based on bootstrap level 20%) like it was about failing to fetch a consensus, rather than failing to like the bridge descriptor it had secretly fetched instead of asking for the consensus it said it was asking for.

comment:2 Changed 5 years ago by arma

I picked 'unspecified' for the milestone because I'm fine with option 3 for at least the short and medium term, unless somebody wants to step up and make this right.

comment:3 Changed 5 years ago by asn

Adding another bootstrap phase for fetching bridge descriptors seems like a reasonable idea to me. Probably not hard to implement either.

comment:4 in reply to:  3 Changed 5 years ago by isis

Cc: isis added

Replying to asn:

Adding another bootstrap phase for fetching bridge descriptors seems like a reasonable idea to me. Probably not hard to implement either.

I'd second that. I think arma's 18% option wouldn't be so difficult, and arma is correct that failing to fetch the consensus vs. failing to fetch bridge descriptors are two different bootstrapping problems.

comment:5 Changed 4 years ago by isis

Milestone: Tor: unspecifiedTor: 0.2.8.x-final
Owner: set to isis
Points: small
Status: newassigned

comment:6 Changed 3 years ago by nickm

Keywords: TorCoreTeam201605 added

Calling all non-needs_information tickets for May.

comment:7 Changed 3 years ago by isis

Severity: Normal
Status: assignedneeds_review

Please see my bug11966 branch.

comment:8 Changed 3 years ago by arma

Reviewer: arma

comment:9 Changed 3 years ago by isabela

Points: small1

comment:10 Changed 3 years ago by nickm

I'm concerned about all the things that look at this number. Can we defer merging this to 0.2.9?

comment:11 in reply to:  10 Changed 3 years ago by isis

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

Replying to nickm:

I'm concerned about all the things that look at this number. Can we defer merging this to 0.2.9?


That's fine by me. I was wondering why it hadn't been bumped to 0.2.9, and why it received the TorCoreTeam201605 tag. (I had assumed you or arma wanted it prioritised for some reason.)

comment:12 Changed 3 years ago by nickm

Keywords: review-group-2 added

Create a review-group-2 from (most of the) tickets in 0.2.8 or 0.2.9 or 029-nickm-says-yes listed as needs_review,

comment:13 Changed 3 years ago by dgoulet

Status: needs_reviewneeds_information

One question. When we trigger the BOOTSTRAP_STATUS_REQUESTING_BRIDGE_DESC control event, we do it in the circuit building function instead of the "requesting bridge desc" function which I assume is directory_get_from_dirserver.

For instance, BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS is used when we realize we need more descriptors rather than when the circuit is built for that request. What if that onehop circuit never gets build, we won't know that it was because we requested a bridge descriptor?

comment:14 in reply to:  13 Changed 3 years ago by isis

Status: needs_informationneeds_review

Replying to dgoulet:

One question. When we trigger the BOOTSTRAP_STATUS_REQUESTING_BRIDGE_DESC control event, we do it in the circuit building function instead of the "requesting bridge desc" function which I assume is directory_get_from_dirserver.


So... what happens is that we just build a circuit to our bridge, without knowing it's key, and then we ask the bridge for the consensus. The bridge responds with its descriptor. Then, usually, because the scheduled events run we call fetch_bridge_descriptors() and the bridge gives us the descriptor again. Then we decide "oh, I actually have the bridge descriptor, now I can ask for the consensus again." When we ask for the consensus again, at this point is the first time that any of the code in directory.c is called, but never before that (because of this fumbling around that happens with fetching the bridge descriptor).

However, if we were to trigger BOOTSTRAP_STATUS_REQUESTING_BRIDGE_DESC in fetch_bridge_descriptors, then it seems like we would want to make this event be at like 3% bootstrapped, because "15%" is "establishing an encrypted directory connection" where the "directory" in question is actually the bridge (for which we already have it's descriptor at this point, because the stupid fumbling) and the point that we get the descriptor is before BOOTSTRAP_STATUS_CONN_DIR=5 (5%).

For instance, BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS is used when we realize we need more descriptors rather than when the circuit is built for that request. What if that onehop circuit never gets build, we won't know that it was because we requested a bridge descriptor?


Ah. That is true.


Okay, there's an alternate version of the patch in my bug11966_v2 branch and it does like this:

May 31 15:11:45.000 [notice] Bootstrapped 0%: Starting
May 31 15:11:45.000 [notice] Delaying directory fetches: No running bridges
May 31 15:11:46.000 [notice] Bootstrapped 3%: Asking for bridge descriptors
May 31 15:11:46.000 [notice] Bootstrapped 5%: Connecting to directory server
May 31 15:11:46.000 [notice] Bootstrapped 10%: Finishing handshake with directory server
May 31 15:11:46.000 [notice] Learned fingerprint 8F347C5673390E46642B06A7B1F4088B59437AD0 for bridge 127.0.0.1:5009.
May 31 15:11:46.000 [notice] Bootstrapped 15%: Establishing an encrypted directory connection
May 31 15:11:46.000 [notice] new bridge descriptor 'test009br' (fresh): $8F347C5673390E46642B06A7B1F4088B59437AD0~test009br at 127.0.0.1
May 31 15:11:46.000 [notice] I learned some more directory information, but not enough to build a circuit: We have no usable consensus.
May 31 15:11:47.000 [notice] Bootstrapped 20%: Asking for networkstatus consensus
May 31 15:11:47.000 [notice] Bootstrapped 25%: Loading networkstatus consensus

(FWIW, I still like the 18% method better but I don't really care either way.)

comment:15 Changed 3 years ago by dgoulet

Status: needs_reviewmerge_ready

Ok! Thanks for the explanation, now it's clearer. :)

Yeah I think the 18% step alone is fine. Our circuit is opened and we are about to request the bridge descriptor followed by 20% which is getting the consensus using that bridge. Sounds good.

The branch bug11966 in your repo lgtm!

comment:16 Changed 3 years ago by arma

A) This wants a control-spec.txt patch too (Section 5.5 at the bottom).

B) + BOOTSTRAP_STATUS_REQUESTING_BRIDGE_DESC=3,
Doesn't this want to be =18? Oh, I see there is discussion above on this topic. I need to read that discussion more, but I hope to not buy it. If two of us like the 18% approach more, but there are bugs, we should explore and fix those bugs.

C) The change in circuitbuild.c now no longer triggers BOOTSTRAP_STATUS_REQUESTING_STATUS in some cases? And it's not clear that we will necessarily get back into circuit_send_next_onion_skin() later, to trigger it? So does that mean there are now cases where we accidentally skip this bootstrap phase?

comment:17 in reply to:  16 Changed 3 years ago by dgoulet

Replying to arma:

A) This wants a control-spec.txt patch too (Section 5.5 at the bottom).

Indeed

B) + BOOTSTRAP_STATUS_REQUESTING_BRIDGE_DESC=3,
Doesn't this want to be =18? Oh, I see there is discussion above on this topic. I need to read that discussion more, but I hope to not buy it. If two of us like the 18% approach more, but there are bugs, we should explore and fix those bugs.

v2 branch is not ideal. The right branch is bug11966. (same for C).

comment:18 Changed 3 years ago by dgoulet

Keywords: TorCoreTeam201606 added; TorCoreTeam201605 removed
Status: merge_readyneeds_revision

@isis, before this can be merged upstream, a control-spec patch is needed. imo bug11966 is ready for merge but still let's make sure C) (from arma review) is actually addressed.

comment:19 in reply to:  18 Changed 3 years ago by isis

Status: needs_revisionneeds_review

Replying to dgoulet:

@isis, before this can be merged upstream, a control-spec patch is needed. imo bug11966 is ready for merge but still let's make sure C) (from arma review) is actually addressed.


Yeah, (C) only applies to bug11966_v2.

There's a control-spec.txt patch in my torspec.git repo, in the bug11966 branch.

comment:20 Changed 3 years ago by nickm

Keywords: review-group-3 added; review-group-2 removed

These have all had at least one review during review-group-2.

comment:21 Changed 3 years ago by arma

Keywords: review-group-2 added; review-group-3 removed

I think issue (C) is present in bug11966.

(Also, I am sorry for being flaky here. I am not having time to be a good developer this month so far.)

comment:22 Changed 3 years ago by nickm

Keywords: review-group-3 added; review-group-2 removed

comment:23 Changed 3 years ago by nickm

Status: needs_reviewneeds_revision

calling this needs_revision till (C) gets fixed or someone says that bug11966 is fine after all.

comment:24 Changed 3 years ago by isis

Keywords: TorCoreTeam201608 added

Adding to my august tickets.

comment:25 Changed 3 years ago by nickm

Keywords: nickm-deferred-20160905 added
Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

Deferring many tickets that are in needs_revision with no progress noted for a while, where I think they could safely wait till 0.3.0 or later.

Please feel free to move these back to 0.2.9 if you finish the revisions soon.

comment:26 Changed 3 years ago by teor

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

Milestone renamed

comment:27 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:28 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:29 Changed 2 years ago by nickm

Keywords: nickm-deferred-20160905 removed

comment:30 Changed 2 years ago by nickm

Keywords: TorCoreTeam201606 removed

comment:31 Changed 2 years ago by nickm

Keywords: review-group-3 TorCoreTeam201608 removed

comment:32 Changed 2 years ago by nickm

Cc: catalyst added
Keywords: sponsor8-maybe bootstrap added

comment:33 Changed 2 years ago by mcs

Cc: brade mcs added

comment:34 Changed 2 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Sponsor: Sponsor8-can

comment:35 Changed 2 years ago by isis

If there's someone who is primarily Sponsor8 funded who'd like to work on this, lmk. Otherwise I can revisit my patch and probably call it SponsorM-can.

comment:36 Changed 2 years ago by nickm

Defer more needs_revision items to 0.3.3

comment:37 Changed 2 years ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

comment:38 Changed 19 months ago by nickm

Keywords: 033-triage-20180320 added

Marking all tickets reached by current round of 033 triage.

comment:39 Changed 19 months ago by nickm

Keywords: 033-removed-20180320 added

Mark all not-already-included tickets as pending review for removal from 0.3.3 milestone.

comment:40 Changed 19 months ago by nickm

Milestone: Tor: 0.3.3.x-finalTor: unspecified

These tickets were marked as removed, and nobody has said that they can fix them. Let's remember to look at 033-removed-20180320 as we re-evaluate our triage process, to see whether we're triaging out unnecessarily, and to evaluate whether we're deferring anything unnecessarily. But for now, we can't do these: we need to fix the 033-must stuff now.

comment:41 Changed 12 months ago by nickm

Cc: ahf dgoulet added; isis removed
Milestone: Tor: unspecifiedTor: 0.3.6.x-final
Owner: isis deleted
Parent ID: #25502
Status: needs_revisionassigned

If we do this, I think it falls under #25502 ? Apparently there is code here that we might be able to use.

comment:42 Changed 12 months ago by nickm

Status: assignedneeds_revision

comment:43 in reply to:  41 Changed 12 months ago by catalyst

Parent ID: #25502#28018

Replying to nickm:

If we do this, I think it falls under #25502 ? Apparently there is code here that we might be able to use.

The bug11966 branch seems to be about bootstrap reporting, so this probably belongs under #28018.

comment:44 Changed 12 months ago by catalyst

#27308 is the more general case of this kind of problem.

comment:45 Changed 12 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:46 Changed 9 months ago by gaba

@catalyst Can we close this ticket as it could be resolved when addressing #27308 ?

comment:47 Changed 9 months ago by gaba

Sponsor: Sponsor8-canSponsor19

comment:48 Changed 8 months ago by nickm

Keywords: 040-deferred-20190220 added
Milestone: Tor: 0.4.0.x-finalTor: unspecified

Deferring 51 tickets from 0.4.0.x-final. Tagging them with 040-deferred-20190220 for visibility. These are the tickets that did not get 040-must, 040-can, or tor-ci.

comment:49 Changed 5 months ago by catalyst

Keywords: ex-28018-child ex-sponsor19 added
Parent ID: #28018
Sponsor: Sponsor19

Unparent remaining open children of #28018.

Note: See TracTickets for help on using tickets.