[pkg-aa-profiles-team] a bunch of improvements i don't know where to throw
intrigeri
intrigeri at debian.org
Thu Jun 19 17:49:03 UTC 2014
Hi anarcat,
first, thanks a lot for trying out this stuff!
anarcat wrote (19 Jun 2014 14:28:09 GMT) :
> I have installed apparmor-profiles and dumped all the "extras" into
> /etc/apparmor.d (yes, i'm like that).
Wow, yeah, you're like that. Never dared doing that given what the
README in that directory says. For the profiles taken from
/usr/share/doc/apparmor-profiles/extras/, pkg-aa-profiles-team hasn't
volunteered to maintain these in Debian, and AFAIK probably nobody
really has; that's why they are in /usr/share/doc :)
So, I think the best place to report issues is the upstream
mailing-list: apparmor at lists.ubuntu.com. They're very nice and all,
and they love eating pull requests and patches for breakfast.
That's why there'll be a lot of "Want to take this upstream?" below.
> First off, one thing that is totally broken is the "chromium"
> abstraction from lightdm: that just needs to go, otherwise we get silly
> warnings that make aa-logprof basically unusable. I lost the error
> message, but removing
> apparmor.d/abstractions/ubuntu-browsers.d/chromium-browser
> and fixing apparmor.d/lightdm-guest-session so it doesn't include it
> fixed it for me.
I believe I've had this fixed in the latest lightdm package (by simply
dropping the lightdm-guest-session profile, since it attempts to
confine functionality that is not shipped in the Debian package
anyway). I'd love to see this confirmed.
> The second thing is that aa-logprof really makes it difficult to review
> changes to apparmor.d with versionning tools.
I have to shamelessly admit I've never used aa-logprof.
> Part of this is because the apparmor.d files use arbitrary order for
> the file flags, so we get stuff like:
> [...]
> That's probably not all of them, and it's really silly: we should
> probably just sanitise those files to the same format as outputted by
> aa-logprof using some tool before they are committed to the package.
I see what you mean, and I don't want you to suffer because of such
inconsistencies. I think your proposed solution makes sense. Shall we
(pkg-aa-profiles) take this matter upstream, or do you want to give it
a try?
> Finally, we use dash. Everywhere. Almost every package out there I could
> find, I had to do:
> /bin/bash mixr,
> + /bin/dash mixr,
Indeed:
$ rgrep --files-with-match bin/bash /usr/share/doc/apparmor-profiles/extras/ | wc -l
21
$ rgrep --files-with-match /bin/bash /usr/share/doc/apparmor-profiles/extras/ \
| xargs grep --files-with-match /bin/dash
1
... this tells a *lot* about the current state of maintenance of these
"extra" profiles, especially considering Ubuntu has switched to
dash-as-default-/bin/sh way before Debian.
> (it's also terrifying to see the number of things that call bash...)
Yeah, that's one of the "interesting" side-effects of using a MAC tool
such as AppArmor: on gets to discover what external tools our programs
actually call...
> So here's my gigantic patch of messed up improvements. I'm sorry that I
> can't submit this in a more piecemeal fashion, but I hope that can be
> picked up and factored in at some point. I did try to do a significant
> amount of work to avoid noise in the diff, so it should still be pretty
> readable.
Thanks a lot. I'm commenting inline below.
> --- a/apparmor.d/abstractions/p11-kit
> +++ b/apparmor.d/abstractions/p11-kit
> @@ -12,6 +12,7 @@
> /etc/pkcs11/pkcs11.conf r,
> /etc/pkcs11/modules/ r,
> /etc/pkcs11/modules/* r,
> + /usr/share/p11-kit/modules/ r,
Fixed upstream already.
> --- a/apparmor.d/etc.cron.daily.logrotate
> +++ b/apparmor.d/etc.cron.daily.logrotate
> [...]
Looks alright at first glance. Want to submit it upstream?
> --- a/apparmor.d/program-chunks/postfix-common
> +++ b/apparmor.d/program-chunks/postfix-common
> @@ -15,7 +15,9 @@
> capability sys_chroot,
> /etc/postfix/*.cf r,
> + /etc/postfix/my* r,
No idea what's this one about. Upstream would want a rationale,
I guess, unless they know Postfix better than me.
> + /{,var/spool/postfix/}etc/localtime r,
Looks good, worth upstreaming!
> --- a/apparmor.d/usr.bin.man
> +++ b/apparmor.d/usr.bin.man
> [...]
> + #include <abstractions/lightdm>
Uh?
> [...]
> +
> +
> + /home/*/.lesshst r,
> + /root/.lesshst r,
These ones want to be merged using @{HOME}, and prefixed by "owner".
> /usr/lib/man-db/man Px,
> + /var/cache/man/** mrwkix,
"ix", really? I have no executable file there on my system.
"m" surprises me a bit too, but why not.
> --- a/apparmor.d/usr.bin.procmail
> +++ b/apparmor.d/usr.bin.procmail
> @@ -24,6 +24,10 @@
> # users want procmail to do many things:
> @{HOME}/** rwl,
> + /var/mail/* w,
The included user-mail abstraction grants 'rwl' access to
/var/spool/mail/*, which is (at least on my system) a symlink to
/var/mail. So, I think this belongs to that abstraction instead.
> + # necessary on my postfix setup
> + /var/spool/postfix/* w,
> + /var/spool/postfix/*/ w,
The great news is that profiles managed with dh_apparmor have this
fancy "#include local/..." facility. Of course, it's not the case with
profiles shipped in /usr/share/doc.
Now, for the rest of the Postfix profiles, everything looks good to me
(as in: I trust you that it's needed to make things work, and doesn't
look scary) => should be upstreamed. I'm only commenting on things I'm
less sure about.
> --- a/apparmor.d/usr.lib.postfix.bounce
> +++ b/apparmor.d/usr.lib.postfix.bounce
> @@ -21,9 +21,11 @@
> /usr/lib/postfix/bounce rmix,
> + /{var/spool/postfix/,}active/* rwk,
> /{var/spool/postfix/,}active/[0-9A-F]/[0-9A-F]/* rwl,
> /{var/spool/postfix/,}active/[0-9A-F]/[0-9A-F]/ rwl,
> /{var/spool/postfix/,}active/[0-9A-F]/ rwl,
> + /{var/spool/postfix/,}bounce/* rwk,
Can't these two ones be restrained a bit more, e.g. similarly to the
existing lines?
> --- a/apparmor.d/usr.lib.postfix.smtpd
> +++ b/apparmor.d/usr.lib.postfix.smtpd
> @@ -37,6 +38,7 @@
> /etc/postfix/smtpd_scache.pag rw,
> /etc/postfix/main.cf r,
> /etc/postfix/prng_exch rw,
> + /etc/postfix/my* r,
Same as above, needs a rationale.
> diff --git a/apparmor.d/usr.sbin.sshd b/apparmor.d/usr.sbin.sshd
> index 67da06d..a547262 100644
> --- a/apparmor.d/usr.sbin.sshd
> +++ b/apparmor.d/usr.sbin.sshd
> @@ -21,6 +21,7 @@
> #include <abstractions/consoles>
> #include <abstractions/nameservice>
> #include <abstractions/wutmp>
> + #include <abstractions/openssl>
I'm surprised. Any idea why?
> @@ -31,6 +32,7 @@
> capability setgid,
> capability setuid,
> capability audit_control,
> + capability audit_write,
If you say so :)
> @@ -43,7 +45,7 @@
> /proc/*/oom_adj rw,
> /proc/*/oom_score_adj rw,
> /usr/sbin/sshd mrix,
> - /var/log/btmp r,
> + /var/log/btmp rw,
> /{,var/}run w,
> /{,var/}run/sshd{,.init}.pid wl,
> I'll keep on trying to improve on this as I go along, but really, it's
> annoyingly difficult to do so when there's so much noise in the diff.
> Fixing the order of the flags in apparmor-profiles seems like a must if
> we want this maintainable in the long term.
Yes. It seems that the "extras" upstream profiles desperately need
users (if there were, these bugs would have been spotted earlier), and
likely interested people to send patches. Maybe we've just
found someone?
TBH, I personally won't spend much time on these profiles until the
Jessie freeze: for Jessie, I'd rather instead focus on integrating
into Debian existing well-tested and mature profiles, such as some of
the ones Ubuntu turns on by default. But I'm happy to act as
a first-level filter, like I just did, if it may help. I'd be
delighted to see these profiles reach a state when I can just drop it
and see it work, to start with :)
Thanks again!
Cheers,
--
intrigeri
More information about the Pkg-aa-profiles-team
mailing list