Hi, as mentioned on #761403: Another change that we'll want to make is to use the AppArmorProfile= unit file directive, to avoid a regression for AppArmor users. This option only exists in systemd v210+ (and I had it added precisely to support this usecase, actually), but is currently disabled in the Debian package: #760526. The systemd maintainers seem to be open to enable it. (Will mark this bug as blocked by #761403 and #760526 once I get its number.) Cheers, -- intrigeri
Hi weasel, this does the job for me: --- a/debian/tor.service +++ b/debian/tor.service @@ -17,11 +17,13 @@ Restart=on-failure LimitNOFILE=65536 # Hardening +AppArmorProfile=system_tor PrivateTmp=yes PrivateDevices=yes ProtectHome=yes ProtectSystem=full ReadOnlyDirectories=/ +ReadWriteDirectories=-/proc ReadWriteDirectories=-/var/lib/tor ReadWriteDirectories=-/var/log/tor ReadWriteDirectories=-/var/run I've explained on https://trac.torproject.org/projects/tor/ticket/16782 why write access to /proc is needed. I've confirmed that works fine both with and without AppArmor enabled. I've also tested it with obfs4proxy, and with "Sandbox 1" (independently since these two are not compatible, but both with AppArmor enabled). Note that it requires systemd >= 218-4, which has been in testing since the end of June. I'll let you judge if a versioned dependency is needed for a nicer upgrade path from Jessie and/or for backports. Cheers, -- intrigeri
The Debian package for Tor now supports multiple instances. https://gitweb.torproject.org/debian/tor.git/tree/debian/tor-instance-create.8.txt https://gitweb.torproject.org/debian/tor.git/tree/debian/tor-instance-create https://gitweb.torproject.org/debian/tor.git/tree/debian/systemd Any idea how to best deal with apparmor here? Should we create one /etc/apparmor.d/system_tor like system for each instance? Can we create them at service start time or do we have to create them at instance creation time? Is there some templating magic we can use? Cheers,
Hi, Peter Palfrader wrote (19 Oct 2015 17:43:23 GMT) : Meta: I've been working for years to have the move to systemd not introduce a regression for us on the AppArmor front, and it currently does ⇒ I'd love to see essentially the regression fixed now for the *default* instance (that is, applying essentially the patch I've proposed on this bug applied, modulo it is now about tor@default.service), and then deal separately with confining the non-default instances. In other words, let's first deal with regressions, then polish newly introduced features. What do you think? If we agree on that I'll prepare/test/submit an updated patch and will clone+retitle this bug for the "confine non-default instances" bits. but here's a first step. If AppArmorProfile= is applied _after_ ExecStartPre=, then we can probably use the latter to dynamically generate a per-instance profile from within a unit file itself. This would be ideal, and I think we should try that first. Else, we would have to generate the profile at instance creation time. I'd rather not to go this way: I don't want to see all non-default instances broken some day on AppArmor-enabled Debian systems because of obsolete profiles generated many months ago. I'd rather see non-default instances not confined with AppArmor at all, or using a single profile and relying on DAC only for isolating instances from each other. But hopefully we won't even have to think about this case :) I don't think so. Cheers,
fyi, from the debian-0.2.7 branch:
From fcdeff2c1d84dd984d61bab73bb81ef08b31674e Mon Sep 17 00:00:00 2001
From: Peter Palfrader <peter@palfrader.org>
Date: Tue, 20 Oct 2015 17:15:18 +0200
Subject: [PATCH] Enable apparmor support for the default tor service (re:
#761404).
Apparmor is not yet being enabled for any other tor instance.
---
debian/changelog | 4 +++-
debian/systemd/tor@default.service | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/debian/changelog b/debian/changelog
index 18b3cd5..c603cf8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -5,8 +5,10 @@ tor (0.2.7.3-rc-1.1) experimental; urgency=medium
* Support multiple instances (closes: #791393).
* Split systemd service timeout into start and stop timeout, and raise
them to 120 and 60 seconds from 45 (closes: tor#16398).
+ * Enable apparmor support for the default tor service (re: #761404).
+ Apparmor is not yet being enabled for any other tor instance.
- -- Peter Palfrader <weasel@debian.org> Tue, 20 Oct 2015 17:09:01 +0200
+ -- Peter Palfrader <weasel@debian.org> Tue, 20 Oct 2015 17:14:23 +0200
tor (0.2.7.3-rc-1) experimental; urgency=medium
diff --git a/debian/systemd/tor@default.service b/debian/systemd/tor@default.service
index 7910d24..52ba846 100644
--- a/debian/systemd/tor@default.service
+++ b/debian/systemd/tor@default.service
@@ -20,12 +20,14 @@ Restart=on-failure
LimitNOFILE=65536
# Hardening
+AppArmorProfile=system_tor
NoNewPrivileges=yes
PrivateTmp=yes
PrivateDevices=yes
ProtectHome=yes
ProtectSystem=full
ReadOnlyDirectories=/
+ReadWriteDirectories=-/proc
ReadWriteDirectories=-/var/lib/tor
ReadWriteDirectories=-/var/log/tor
ReadWriteDirectories=-/var/run
I really dislike the fact that there now is feature disparity between the default instance and the others. Hopefully we can fix this quickly. That'd be great. Can profiles live outside of /etc/apparmor?
Peter Palfrader wrote (20 Oct 2015 15:16:36 GMT) : Thanks!
Hi,
Peter Palfrader wrote (20 Oct 2015 18:50:38 GMT) :
I have good news. It works, here's a PoC:
$ cat /etc/systemd/system/tor.service.d/apparmor.conf
[Service]
ExecStartPre=/bin/sh -c "/bin/sed \
-e 's/profile system_tor/profile default_system_tor/' \
/etc/apparmor.d/system_tor \
| /sbin/apparmor_parser --skip-cache --replace"
AppArmorProfile=default_system_tor
ReadWriteDirectories=-/proc
# apparmor_parser needs write access there:
ReadWriteDirectories=-/sys/kernel/security/apparmor
I've tested this on 0.2.7.3-rc-1, after making sure sure systemd won't
be using the profile that was already loaded by previous attempts:
/bin/sed -e 's/profile system_tor/profile default_system_tor/' \
/etc/apparmor.d/system_tor \
| sudo /sbin/apparmor_parser --skip-cache --remove
sudo aa-status
And after `sudo service tor restart', aa-status(8) confirms that my
system Tor daemon runs under the 'default_system_tor' profile, that is
necessarily the one we've generated and loaded on-the-fly :)
Now, the ExecStartPre= that generates and loads the profile should be
turned into a no-op when AppArmor is not enabled, e.g. re-using code
we have in /etc/init.d/tor. And it would be nice to move it to
a dedicated executable, instead of being inlined in the systemd unit.
Can you (please) take it over from this point and integrate it, in
your preferred way, with the rest of the packaging?
A profile can live wherever we want, *but* it must be loaded into the
kernel (with apparmor_parser(8)) before we can use it; that is, before
systemd calls the aa_change_onexec libapparmor function. That's why
AppArmorProfile= takes a profile name, not a file name. We actually
don't need any file at all, since apparmor_parser can consume stdin,
as demonstrated by the PoC above.
Cheers,
Hi, My system copy of tor wasn't starting in the background. It was because I'd put some customizations into /etc/torrc.custom, (as suggested by the comments at the bottom of /etc/tor/torrc. However the apparmor profile in (abstractions/system_tor) limit tor to be able to only read /etc/tor/. Could either the config file suggest using /etc/tor/torrc.custom, or modifying the apparmor profile to allow reading /etc/torrc.custom. I wasn't sure if this should go to this open bug, or get its own new wishlist bug. Diane
Hi Diane, Diane Trout: Thanks for this report. Changing the recommended path would be painful for those who have already followed the previous set of recommendations and I trust weasel to have chosen these paths carefully. So IMO we should simply adjust the AppArmor profile:--- a//etc/apparmor.d/abstractions/tor 2018-01-16 09:49:46.000000000 +0000 +++ b//etc/apparmor.d/abstractions/tor 2018-01-29 08:49:34.583943603 +0000 @@ -24,6 +24,9 @@ /sys/devices/system/cpu/** r, /etc/tor/* r, + /etc/torrc.custom r, + /etc/torrc.d/ r, + /etc/torrc.d/* r, /usr/share/tor/** r, /usr/bin/obfsproxy PUx, Please test and report back :) I believe this is off-topic on this bug report so I'm cloning it to a new one. Please follow-up on the new one. Cheers,
Hi Peter! intrigeri: Is there anything I can do to increase the chances the fix I've proposed is applied to the Debian packaging? Cheers,
Currently tor can't read configs from under /etc/tor/ sub directories like /etc/tor/torrc.d/* because of apparomor restrictions. Let allow this to make "%include /etc/tor/torrc.d/*.conf" to work: diff --git a/tor b/tor index 0b6c08e..49f0a25 100644 --- a/tor +++ b/tor @@ -24,7 +24,7 @@ /sys/devices/system/cpu/ r, /sys/devices/system/cpu/** r, - /etc/tor/* r, + /etc/tor/** r, /usr/share/tor/** r, /usr/bin/obfsproxy PUx,