Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#2250 closed defect (fixed)

TorTestingNetwork vs. GETINFO config-text

Reported by: weasel Owned by:
Priority: Low Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version: Tor: 0.2.2.19-alpha
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

So,

if one has a Tor instance running with TorTestingNetwork set to 1, and sends it a GETINFO config-text on the control port, tor will assert(0).

| Dec 05 00:34:26.000 [err] config_dump(): Bug: validate_fn said: TestingV3AuthInitialVotingInterval may only be changed in testing Tor networks!
| Dec 05 00:34:26.000 [err] config_dump(): Bug: Failed to validate default config.
| Dec 05 00:34:26.000 [err] config_dump(): Bug: config.c:2750: config_dump: Assertion 0 failed; aborting.

[the first log line is mine.]

The problem results from several things, as always.

When TorTestingNetwork is set, Tor overwrites some of the default values, that is it directly modifies the initval attributes of the _option_vars elements in the global options_format. It does this in options_init_from_string() around line 4230. One of those options that are changed is TestingV3AuthInitialVotingInterval.

Also, in options_validate() we verify that TestingV3AuthInitialVotingInterval is 30*60 (the non-testing default).

Now, GETINFO config-text causes config_dump() to be called. That function wants to get a list of default values, so it can ignore them later on. For that it first gets a list of all the default values. It does that by getting a fresh config and initialising it with the (modified) defaults. Then it goes to check if Tor likes the default config.

Unfortunately we don't like the default config we just created - the default is TorTestingNetwork 0 and TestingV3AuthInitialVotingInterval not 30*60 (because we changed initval before) so we blow up (the check is around line 3690).

Note that just setting TestingTorNetwork in the config to be checked (so that we like the new non-default values) is not sufficient - the next obstacle is that TestingTorNetwork only is allowed with non-default directory authorities.

Cheers,
weasel

Not enough. and would also require special handling in config_dump())

|  static config_var_t testing_tor_network_defaults[] = {
|    V(ServerDNSAllowBrokenConfig,  BOOL,  "1"),
| +  V(TestingTorNetwork,           BOOL,     "1"),
|    V(DirAllowPrivateAddresses,    BOOL,     "1"),

Child Tickets

Change History (7)

comment:1 Changed 9 years ago by nickm

Milestone: Tor: 0.2.2.x-final
Priority: normalminor

If the fix isn't too hard, we should aim for 0.2.2.x; otherwise, it's okay IMO to do this in 0.2.3.x.

comment:2 Changed 9 years ago by Sebastian

Status: newneeds_review

Here's an ugly, but easy, fix/hack. We should fix it for real (by not having TestingTorNetwork change defaults) later imo.

Fix is in branch bug2250 in my repository. Doesn't merge cleanly into master, but the fix should be trivial.

comment:3 Changed 9 years ago by nickm

Needs a changes file; seems plausible that it would work. (Is it tested?)

Also, I think _UsingTestingTorNetwork is a bit of a confusing name; how about _UsingTestNetworkDefaults or something to indicate that what we care about is which set of defaults are in play.

comment:4 Changed 9 years ago by Sebastian

Yes, it is tested. I pushed a new commit on top of 2250 to address your concerns.

comment:5 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged it: thwunk!

comment:6 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:7 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.