Open Stack

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/17/2015 07:49 PM, Salvatore Orlando wrote:
> == 2. filling in admin context with admin roles ==
> 
> Admin context object is filled with .roles attribute that is a list
> of roles considered granting admin permissions [4]. The attribute
> would then be used by plugins that would like to do explicit policy
> checks. As per Salvatore, this attribute can probably be dropped
> now that all plugins and services don't rely on it (Salvatore
> mentioned lbaas mixins as the ones that previously relied on it,
> but are now not doing it since service split from neutron tree
> (?)).
> 
> The problem with dropping the .roles attribute from context object
> in Liberty is that we, as a responsible upstream with lots of
> plugins maintained out-of-tree (see the ongoing vendor
> decomposition effort) would need to support the attribute while
> it's marked as deprecated for at least one cycle, meaning that if
> we don't get those oslo.policy internals we rely on in Liberty, we
> would need to postpone the switch till Mizzle, or rely on private
> symbols during the switch (while a new release of oslo.policy can
> easily break us).
> 
> (BTW the code to extract admin roles is not really robust and has 
> bugs, f.e. it does not handle AndChecks that could be used in 
> context_is_admin. In theory, 'and' syntax would mean that both
> roles are needed to claim someone is an admin, while the code to
> extract admin roles handles 'and' the same way as 'or'. For the
> deprecation time being, we may need to document this limitation.)
> 
> 
>> Roles are normally populated by the keystone middleware. I'm not
>> sure whether we want to drop them altogether from the context,
>> but I would expect the policy engine to use them even after
>> switching to oslo.policy - Plugins should never leverage this
>> kind of information. I am indeede tempted to drop roles and other
>> AAA-related info from the context when it's dispatched to the
>> plugin. This should be done carefully - beyond breaking some
>> plugins it might also impact the notifiers we use to communicate
>> with nova.
> 
>> The function which artificially populates roles in the context
>> was built for artificial contextes, which in some cases were
>> created by plugins performing db operations at startup. I would
>> check if we still have this requirement, and if not remove the
>> function.
> 

Ouch, I failed in wording above (ETOOMANYWORDS?). I only meant
dropping explicit admin role population from the context object. If
there are any reasons to drop the whole attribute, they are irrelevant
to oslo.policy adoption discussion, and are worth a separate thread,
if at all. Thanks for keeping me honest on the non-sense above!

> 
> 
> == 3. aggregating core, attribute and subattribute policies ==
> 

[...]

>> Policies on subattributes are an abomination of nature and they
>> should go.

Not sure they can easily go now, without breaking existing setups. I
wouldn't require existing deployments to convert their policies unless
we are completely locked otherwise.

>> The problem however is that this needs first a rethink about
>> some API extensions - namely the one for external gateway modes. 
>> However, as you say we can't reablockedlly do without policies on
>> attributes at the moment. Policies like the following:
> 
>> "create_subnetpool:shared": "rule:admin_only"
> 
>> Led us to implement [1], which uses the "symbols" which were now
>> made private.

You probably forgot to define [1] in your email. At least [1] in the
original email seems irrelevant to attribute matching in neutron.

>> That logic is specific for Neutron, which adds semantic value to
>> the policy target. As Ihar says, imposing this on all the other
>> projects might not be welcome and in some cases break the project
>> themselves.
> 
> 
> So the question to oslo.policy maintainers is: whether all that is 
> said above makes sense, and if so, whether we may now consider 
> exposing those private symbols (+ maybe OrCheck, NotCheck, and
> other primitives that are logically bound to AndCheck) as part of
> public API. If community agrees with my analysis and justification
> for the change, I am happy to propose a patch that would do just
> that.
> 
> 
>> Making this possilbe would be the quickest path for Neutron. 
>> However if the oslo_policy team took this decision it must have
>> been for a solid design reasoning. It is tough to ask to revise a
>> design decision for a single user of a library. Unfortunately a
>> particular Neutron developer took the liberty of playing with
>> authZ policies - I bet he was even proud of what he did: leaving 
>> the project with more technical debt. That particular developer
>> should be dealt with appropriately, but this is another story.
> 

Do we need a separate neutron-troika gerrit group to handle those cases?

>> I think that the only alternative to making those symbols public
>> in oslo_policy is for Neutron to perform attribute and
>> sub-attribute authZ checks in a different fashion (perhaps an
>> additional engine). This will be a backward incompatible
>> configuration change as deployers will have to replace their
>> policy.json file. Probably scripts might be provided to ease the
>> transition, but it's not going to be simple.
> 

Not sure why you think that it would require policy.json conversion.
We could just validate actions against their basic rules (like
'create_network: ...') via oslo.policy, and add a separate
neutron-specific matching just for attribute and sub-attribute matching.

If we get to that point of despair, we are maybe better off just
dropping oslo.policy engine completely, now that we would need to
maintain our own anyway.

That said, I really hope we don't consider this option outside this
thread, and instead claim everyone proposing things like that being a
war criminal and a traitor (again, neutron-troika would be of help).

/Ihar
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBAgAGBQJVNLLnAAoJEC5aWaUY1u57RhAH/2vzlaojHk/89NE+t92IETOx
OcRdheOyuwzCNxQaYip+BqWVLgjuLo7OHuirtPdDjhpBptNxEy+EwpPY8VG65bEm
YQhCIegTZbB0RqrosUMP6RRvBraxJhyPS+Uvona2HVOaqiFoh7TBT2JO2mZY/Ohu
dJ3TNqXDF5o0tez60XfWSJ58SGklqoFdypy703ZPaODJ8pWlCpb9oo79r8+Dgqzk
tI7Zp3RDZ8ZuHsVSoaIF/eS30+UZV2riBZ9AFiJ80mON7mDuVD1rOz5CuWaM6WPD
tkPKlPFKN0zmcswb1NZRtR17U3Ao7ajysj/D7VNARQX7HFNntjRjPzGL3OiyExI=
=7hmF
-----END PGP SIGNATURE-----