interfaces/builtin,docs,snap: add the pulseaudio interface

[zyga]

This patch adds Classic-only interface for pulseaudio. This interface
allows applications with connected slots to talk to the pulseaudio
socket and to access the shared memory files created by pulse.

The new interface is implicitly added to the OS snap, it is also
automatically connected to clients.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

[jdstrand]

Please use 'rk' instead. Touch policy doesn't require 'w' and the 'mlix' accesses are wrong.

[jdstrand]

Also, use: '/{run,dev}/' instead of '/dev/'. Some Personal devices will use /run/ instead.

[zyga]

Done

[jdstrand]

This should be:
owner /{,var/}run/user/*/pulse/ r,
owner /{,var/}run/user/*/pulse/native rwk,

On Touch we also had:
owner /{,var/}run/user/*/pulse/ w, # shouldn't be needed, but rmdir fail otherwise
but let's hold off on that write access for the directory until we know we need it.

[jdstrand]

Honestly, I'm surprised you don't also need:
owner @{HOME}/.pulse/ r,
owner @{HOME}/.pulse/* rk,

[zyga]

$HOME is the snap's HOME directory

[jdstrand]

Note that this is actually pretty dangerous because it means that any app can be uploaded, be installed, be autoconnected, run in the background and record audio without the user having any idea it is happening. On Touch pulseaudio was adjusted to integrate with trust-store and prompt the user when an application tried to record audio (playback was always allowed) and this feature was borne out of both security team and customer requirements. This works well on Touch because there is a guaranteed UI that can be used, the user is in full control via System Settings and there are no background services on Touch.

Snappy changes all of that-- the pulseaudio on classic doesn't have the trust-store integration (though it could) and snappy allows background services and there may be no UI for IoT systems. IMO for this to be auto-connectable it should be limited to playback only, which will require an SRU for pulseaudio on classic to disallow recording if the security label of the connecting process starts with "snap.". The patch to Touch's stable-phone-overlay should be adaptable enough for this.

That buys us time regarding how to design safe recording (perhaps it is a different interface or an attribute of the pulseaudio interface, which could be implemented as on interface connect/disconnect for recording, snapd updates pulseaudio's trust-store, then much of the existing code could be reused. Other implementations are possible).

[zyga]

I think this is reasonable. We should perhaps consider SRU-ing the trust store as well so that applications like firefox can use this for media recording (hangouts, certain type of games, etc).

[niemeyer]

Just for pondering, if we wanted to split the recording from the playback interface, how could we do that?

[jdstrand]

@niemeyer (cc @vosst ) - this is what I was getting at in my last paragraph: "That buys us time regarding how to design safe recording (perhaps it is a different interface or an attribute of the pulseaudio interface, which could be implemented as on interface connect/disconnect for recording, snapd updates pulseaudio's trust-store, then much of the existing code could be reused. Other implementations are possible)."

To put more directly, we need pulseaudio to make a choice on whether to allow playback and/or recording. The patch for Touch does this by (in essence):

1. allowing anyone to playback
2. if recording, using libapparmor to get the security label
3. cross-checking that security label with trust-store (a dynamically linked library)
4. checking trust-store to see if the security-label exists in the db. If not, prompt the user to see if this app should be allowed to record and save the answer, otherwise go to the next step
5. trust-store checking the user's response and allows/disallows recording based on that

(the user can later adjust the microphone settings via System Settings)

Considering the above, to support splitting recording from playback, pulseaudio needs to either be told what is allowed to record or query the system on what is allowed to record.

For the first and with the current pulseaudio trust-store patchset, it should be relatively straightforward for, upon record interface connect, snapd to add an entry to pulseaudio's trust-store db (an sqlite db iirc) to say the app may record, and upon disconnect to say the app may not record. We would probably upon install of the snap also want to populate pulseaudio's trust-store with 'may not record' for this snap so we would avoid prompting (since we won't autoconnect).

Alternatively we can turn this around and have pulseaudio still use trust-store, but adjust trust-store to talk to snapd to ask if the client process' snap is connected or not. Or we replace trust-store with a small library that talks to snapd.

Both approaches have their merits. The first approach is probably faster, doesn't require a new snapd API and won't require giving pulseaudio access to the sensitive snapd-control interface. The 2nd approach feels cleaner but would require careful design so that snaps providing slots like this don't have access to the entire rest API.

I think it is important to understand that the path chosen here will have a direct impact on Ubuntu Personal, since many 'trusted helpers' on Touch are like pulseaudio and use trust-store in some manner. It should also be understood that there are customer requirements for Touch/Personal that state there must be a UI prompt for first time access (in the manner implemented) for microphone, video recording, and location, so while right this second we wouldn't have to implement a UI prompt, the design should support this going forward. I highly recommend talking to tvoss since he is the technical architect for Touch/Personal, understands the requirements for it and designed trust-store and its prompting.

[niemeyer]

Thanks for these insights, @jdstrand. I'm tempted to actually move on with a simpler pulseaudio interface that allows both playback and recording, and leave the recording constraint for later. But we should at least understand how we want that to work in the future so we're not putting ourselves in a corner. I will talk to tvoss before moving forward.

[jdstrand]

@niemeyer - @morphis created a PoC pulseaudio plugin that examines the apparmor label of the connecting process and if it starts with 'snap.', disallow recording. Doing this in the immediate term would satisfy the security concerns until the proper mediation for recording is available.

[niemeyer]

Let's please figure what we want to do about those before moving this interface in.

[niemeyer]

This has been discussed and sorted at the sprint. The agreement is to move on with this as-is, using the native path format, and also to open up tag-prefixed read/writes within the process itself so that applications have a way to be unblocked for their own needs.

In other words, cross-application will now require interfaces, which matches with our practices, but internal usage can be properly confined without talking to anyone by using the proper path.

[jdstrand]

IIUC you are referring to the shm accesses and I believe what you outlined here is allow this shm access for pulseaudio and to also allow what is in #1135. If not, please respond in whichever PR you feel is the correct place to respond. :)

[zyga]

Simon has now patched pulseaudio to disable recording. We are closer to being able to land this for everyone: https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/1583057

[jdstrand]

@zyga and @morphis - great news on disabling recording. @niemeyer and I discussed this in IRC today and feel that pulseaudio interface should not be blocked on the pulseaudio SRU. Instead, commit it then land pulseaudio SRU soon. This way everyone gets playback now, recroding gets blocked soon and then we can design a pulseaudio-record interface or 'record' attribute in the fullness of time.

[morphis]

@jdstrand Sounds good to me. I also have a first shot on getting the slot side of this ready. Will push a PR on top of this in a bit.