Opened 10 months ago

Closed 6 months ago

#26913 closed defect (fixed)

DataDirectoryGroupReadable enabled does not have effect

Reported by: maha Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version: Tor: 0.3.3.9
Severity: Normal Keywords: regression, 035-roadmap-proposed, 033-backport, 034-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

On RedHat based systems the defaultrc includes DataDirectoryGroupReadable set to 1. But when starting up the daemon this is ignored and chmod of /var/lib/tor is set back to 0700.

This can be demostrated by the following test using vagrant:

$ vagrant up
Bringing machine 'default' up with 'virtualbox' provider...
==> default: Importing base box 'centos/7'...
==> default: Matching MAC address for NAT networking...
==> default: Checking if box 'centos/7' is up to date...
==> default: Setting the name of the VM: tor-bug_default_1532356217662_9318
==> default: Fixed port collision for 22 => 2222. Now on port 2200.
==> default: Clearing any previously set network interfaces...
==> default: Preparing network interfaces based on configuration...
    default: Adapter 1: nat
==> default: Forwarding ports...
    default: 22 (guest) => 2200 (host) (adapter 1)
==> default: Booting VM...
==> default: Waiting for machine to boot. This may take a few minutes...
    default: SSH address: 127.0.0.1:2200
    default: SSH username: vagrant
    default: SSH auth method: private key
    default: 
    default: Vagrant insecure key detected. Vagrant will automatically replace
    default: this with a newly generated keypair for better security.
    default: 
    default: Inserting generated public key within guest...
    default: Removing insecure key from the guest if it's present...
    default: Key inserted! Disconnecting and reconnecting using new SSH key...
==> default: Machine booted and ready!
==> default: Checking for guest additions in VM...
    default: No guest additions were detected on the base box for this VM! Guest
    default: additions are required for forwarded ports, shared folders, host only
    default: networking, and more. If SSH fails on this machine, please install
    default: the guest additions and repackage the box to continue.
    default: 
    default: This is not an error message; everything may continue to work properly,
    default: in which case you may ignore this message.
==> default: Rsyncing folder: /home/mh/fedora/tor-bug/ => /vagrant
==> default: Running provisioner: shell...
    default: Running: inline script
    default: Installing tor
    default: Loaded plugins: fastestmirror
    default: Determining fastest mirrors
    default:  * base: mirror.spreitzer.ch
    default:  * extras: mirror.spreitzer.ch
    default:  * updates: mirror.spreitzer.ch
    default: Resolving Dependencies
    default: --> Running transaction check
    default: ---> Package tor.x86_64 0:0.3.3.9-1.el7 will be installed
    default: --> Processing Dependency: torsocks for package: tor-0.3.3.9-1.el7.x86_64
    default: --> Running transaction check
    default: ---> Package torsocks.x86_64 0:2.2.0-1.el7.centos will be installed
    default: --> Finished Dependency Resolution
    default: 
    default: Dependencies Resolved
    default: 
    default: ================================================================================
    default:  Package       Arch        Version                   Repository            Size
    default: ================================================================================
    default: Installing:
    default:  tor           x86_64      0.3.3.9-1.el7             maha-tor-latest      2.8 M
    default: Installing for dependencies:
    default:  torsocks      x86_64      2.2.0-1.el7.centos        maha-tor-latest       65 k
    default: 
    default: Transaction Summary
    default: ================================================================================
    default: Install  1 Package (+1 Dependent package)
    default: 
    default: Total download size: 2.9 M
    default: Installed size: 13 M
    default: Downloading packages:
    default: Public key for torsocks-2.2.0-1.el7.centos.x86_64.rpm is not installed
    default: warning: /var/cache/yum/x86_64/7/maha-tor-latest/packages/torsocks-2.2.0-1.el7.centos.x86_64.rpm: Header V3 RSA/SHA1 Signature, key ID fe1432b1: NOKEY
    default: --------------------------------------------------------------------------------
    default: Total                                              1.4 MB/s | 2.9 MB  00:02     
    default: Retrieving key from https://copr-be.cloud.fedoraproject.org/results/maha/tor-latest/pubkey.gpg
    default: Importing GPG key 0xFE1432B1:
    default:  Userid     : "maha_tor-latest (None) <maha#tor-latest@copr.fedorahosted.org>"
    default:  Fingerprint: ddc6 1efd 56fa 03e5 e2d8 fa26 03f9 1145 fe14 32b1
    default:  From       : https://copr-be.cloud.fedoraproject.org/results/maha/tor-latest/pubkey.gpg
    default: Running transaction check
    default: Running transaction test
    default: Transaction test succeeded
    default: Running transaction
    default:   Installing : torsocks-2.2.0-1.el7.centos.x86_64                           1/2
    default:  
    default:   Installing : tor-0.3.3.9-1.el7.x86_64                                     2/2
    default:  
    default:   Verifying  : torsocks-2.2.0-1.el7.centos.x86_64                           1/2
    default:  
    default:   Verifying  : tor-0.3.3.9-1.el7.x86_64                                     2/2
    default:  
    default: 
    default: Installed:
    default:   tor.x86_64 0:0.3.3.9-1.el7                                                    
    default: 
    default: Dependency Installed:
    default:   torsocks.x86_64 0:2.2.0-1.el7.centos                                          
    default: 
    default: Complete!
    default: 
    default: ls -la /var/lib/tor
    default: total 4
    default: drwxr-x---.  2 toranon root    6 Jul 14 09:59 .
    default: drwxr-xr-x. 29 root    root 4096 Jul 23 14:31 ..
    default: 
    default: Grep Data
    default: /etc/tor/torrc:## things in $HOME/.tor on Unix, and in Application Data\tor on Windows.
    default: /etc/tor/torrc:#DataDirectory /var/lib/tor
    default: /usr/share/tor/defaults-torrc:DataDirectory /var/lib/tor
    default: /usr/share/tor/defaults-torrc:DataDirectoryGroupReadable 1
    default: 
    default: starting tor
    default: 
    default: tor logs
    default: -- Logs begin at Mon 2018-07-23 14:30:24 UTC, end at Mon 2018-07-23 14:31:08 UTC. --
    default: Jul 23 14:31:07 localhost.localdomain systemd[1]: Starting Anonymizing overlay network for TCP...
    default: Jul 23 14:31:08 localhost.localdomain tor[2563]: Jul 23 14:31:08.126 [notice] Tor 0.3.3.9 (git-45028085ea188baf) running on Linux with Libevent 2.0.21-stable, OpenSSL 1.0.2k-fips, Zlib 1.2.7, Liblzma N/A, and Libzstd N/A.
    default: Jul 23 14:31:08 localhost.localdomain tor[2563]: Jul 23 14:31:08.127 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
    default: Jul 23 14:31:08 localhost.localdomain tor[2563]: Jul 23 14:31:08.127 [notice] Read configuration file "/usr/share/tor/defaults-torrc".
    default: Jul 23 14:31:08 localhost.localdomain tor[2563]: Jul 23 14:31:08.127 [notice] Read configuration file "/etc/tor/torrc".
    default: Jul 23 14:31:08 localhost.localdomain tor[2563]: Jul 23 14:31:08.135 [warn] Fixing permissions on directory /var/lib/tor
    default: Jul 23 14:31:08 localhost.localdomain tor[2563]: Configuration was valid
    default: Jul 23 14:31:08 localhost.localdomain systemd[1]: Started Anonymizing overlay network for TCP.
    default: 
    default: ls -la /var/lib/tor
    default: total 4
    default: drwx------.  2 toranon root    6 Jul 14 09:59 .
    default: drwxr-xr-x. 29 root    root 4096 Jul 23 14:31 ..

Using the following Vagrantfile:

$ cat Vagrantfile 
script = <<-SCRIPT
curl -s -o /etc/yum.repos.d/maha-tor-latest-epel-7.repo https://copr.fedorainfracloud.org/coprs/maha/tor-latest/repo/epel-7/maha-tor-latest-epel-7.repo
echo Installing tor
yum install tor -y
echo 'Log debug stderr' >> /etc/tor/torrc
echo
echo ls -la /var/lib/tor
ls -la /var/lib/tor
echo
echo "Grep Data"
grep Data /etc/tor/torrc /usr/share/tor/defaults-torrc
echo
echo starting tor
systemctl start tor
echo
echo tor logs
journalctl -u tor -n 2000 --no-pager
echo
echo ls -la /var/lib/tor
ls -la /var/lib/tor
SCRIPT

Vagrant.configure("2") do |config|
  config.vm.box = "centos/7"
  config.vm.provision "shell", inline: script
end

Child Tickets

Change History (9)

comment:1 Changed 10 months ago by teor

Keywords: regression? 035-roadmap-proposed added
Milestone: Tor: 0.3.5.x-final
Status: newneeds_information

Thanks for this bug report!

The code seems to be correct:
https://github.com/torproject/tor/blob/c08b7b10c520db08e20e6ac5630f0a668d5ecf3a/src/lib/fs/dir.c#L247
https://github.com/torproject/tor/blob/3c490190163e227d37eb989b41df152e8500e059/src/app/config/config.c#L1372

Can you please run the tor unit tests on your platform, and tell us if any of the test_checkdir tests fail?
https://github.com/torproject/tor/blob/8b0920bb6f0d52402026d38dcc8405c0ff263dbb/src/test/test_checkdir.c#L74

We might also need to add some debugging statements to check_private_dir function, so we can find out exactly what it's doing:
https://github.com/torproject/tor/blob/c08b7b10c520db08e20e6ac5630f0a668d5ecf3a/src/lib/fs/dir.c#L247

comment:2 Changed 10 months ago by redfish

I also hit this. Workaround: define CacheDirectory in your torrc, for example: CacheDirectory /var/cache/tor (and, just in case, create it before starting Tor: mkdir /var/cache/tor && chmod 700 /var/cache/tor).

The wrong permission settings happens when DataDirectory == CacheDirectory, which happens by default if CacheDirectory is undefined. The call that breaks permissions is this one:
It's https://github.com/torproject/tor/blob/3c490190163e227d37eb989b41df152e8500e059/src/app/config/config.c#L1557

It's tricky to suggest the rightTM fix. I think the easiest and foolproof fix is to change the default for CacheDirectory to be "DataDirectory"/cache (or even /var/cache/tor, if tor generally has permissions to create it in /var/cache on its own). Next option for a fix is to log a warning during configuration validity check if (DataDirectory == CacheDirectory and DataDirectoryGroupReadable != CacheDirectoryGroupReadable), and maybe even fail hard rejecting the config as inconsistent.

It regressed, because CacheDirectory stuff was added somewhat recently in #22703, so people with old configs without this var defined will all be affected.

PS. Two years after #19953: same bug (albeit for a different reason) and same fixer, lol.

Edit: close superscript

Last edited 10 months ago by teor (previous) (diff)

comment:3 Changed 10 months ago by teor

Keywords: regression 033-backport 034-backport added; regression? removed

I like your second suggestion, because your first suggestion breaks compatibility with tools that expect CacheDirectory == DataDirectory.

Just a minor tweak:

When CacheDirectory is the same value as DataDirectory:

  • CacheDirectoryGroupReadable should default to DataDirectoryGroupReadable, and
  • If CacheDirectoryGroupReadable doesn't match DataDirectoryGroupReadable, log an error and exit.

This fix handles torrcs that only set DataDirectory and DataDirectoryGroupReadable.

comment:4 Changed 10 months ago by teor

Status: needs_informationnew

comment:5 Changed 8 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:6 Changed 8 months ago by nickm

Status: acceptedneeds_review

How about branch bug26913_033 in my public repository? PR at https://github.com/torproject/tor/pull/345

comment:7 Changed 8 months ago by dgoulet

Reviewer: dgoulet
Status: needs_reviewmerge_ready

lgtm; Travis is failing but doesn't seem related to the code itself.

comment:8 Changed 8 months ago by nickm

Milestone: Tor: 0.3.5.x-finalTor: 0.3.4.x-final

Merged to master; marking for possible backport.

comment:9 Changed 6 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: 0.3.3.x-final
Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.3 and forward.

Note: See TracTickets for help on using tickets.