#20269 closed defect (fixed)

bridge users ignore their cached consensus file on startup

Reported by: arma Owned by: arma
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 029-proposed, nickm-deferred-20161017
Cc: karsten, teor Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:

Description (last modified by arma)

When a client has UseBridges set to 1, and it has a cached microdesc consensus in its datadir, when it starts up it loads that consensus file from disk, and calls networkstatus_set_current_consensus() on it.

That function checks

  if (flav != usable_consensus_flavor() &&
      !directory_caches_dir_info(options)) {
    /* This consensus is totally boring to us: we won't use it, and we won't
     * serve it.  Drop it. */
    goto done;
  }

and in this case, usable_consensus_flavor() checks we_use_microdescriptors_for_circuits() which decides

    /* If we are configured to use bridges and none of our bridges
     * know what a microdescriptor is, the answer is no. */
    if (options->UseBridges && !any_bridge_supports_microdescriptors())
      return 0;

That is, if you have bridges but you haven't marked any of them up yet, then you default to using the old-flavor consensus, so when you read your microdesc-consensus from disk, you quietly discard it. This bug results in bridge users always fetching a new consensus at start, even when they don't need to.

(Bug reported by [a person who was helpful at first but now seems to be trying to distract me and prevent me from fixing Tor bugs].)

Child Tickets

Change History (21)

comment:1 in reply to:  description Changed 14 months ago by arma

Cc: karsten added

Replying to arma:

This bug results in bridge users always fetching a new consensus at start, even when they don't need to.

I am cc'ing Karsten here, since this bug influences our metrics stats. (If many bridge-using clients restart a lot, we inflate our count of bridge users.)

comment:2 Changed 14 months ago by arma

Status: newneeds_review

My bug20269 branch fixes this issue.

comment:3 Changed 14 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.9.x-final

can consider in 029

comment:4 Changed 14 months ago by cypherpunks

Bug reported by

Please remove this line. I'm sorry about poking code and report anything again. I promise never ever report anything, can't promise I'll stop poke code, however, at least for first time.

-- lurk and surf. be safe

comment:5 Changed 14 months ago by cypherpunks

-- lurk and surf. be safe

We no need folks with a tremendously demotivating hobby. go away and never come back.
amen.

comment:6 Changed 14 months ago by arma

Description: modified (diff)

comment:7 Changed 14 months ago by nickm

Cc: teor added
Reviewer: nickm

Roger, what happens with this patch if a client doesn't actually have any bridges that support microdescriptors? Is that now impossible?

Other than that, I think we can merge this (if not the rest of #6769) into 0.2.9

comment:8 in reply to:  7 Changed 14 months ago by teor

Status: needs_reviewmerge_ready

Replying to nickm:

Roger, what happens with this patch if a client doesn't actually have any bridges that support microdescriptors? Is that now impossible?

Yes, every supported directory cache version supports microdescriptors (and if some custom version doesn't, it will be a useless bridge):

 Tor 0.2.3.25, the first stable release in the 0.2.3 branch, features
  significantly reduced directory overhead (via microdescriptors),

And in 0.2.7.1-alpha:

    - Tor no longer contains checks for ancient directory cache versions
      that didn't know about microdescriptors.

So we could have removed this check back in 0.2.7.

Other than that, I think we can merge this (if not the rest of #6769) into 0.2.9

Yes, it is safe to default to microdesc consensuses.

comment:9 Changed 14 months ago by nickm

Okay, but what will actually happen if a new client (with this patch) tries to run with an 0.2.2.x bridge? That will break, right?

If so, are we okay with that breaking?

comment:10 in reply to:  9 ; Changed 14 months ago by teor

Replying to nickm:

Okay, but what will actually happen if a new client (with this patch) tries to run with an 0.2.2.x bridge? That will break, right?

If so, are we okay with that breaking?

Yes, but we should log a warning to the client saying the bridge is too old.
(We should reject the bridge's descriptor in 0.2.9 anyway, as it doesn't have an ntor key.)

comment:11 Changed 14 months ago by nickm

Owner: set to arma
Status: merge_readyassigned

setting owner.

comment:12 Changed 14 months ago by nickm

Status: assignedneeds_revision

move into needs_reviewrevision to address this:

Yes, but we should log a warning to the client saying the bridge is too old.

Last edited 14 months ago by nickm (previous) (diff)

comment:13 Changed 14 months ago by nickm

Keywords: review-group-10 added

Add needs_review 0.2.9 tickets, plus ones that have been in needs_revision for less than a week, to review-group-10.

comment:14 Changed 13 months ago by nickm

Keywords: nickm-deferred-20161017 added
Milestone: Tor: 0.2.9.x-finalTor: 0.3.0.x-final

I am fairly sure that these are neither regressions nor major problems. So, deferring from 0.2.9. Please let me know if I'm wrong.

comment:15 Changed 13 months ago by nickm

Keywords: review-group-11 added

Moving these to review-group-11.

comment:16 Changed 13 months ago by nickm

Keywords: review-group-10 removed

comment:17 in reply to:  10 Changed 12 months ago by arma

Replying to teor:

Replying to nickm:

Okay, but what will actually happen if a new client (with this patch) tries to run with an 0.2.2.x bridge? That will break, right?

If so, are we okay with that breaking?

Yes, but we should log a warning to the client saying the bridge is too old.
(We should reject the bridge's descriptor in 0.2.9 anyway, as it doesn't have an ntor key.)

It's worse than that: Tor 0.3.0.0 doesn't finish its TLS handshake with the 0.2.2.x bridge.

I ran an 0.2.2.39 bridge and an 0.3.0.0 client.

Here's what my (modern) client says:

Nov 13 01:09:16.852 [warn] Problem bootstrapping. Stuck at 10%: Finishing handshake with directory server. (IOERROR; IOERROR; count 1; recommendation warn; host 0000000000000000000000000000000000000000 at 128.31.0.39:9005)
Nov 13 01:09:16.852 [warn] 1 connections have failed:
Nov 13 01:09:16.852 [warn]  1 connections died in state handshaking (Tor, v3 handshake) with SSL state SSL negotiation finished successfully in OPEN
Nov 13 01:09:17.398 [warn] Problem bootstrapping. Stuck at 10%: Finishing handshake with directory server. (IOERROR; IOERROR; count 2; recommendation warn; host 0000000000000000000000000000000000000000 at 128.31.0.39:9005)
Nov 13 01:09:17.398 [warn] 2 connections have failed:
Nov 13 01:09:17.398 [warn]  2 connections died in state handshaking (Tor, v3 handshake) with SSL state SSL negotiation finished successfully in OPEN

And here's what the (ancient) bridge says:

Nov 13 01:09:17.362 [debug] tor_tls_handshake(): Completed V2 TLS handshake with client; waiting for renegotiation.
Nov 13 01:09:17.362 [debug] connection_tls_continue_handshake(): Done with initial SSL handshake (server-side). Expecting renegotiation.
Nov 13 01:09:17.386 [debug] conn_read_callback(): socket 121 wants to read.
Nov 13 01:09:17.386 [debug] connection_read_to_buf(): 121: starting, inbuf_datalen 0 (0 pending in tls object). at_most 16384.
Nov 13 01:09:17.386 [debug] connection_read_to_buf(): After TLS read of 9: 510 read, 1179 written
Nov 13 01:09:17.386 [info] connection_or_process_inbuf(): Accumulated too much data (9 bytes) on nonopen OR connection from a.b.c.d:43856 in state waiting for renegotiation (TLS); closing.
Nov 13 01:09:17.386 [debug] conn_close_if_marked(): Cleaning up connection (fd 121).

So unless we want to put some energy into figuring out how to resume supporting 0.2.2.x bridges and relays (in which case we should open a separate ticket for that), I suggest we merge this one and call it done.

comment:18 Changed 12 months ago by nickm

Keywords: review-group-11 removed

comment:19 Changed 12 months ago by arma

Status: needs_revisionmerge_ready

Marking as ready for merge -- this patch doesn't make things worse for people who try to use 0.2.2.x bridges, because people like that are already in much worse shape than you think.

comment:20 Changed 12 months ago by nickm

It looks like Roger's bug20269 branch is merged in master. Is there something else to merge here?

comment:21 Changed 12 months ago by arma

Resolution: fixed
Status: merge_readyclosed

It is beautiful!

I shall close as done.

Note: See TracTickets for help on using tickets.