Tor left in broken state on startup

[seeess]

Sep 21 20:05:21.660 [notice] Tor 0.2.1.5-alpha (r16710) opening log file.
Sep 21 20:05:21.661 [warn] /var/lib/tor/cached-status is not owned by this user (root, 0) but by username (1001). Perhaps you are running Tor as the wrong user?
Sep 21 20:05:21.661 [warn] Couldn't access/create private data directory "/var/lib/tor/cached-status"
Sep 21 20:05:21.661 [err] set_options(): Bug: Acting on config options left us in a broken state. Dying.

The directory initially wasnt owned by the correct user and it complained in system.out/err correctly so i could fix it,
but i forgot everything under it had the wrong perms too. But when i tried to start it the second time it just failed to launch
without anything significant printed out in system.out/err. I had to check the log for the above msg

[Automatically added by flyspray2trac: Operating System: Other Linux]

[arma]

So what's the bug? That Tor didn't complain on stdout when it died? This is
what logs are for. :)

Or that Tor refused to start if your datadir has funny permissions? I don't
see Tor can automatically fix that, since it's the wrong ownership, not wrong
mode.

[arma]

seeess> yeah i dont have run as daemon but it doesnt print to std out that it
died, so i kinda had to know that i expected it to not dump me to a terminal.
i dunno i just figured you'd want it to match the same behavior of what
happens when the dir is set wrong i dont really care
seeess> i'll try to find a real bug heh

I think this actually is a bug. Patch attached shortly.

[arma]

Here's my plan:

Index: config.c
===================================================================
--- config.c (revision 16917)
+++ config.c (working copy)
@@ -1100,6 +1100,22 @@

if (!running_tor)

goto commit;

+ if (directory_caches_v2_dir_info(options)) {
+ size_t len = strlen(options->DataDirectory)+32;
+ char *fn = tor_malloc(len);
+ tor_snprintf(fn, len, "%s"PATH_SEPARATOR"cached-status",
+ options->DataDirectory);
+ if (check_private_dir(fn, CPD_CREATE) < 0) {
+ char buf[1024];
+ int tmp = tor_snprintf(buf, sizeof(buf),
+ "Couldn't access/create private data directory \"%s\"", fn);
+ *msg = tor_strdup(tmp >= 0 ? buf : "internal error");
+ tor_free(fn);
+ goto done;
+ }
+ tor_free(fn);
+ }
+

mark_logs_temp(); /* Close current logs once new logs are open. */
logs_marked = 1;
if (options_init_logs(options, 0)<0) { /* Configure the log(s) */

@@ -1166,8 +1182,6 @@

options_act(or_options_t *old_options)
{

config_line_t *cl;

• char *fn;
• size_t len;

or_options_t *options = get_options();
int running_tor = options->command == CMD_RUN_TOR;
char *msg;

@@ -1203,20 +1217,6 @@

return -1;

}

• if (running_tor && directory_caches_v2_dir_info(options)) {
• len = strlen(options->DataDirectory)+32;
• fn = tor_malloc(len);
• tor_snprintf(fn, len, "%s"PATH_SEPARATOR"cached-status",
• options->DataDirectory);
• if (check_private_dir(fn, CPD_CREATE) != 0) {
• log_warn(LD_CONFIG,
• "Couldn't access/create private data directory \"%s\"", fn);
• tor_free(fn);
• return -1;
• }
• tor_free(fn);
• }

-

/* Load state */
if (! global_state && running_tor) {

if (or_state_load())

[arma]

The other option for my plan is to put the new clause above the

if (!running_tor)

line, and change the check_private_dir() arguments to be like the
other time we call it.

Is it necessary to check some random subdirectory when we're starting
as something other than Tor?
(versus)
Is it good practice to check it anyway, even if we don't have any use
cases right now where we actually use it?

I'm inclined to move it up (and check it anyway), since it doesn't hurt
much and one day we might like it to check the permissions.

[nickm]

Your inclination sounds sensible; I say go for it.

[arma]

Committed as r16998

[arma]

flyspray2trac: bug closed.