Open Stack

On 20 April 2015 at 10:03, Ihar Hrachyshka <ihrachys at redhat.com> wrote:

> -----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!
>

Nah it was me being ambiguous in my reply.
We need roles in the context. Otherwise most policy checks would not be
applicable anymore.
I was making a point that a function that loads and stores them in context
generated with
operation like get_admin_context or ContextBase.elevated is not needed
anymore.
Indeed there is a note in the code pointing that out [1].
And here's the patch for doing it [2] (needs some more work)

[1]
http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n472
[2] https://review.openstack.org/#/c/175238/1



>
> >
> >
> > == 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.
>

Sure. That was just the personal opinion of the developer who was asked to
introduce them.


> >> 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.
>

I was pointing out this (no reference to avoid confusion):
http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n152

>
> >> 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.
>

This is what I meant. policy.json is a configuration file, and it would be
changed in a backward incompatible way. If any operator did any change to
attribute-level policies, this change is likely to cause a breakage for
them.


>
> 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).
>

Are you suggesting I am a criminal and a traitor? I'm off to Mexico then ;)

>
> /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-----
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150420/dd048983/attachment.html>