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
