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

[Ahmed S. Darwish]

Hi David,

[ Thanks a lot for chiming back in! ]

On Fri, Jun 24, 2016 at 05:21:53PM +0200, David Henningsson wrote:
> 
> 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.
>

Can we solve the above two points by by introducing a pulse module
dependencies feature, just like what the linux kernel does? [1]

If so, we can do the following:

(a) Introduce module dependencies

(b) Put the access-control hooks inside protocol-native itself,
    and thus remove the 1-to-1 mappings [2] and avoid making core
    dependent on protocol-native (even implicitly)

(c) Make all policy modules, which contains all the hooks
    implementations, hard-dependent on protocol-native

In the first point raised, to unload protocol-native, user will
have to unload the dependent protocol-native-policy-X first. So if
the user loads protocol-native again, he/she will know that the
policy module will need to be reloaded as well ..

In the second point raised, hopefully module dependencies will
simplify the relationship: use-after-free errors can be eliminated
given proper cleanup by the policy modules ..

Any thoughts on this?

thanks,

[1] http://man7.org/linux/man-pages/man5/modules.dep.5.html

[2] For details on the 1-to-1 mapping, check the pa_access_hook
    enumerations + these hooks placement inside struct pa_core,
    at patch #3 and the map_table[] at patch #4

-- 
Darwish
http://darwish.chasingpointers.com