Opened 11 years ago

Last modified 7 years ago

#820 closed defect (Fixed)

Tor left in broken state on startup

Reported by: seeess Owned by:
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.2.1.4-alpha
Severity: Keywords:
Cc: seeess, nickm, arma Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

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]

Child Tickets

Change History (8)

comment:1 Changed 11 years ago by 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.

comment:2 Changed 11 years ago by 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.

comment:3 Changed 11 years ago by 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())

comment:4 Changed 11 years ago by 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.

comment:5 Changed 11 years ago by nickm

Your inclination sounds sensible; I say go for it.

comment:6 Changed 11 years ago by arma

Committed as r16998

comment:7 Changed 11 years ago by arma

flyspray2trac: bug closed.

comment:8 Changed 7 years ago by nickm

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