[pulseaudio-discuss] [PATCH 0/5] Add access control to protocol-native

[David Henningsson]

On 2016-06-24 02:30, Ahmed S. Darwish wrote:
> Hi,
>
> Apologies for resurrecting this year-old thread. [*] Hopefully we
> can finish this, with Flatpak integration as an access-control
> module, before v10.0 ;-) [1] [2] [3]
>
> Some questions below..
>
> On 2015-07-17 13:02, David Henningsson wrote:
>>
>> On 2015-04-07 17:13, Wim Taymans wrote:
>>> This is a new patch series that preplaces the previous patches regarding
>>> access control.
>>>
> ...
>>>
>>> There is an example access control module that shows how one could implement
>>> client specific access control checks.
>>>
>>
> ...
>>
>> A few design thoughts that I think we need to resolve first before
>> reviewing details:
>>
>> 1. Coupling between core and native-protocol. Right now the hooks are in
>> core, and mapped 1-to-1 (or? are there exceptions?) to PA_COMMAND_*.
>>
>> Other options would be:
>>    - Just have a "pa_hook access[PA_COMMAND_MAX]" in pa_core instead?
>> Then we could skip the long list of PA_ACCESS_HOOK_ constants. However,
>> the increased dependency between the native protocol and the pa_core
>> object might be undesirable.
>>    - On the other side, we could have the "pa_hook
>> access[PA_COMMAND_MAX]" in pa_native_protocol instead. However, native
>> protocol instances could - at least in theory - come and go, so they
>> probably need stored somewhere more reachable anyhow...
>>
>> To sum up, I feel that since the hooks are specific to the native
>> protocol, they should be put closer to that protocol, but I can't point
>> to a better place to put it.
>>
>
> After looking at this, isn't protocol-native a right place for
> the hooks? We're only protecting the protocol-native commands
> anyway.
>
> There's also a precedent in having hooks in pa_protocol_native.
> Namely at protocol-native.c:
>
>     struct pa_native_protocol {
>         PA_REFCNT_DECLARE;
>         pa_core *core;
>         ...
>         pa_hook hooks[PA_NATIVE_HOOK_MAX];  // <-- here
>         ...
>     };
>
>     pa_hook *pa_native_protocol_hooks(pa_native_protocol *p) {
>         ...
>         return p->hooks;
>     }
>
> And modules can get access to the hooks directly using the
> pa_native_protocol_hooks() accessor above. This is how it's done
> in module  device-manager, module device-restore, and module
> stream-restore, etc.
>
> Anything blocking us from doing the same for the access control
> hooks? This would indeed save us from the 1-to-1 mapping table.
>
> _ at diwic_: You mention that native protocol instances could at
> least in theory come and go. But if that happened, what would be
> the dangers of freeing the hooks along with protocol-native and
> be done with it? In a sense, the access-control hooks themselves
> won't make any sense without protocol-native anyway?

Suppose the following chain of events:

  1. Protocol-native loads
  2. Access control module loads, puts hooks into existing 
protocol-native instance
  3. Protocol-native unloads
  4. Another Protocol-native loads

At this point the new protocol-native is unprotected, which is probably 
not what you want?

Also we have to make sure we don't get use-after-free errors, no matter 
which order the modules are unloaded. But that's certainly solvable, 
just an additional thing to remember.

>
>
> Regards,
>
> [*] Full thread originally archived at http://goo.gl/8h7876
>
> [1] Simple kernel based mechanism to know if a client is inside
>      a Flatpak, from a PID which we can get through the unix
>      socket credentials passing, is here: https://goo.gl/OCfmBW
>
> [2] Initial repo here
>      https://github.com/a-darwish/pulseaudio/commits/access-control-v1
>
> [3] Wim Tay was kind enough to welcome building on top of this
>      patchset! Thanks a lot Wim :-)
>