Opened 5 months ago

Closed 2 months ago

#29702 closed defect (fixed)

Stop using configs from the local tor install when we launch tor for tests

Reported by: teor Owned by: rl1987
Priority: High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.3.5.1-alpha
Severity: Normal Keywords: teor-merge, technical-debt, tor-ci, tor-test, 035-backport, 040-backport, security
Cc: rl1987 Actual Points: 0.1
Parent ID: Points: 1
Reviewer: nickm Sponsor:

Description

We should stop using paths in /usr when we're launching tor for build tests.

We don't get full logs for all the tor processes we launch, but these scripts launch tor, so they're probably affected:

  • test_rebind.sh
  • test_key_expiration.sh
  • test_keygen.sh
  • test_zero_length_keys.sh

test-network.sh is also affected, but we'll deal with that in chutney in #29701.

For example, on my machine, test_rebind.sh does:

2019-03-05 11:00:11.573 Tor logged: "Mar 05 11:00:11.571 [notice] Configuration file "/usr/local/etc/tor/torrc" not present, using reasonable defaults.", waiting for "Opened Control listener on"
2019-03-05 11:00:11.596 Tor logged: "Mar 05 11:00:11.591 [notice] Parsing GEOIP IPv4 file /usr/local/share/tor/geoip.", waiting for "Opened Socks listener"
2019-03-05 11:00:11.899 Tor logged: "Mar 05 11:00:11.899 [notice] Parsing GEOIP IPv6 file /usr/local/share/tor/geoip6.", waiting for "Opened Socks listener"

Child Tickets

Change History (23)

comment:1 Changed 5 months ago by rl1987

Cc: rl1987 added

comment:2 Changed 5 months ago by rl1987

Owner: set to rl1987
Status: newaccepted

comment:3 Changed 5 months ago by rl1987

test_keygen.sh does the following:

 85 touch "${DATA_DIR}/empty_torrc"                                                 
 86                                                                                 
 87 QUIETLY="--hush"                                                                
 88 SILENTLY="--quiet"                                                              
 89 TOR="${TOR_BINARY} ${QUIETLY} --DisableNetwork 1 --ShutdownWaitLength 0 --ORPort 12345 --ExitRelay 0 -f ${DATA_DIR}/empty_torrc"

comment:4 Changed 5 months ago by rl1987

So does test_key_expiration.sh; test_zero_length_keys.sh launches zero_length_keys.sh, which does this as well.

comment:5 Changed 5 months ago by rl1987

Status: acceptedneeds_review

Doing the same in test_rebind.py: https://github.com/torproject/tor/pull/793

comment:6 Changed 5 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.1.x-final

comment:7 Changed 5 months ago by asn

Reviewer: mikeperry

comment:8 Changed 5 months ago by teor

Reviewer: mikeperry

Remove Mike as reviewer, because he's overloaded.
We'll work out what to do with these tickets in the weekly meeting.

comment:9 Changed 4 months ago by asn

Reviewer: nickm

comment:10 Changed 4 months ago by nickm

Status: needs_reviewmerge_ready

LGTM. I've opened #30102 to make sure that we don't reintroduce this kind of bug.

comment:11 Changed 4 months ago by nickm

Keywords: 035-backport 040-backport added

FWIW though, I think we should backport this as far as 0.3.5. So let's rebase before merging.

comment:12 Changed 4 months ago by teor

Actual Points: 0.1
Keywords: asn-merge teor-merge tor-ci added
Milestone: Tor: 0.4.1.x-finalTor: 0.4.0.x-final
Version: Tor: 0.3.5.1-alpha

Rebased to 0.3.5 as https://github.com/torproject/tor/pull/924

Waiting for CI.

comment:13 Changed 4 months ago by teor

Status: merge_readyneeds_revision

These changes are a good start, but they don't resolve the original issue:

$ make check
$ grep usr src/test/test_rebind.sh.log 
2019-04-10 19:09:45.714 Tor logged: "Apr 10 19:09:45.709 [notice] Parsing GEOIP IPv4 file /usr/local/share/tor/geoip.", waiting for "Opened Socks listener"
2019-04-10 19:09:46.128 Tor logged: "Apr 10 19:09:46.127 [notice] Parsing GEOIP IPv6 file /usr/local/share/tor/geoip6.", waiting for "Opened Socks listener"

I think we'll either need to change Tor's default share path at compile-time:

./configure --datarootdir=src/config

Or explicitly supply GeoIP and GeoIPv6 config lines in the defaults or user torrcs. (Or both, so that we catch future issues like this.)

We might also want to set configure --prefix to an empty directory, to catch future issues like this.

We also forgot to use an empty file for the defaults-torrc, so I added a commit to do that in all the scripts.

Whatever we do, we should make sure we fix this issue in Travis and Appveyor.

comment:15 Changed 3 months ago by nickm

Keywords: 041-should added
Milestone: Tor: 0.4.0.x-finalTor: 0.4.1.x-final

comment:16 Changed 3 months ago by nickm

Keywords: security added

comment:17 Changed 3 months ago by nickm

Keywords: 041-must added

comment:18 Changed 3 months ago by nickm

Priority: MediumHigh

comment:19 Changed 3 months ago by nickm

Status: needs_revisionmerge_ready

PR 924 above looks good to me. I have confirmed that it works with all kinds of junk in my torrc, the system torrc, and the system torrc-defaults.

I have opened #30526 to handle the geoip issue. I think we should merge this branch as it is, and backport to 0.3.5, since it is undesirable to launch Tor with a user's configuration for testing purposes.

comment:20 Changed 3 months ago by asn

Keywords: asn-merge removed

merged to 040 and onwards.

comment:21 Changed 3 months ago by nickm

Keywords: 041-should 041-must removed

comment:22 Changed 3 months ago by nickm

Milestone: Tor: 0.4.1.x-finalTor: 0.3.5.x-final

Marking for 035 backport

comment:23 Changed 2 months ago by teor

Resolution: fixed
Status: merge_readyclosed

Backported to 0.3.5.

Note: See TracTickets for help on using tickets.