Opened 14 months ago

Last modified 2 weeks ago

#8368 needs_review enhancement

Add tor.service (for systemd) to upstream package

Reported by: jamielinux Owned by:
Priority: normal Milestone: Tor: 0.2.5.x-final
Component: Tor Version:
Keywords: tor-relay systemd 025-triaged Cc: intrigeri@…
Actual Points: Parent ID:
Points:

Description

In Fedora we have a custom systemd service file for running Tor. We are encouraged to push changes upstream, and thus I am proposing that it be included as part of the upstream tarball. I have pasted the contents below, but please do advise on whether this can be improved:

$ cat tor.service
[Unit]
Description = Anonymizing overlay network for TCP
After = syslog.target network.target nss-lookup.target

[Service]
Type = simple
ExecStart = /usr/bin/tor --runasdaemon 0 --defaults-torrc /usr/share/tor/defaults-torrc -f /etc/tor/torrc --quiet
ExecReload = /bin/kill -HUP ${MAINPID}
ExecStop = /bin/kill -INT ${MAINPID}
TimeoutSec = 30
Restart = on-failure
LimitNOFILE = 4096

[Install]
WantedBy = multi-user.target

Child Tickets

Attachments (1)

0001-Add-contrib-tor.service-for-use-with-systemd.patch (1.4 KB) - added by jamielinux 14 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 14 months ago by jamielinux

Actually, the ExecStart line should probably just say this:

ExecStart = /usr/bin/tor --runasdaemon 0 -f /etc/tor/torrc --quiet

comment:2 Changed 14 months ago by nickm

  • Keywords tor-relay added
  • Milestone set to Tor: 0.2.5.x-final
  • Status changed from new to needs_review

If we're going to ship this with Tor, it should probably have a comment at the top explaining what it is and how to use it.

I'm not very knowledgeable about how Unixes launch things nowadays -- is this Fedora-specific, or will this file work with lots of other systems?

(If the latter, has anybody else written a systemd file that we could also look at to see if it has any ideas that this one might be missing?)

comment:3 follow-up: Changed 14 months ago by jamielinux

If we're going to ship this with Tor, it should probably
have a comment at the top explaining what it is and how
to use it.

Sure. Something like:

# This is a service file for use with the systemd init daemon. It should be
# placed with other systemd units, usually at /usr/lib/systemd/system/tor.service

I'm not very knowledgeable about how Unixes launch things nowadays
-- is this Fedora-specific, or will this file work with lots of
other systems?

It should work with any other distribution that has implemented systemd, which now includes Arch Linux (default init), OpenSUSE (default init in 12.1), soon Mageia (will be default init from version 3) and probably others.

(If the latter, has anybody else written a systemd file that we
could also look at to see if it has any ideas that this one might
be missing?)

Yes that's a good idea. The Arch Linux one is here:
https://projects.archlinux.org/svntogit/community.git/tree/trunk/tor.service?h=packages/tor

They do things a bit differently but dropping the service file proposed here into Arch Linux would work fine, and vice versa.

I'm also not sure what the best value for LimitNOFILE is yet. The sysv initscripts distributed at the moment have some logic to raise ulimit based on the file descriptors available.

comment:4 in reply to: ↑ 3 Changed 14 months ago by arma

Replying to jamielinux:

I'm also not sure what the best value for LimitNOFILE is yet. The sysv initscripts distributed at the moment have some logic to raise ulimit based on the file descriptors available.

Something like https://gitweb.torproject.org/debian/tor.git/blob/HEAD:/debian/tor.init#l38 ?

If you want to pick just one ulimit -n, 32768 is a good choice.

4096 is way too low for fast relays.

comment:5 Changed 14 months ago by nickm

It looks as if we're converging on a thing to ship here. The ideal format to get this in would be a patch generated by "git format-patch", containing a file for the "changes" directory, as described in doc/HACKING. Next-best would be just a patch.

comment:6 Changed 14 months ago by jamielinux

If you want to pick just one ulimit -n, 32768 is a good choice.

4096 is way too low for fast relays.

Thanks for the advice. I've fixed this in git for the Fedora package and have fixed in the patch attached.

The ideal format to get this in would be a patch generated by
"git format-patch", containing a file for the "changes" directory,
as described in doc/HACKING.

Sure, I've attached a patch.

comment:7 Changed 14 months ago by jamielinux

We've been considering hardening the tor systemd service and I paste our current draft below. We're working on further hardening and testing at the moment.

I do have one question. If limiting the tor service to specific devices (with DeviceAllow), does it actually need /dev/random?

[Service]
...
PrivateTmp = yes
LimitNPROC = 2
DeviceAllow = /dev/null rw
DeviceAllow = /dev/urandom r
DeviceAllow = /dev/random r
InaccessibleDirectories = /
ReadOnlyDirectories = /etc /usr
ReadWriteDirectories = /var/lib/tor /var/log/tor

comment:8 Changed 14 months ago by nickm

If you have a working /dev/urandom, Tor shouldn't need /dev/random.

comment:9 Changed 8 months ago by Joshua

  • Priority changed from minor to normal

Are the arguments "--runasdaemon 0" and "--quiet" really needed? If possible they should be omitted, because they overwrite the settings in the torrc file, making it impossible to change them.

I've got this problem at the moment with the SysVinit script, which uses the deprecated TorCtl. TorCtl forces the argument '--log \"notice file $LOGFILE\"' upon the user, which results in my logging settings in /etc/torrc being ignored. This being very annoying to me, I think the incorporation of a cleaner systemd script is not "minor"; I've changed the priority to "normal".

comment:10 Changed 8 months ago by intrigeri

  • Cc intrigeri@… added

comment:11 Changed 8 months ago by intrigeri

According to Michael Stapelberg (who is maintaining systemd in Debian), --quiet should not be used in this case: it loses some valuable input that would otherwise be collected by systemd and displayed with "systemctl status tor" (especially useful if it fails to start, actually).

For the record, I've been working on a systemd profile too. I will merge with the one proposed here. The main area I've been working on is the AppArmor integration (currently done with aa-exec in the Debian initscript), first with a rough shell wrapper, and working with systemd upstream to add native AppArmor profile loading support so that we can get rid of the wrapper eventually.

comment:12 Changed 8 weeks ago by nickm

  • Keywords systemd added

comment:13 Changed 8 weeks ago by misc

I would remove --quiet and set it in the configuration if needed. For --runasdaemon, this is needed for Type=simple, so changing that setting would break the service file ( ie, if it fork when we tell to not fork ), but it can be dropped if we switch to Type=notify as proposed in another bug report.

I am not sure ExecStop is needed, since by default, a SIGTERM is sent. If that's not sufficient, I would say using KillSignal would be better that explicitly using kill, 1 less fork :)

comment:14 Changed 2 weeks ago by andrea

  • Keywords 025-triaged added
Note: See TracTickets for help on using tickets.