Opened 3 years ago

Closed 3 years ago

#13161 closed defect (fixed)

[patch] getting chutney working with tor 0.2.6-alpha on OS X

Reported by: teor Owned by: teor
Priority: Low Milestone:
Component: Core Tor/Chutney Version: Tor: unspecified
Severity: Keywords: tor-authority chutney needs-test
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I had several issues getting chutney working on OS X 10.9 with tor 0.2.6.?-alpha. I've attached bug-related patches to this bug. I have created another issue for dependent enhancements. I have also created a bug for my remaining issue with the chutney bridges* flavours accessing the global tor network, rather than the chutney network.

tor git d6b2a1709d28c656dadc019fb24145e6ac400771
chutney git 60b2d18d81904e6a71a352dfa8b5cb73f4e04003

Getting chutney working

01-tor-TestingDirAuthVoteExit.patch

The authorities configured by chutney didn't apply the "Exit" flag to the exits in the network until about 35 minutes had passed. This patch creates a test config TestingDirAuthVoteExit which makes authorities mark particular relays as exits, similar to TestingDirAuthVoteGuard. (There may be a better way of getting the authorities to mark exits within seconds rather than half an hour.)

02-chutney-authority-force-vote-exit-guard.patch

Use tor-TestingDirAuthVoteExit.patch and TestingDirAuthVoteGuard to make all authorities vote all relays as exits and guards. This ensures that tor's make test-network will work. But it's a sledgehammer approach.

03-chutney-explicit-exit.patch

It looks like tor exits need to be explicitly configured to accept all ports on localhost (they won't do it by default).

04-chutney-basic-decreased-guards-more-relays.patch

Since the guard percentage has been reduced from 50% to 25% of relays, we need more relays to compensate.

Fixing chutney issues

05-chutney-kill-old-nets-other-flavours.patch

If you change the chutney flavour between runs, tor processes from the previous run sometimes don't get terminated when the next chutney runs. This patch fixes this issue by using pidfiles, and, in the extreme case of kill-zombies.sh, ps and path grepping (which is a hack).

06-test-network-wait-25s.patch (optional)

The chutney network seems more reliable if I wait for 25 seconds instead of 18. YMMV

07-test-network-echo.patch

The default shell on OS X is bash, which has a builtin echo. When called in "sh" mode, this echo does not accept "-n". This patch uses "/bin/echo -n" instead.

Debugging tor launched by chutney

08-chutney-enable-debugger.patch

This patch sets DisableDebuggerAttachment to 0, because if you're running a test network, you probably don't care about a debugger. (I may be wrong about this.)

09-chutney-TorNet-not-RunAsDaemon-comments.patch

I found it hard to debug tor processes launched by chutney when RunAsDaemon was set. So I wrote code that allowed the use of poll() instead of wait() to catch launch failures. This patch includes these changes in the comments - the functionality is the same as the git version of chutney. (There may be a better way of making this an option.)

Child Tickets

Attachments (10)

02-chutney-authority-force-vote-exit-guard.patch (705 bytes) - added by teor 3 years ago.
Change the chutney authority template to force-vote everything an "Exit" flag
03-chutney-explicit-exit.patch (2.1 KB) - added by teor 3 years ago.
Create a template for "exit", explicitly permitting localhost
04-chutney-basic-decreased-guards-more-relays.patch (550 bytes) - added by teor 3 years ago.
Since we've decreased the Guard proportion, increase the number of relays
05-chutney-kill-old-nets-other-flavours.patch (1.3 KB) - added by teor 3 years ago.
Make sure we kill the old network, even if it has changed flavour
06-test-network-wait-25s.patch (578 bytes) - added by teor 3 years ago.
Make test-network.sh wait 25 seconds rather than 18, to increase reliability
07-test-network-echo.patch (748 bytes) - added by teor 3 years ago.
Use an echo that recognises "-n" in test-network.sh
08-chutney-enable-debugger.patch (457 bytes) - added by teor 3 years ago.
Enable the debugger by default in chutney configs
09-chutney-TorNet-not-RunAsDaemon-comments.patch (1.6 KB) - added by teor 3 years ago.
Add commented-out code that avoids the need for RunAsDaemon (for debugging)
01-tor-TestingDirAuthVoteExit.patch (2.0 KB) - added by teor 3 years ago.
Add TestingDirAuthVoteExit like TestingDirAuthVoteGuard, include config.c and dirserv.c this time
06-test-network-configurable-wait.patch (804 bytes) - added by teor 3 years ago.
Add configurable wait time to test-network.sh using --delay

Download all attachments as: .zip

Change History (33)

Changed 3 years ago by teor

Change the chutney authority template to force-vote everything an "Exit" flag

Changed 3 years ago by teor

Create a template for "exit", explicitly permitting localhost

Changed 3 years ago by teor

Since we've decreased the Guard proportion, increase the number of relays

Changed 3 years ago by teor

Make sure we kill the old network, even if it has changed flavour

Changed 3 years ago by teor

Make test-network.sh wait 25 seconds rather than 18, to increase reliability

Changed 3 years ago by teor

Attachment: 07-test-network-echo.patch added

Use an echo that recognises "-n" in test-network.sh

Changed 3 years ago by teor

Enable the debugger by default in chutney configs

Changed 3 years ago by teor

Add commented-out code that avoids the need for RunAsDaemon (for debugging)

Changed 3 years ago by teor

Add TestingDirAuthVoteExit like TestingDirAuthVoteGuard, include config.c and dirserv.c this time

comment:1 Changed 3 years ago by teor

Status: newneeds_review

The previous 01-tor-TestingDirAuthVoteExit.patch only contained the changes from config.h. This was my mistake.

This patch includes the changes from config.c and dirserv.c as well as config.h.

Changed 3 years ago by teor

Add configurable wait time to test-network.sh using --delay

comment:2 Changed 3 years ago by teor

Rather than just arbitrarily increasing the wait time to 25s, the latest patch makes it configurable - see 06-test-network-configurable-wait.patch

comment:3 Changed 3 years ago by nickm

Status: needs_reviewneeds_revision

Hmmm. A few general notes:

  • Would you be able to generate future patches with "git format-patch" rather than "git diff" or whatever you're doing now? If you use "git format-patch", I can merge them directly with "git am" and I don't need to re-enter the commit messages.
  • For future patches with changes to Tor, could you write a changelog fragment file for the "changes" directory, as described in doc/HACKING? That'll also make it faster for me to apply patches.

Patch-by-patch:

  • 01 looks okay, but the option needs to be documented in doc/tor.1.txt
  • 02 will break chutney with older versions of Tor. This happens every time we add a new option and have chutney use it. I wonder if we could make chutney smarter about which options to include depending on which version of Tor it's configuring to use.
  • 03 looks fine
  • 04 looks fine
  • 05 looks okay; I wonder if chutney should do this kind of thing itself if it's about to overwrite a running network.
  • 06 looks fine
  • 07 looks fine
  • 08 looks fine
  • 09 looks fine; I agree that if we're goign to merge it, though, it should be an option or something.

comment:4 Changed 3 years ago by teor

Sure, Nick, I'm still getting used to using git and everything.

comment:5 Changed 3 years ago by teor

Re: 01-tor-TestingDirAuthVoteExit

I had documented this, but obviously didn't pick it up in the patch. Oops!

comment:6 Changed 3 years ago by teor

Nick, do I need to document the new TestingDirAuthVoteExit option in both doc/tor.1.txt and doc/tor.1.in ?
Or does asciidoc take care of the conversion from tor.1.txt to tor.1.in for me?

comment:7 Changed 3 years ago by teor

Ok, I see it does now.

comment:8 Changed 3 years ago by teor

Progress on tor patches:

Repository: ​​​https://github.com/teor2345/tor.git

Branch: issue-13161-TestingDirAuthVoteExit

27f30040f63d13234d07e58cf3d0d2a3cbd93ce5 Add TestingDirAuthVoteExit option
Includes 01, tor.1.txt documentation, and changes file

Branch: issue-13161-test-network

7c0215f8cafd2b7e1d251439b97ad97ec46e6211 use "/bin/echo -n" rather than builtin echo
Includes 07 and changes file

bae73343902072c24469d42deffe90b22b7fff6e add --delay option to test-network.sh
Improves on 06 by making it configurable

Or you can pick all 3 changes out of issue-13161-integrated-testing if you prefer.

TODO:

  • chutney patches for this issue
  • re-testing of integrated set of changes

comment:9 Changed 3 years ago by teor

Progress on chutney patches:

Repository: https://github.com/teor2345/chutney.git

Branch: issue-13161-check-torrc-options
Commit: Stop using unsupported torrc options using tor --list-torrc-options
This resolves nickm's feature request based on my original patch 02.
Caveats: if the supported option values change, we can't tell the good from the bad.

Branch: issue-13161-typos
A branch for any typos I might find in chutney during issue 13161
Commit: 1 typo found

TODO:

  • chutney patches 02-09 for this issue, and 10 & 11 in #13162
  • re-testing the integrated set of changes

comment:10 Changed 3 years ago by teor

Progress on chutney patches:

Repository: ​https://github.com/teor2345/chutney.git

Branch: issue-13161-ensure-kill-chutney-tors
Commit: Ensure chutney kills leftover tor processes
Use 3 different methods:

  • kill leftover nodes by pid file in tools/bootstrap.sh,
  • kill current nodes by pid file in tools/kill-current-nodes.sh
  • kill all nodes by grepping command lines in tools/kill-all-nodes.sh

Caveats: if you're reduced to grepping process command-lines, don't do it on a production machine.

TODO:

  • chutney patches 02-04, 08 & 09 for this issue, and 10 & 11 in #13162
  • re-testing the integrated set of changes

comment:11 Changed 3 years ago by teor

Status: needs_revisionneeds_review

tor & chutney patches ready for review:

Repository: ​​​​https://github.com/teor2345/tor.git

Branch: issue-13161-TestingDirAuthVoteExit
Add TestingDirAuthVoteExit option
Includes 01, tor.1.txt documentation, and changes file

Branch: issue-13161-test-network
use "/bin/echo -n" rather than builtin echo
Includes 07 and changes file

add --delay option to test-network.sh
Improves on 06 by making it configurable

Repository: ​https://github.com/teor2345/chutney.git

Branch: issue-13161-check-torrc-options
Stop using unsupported torrc options using tor --list-torrc-options
This resolves nickm's feature request (based on original patch 02) to not break previous versions of tor by using new torrc options in chutney templates.

Check RunAsDaemon before using wait()
If the torrc has RunAsDaemon set to 1, use wait(). Otherwise, use poll().
Based on original patch 09.

Merged branch issue-13161-typos
I only found one typo, so I merged it into the TorNet.py branch

Branch: issue-13161-ensure-kill-chutney-tors
Ensure chutney kills leftover tor processes, both in bootstrap and as separate scripts.
Based on original patch 05.

Branch: issue-13161-template-tweaks
Ensure chutney relays can exit to localhost IPv4 and IPv6
Based on original patch 03.

Enable Debugging of chutney-launched tor nodes
Based on original patch 08.

Increase relays in basic flavour to compensate for smaller guard fraction
Based on original patch 04.

Force authorities to vote Exit & Guard for all nodes
Requires the corresponding tor option patch in issue-13161-TestingDirAuthVoteExit
Based on original patch 02.

Create a middle flavour, and size variants for basic and middle
Based on patches 10 & 11 in #13162.

Results:
basic and middle flavours, and some of the medium-sized variants, work on my machine (OS X 10.9).
make test-network works on my machine, even with the default of 18 seconds. YMMV.

comment:12 Changed 3 years ago by teor

I'm seeing the following warning in my tor logs, probably because of #13161 or #13163.

"[Warning] const routerstatus_t *directory_pick_generic_dirserver(dirinfo_type_t, int, uint8_t)(): Bug: Called when we have UseBridges set."

Can someone help me work out what I've done wrong or incompletely?

comment:13 Changed 3 years ago by teor

It's a bug in commit 3 of 3 in #13163. Dealt with there.

comment:14 Changed 3 years ago by nickm

Merged the tor patches; will look at the chutney ones next.

comment:15 Changed 3 years ago by nickm

Status: needs_reviewneeds_revision

issue-13161-check-torrc-options :

  • Looks okay. If we do any more option handling than this, though, we should probably exract this torrc-handling logic to a new function.

issue-13161-ensure-kill-chutney-tors

  • I'm a little worried that this one will potentially kill processes that are not Tor.

issue-13161-template-tweaks

  • Looks good to me.

I've merged template-tweaks and check-torrc-options. I don't think I'm brave enough to merge ensure-kill-chutney-tors unless we can somehow be sure that we aren't killing anything we shouldn't be killing.

comment:16 Changed 3 years ago by teor

Indeed. Automated killing is generally frowned upon, unless guilt can be conclusively established.

All you'd need to be doing is running a compile at the same time, and there would be carnage among innocent processes. (I did this to myself while developing that branch. Only reason I didn't kill my tor routers was they had 3-digit pids!)

The existing, canonical solution: use the Tor control protocol to kill the tor processes (or confirm their existence), and match both pid, port, and Tor control password. That would be safe(r).

comment:17 Changed 3 years ago by teor

Resolution: fixed
Status: needs_revisionclosed

Actually, #13331 fixes this "the right way" for tor's test-network.sh: stop the network right after you've finished using it.

For all other usage mechanisms, that's up to the user.

comment:18 Changed 3 years ago by teor

Resolution: fixed
Status: closedreopened

When I increased relay numbers to at least 8 to compensate for a guard fraction of 25%, I forgot mixed, bridges, and bridges+ipv6. I've updated the relevant branch with these (minor) changes.

Repository: ​​https://github.com/teor2345/chutney.git

Branch: issue-13161-template-tweaks
Increase relays to compensate for smaller guard fraction: mixed, bridges, bridges+ipv6.

comment:19 Changed 3 years ago by nickm

Resolution: fixed
Status: reopenedclosed

Okay; merged that. Thanks!

comment:20 Changed 3 years ago by teor

Keywords: needs-test added
Resolution: fixed
Status: closedreopened

issue-13161-TestingDirAuthVoteExit & Guard could do with unit tests. I'll do these.

comment:21 Changed 3 years ago by teor

Owner: changed from nickm to teor
Status: reopenedassigned

comment:22 Changed 3 years ago by teor

Priority: normalminor

These are already tested pretty well through chutney.

comment:23 Changed 3 years ago by nickm

Resolution: fixed
Status: assignedclosed

Okay. Closing this ticket then; please reopen if you think it's appropriate.

Note: See TracTickets for help on using tickets.