Opened 7 years ago

Closed 7 years ago

#6304 closed defect (fixed)

CircuitBuildTimeout default value does not match man page

Reported by: dcf Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: documentation tor-client
Cc: andrea Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Running tor with

LearnCircuitBuildTimeout 0

and not specifiying any CircuitBuildTimeout gives a misleading warning about a timeout of 0:

Jul 05 02:48:16.974 [warn] CircuitBuildTimeout is shorter (0 seconds) than recommended (10 seconds), and LearnCircuitBuildTimeout is disabled.  If tor isn't working, raise this value or enable LearnCircuitBuildTimeout.

The ticket that resulted in this warning is #5452.

Child Tickets

Attachments (1)

0001-Make-the-default-CircuitBuildTimeout-match-the-man-p.patch (1.5 KB) - added by rransom 7 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 7 years ago by arma

Milestone: Tor: 0.2.3.x-final

comment:2 Changed 7 years ago by dcf

Now I'm not so sure that this is only a problem with the warning. I reliably fail to connect using flash proxies using this command:

$ tor ClientTransportPlugin "websocket socks4 tor-facilitator.bamsoftware.com:9999" UseBridges 1 Bridge "websocket 0.0.1.0:1" LearnCircuitBuildTimeout 0 SocksPort 9051
<...>
Jul 10 10:50:48.000 [notice] Bootstrapped 5%: Connecting to directory server.
Jul 10 10:50:48.000 [notice] Heartbeat: Tor's uptime is 0:00 hours, with 0 circuits open. I've sent 0 kB and received 0 kB.
Jul 10 10:50:48.000 [notice] Bootstrapped 10%: Finishing handshake with directory server.
Jul 10 10:50:48.000 [warn] Problem bootstrapping. Stuck at 10%: Finishing handshake with directory server. (DONE; DONE; count 1; recommendation warn)
Jul 10 10:50:48.000 [warn] 1 connections have failed:
Jul 10 10:50:48.000 [warn]  1 connections died in state handshaking (TLS) with SSL state unknown state in HANDSHAKE

But adding what should be the default timeout allows me to connect:

$ tor ClientTransportPlugin "websocket socks4 tor-facilitator.bamsoftware.com:9999" UseBridges 1 Bridge "websocket 0.0.1.0:1" LearnCircuitBuildTimeout 0 CircuitBuildTimeout 60 SocksPort 9051
<...>
Jul 10 10:52:42.000 [notice] Tor has successfully opened a circuit. Looks like client functionality is working.
Jul 10 10:52:42.000 [notice] Bootstrapped 100%: Done.

This persists even after totally deleting my ~/.tor directory (I thought a short timeout might be cached there). You should be able to run these flash proxy commands without installing any flash proxy software.

comment:3 Changed 7 years ago by rransom

Milestone: Tor: 0.2.3.x-finalTor: 0.2.2.x-final
Priority: minormajor
Status: newneeds_review
Summary: Too-low CircuitBuildTimeout warning is misleading with default timeoutCircuitBuildTimeout default value does not match man page

This warning happened to me a few days ago, while setting up Tor on a laptop. It is accurate.

comment:4 Changed 7 years ago by nickm

Status: needs_reviewneeds_revision

NAK. I believe that "0" is supposed to mean "use the default". See the logic in circuit_build_times_get_initial_timeout().

That said, the logic is screwy, and it looks like should change the outer "else" case of that function so that if LearnCircuitBuildTimeout and CircuitBuildTimeout, we fall back to circuit_build_times_initial_timeout.

comment:5 Changed 7 years ago by nickm

Keywords: documentation added
Milestone: Tor: 0.2.2.x-finalTor: 0.2.3.x-final

comment:6 Changed 7 years ago by nickm

Keywords: tor-client added

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:8 Changed 7 years ago by nickm

Status: needs_revisionneeds_review

Just closed #8444 as a duplicate of this.

I've got a possible fix as branch "bug6034_v2" in my public repository.

comment:9 Changed 7 years ago by aagbsn

Oh, I might have just tested the wrong branch (8444_v1). I got the following error when trying to run with tor2webmode. I don't see bug6034_v2 in your repository yet.

Mar 10 02:34:12.143 [notice] Tor v0.2.3.18-rc-dev (git-43d224233f8b6a88) running on Linux.
Mar 10 02:34:12.144 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Mar 10 02:34:12.144 [notice] Read configuration file /home/user/code/tor/torrc.
Mar 10 02:34:12.149 [notice] Tor2webMode is enabled; turning LearnCircuitBuildTimeout off.
Mar 10 02:34:12.149 [warn] CircuitBuildTimeout is shorter (0 seconds) than recommended (10 seconds), and LearnCircuitBuildTimeout is disabled.  If tor isn't working, raise this value or enable LearnCircuitBuildTimeout.
Mar 10 02:34:12.150 [notice] Initialized libevent version 2.0.18-stable using method epoll (with changelist). Good.
Mar 10 02:34:12.150 [notice] Opening Socks listener on 127.0.0.1:9333
Mar 10 02:34:12.000 [err] This copy of Tor was not compiled to run in 'tor2web mode'. It cannot be run with the Tor2webMode torrc option enabled. To enable Tor2webMode recompile with the --enable-tor2webmode option.
Mar 10 02:34:12.000 [err] set_options(): Bug: Acting on config options left us in a broken state. Dying.

comment:10 Changed 7 years ago by nickm

That's the error you get when you enable Tor2WebMode without configuring with --enable-tor2webmode option.

comment:11 Changed 7 years ago by aagbsn

oops. mild dyslexia strikes again... your branch (bug6304_v2) successfully builds a circuit :-)

(and I am pretty sure I built bug8444_v1 with --enable-tor2webmode, because I did it twice to make sure)

comment:12 Changed 7 years ago by nickm

Keywords: 023-backport added
Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final

Let's try this in 0.2.4 first. Can somebody review the code?

comment:13 in reply to:  12 Changed 7 years ago by dcf

Replying to nickm:

Let's try this in 0.2.4 first. Can somebody review the code?

I looked at commit d13b996d and it doesn't immediately make sense to me. I don't see why different conditions are used in different places. Simplifying a bit,

  if (get_options()->LearnCircuitBuildTimeout) {
    if (get_options()->CircuitBuildTimeout)
      timeout = get_options()->CircuitBuildTimeout*1000;
    else
      timeout = circuit_build_times_initial_timeout();
  } else {
    if (get_options()->CircuitBuildTimeout > 0)
      timeout = get_options()->CircuitBuildTimeout*1000;
    else
      timeout = circuit_build_times_initial_timeout();
  }

In the first branch of the if (LearnCircuitBuildTimeout is true), we check CircuitBuildTimeout != 0. But in the second branch (LearnCircuitBuildTimeout is false), we rather check CircuitBuildTimeout > 0. I realize there must be some difference in the two branches, or the outer if could disappear, but it's not obvious to me why the conditions differ here. Can CircuitBuildTimeout be negative (-1 means uninitialized or something)?

comment:14 Changed 7 years ago by nickm

Actually, it's 0 that means "uninitialized" here. The >0 vs true tests could be made more uniform.

This code wouldn't actually be too hard to simplify, into something like what I have now in "bug6304_v3". I didn't do that in the initial version, since I wanted the patch to be easier to follow. I *think* that commit 7543fb61e34e92 changes nothing, but it's a nonobvious refactor.

comment:15 Changed 7 years ago by nickm

Keywords: 023-backport removed

Removing 023-backport. Added another commit to "bug6304_v3", to fix the actual warning message that provoked this ticket originally.

comment:16 Changed 7 years ago by andrea

I think this is fine.

comment:17 Changed 7 years ago by dcf

Looks good to me.

comment:18 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged to 0.2.4.

Note: See TracTickets for help on using tickets.