Bug#857798: Please add an AppArmor profile for Pulseaudio

[Felipe Sateler]

On Wed, Mar 15, 2017 at 1:57 PM, Ulrike Uhlig  wrote:
> Hi Felipe,
>
 + # install apparmor profile
 + cp debian/apparmor/usr.bin.pulseaudio
 debian/pulseaudio/etc/apparmor.d/usr.bin.pulseaudio

 This would install the file with whatever umask is currently set.
>>>
>>> Thanks for making this clear.
>>> Yes. root:root 644 is correct.
>>
>> Thanks. I have changed this to install -m 644 instead of cp.
>
> Perfect.
>
>> BTW, I still would like an answer to this question:
>>
>> Wouldn't that benefit be best achieved if the profile was shipped
>> by (pulse) upstream?
>>
>> AFAICT, this file should be distro-agnostic, so it should be safe to
>> ship in the upstream package, wouldn't it?
>
> The apparmor profile itself could indeed be part of the upstream package.
>
> Currently, these profiles are worked on collectively by people from
> Ubuntu, Debian/Tails and OpenSuSe and we use a shared Git repository
> between our three distributions.
>
> For torbrowser-launcher we upstreamed the profile for example, also
> because upstream is very responsive about patches. But I have no other
> examples in mind where this would be the case.
>
> Would you care to ask upstream if they'd like to include it?

Better late than never, I have asked upstream and they are receptive
to adding the profile there. Could you please propose a patch on the
upstream mailing list?

https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


-- 

Saludos,
Felipe Sateler

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Ulrike Uhlig]

Hi Felipe,

>>> + # install apparmor profile
>>> + cp debian/apparmor/usr.bin.pulseaudio
>>> debian/pulseaudio/etc/apparmor.d/usr.bin.pulseaudio
>>>
>>> This would install the file with whatever umask is currently set.
>>
>> Thanks for making this clear.
>> Yes. root:root 644 is correct.
> 
> Thanks. I have changed this to install -m 644 instead of cp.

Perfect.

> BTW, I still would like an answer to this question:
> 
> Wouldn't that benefit be best achieved if the profile was shipped
> by (pulse) upstream?
> 
> AFAICT, this file should be distro-agnostic, so it should be safe to
> ship in the upstream package, wouldn't it?

The apparmor profile itself could indeed be part of the upstream package.

Currently, these profiles are worked on collectively by people from
Ubuntu, Debian/Tails and OpenSuSe and we use a shared Git repository
between our three distributions.

For torbrowser-launcher we upstreamed the profile for example, also
because upstream is very responsive about patches. But I have no other
examples in mind where this would be the case.

Would you care to ask upstream if they'd like to include it?

Cheers!
ulrike

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Felipe Sateler]

On Wed, Mar 15, 2017 at 11:56 AM, Ulrike Uhlig  wrote:
> Control: tags + patch
>
> Hi!
>
> Felipe Sateler:
>> On Wed, Mar 15, 2017 at 11:23 AM, Ulrike Uhlig  wrote:
>>> tags + patch
>>>
>>> Hi,
>>>
> I'll try to prepare a patch to make it easier for you to integrate it.

 That would be great.
>>>
>>> Please find a patch attached.
>>
>> Thanks.
>>
>>>
>>> The will simply to copy the file to /etc/apparmor.d/ and only if the
>>> user has AppArmor installed and enabled, this will then confine the
>>> pulseaudio executable. Furthremore, dh_apparmor should create an empty
>>> file /etc/apparmor.d/local/usr.bin.pulseaudio which can be used for
>>> local overrides.
>>
>>
>> + # install apparmor profile
>> + cp debian/apparmor/usr.bin.pulseaudio
>> debian/pulseaudio/etc/apparmor.d/usr.bin.pulseaudio
>>
>> This would install the file with whatever umask is currently set.
>
> Thanks for making this clear.
>
>> Which permissions should the file have? root:root 644 ?
>
> Yes. root:root 644 is correct.

Thanks. I have changed this to install -m 644 instead of cp.

BTW, I still would like an answer to this question:

Wouldn't that benefit be best achieved if the profile was shipped
by (pulse) upstream?

AFAICT, this file should be distro-agnostic, so it should be safe to
ship in the upstream package, wouldn't it?

-- 

Saludos,
Felipe Sateler

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Ulrike Uhlig]

Control: tags + patch

Hi!

Felipe Sateler:
> On Wed, Mar 15, 2017 at 11:23 AM, Ulrike Uhlig  wrote:
>> tags + patch
>>
>> Hi,
>>
 I'll try to prepare a patch to make it easier for you to integrate it.
>>>
>>> That would be great.
>>
>> Please find a patch attached.
> 
> Thanks.
> 
>>
>> The will simply to copy the file to /etc/apparmor.d/ and only if the
>> user has AppArmor installed and enabled, this will then confine the
>> pulseaudio executable. Furthremore, dh_apparmor should create an empty
>> file /etc/apparmor.d/local/usr.bin.pulseaudio which can be used for
>> local overrides.
> 
> 
> + # install apparmor profile
> + cp debian/apparmor/usr.bin.pulseaudio
> debian/pulseaudio/etc/apparmor.d/usr.bin.pulseaudio
> 
> This would install the file with whatever umask is currently set.

Thanks for making this clear.

> Which permissions should the file have? root:root 644 ?

Yes. root:root 644 is correct.

Cheers!
u.

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Felipe Sateler]

On Wed, Mar 15, 2017 at 11:23 AM, Ulrike Uhlig  wrote:
> tags + patch
>
> Hi,
>
>>> I'll try to prepare a patch to make it easier for you to integrate it.
>>
>> That would be great.
>
> Please find a patch attached.

Thanks.

>
> The will simply to copy the file to /etc/apparmor.d/ and only if the
> user has AppArmor installed and enabled, this will then confine the
> pulseaudio executable. Furthremore, dh_apparmor should create an empty
> file /etc/apparmor.d/local/usr.bin.pulseaudio which can be used for
> local overrides.


+ # install apparmor profile
+ cp debian/apparmor/usr.bin.pulseaudio
debian/pulseaudio/etc/apparmor.d/usr.bin.pulseaudio

This would install the file with whatever umask is currently set.
Which permissions should the file have? root:root 644 ?

-- 

Saludos,
Felipe Sateler

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Ulrike Uhlig]

tags + patch

Hi,

>> I'll try to prepare a patch to make it easier for you to integrate it.
> 
> That would be great.

Please find a patch attached.

The will simply to copy the file to /etc/apparmor.d/ and only if the
user has AppArmor installed and enabled, this will then confine the
pulseaudio executable. Furthremore, dh_apparmor should create an empty
file /etc/apparmor.d/local/usr.bin.pulseaudio which can be used for
local overrides.

FYI I've not tried to build the package with this modification.

Let me know if it works out :)

Cheers!
ulrike
diff --git a/apparmor/usr.bin.pulseaudio b/apparmor/usr.bin.pulseaudio
new file mode 100644
index 000..23113ac
--- /dev/null
+++ b/apparmor/usr.bin.pulseaudio
@@ -0,0 +1,117 @@
+# Origin: https://git.launchpad.net/apparmor-profiles/tree/ubuntu/17.04/usr.bin.pulseaudio
+# Last commit: b0d658f9caba715e54b6efd41e298fd9d4511bd9
+#include 
+
+/usr/bin/pulseaudio {
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+
+  dbus send
+   bus=system
+   path=/org/freedesktop/RealtimeKit1
+   interface=org.freedesktop.RealtimeKit1
+   member={MakeThreadRealtime,MakeThreadHighPriority}
+   peer=(name=org.freedesktop.RealtimeKit1),
+
+  dbus send
+   bus=system
+   path=/org/freedesktop/RealtimeKit1
+   interface=org.freedesktop.DBus.Properties
+   member=Get,
+
+  unix (connect, receive, send) type=stream peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
+  ptrace (read,trace) peer=@{profile_name},
+
+  /usr/bin/pulseaudio mixr,
+
+  /etc/pulse/ r,
+  /etc/pulse/* r,
+  /etc/udev/udev.conf r,
+  /etc/timidity/.pulse_cookie w,
+
+  owner @{HOME}/.esd_auth rwk,
+  owner @{HOME}/.pulse-cookie rwk,
+  owner @{HOME}/.config/pulse/cookie rwk,
+  owner @{HOME}/{.config/pulse,.pulse}/ rw,
+  owner @{HOME}/{.config/pulse,.pulse}/* rw,
+
+  owner /run/pulse/ rw,
+  owner /run/pulse/.pulse-cookie rwk,
+  owner /run/pulse/dbus-socket rwk,
+  owner /run/pulse/native rwk,
+  owner /run/pulse/pid rwk,
+  owner /run/user/[0-9]*/pulse/  rw,
+  owner /run/user/[0-9]*/pulse/* rwk,
+  /run/udev/data/+sound:card* r,
+  /run/udev/data/c116:[0-9]* r,
+  /run/udev/data/c14:[0-9]* r,
+
+  # logind
+  /run/systemd/users/[0-9]* r,
+  /run/user/[0-9]*/dconf/user k,
+
+  /sys/bus/ r,
+  /sys/class/ r,
+  /sys/class/sound/ r,
+  /sys/devices/pci[0-9]*/**/*class r,
+  /sys/devices/pci[0-9]*/**/uevent r,
+  /sys/devices/system/cpu/ r,
+  /sys/devices/system/cpu/online r,
+  /sys/devices/virtual/dmi/id/bios_vendor r,
+  /sys/devices/virtual/dmi/id/board_vendor r,
+  /sys/devices/virtual/dmi/id/sys_vendor r,
+  /sys/devices/virtual/sound/**/uevent r,
+
+  /usr/share/alsa/** r,
+  /usr/share/applications/ r,
+  /usr/share/applications/* r,
+  /usr/share/pulseaudio/** r,
+  /usr/lib/pulse-[1-9]*.[0-9]/modules/*.so mr,
+  /usr/lib/pulseaudio/pulse/gconf-helper Cx,
+
+  owner /var/lib/gdm3/.config/pulse/ rw,
+  owner /var/lib/gdm3/.config/pulse/* rw,
+  owner /var/lib/gdm3/.config/pulse/cookie rwk,
+
+  owner /var/lib/lightdm/.Xauthority r,
+  owner /var/lib/lightdm/.esd_auth rwk,
+  owner /var/lib/lightdm/.config/pulse/cookie rwk,
+  owner /var/lib/lightdm/.config/pulse/ rw,
+  owner /var/lib/lightdm/.config/pulse/* rw,
+
+  # are these needed?
+  /var/lib/pulse/ rw,
+  /var/lib/pulse/*-default-sink rw,
+  /var/lib/pulse/*-default-source rw,
+  /var/lib/pulse/*.tdb rw,
+
+  owner @{PROC}/@{pid}/fd/ r,
+  owner @{PROC}/@{pid}/maps r,
+  owner @{PROC}/@{pid}/stat r,
+
+  owner /tmp/pulse-*/pid rwk,
+  owner /tmp/pulse-*/native rwk,
+  owner /tmp/pulse-*/autospawn.lock rwk,
+  owner /run/user/*/pulse/autospawn.lock rwk,
+
+  owner /tmp/orcexec.* mrw,
+  owner /{,var/}run/user/[0-9]*/orcexec.* mrw,
+  # needed if /tmp is mounted noexec:
+  owner @{HOME}/orcexec.* mrw,
+
+  owner /tmp/.esd-@{pid}*/ rw,
+  owner /tmp/.esd-@{pid}*/socket rw,
+
+  profile /usr/lib/pulseaudio/pulse/gconf-helper {
+#include 
+
+/usr/lib/pulseaudio/pulse/gconf-helper mr,
+  }
+
+  # Site-specific additions and overrides. See local/README for details.
+  #include 
+}
diff --git a/debian/rules b/debian/rules
index a16fea9..350fd5a 100755
--- a/debian/rules
+++ b/debian/rules
@@ -60,6 +60,9 @@ override_dh_shlibdeps:
 
 override_dh_install:
 	dh_install --fail-missing
+	# install apparmor profile
+	cp debian/apparmor/usr.bin.pulseaudio debian/pulseaudio/etc/apparmor.d/usr.bin.pulseaudio
+	dh_apparmor --profile-name=usr.bin.pulseaudio -ppulseaudio
 
 override_dh_installdocs:
 	dh_installdocs -A NEWS README AGPL

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Ulrike Uhlig]

Hi Felipe,

thank you for your answer.

Felipe Sateler:
> On Wed, Mar 15, 2017 at 5:07 AM, Ulrike Uhlig  wrote:
>> Package: pulseaudio

> I have some doubts:
> 
> 1. What is the benefit of shipping the profile info in pulseaudio
> versus shipping it in the apparmor-profiles package?

The ultimate aim of the Debian AppArmor team is to have all profiles
shipped in their respective packages. Why? Because the package
maintainers are the ones who know how their package should work and they
are ideally placed to see when something is wrong.

This is also what Ubuntu is doing by the way. They have enabled AppArmor
by default since years to provide users with Mandatory Access Control.

Furthermore, the apparmor-profiles-extra package is supposed to disappear.

> 2. Wouldn't that benefit be best achieved if the profile was shipped
> by (pulse) upstream?

> I'm wary of being in charge of stuff I don't use, and I would think

You should use this kind of stuff ;)
It's super easy to setup see https://wiki.debian.org/AppArmor/HowToUse

> upstream would be as well. Would apparmor maintainers be willing to
> step in to help when problems appear with the profile?

Absolutely. To help you here, we (the AppArmor team) have set up this
documentation: https://wiki.debian.org/AppArmor/Debug If ever people
report bugs against Pulseaudio related to AppArmor, you can invoke help
by the AppArmor team by usertagging such bugs so they will appear on our
radar.

Furthermore, the upstream authors are very responsive, and I'm convinced
they react quickly. FYI upstream can be contacted through
appar...@lists.ubuntu.com

>> I'll try to prepare a patch to make it easier for you to integrate it.
> That would be great.

Ack.

Cheers!
ulrike

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Felipe Sateler]

Control: tags -1 moreinfo

Hi,

On Wed, Mar 15, 2017 at 5:07 AM, Ulrike Uhlig  wrote:
> Package: pulseaudio
> Severity: normal
>
> Hi,
>
> as you might know, AppArmor confines programs according to a set of
> rules that specify what files a given program can access. This approach
> helps protect the system against both known and unknown vulnerabilities.
> In several distributions such as Ubuntu or Tails, AppArmor is enabled by
> default.
>
> There is an AppArmor profile for Pulseaudio available upstream:
> https://git.launchpad.net/apparmor-profiles/tree/ubuntu/17.04/usr.bin.pulseaudio
> I've asked the original authors if this profile is ready to be included
> and they confirmed. In any case, this profile is only active if people
> have installed AppArmor in first case, so it should never break the
> package for users without AppArmor.
>
> The profile can be included in the Pulseaudio packaging quite easily.
> All the necessary steps are documented here:
> https://wiki.debian.org/AppArmor/Contribute/FirstTimeProfileImport
>
> Please also see examples in the packages torbrowser-launcher or in
> Icedove
> (https://anonscm.debian.org/cgit/pkg-mozilla/icedove.git/tree/debian).

I have some doubts:

1. What is the benefit of shipping the profile info in pulseaudio
versus shipping it in the apparmor-profiles package?
2. Wouldn't that benefit be best achieved if the profile was shipped
by (pulse) upstream?

I'm wary of being in charge of stuff I don't use, and I would think
upstream would be as well. Would apparmor maintainers be willing to
step in to help when problems appear with the profile?

>
> I'll try to prepare a patch to make it easier for you to integrate it.

That would be great.

-- 

Saludos,
Felipe Sateler

Bug#857798: Please add an AppArmor profile for Pulseaudio

[Ulrike Uhlig]

Package: pulseaudio
Severity: normal

Hi,

as you might know, AppArmor confines programs according to a set of
rules that specify what files a given program can access. This approach
helps protect the system against both known and unknown vulnerabilities.
In several distributions such as Ubuntu or Tails, AppArmor is enabled by
default.

There is an AppArmor profile for Pulseaudio available upstream:
https://git.launchpad.net/apparmor-profiles/tree/ubuntu/17.04/usr.bin.pulseaudio
I've asked the original authors if this profile is ready to be included
and they confirmed. In any case, this profile is only active if people
have installed AppArmor in first case, so it should never break the
package for users without AppArmor.

The profile can be included in the Pulseaudio packaging quite easily.
All the necessary steps are documented here:
https://wiki.debian.org/AppArmor/Contribute/FirstTimeProfileImport

Please also see examples in the packages torbrowser-launcher or in
Icedove
(https://anonscm.debian.org/cgit/pkg-mozilla/icedove.git/tree/debian).

I'll try to prepare a patch to make it easier for you to integrate it.

Cheers!
u.