Opened 8 years ago

Closed 6 years ago

Last modified 5 years ago

#1101 closed defect (fixed)

getinfo config-file doesn't handle relative paths well

Reported by: arma Owned by:
Priority: High Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version: 0.2.1.19
Severity: Keywords: easy
Cc: arma, Sebastian, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by nickm)

Here's how getinfo config-file is supposed to go:

getinfo config-file
250-config-file=/usr/local/etc/tor/torrc
250 OK

But I started my tor like "path/to/tor -f moria2-orrc", so getinfo
config-file simply returns "moria2-orrc" with no hint about where
it is. That means controllers like arm don't know where to look.

Should we change Tor so it prepends an absolute path when answering
the controller?

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (19)

comment:1 Changed 7 years ago by nickm

Keywords: easy added

comment:2 Changed 7 years ago by nickm

Description: modified (diff)
Milestone: Tor: 0.2.3.x-final

Yes, I think so.

comment:3 Changed 7 years ago by atagar

This is also an issue for the COOKIEFILE field of PROTOCOLINFO responses. For instance, TBB returns:
COOKIEFILE="./Data/Tor/control_auth_cookie"

Which is... difficult for controllers to handle.

comment:4 Changed 6 years ago by nickm

Priority: minormajor

At the dev meeting, atagar pointed out that this is a major annoyance for making arm actually work under some interesting circumstances. Bumping to major.

The only tricky part here will be making sure that we absolute-ify the filenames pretty early, in case we're daemonizing (which makes us chdir iirc)

comment:5 Changed 6 years ago by atagar

I just realized that according to the spec this is a bug with tor. From control-spec section 3.21 (PROTOCOLINFO)...

AuthCookieFile specifies the absolute path and filename of the
authentication cookie

comment:6 Changed 6 years ago by krkhan

rransom_ explained on IRC how realpath() is not a good choice for this since it expands symlinks as well.

I still have a question though. What's so bad about traversing symlinks if a SIGHUP shall reach load_torrc_from_disk() again anyway?

Still, as per our discussion here's an initial commit which prepends CWD on POSIX systems for filenames which do not start with '/' or '.'.

comment:7 Changed 6 years ago by nickm

Still, as per our discussion here's an initial commit which prepends CWD on POSIX systems for filenames which do not start with '/' or '.'.

Hm. Why not prepend CWD to paths that start with "." ? After all, "./torrc" isn't an absolute path.

comment:8 in reply to:  7 Changed 6 years ago by krkhan

Replying to nickm:

Hm. Why not prepend CWD to paths that start with "." ? After all, "./torrc" isn't an absolute path.

Because the name might start with "..". Something like "../xyz/torrc". Although yes, "./" should perhaps be CWD'ed as well.

comment:9 Changed 6 years ago by rransom

Status: newneeds_review

See bug1101 ( https://git.torproject.org/rransom/tor.git bug1101 ) for some fixups. (Make sure whoever autosquashes this branch updates the commit message; this branch now makes paths which begin with “.” absolute, too.)

comment:10 Changed 6 years ago by rransom

(pushed another fixup)

comment:11 Changed 6 years ago by nickm

Looking better. Do we care about stuff like the case where CWD ends with a / ? I say no.

Also, on windows, _fullpath seems pretty promising.

comment:12 in reply to:  11 Changed 6 years ago by rransom

Replying to nickm:

Looking better. Do we care about stuff like the case where CWD ends with a / ? I say no.

Yes, we do care. (We could be running in / .) Fortunately, the Tcl 8.5 filename(n) man page says: Multiple adjacent slash characters are interpreted as a single separator. (If we're lucky, that's even part of a Unixoid standard.)

Also, on windows, _fullpath seems pretty promising.

I've heard of that function (from Shondoit). I suppose I should read that function's documentation and use it.

comment:13 Changed 6 years ago by rransom

I've pushed a completely untested commit that might use _fullpath properly on Windows.

comment:14 Changed 6 years ago by nickm

Resolution: Nonefixed
Status: needs_reviewclosed

Seems okay to me. I'll squash, add a changes file, and merge it now, and test it the next time I'm at a windows box.

comment:15 Changed 6 years ago by atagar

Resolution: fixed
Status: closedreopened

Sorry to reopen but the far more substantial bug that I brought up earlier (relative auth cookie paths) was not addressed with this. I checked both the main repository and nickm's and all I can find is...

8b4e7f9 nickm@… - Merge branch 'bug1101_squashed'
e0651bb nickm@… - Changes file for bug1101
a1c1fc7 krkhan@… - Prepend cwd for relative config file paths.

... which only concerns the torrc. I suppose this is my bad for treating this as a general "tor provides relative paths to controllers" ticket. Would you like me to open a separate one for auth cookie paths or do we want to track it on this one?

-Damian

PS. Damn, I was really hoping that this had been fixed - its been a huge pita to code around...

comment:16 Changed 6 years ago by nickm

Argh. Yes, please open a new ticket. (This is what happens when a ticket's meaning changes.)

comment:17 Changed 6 years ago by nickm

(When the new ticket opens, please mention branch absolute_cookie_file in my public repository

comment:18 Changed 6 years ago by atagar

Resolution: fixed
Status: reopenedclosed

comment:19 Changed 5 years ago by nickm

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