Can't have relative DataDirectory with CookieAuthentication enabled
DataDirectory ./datadir-3hop
Log notice file ./datadir-3hop/notice.log
RunAsDaemon 1
ControlPort 9980
CookieAuthentication 1leads to
Apr 29 12:29:03.000 [notice] Tor 0.3.0.6 (git-47d2e4f06ec26a79) opening new log file.
Apr 29 12:29:02.994 [warn] OpenSSL version from headers does not match the version we're running with. If you get weird crashes, that might be why. (Compiled with 1000105f: OpenSSL 1.0.1e 11 Feb 2013; running with 1000105f: OpenSSL 1.0.1e-fips 11 Feb 2013).
Apr 29 12:29:03.013 [notice] Tor 0.3.0.6 (git-47d2e4f06ec26a79) running on Linux with Libevent 2.0.21-stable, OpenSSL 1.0.1e-fips and Zlib 1.2.7.
Apr 29 12:29:03.013 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Apr 29 12:29:03.014 [notice] Read configuration file "/home/mtraudt/src/qtime-with-load-testing/torrc-3hop".
Apr 29 12:29:03.015 [warn] Path for DataDirectory (./datadir-3hop) is relative and will resolve to /home/mtraudt/src/qtime-with-load-testing/./datadir-3hop. Is this what you wanted?
Apr 29 12:29:03.016 [notice] Opening Socks listener on 127.0.0.1:9050
Apr 29 12:29:03.016 [notice] Opening Control listener on 127.0.0.1:9980
Apr 29 12:29:03.000 [warn] Couldn't open "./datadir-3hop/control_auth_cookie.tmp" (./datadir-3hop/control_auth_cookie) for writing: No such file or directory
Apr 29 12:29:03.000 [warn] Error writing auth cookie to "./datadir-3hop/control_auth_cookie".
Apr 29 12:29:03.000 [warn] Error creating control cookie authentication file.
Apr 29 12:29:03.000 [err] set_options(): Bug: Acting on config options left us in a broken state. Dying. (on Tor 0.3.0.6 47d2e4f06ec26a79)Setting DataDirectory to the absolute path in the torrc allows Tor to start
Activity
changed milestone to %Tor: 0.3.2.x-final
I've looked into this for a bit and found the cause of the issue:
In the function options_act (in config.c), finish_daemon is called and finish_daemon changes the current directory to the DataDirectory.
Later on options_act, init_control_cookie_authentication is called, which will try to create the control_auth_cookie file inside the DataDirectory.
Because DataDirectory is a relative path, it will try to create the control_auth_cookie file inside CURRENT_DIR/DataDirectory.
The problem is that, because CURRENT_DIR was changed to DataDirectory by finish_daemon, it will try to create the control_auth_cookie file inside DataDirectory/DataDirectory which does not exist, resulting in the error we see on the logs.
There may more bugs similar to this, as any call to get_datadir_fname after CURRENT_DIR is changed will result in the wrong path when a relative path is used for the DataDirectory.
Thanks for looking into this, Jigsaw52!
I think we should make get_datadir_fname() return an absolute path. We need to do this when we change CURRENT_DIR, but maybe we should always do it for consistency.
(Removing the version, because this bug has been in tor for a long time.)
Trac:
Version: Tor: 0.3.0.6 to N/A
Replying to Jigsaw52:
There may more bugs similar to this, as any call to get_datadir_fname after CURRENT_DIR is changed will result in the wrong path when a relative path is used for the DataDirectory.
Replying to [comment:3 teor]
I think we should make get_datadir_fname() return an absolute path. We need to do this when we change CURRENT_DIR, but maybe we should always do it for consistency.
(IMHO and without looking into it any more than the bug reports I've already made) it is starting to sound like this is also causing #22102 (moved). Perhaps there's a whole sea of buggy assumptions that could be squashed in one fell swoop.
Replying to pastly:
Replying to Jigsaw52:
There may more bugs similar to this, as any call to get_datadir_fname after CURRENT_DIR is changed will result in the wrong path when a relative path is used for the DataDirectory.
Replying to [comment:3 teor]
I think we should make get_datadir_fname() return an absolute path. We need to do this when we change CURRENT_DIR, but maybe we should always do it for consistency.
(IMHO and without looking into it any more than the bug reports I've already made) it is starting to sound like this is also causing #22102 (moved). Perhaps there's a whole sea of buggy assumptions that could be squashed in one fell swoop.
There's at least one more for the PidFile, too.
I wrote a patch to make get_datadir_fname() always return an absolute path. I save the current directory in the tor_init function and use it to create the absolute path.
Here is my branch: https://github.com/Jigsaw52/tor/tree/relative_path_fix_22101
Also, I believe this is also causing #20456 (moved).
Trac:
Status: new to needs_reviewTrac:
Cc: N/A to danielpinto52@gmail.com
What about the third case of the options_get_datadir_fname2_suffix(), where we just copy the entire DataDirectory into the fname output?
On the whole, let me suggest an alternative fix to this problem: There are over 30 places in the codebase where we refer to options->DataDirectory in one way or another. What if they were all replaced with a function like
get_data_directory(), and options->DataDirectory were only used inside functions that cared what the user actually typed? That one function could be the place that makes DataDirectory an absolute path.Trac:
Status: needs_review to needs_revisionI followed your suggestion and updated the code get_data_directory. I used the same branch: https://github.com/Jigsaw52/tor/tree/relative_path_fix_22101 Also, #22102 (moved) is not caused by this.
Trac:
Status: needs_revision to assigned
Owner: N/A to Jigsaw52I've merged and refactored my code for this fix and the fix for #22102 (moved).
The main changes are:
- I created the get_data_directory() function suggested above and replaced instances of DataDirectory with it where possible.
- I've changed make_path_absolute() to use the tor starting directory instead of the current directory to make an absolute path and moved it to util.c.
- I've changed expand_filename() to return an absolute path.
The branch is here: https://github.com/Jigsaw52/tor/tree/relative_paths_fixes
This fixes #22101 (moved), #22102 (moved) and #20456 (moved).
The issue is that RunAsDaemon is set as well, right? So the problem is (a) relative paths plus (b) setting runasdaemon?
In the past, our answer was "you cannot have relative paths and also set runasdaemon."
Trying to do both has certainly confused people in the past about what behavior they ought to get. (Some behavior will now be relative to the datadirectory, and some of it won't? That cannot end well, right?)
Yes, the problem only happens when RunAsDaemon is set.
What happens is that, when RunAsDaemon is set, the relative paths are relative to the DataDirectory but when RunAsDaemon is not set, they are relative to the tor starting directory.
I tried to fix the issue by making the relative paths always relative to the tor starting directory.
-
Replying to nickm:
Huh, I'm not sure whether or not that's behavior people would have expected. What do you think, Roger? Should everything be relative to the datadirectory, the cwd, or what?
I'm also not certain what behavior we should expect people to expect.
I would be ok with "if you want to use relative paths, you'd best specify them from wherever you are starting your Tor".
The trouble is, we already have users, and they have already done whatever they've done. So if we change the behavior on them, we're going to be breaking things for those users who rely on the current behavior. And I'm sure somewhere out there is a person who thinks that relative paths, relative to the datadir, makes perfect sense.
Things aren't so bad since this only applies when you set runasdaemon, and I think it's mostly packages that set runasdaemon. But even there, is there somebody who wrote
HiddenServiceDir hidservin their /etc/tor/torrc on debian? Probably?I wonder if there are any reasonable migation paths, like warning people about things for a while first.
-
Replying to arma:
Is Tor applying its "make the relative path absolute" algorithm twice here? That would be a bug, and one worth fixing.
In particular, check out how init_cookie_authentication() in config.c just takes an fname argument, and that comes straight out of get_controller_cookie_file_name() in control.c. And when
CookieAuthFileisn't set, it usesget_datadir_fname("control_auth_cookie")instead. The get_datadir_fname mess in config.h is deeply confusing to me, but, is it prepending the datadir onto the control_auth_cookie name, after it has done a chdir() to the datadir?I'm having trouble getting strace -f to cooperate with me, but I think in pastly's case Tor is trying to write to ./datadir-3hop/datadir-3hop/control_auth_cookie.tmp which is why it fails. And that's a bug in how we handle the default control_auth_cookie filename. Which should be fixable separate from the bigger-picture stuff above.
-
Though. If you change pastly's torrc to no longer include
CookieAuthentication 1, Tor starts up, but its log file has a bunch of other complaints, e.g.Jun 26 17:23:50.000 [warn] Couldn't open "./datadir-3hop/unverified-microdesc-consensus.tmp" (./datadir-3hop/unverified-microdesc-consensus) for writing: No such file or directory Jun 26 17:23:50.000 [warn] Couldn't open "./datadir-3hop/cached-certs.tmp" (./datadir-3hop/cached-certs) for writing: No such file or directory Jun 26 17:23:50.000 [warn] Error writing certificates to disk. Jun 26 17:23:50.000 [warn] Failed to unlink ./datadir-3hop/unverified-microdesc-consensus: No such file or directoryIt looks like get_datadir_fname() in general can't handle having a relative path for a datadir.
-
Replying to arma:
But! I have a fine 'state' file sitting in ./datadir-3hop/
And that appears to be written, in or_state_save(), using
get_datadir_fname("state").So now I'm just confused.
I think we might want to do a full refactoring here. (Easy for me to say.)
Ah ha!
Jun 26 17:26:56.000 [warn] Unable to write state to file "./datadir-3hop/state"; will try again laterI guess Tor wrote a state file before it did the chdir, and then after the chdir it tried to write the other state file location and failed.
Ok, my theory remains intact!
Trac:
Cc: danielpinto52@gmail.com to danielpinto52@gmail.com, mcs-
Replying to arma:
It looks like get_datadir_fname() in general can't handle having a relative path for a datadir.
Nick suggested that I open a new ticket if this turns into a different bug, so I did: #22731 (moved).
I believe if we do that ticket as described, we can close #22101 (moved) and #20456 (moved).
-
Marking this as needs_revision and child of #22731 (moved). Once #22731 (moved) is done, we can close this ticket.
Trac:
Status: needs_review to needs_revision
Parent: N/A to #22731 (moved) mentioned in issue #22102 (moved)
mentioned in issue #22731 (moved)
moved to tpo/core/tor#22101 (closed)
mentioned in issue tpo/core/tor#22102 (closed)
mentioned in issue tpo/core/tor#22731 (closed)
