Regression breaks 'SETCONF HiddenServiceDir'

changed milestone to %Tor: 0.3.0.x-final

added actualpoints::0.4 component::core tor/tor milestone::Tor: 0.3.0.x-final points::0.1 priority::very high resolution::fixed severity::major status::closed tor-hs type::defect version::tor 0.3.0.0-alpha-dev labels

Trac:
Keywords: N/A deleted, tor-hs regression added
Milestone: N/A to Tor: 0.2.9.x-final

Teor?

Trac:
Cc: N/A to teor

Dec 02 14:22:39.000 [warn] Hidden service ("/tmp/hs_dir") with no ports configured.
Dec 02 14:22:39.000 [warn] options_act(): Bug: Previously validated hidden services line could not be added! (on Tor 0.3.0.0-alpha-dev e6facbfe7abc50de)
Dec 02 14:22:39.000 [err] set_options(): Bug: Acting on config options left us in a broken state. Dying. (on Tor 0.3.0.0-alpha-dev e6facbfe7abc50de)

This appears to crash master, but not to crash 0.2.9. So this might not be because of Teor's 0.2.9 patch yesterday.

Asn, are you sure that this belongs in the 0.2.9 milestone? 0.2.9 isn't crashing for me when I do this.

This doesn't fail for me on 0.2.9 either. Looks like a result of #20559 (moved), which added stricter checks. We obviously need to handle failures at a higher level, rather than letting them get all the way to options_act().

Trac:
Version: N/A to Tor: 0.3.0.0-alpha-dev
Points: N/A to 0.1
Keywords: tor-hs regression deleted, tor-hs added
Milestone: Tor: 0.2.9.x-final to Tor: 0.3.0.x-final

(The directory creation appears unnecessary, the crash occurs regardless for me.)

Please see my branch bug20860-v3 on https://github.com/teor2345/tor.git

The root cause of this bug is that some checks were only getting performed when we acted on the options, and not during validation. This is problematic, because if tor passes validation but fails when acting on options, it asserts and dies.

This patch resolves this issue by performing more of the checks during validation.

Now the results are:

SETCONF HiddenServiceDir="/tmp/hs_dir"
513 Unacceptable option value: Failed to configure rendezvous options. See logs for details.

With the log message:

Dec 03 09:14:21.000 [warn] Hidden service ("/tmp/hs_dir") with no ports configured.
Dec 03 09:14:21.000 [warn] Controller gave us config lines that didn't validate: Failed to configure rendezvous options. See logs for details.

(Previously, this would appear to succeed:

250 OK

but actually fail, and log failure messages to the log.)

When a valid service is added, the output is:

SETCONF HiddenServiceDir="/tmp/hs_dir" HiddenServicePort=80
250 OK

When a duplicate hidden service is added, the output is:

SETCONF HiddenServiceDir="/tmp/hs_dir" HiddenServicePort=80 HiddenServiceDir="/tmp/hs_dir" HiddenServicePort=80
513 Unacceptable option value: Failed to configure rendezvous options. See logs for details.

Dec 03 09:16:53.000 [warn] Another hidden service is already configured for directory "/tmp/hs_dir".
Dec 03 09:16:53.000 [warn] Controller gave us config lines that didn't validate: Failed to configure rendezvous options. See logs for details.

There is still a bug in check_private_dir where tor will crash if any directory configured in the options can't be created, but that's been present since at least 0.2.7, and is due to missing checks on the parent directory:

    /* XXXX In the case where check==CPD_CHECK, we should look at the
     * parent directory a little harder. */

So, for example:

SETCONF HiddenServiceDir="/" HiddenServicePort=80

will likely cause a crash.

Trac:
Status: new to needs_review
Actualpoints: N/A to 0.4

This looks plausible to me. Merging.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Thanks! Stem tests updated to account for this. Works great.

closed

changed time estimate to 48m

added 3h 12m of time spent

moved to tpo/core/tor#20860 (closed)

Regression breaks 'SETCONF HiddenServiceDir'

Child items 0

Activity