On 2016-01-12 07:35 PM, Seth Arnold wrote:
> Thanks! I have some thoughts inline.
I should have made it explicit that this started as a copy of the
Firefox profile. I tried to kept them relatively in sync.
> Diff comments:
>
>> === added file 'ubuntu/16.04/usr.bin.thunderbird'
>> --- ubuntu/16.04/usr.bin.thunderbird 1970-01-01 00:00:00 +0000
>> +++ ubuntu/16.04/usr.bin.thunderbird 2016-01-12 22:16:34 +0000
>> @@ -0,0 +1,274 @@
>> +# vim:syntax=apparmor
>> +# Author: Simon Deziel <simon.deziel at gmail_com>
>> +# This apparmor profile is provided as-is
>> +
>> +# Declare an apparmor variable to help with overrides
>> +@{MOZ_LIBDIR}=/usr/lib/thunderbird
>> +
>> +#include <tunables/global>
>> +
>> +# We want to confine the binaries that match:
>> +# /usr/lib/thunderbird/thunderbird
>> +# /usr/lib/thunderbird/thunderbird
>> +# but not:
>> +# /usr/lib/thunderbird/thunderbird.sh
>> +/usr/lib/thunderbird/thunderbird{,*[^s][^h]} {
>
> I don't understand what the first two "we want to match" lines mean, they look identical to me no matter how much I squint :) -- but I really dislike this profile name. If the attachment specification has to be this complicated, please give the profile a specific profile name like "thunderbird":
Honestly, I never understood the need for this complicated name.
> profile thunderbird /usr/lib/whatnot { ...
>
> We made the mistake of giving firefox a terrible profile name and it upsets me every time I see it. Maybe we can fix it before 16.04 LTS is released...
FF's profile doesn't have it and maybe never had. I think I wanted the
failed thumbnails to not be used because they could be "poisoned". I'll
drop it for the sake of uniformity with FF.
>> +
>> + /etc/mtab r,
>> + /etc/fstab r,
>> +
>> + # Needed for the crash reporter
>> + owner @{PROC}/[0-9]*/environ r,
>> + owner @{PROC}/[0-9]*/auxv r,
>> + /etc/lsb-release r,
>> + /usr/bin/expr ix,
>> + /sys/devices/system/cpu/ r,
>> + /sys/devices/system/cpu/** r,
>> +
>> + # about:memory
>> + owner @{PROC}/[0-9]*/statm r,
>> + owner @{PROC}/[0-9]*/smaps r,
>> +
>> + # Needed for container to work in xul builds
>> + /usr/lib/xulrunner-*/plugin-container ixr,
>> +
>> + # allow access to documentation and other files the user may want to look
>> + # at in /usr and /opt
>> + /usr/ r,
>> + /usr/** r,
>> + /opt/ r,
>> + /opt/** r,
>> +
>> + # so browsing directories works
>> + / r,
>> + /**/ r,
>> +
>> + # per-user thunderbird configuration
>> + owner @{HOME}/.thunderbird/ rw,
>> + owner @{HOME}/.thunderbird/** rw,
>> + owner @{HOME}/.thunderbird/**/storage.sdb k,
>> + owner @{HOME}/.thunderbird/**/*.{db,parentlock,sqlite}* k,
>> + owner @{HOME}/.thunderbird/plugins/** rm,
>> + owner @{HOME}/.thunderbird/**/plugins/** rm,
>> + owner @{HOME}/.cache/thunderbird/ rw,
>> + owner @{HOME}/.cache/thunderbird/** rw,
>> +
>> + #
>> + # Extensions
>> + # /usr/share/.../extensions/... is already covered by '/usr/** r', above.
>> + # Allow 'x' for downloaded extensions, but inherit policy for safety
>> + owner @{HOME}/.thunderbird/**/extensions/** mixrw,
>> + owner @{HOME}/.mozilla/extensions/** mixr,
>> + /usr/share/xul-ext/**/*.sqlite rk,
>> + /usr/lib/xul-ext/**/*.sqlite rk,
>> + /usr/lib/thunderbird-addons/extensions/**/*.sqlite rk,
>> +
>> + deny @{MOZ_LIBDIR}/update.test w,
>> + deny /usr/lib/mozilla/extensions/**/ w,
>> + deny /usr/lib/xulrunner-addons/extensions/**/ w,
>> + deny /usr/share/mozilla/extensions/**/ w,
>> + deny /usr/share/mozilla/ w,
>> +
>> + # Miscellaneous (to be abstracted)
>> + # Ideally these would use a child profile. They are all ELF executables
>> + # so running with 'Ux', while not ideal, is ok because we will at least
>> + # benefit from glibc's secure execute.
>> + /usr/bin/mkfifo Uxr, # investigate
>> + /bin/ps Uxr,
>> + /bin/uname Uxr,
>> +
>> + /usr/bin/gpg Cx -> gpg,
>> +
>> + profile gpg {
>> + #include <abstractions/base>
>> +
>> + # Required to import keys from keyservers
>> + #include <abstractions/nameservice>
>> + #include <abstractions/p11-kit>
>> +
>> + # For smartcards?
>> + /dev/bus/usb/ r,
>> + /dev/bus/usb/[0-9]*/ r,
>> + /dev/bus/usb/[0-9]*/[0-9]* r,
>> +
>> + # LDAP key servers
>> + /etc/ldap/ldap.conf r,
>> +
>> + /usr/bin/gpg mr,
>> + /usr/lib/gnupg/gpgkeys_* ix,
>> + owner @{HOME}/.gnupg r,
>> + owner @{HOME}/.gnupg/gpg.conf r,
>> + owner @{HOME}/.gnupg/random_seed rwk,
>> + owner @{HOME}/.gnupg/pubring.gpg{,~} rw,
>> + owner @{HOME}/.gnupg/secring.gpg rw,
>> + owner @{HOME}/.gnupg/trustdb.gpg rw,
>> + owner @{HOME}/.gnupg/*.gpg.{lock,tmp} rwl,
>> + owner @{HOME}/.gnupg/.#*[0-9] rw,
>> + owner @{HOME}/.gnupg/.#*[0-9]x rwl,
>> + owner /run/user/[0-9]*/keyring-*/gpg rw,
>> +
>> + owner @{HOME}/** r,
>> + owner /tmp/** r,
>> + owner /tmp/encfile wr,
>
> Well now, this is interesting. Normally when you see a hardcoded path in /tmp/ like this you also find an exploitable security bug. We might have Yama to prevent this but other people may not. I wonder if Kurt's seen this yet.
IIRC, TB uses /tmp/encfile first then if it exists it uses encfile-x and
keep incrementing x.
On 2016-01-12 07:35 PM, Seth Arnold wrote:
> Thanks! I have some thoughts inline.
I should have made it explicit that this started as a copy of the
Firefox profile. I tried to kept them relatively in sync.
> Diff comments: 16.04/usr. bin.thunderbird ' 16.04/usr. bin.thunderbird 1970-01-01 00:00:00 +0000 16.04/usr. bin.thunderbird 2016-01-12 22:16:34 +0000 LIBDIR} =/usr/lib/ thunderbird thunderbird/ thunderbird thunderbird/ thunderbird thunderbird/ thunderbird. sh thunderbird/ thunderbird{ ,*[^s][ ^h]} {
>
>> === added file 'ubuntu/
>> --- ubuntu/
>> +++ ubuntu/
>> @@ -0,0 +1,274 @@
>> +# vim:syntax=apparmor
>> +# Author: Simon Deziel <simon.deziel at gmail_com>
>> +# This apparmor profile is provided as-is
>> +
>> +# Declare an apparmor variable to help with overrides
>> +@{MOZ_
>> +
>> +#include <tunables/global>
>> +
>> +# We want to confine the binaries that match:
>> +# /usr/lib/
>> +# /usr/lib/
>> +# but not:
>> +# /usr/lib/
>> +/usr/lib/
>
> I don't understand what the first two "we want to match" lines mean, they look identical to me no matter how much I squint :) -- but I really dislike this profile name. If the attachment specification has to be this complicated, please give the profile a specific profile name like "thunderbird":
Honestly, I never understood the need for this complicated name.
> profile thunderbird /usr/lib/whatnot { ...
>
> We made the mistake of giving firefox a terrible profile name and it upsets me every time I see it. Maybe we can fix it before 16.04 LTS is released...
That would be great. I will try with TB ASAP.
>> + #include <abstractions/ audio> aspell> cups-client> dbus-accessibil ity> dbus-session> gnome> nameservice> p11-kit> private- files> ssl_certs> ubuntu- browsers> ubuntu- helpers> lib/thunderbird /thunderbird{ ,*[^s][ ^h]}, profile_ name}
>> + #include <abstractions/
>> + #include <abstractions/
>> + # TODO: finetune this for required accesses
>> + #include <abstractions/dbus>
>> + #include <abstractions/
>> + #include <abstractions/
>> + #include <abstractions/
>> + #include <abstractions/ibus>
>> + #include <abstractions/
>> + #include <abstractions/
>> + #include <abstractions/
>> + #include <abstractions/
>> + #include <abstractions/
>> + #include <abstractions/
>> +
>> + # for crash reports?
>> + ptrace (read,trace) peer=/usr/
>
> This could be peer=@{
Good point, should also be replicated in FF's profile.
>> + /.{cache, config} /dconf/ user rw, [0-9]*/ dconf/user rw, /.config/ gtk-3.0/ bookmarks r, /.local/ share/gvfs- metadata/ * r, virtual/ block/dm- [0-9]*/ uevent r, ubuntu- browsers. d/firefox> /[0-9]* /net/if_ inet6 r, /[0-9]* /net/ipv6_ route r, /[0-9]* /net/dev r, /[0-9]* /net/wireless r, *buntu/ applications/ defaults. list r, # for all derivatives defaults. list r, xubuntu/ applications/ defaults. list r, /.local/ share/applicati ons/defaults. list r, /.local/ share/applicati ons/mimeapps. list r, /.local/ share/applicati ons/mimeinfo. cache r, wildmidi. cfg r, 2.0*/ r, 2.0*/** r, thunderbird- addons/ ** w, xulrunner- addons/ ** w, xulrunner- */components/ *.tmp w,
>> + # Pulseaudio
>> + /usr/bin/pulseaudio Pixr,
>> +
>> + owner @{HOME}
>> + owner /run/user/
>> + owner @{HOME}
>> + deny owner @{HOME}
>> +
>> + # potentially extremely sensitive files
>> + audit deny @{HOME}/.gnupg/** mrwkl,
>> + audit deny @{HOME}/.ssh/** mrwkl,
>> +
>> + # rw access to HOME is useful when sending/receiving attachments
>> + owner @{HOME}/** rw,
>> +
>> + # Required for LVM setups
>> + /sys/devices/
>> +
>> + # Addons (too lax for thunderbird)
>> + ##include <abstractions/
>> +
>> + # for networking
>> + network inet stream,
>> + network inet6 stream,
>> + @{PROC}
>> + @{PROC}
>> + @{PROC}
>> + @{PROC}
>> +
>> + # should maybe be in abstractions
>> + /etc/ r,
>> + /etc/mime.types r,
>> + /etc/mailcap r,
>> + /etc/xdg/
>> + /etc/xfce4/
>> + /usr/share/
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner /tmp/** m,
>> + owner /var/tmp/** m,
>> + /tmp/.X[0-9]*-lock r,
>> + /etc/udev/udev.conf r,
>> + # Doesn't seem to be required, but noisy. Maybe allow 'r' for 'b*' if needed.
>> + # Possibly move to an abstraction if anything else needs it.
>> + deny /run/udev/data/** r,
>> +
>> + /etc/timezone r,
>> + /etc/wildmidi/
>> +
>> + # thunderbird specific
>> + /etc/thunderbird/ r,
>> + /etc/thunderbird/** r,
>> + /etc/xul-ext/** r,
>> + /etc/xulrunner-
>> + /etc/xulrunner-
>> + /etc/gre.d/ r,
>> + /etc/gre.d/* r,
>> +
>> + # noisy
>> + deny @{MOZ_LIBDIR}/** w,
>> + deny /usr/lib/
>> + deny /usr/lib/
>> + deny /usr/lib/
>> + deny /.suspended r,
>
> Wow. What the heck are they even trying to do??
Really no clue. Again it was copied verbatim from FF's profile.
>> + deny /boot/initrd.img* r, fontconfig/ w, /.local/ share/recently- used.xbel r, gconftool- 2 x, /[0-9]* /cmdline r, /[0-9]* /mountinfo r, /[0-9]* /task/[ 0-9]*/stat r, /[0-9]* /status r, pci[0-9] */**/uevent r, /.thumbnails/ */*.png r, /.cache/ thumbnails/ */*.png r, /.cache/ thumbnails/ fail/*. png r,
>> + deny /boot/vmlinuz* r,
>> + deny /var/cache/
>> + deny @{HOME}
>> + deny @{HOME}/.* r,
>> +
>> + # TODO: investigate
>> + deny /usr/bin/
>> +
>> + # These are needed when a new user starts thunderbird and thunderbird.sh is used
>> + @{MOZ_LIBDIR}/** ixr,
>> + /usr/bin/basename ixr,
>> + /usr/bin/dirname ixr,
>> + /usr/bin/pwd ixr,
>> + /sbin/killall5 ixr,
>> + /bin/which ixr,
>> + /usr/bin/tr ixr,
>> + @{PROC}/ r,
>> + @{PROC}
>> + @{PROC}
>> + @{PROC}/[0-9]*/stat r,
>> + owner @{PROC}
>> + @{PROC}
>> + @{PROC}/filesystems r,
>> + /sys/devices/
>> + owner @{HOME}
>> + owner @{HOME}
>> + deny @{HOME}
>
> What's wrong with allowing this?
FF's profile doesn't have it and maybe never had. I think I wanted the
failed thumbnails to not be used because they could be "poisoned". I'll
drop it for the sake of uniformity with FF.
>> + /[0-9]* /environ r, system/ cpu/ r, system/ cpu/** r, /[0-9]* /statm r, /[0-9]* /smaps r, xulrunner- */plugin- container ixr, /.thunderbird/ rw, /.thunderbird/ ** rw, /.thunderbird/ **/storage. sdb k, /.thunderbird/ **/*.{db, parentlock, sqlite} * k, /.thunderbird/ plugins/ ** rm, /.thunderbird/ **/plugins/ ** rm, /.cache/ thunderbird/ rw, /.cache/ thunderbird/ ** rw, .../extensions/ ... is already covered by '/usr/** r', above. /.thunderbird/ **/extensions/ ** mixrw, /.mozilla/ extensions/ ** mixr, xul-ext/ **/*.sqlite rk, xul-ext/ **/*.sqlite rk, thunderbird- addons/ extensions/ **/*.sqlite rk, /update. test w, mozilla/ extensions/ **/ w, xulrunner- addons/ extensions/ **/ w, mozilla/ extensions/ **/ w, nameservice> p11-kit> usb/[0- 9]*/ r, usb/[0- 9]*/[0- 9]* r, gnupg/gpgkeys_ * ix, /.gnupg/ gpg.conf r, /.gnupg/ random_ seed rwk, /.gnupg/ pubring. gpg{,~} rw, /.gnupg/ secring. gpg rw, /.gnupg/ trustdb. gpg rw, /.gnupg/ *.gpg.{ lock,tmp} rwl, /.gnupg/ .#*[0-9] rw, /.gnupg/ .#*[0-9] x rwl, [0-9]*/ keyring- */gpg rw,
>> + /etc/mtab r,
>> + /etc/fstab r,
>> +
>> + # Needed for the crash reporter
>> + owner @{PROC}
>> + owner @{PROC}/[0-9]*/auxv r,
>> + /etc/lsb-release r,
>> + /usr/bin/expr ix,
>> + /sys/devices/
>> + /sys/devices/
>> +
>> + # about:memory
>> + owner @{PROC}
>> + owner @{PROC}
>> +
>> + # Needed for container to work in xul builds
>> + /usr/lib/
>> +
>> + # allow access to documentation and other files the user may want to look
>> + # at in /usr and /opt
>> + /usr/ r,
>> + /usr/** r,
>> + /opt/ r,
>> + /opt/** r,
>> +
>> + # so browsing directories works
>> + / r,
>> + /**/ r,
>> +
>> + # per-user thunderbird configuration
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> +
>> + #
>> + # Extensions
>> + # /usr/share/
>> + # Allow 'x' for downloaded extensions, but inherit policy for safety
>> + owner @{HOME}
>> + owner @{HOME}
>> + /usr/share/
>> + /usr/lib/
>> + /usr/lib/
>> +
>> + deny @{MOZ_LIBDIR}
>> + deny /usr/lib/
>> + deny /usr/lib/
>> + deny /usr/share/
>> + deny /usr/share/mozilla/ w,
>> +
>> + # Miscellaneous (to be abstracted)
>> + # Ideally these would use a child profile. They are all ELF executables
>> + # so running with 'Ux', while not ideal, is ok because we will at least
>> + # benefit from glibc's secure execute.
>> + /usr/bin/mkfifo Uxr, # investigate
>> + /bin/ps Uxr,
>> + /bin/uname Uxr,
>> +
>> + /usr/bin/gpg Cx -> gpg,
>> +
>> + profile gpg {
>> + #include <abstractions/base>
>> +
>> + # Required to import keys from keyservers
>> + #include <abstractions/
>> + #include <abstractions/
>> +
>> + # For smartcards?
>> + /dev/bus/usb/ r,
>> + /dev/bus/
>> + /dev/bus/
>> +
>> + # LDAP key servers
>> + /etc/ldap/ldap.conf r,
>> +
>> + /usr/bin/gpg mr,
>> + /usr/lib/
>> + owner @{HOME}/.gnupg r,
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner @{HOME}
>> + owner /run/user/
>> +
>> + owner @{HOME}/** r,
>> + owner /tmp/** r,
>> + owner /tmp/encfile wr,
>
> Well now, this is interesting. Normally when you see a hardcoded path in /tmp/ like this you also find an exploitable security bug. We might have Yama to prevent this but other people may not. I wonder if Kurt's seen this yet.
IIRC, TB uses /tmp/encfile first then if it exists it uses encfile-x and
keep incrementing x.
I'll "audit" this.
Thanks for the review!
Simon