[pulseaudio-discuss] Access control

[Timothy Hobbs]

Obviously, this question reveals my total ignorance of pulseaudio 
architecture, but why are you implementing access control in pulseaudio 
itself, rather than using a firewall wrapper that parses the info being 
sent down the pulse audio socket and only lets allowed RPC calls through?


On 02/15/2017 09:53 AM, Tanu Kaskinen wrote:
> On Tue, 2017-02-14 at 13:28 +0100, Wim Taymans wrote:
>> On 13 February 2017 at 13:58, Tanu Kaskinen <tanuk at iki.fi> wrote:
>>> On Fri, 2017-01-27 at 17:15 +0100, Wim Taymans wrote:
>>>> Hi All,
>>>>
>>>> I think (1) is quite ugly and requires you to modify many functions
>>>> with a new parameter that
>>>> is not at all related to what the function does.
>>> I disagree with the characterization that the client parameter is not
>>> related to what the function does. If the function does access control,
>>> then the client parameter is very much related to that.
>> Together with things such as logging,configuration and allocation, security
>> is typically a Cross-cutting concern
>> (https://en.wikipedia.org/wiki/Cross-cutting_concern).
>> The methods that operate on objects should not include the parameters for
>> the cross-cutting concern, you would typically keep those stored in a global
>> environment or singleton (like how we have the logging in a singleton, the
>> mempool and configuration in pa_core, ..).
>> We should to the same with the current client, IMHO.
> The existing stuff in pa_core can't be removed and replaced with
> function arguments, as far as I can tell. The current_client field can.
>
> I think having current_client would be completely fine, though, if it
> really was only used to store the client whose request is currently
> being processed. However, in IRC we talked about needing to set
> current_client before calling pa_core_get_sinks() when rescuing
> streams, for example. That's quite clearly using a global variable for
> just passing a function argument. (This particular problem could be
> side-stepped by adding the client argument to pa_core_get_sinks() and
> the other similar functions, but otherwise keep using current_client.
> As I see it, pa_core_get_sinks() just provides a client-specific view
> of the sink list, and it could as well be implemented by storing the
> filtered sink list in pa_client, but maintaining that might be more
> work than creating the filtered list on demand.)
>
> Aren't there also going to be problems when one operation involves
> another operation, if the first operation is allowed for the client but
> second operation isn't? The first operation will fail even though it
> was supposed to be allowed. For example, module-bluetooth-policy will
> automatically switch the card profile from A2DP to HSP when a source
> output appears with media.role=phone. I think that shouldn't be
> prevented just because the client that created the source output
> doesn't have the right to switch card profiles by itself. I guess
> you'll end up setting current_client in module-bluetooth-policy to NULL
>   before the profile switch while the client still has its stream
> creation operation ongoing. (I now realize this wasn't a perfect
> example of the problem I first stated, because the source output
> creation doesn't fail completely. Maybe a better example would have
> been a client that tries to set its stream volume, but it fails because
> flat volumes are enabled and the client doesn't have the permission to
> set sink volumes.)
>
> Another example is loading a module: the client's permissions affect
> what the module can do during the initialization, which from some
> perspective may make sense, but in any case that causes
> inconsistencies, because the client's access limitations stop affecting
> the module as soon as the initialization is completed.
>
> But if you think the current_client approach is the best one, I don't
> want to override your preference.
>