Opened 14 months ago

Closed 2 weeks ago

#27992 closed defect (fixed)

config DataDirectoryGroupReadable 1 is overridden if you set KeyDir == DataDir

Reported by: needle8420 Owned by: nickm
Priority: Low Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version: Tor: 0.3.5.2-alpha
Severity: Minor Keywords: DataDirectoryGroupReadable intro BugSmashFund
Cc: Actual Points: .1
Parent ID: Points:
Reviewer: teor Sponsor:

Description

im trying to run zeronet over tor.

i need group access to the DataDirectory for cookie auth
so /var/lib/tor should have file mode 0750

spoiler: see below for workarounds + bugfix

when i run
# d=$(date +"%F %T"); \
chmod 0750 /var/lib/tor; \
systemctl restart tor; sleep 2; \
journalctl -u tor --since="$d" \
| grep -i permissions; \
stat -c%a /var/lib/tor

i always get
Fixing permissions on directory /var/lib/tor
700

and datadir ends up with filemode 0700
so it is not accessible for other users in the tor group

... though in my torrc i set
DataDirectoryGroupReadable 1

# usermod -a -G tor zeronet
# sudo -u zeronet cat /var/lib/tor/control_auth_cookie
cat: /var/lib/tor/control_auth_cookie: Permission denied

the authcookie filemode is set correctly to 0640
with the config
CookieAuthFileGroupReadable 1

--

workaround 1
run
# chmod 0750 /var/lib/tor
after starting tor

workaround 2
add
CacheDirectoryGroupReadable 1
to your torrc file

workaround 3
add
CacheDirectory = /var/lib/tor/cache
to your torrc file
if your cache dir should not be group readable

why workaround 2 and 3?

cos the error only happens
if CacheDirectory == DataDirectory
which is the default config

--

bugfix

in
src/app/config/config.c
add
if (strcmp(options->KeyDirectory, options->DataDirectory) != 0) {
and
if (strcmp(options->CacheDirectory, options->DataDirectory) != 0) {
around line 1570 and 1590
before calling
check_and_create_data_directory
... and close the parentheses

--

# cat /etc/tor/torrc
Log notice syslog
DataDirectory /var/lib/tor
DataDirectoryGroupReadable 1
ControlPort 9051
CookieAuthentication 1
CookieAuthFileGroupReadable 1
CookieAuthFile /var/lib/tor/control_auth_cookie

Child Tickets

Change History (12)

comment:1 Changed 14 months ago by nickm

Is this a duplicate of #26913 ?

comment:2 Changed 14 months ago by nickm

Milestone: Tor: 0.3.5.x-final

comment:3 Changed 14 months ago by needle8420

Is this a duplicate of #26913 ?

yes it is.

i only found #19953 .... nevermind : ]

comment:4 Changed 14 months ago by needle8420

but ...

#26913 does NOT fix the case of
KeyDirectory == DataDirectory
and
DataDirectoryGroupReadable == 1
and
KeyDirectoryGroupReadable == 0 [default]

cos in that case,
DataDirectory is set to filemode 0750,
but then back to 0700

so KeyDirectoryGroupReadable
should be made an autobool too

this is a very rare case,
cos by default
KeyDirectory = ${DataDirectory}/keys

comment:5 Changed 14 months ago by nickm

Keywords: intro added
Milestone: Tor: 0.3.5.x-finalTor: unspecified
Summary: config DataDirectoryGroupReadable 1 is ignoredconfig DataDirectoryGroupReadable 1 is overridden if you set KeyDir == DataDir

hm, yeah. I wouldn't recommend that configuration, but you're right that we should fix it sooner or later. If somebody wants to adapt the patch from 4fd761a418558c05716b4a04a5306dc67ce53dfe for #26913 so that it covers KeyDir as well, that would be great.

comment:6 Changed 2 weeks ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.3.x-final
Owner: set to nickm
Status: newaccepted

It makes sense to do this as a part of writing the tests for #32427.

comment:7 Changed 2 weeks ago by nickm

Actual Points: .1
Keywords: BugSmashFund added
Points: #32427

comment:8 Changed 2 weeks ago by nickm

Status: acceptedneeds_review

Fix now in parent branch.

comment:9 Changed 2 weeks ago by teor

These changes look fine, this ticket can close when the parent ticket is merged.

comment:10 Changed 2 weeks ago by teor

Status: needs_reviewmerge_ready

comment:11 Changed 2 weeks ago by teor

Reviewer: teor

comment:12 Changed 2 weeks ago by nickm

Points: #32427
Resolution: fixed
Status: merge_readyclosed

Fixed in #32427

Note: See TracTickets for help on using tickets.