Opened 9 months ago
Closed 6 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 9 months ago by
Cc: | rl1987 added |
---|
comment:2 Changed 9 months ago by
Owner: | set to rl1987 |
---|---|
Status: | new → accepted |
comment:3 Changed 9 months ago by
comment:4 Changed 9 months ago by
So does test_key_expiration.sh; test_zero_length_keys.sh launches zero_length_keys.sh, which does this as well.
comment:5 Changed 9 months ago by
Status: | accepted → needs_review |
---|
Doing the same in test_rebind.py: https://github.com/torproject/tor/pull/793
comment:6 Changed 9 months ago by
Milestone: | Tor: unspecified → Tor: 0.4.1.x-final |
---|
comment:7 Changed 9 months ago by
Reviewer: | → mikeperry |
---|
comment:8 Changed 9 months ago by
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 8 months ago by
Reviewer: | → nickm |
---|
comment:10 Changed 8 months ago by
Status: | needs_review → merge_ready |
---|
LGTM. I've opened #30102 to make sure that we don't reintroduce this kind of bug.
comment:11 Changed 8 months ago by
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 8 months ago by
Actual Points: | → 0.1 |
---|---|
Keywords: | asn-merge teor-merge tor-ci added |
Milestone: | Tor: 0.4.1.x-final → Tor: 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 8 months ago by
Status: | merge_ready → needs_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:14 Changed 8 months ago by
My extra commit is in:
https://github.com/torproject/tor/pull/924
comment:15 Changed 7 months ago by
Keywords: | 041-should added |
---|---|
Milestone: | Tor: 0.4.0.x-final → Tor: 0.4.1.x-final |
comment:16 Changed 7 months ago by
Keywords: | security added |
---|
comment:17 Changed 7 months ago by
Keywords: | 041-must added |
---|
comment:18 Changed 7 months ago by
Priority: | Medium → High |
---|
comment:19 Changed 7 months ago by
Status: | needs_revision → merge_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:21 Changed 7 months ago by
Keywords: | 041-should 041-must removed |
---|
comment:22 Changed 7 months ago by
Milestone: | Tor: 0.4.1.x-final → Tor: 0.3.5.x-final |
---|
Marking for 035 backport
comment:23 Changed 6 months ago by
Resolution: | → fixed |
---|---|
Status: | merge_ready → closed |
Backported to 0.3.5.
test_keygen.sh does the following: