<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 20 April 2015 at 10:03, Ihar Hrachyshka <span dir="ltr"><<a href="mailto:ihrachys@redhat.com" target="_blank">ihrachys@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<br>
</span><div><div class="h5">On 04/17/2015 07:49 PM, Salvatore Orlando wrote:<br>
> == 2. filling in admin context with admin roles ==<br>
><br>
> Admin context object is filled with .roles attribute that is a list<br>
> of roles considered granting admin permissions [4]. The attribute<br>
> would then be used by plugins that would like to do explicit policy<br>
> checks. As per Salvatore, this attribute can probably be dropped<br>
> now that all plugins and services don't rely on it (Salvatore<br>
> mentioned lbaas mixins as the ones that previously relied on it,<br>
> but are now not doing it since service split from neutron tree<br>
> (?)).<br>
><br>
> The problem with dropping the .roles attribute from context object<br>
> in Liberty is that we, as a responsible upstream with lots of<br>
> plugins maintained out-of-tree (see the ongoing vendor<br>
> decomposition effort) would need to support the attribute while<br>
> it's marked as deprecated for at least one cycle, meaning that if<br>
> we don't get those oslo.policy internals we rely on in Liberty, we<br>
> would need to postpone the switch till Mizzle, or rely on private<br>
> symbols during the switch (while a new release of oslo.policy can<br>
> easily break us).<br>
><br>
> (BTW the code to extract admin roles is not really robust and has<br>
> bugs, f.e. it does not handle AndChecks that could be used in<br>
> context_is_admin. In theory, 'and' syntax would mean that both<br>
> roles are needed to claim someone is an admin, while the code to<br>
> extract admin roles handles 'and' the same way as 'or'. For the<br>
> deprecation time being, we may need to document this limitation.)<br>
><br>
><br>
>> Roles are normally populated by the keystone middleware. I'm not<br>
>> sure whether we want to drop them altogether from the context,<br>
>> but I would expect the policy engine to use them even after<br>
>> switching to oslo.policy - Plugins should never leverage this<br>
>> kind of information. I am indeede tempted to drop roles and other<br>
>> AAA-related info from the context when it's dispatched to the<br>
>> plugin. This should be done carefully - beyond breaking some<br>
>> plugins it might also impact the notifiers we use to communicate<br>
>> with nova.<br>
><br>
>> The function which artificially populates roles in the context<br>
>> was built for artificial contextes, which in some cases were<br>
>> created by plugins performing db operations at startup. I would<br>
>> check if we still have this requirement, and if not remove the<br>
>> function.<br>
><br>
<br>
</div></div>Ouch, I failed in wording above (ETOOMANYWORDS?). I only meant<br>
dropping explicit admin role population from the context object. If<br>
there are any reasons to drop the whole attribute, they are irrelevant<br>
to oslo.policy adoption discussion, and are worth a separate thread,<br>
if at all. Thanks for keeping me honest on the non-sense above!<br></blockquote><div><br></div><div>Nah it was me being ambiguous in my reply.</div><div>We need roles in the context. Otherwise most policy checks would not be applicable anymore.</div><div>I was making a point that a function that loads and stores them in context generated with</div><div>operation like get_admin_context or ContextBase.elevated is not needed anymore.</div><div>Indeed there is a note in the code pointing that out [1].</div><div>And here's the patch for doing it [2] (needs some more work)</div><div><br></div><div>[1] <a href="http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n472">http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n472</a></div><div>[2] <a href="https://review.openstack.org/#/c/175238/1">https://review.openstack.org/#/c/175238/1</a></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
><br>
><br>
> == 3. aggregating core, attribute and subattribute policies ==<br>
><br>
<br>
</span>[...]<br>
<span class=""><br>
>> Policies on subattributes are an abomination of nature and they<br>
>> should go.<br>
<br>
</span>Not sure they can easily go now, without breaking existing setups. I<br>
wouldn't require existing deployments to convert their policies unless<br>
we are completely locked otherwise.<br></blockquote><div><br></div><div>Sure. That was just the personal opinion of the developer who was asked to introduce them.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
>> The problem however is that this needs first a rethink about<br>
>> some API extensions - namely the one for external gateway modes.<br>
</span>>> However, as you say we can't reablockedlly do without policies on<br>
<span class="">>> attributes at the moment. Policies like the following:<br>
><br>
>> "create_subnetpool:shared": "rule:admin_only"<br>
><br>
>> Led us to implement [1], which uses the "symbols" which were now<br>
>> made private.<br>
<br>
</span>You probably forgot to define [1] in your email. At least [1] in the<br>
original email seems irrelevant to attribute matching in neutron.<br></blockquote><div><br></div><div>I was pointing out this (no reference to avoid confusion): <a href="http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n152">http://git.openstack.org/cgit/openstack/neutron/tree/neutron/policy.py#n152</a> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
>> That logic is specific for Neutron, which adds semantic value to<br>
>> the policy target. As Ihar says, imposing this on all the other<br>
>> projects might not be welcome and in some cases break the project<br>
>> themselves.<br>
><br>
><br>
> So the question to oslo.policy maintainers is: whether all that is<br>
> said above makes sense, and if so, whether we may now consider<br>
> exposing those private symbols (+ maybe OrCheck, NotCheck, and<br>
> other primitives that are logically bound to AndCheck) as part of<br>
> public API. If community agrees with my analysis and justification<br>
> for the change, I am happy to propose a patch that would do just<br>
> that.<br>
><br>
><br>
>> Making this possilbe would be the quickest path for Neutron.<br>
>> However if the oslo_policy team took this decision it must have<br>
>> been for a solid design reasoning. It is tough to ask to revise a<br>
>> design decision for a single user of a library. Unfortunately a<br>
>> particular Neutron developer took the liberty of playing with<br>
>> authZ policies - I bet he was even proud of what he did: leaving<br>
>> the project with more technical debt. That particular developer<br>
>> should be dealt with appropriately, but this is another story.<br>
><br>
<br>
</span>Do we need a separate neutron-troika gerrit group to handle those cases?<br>
<span class=""><br>
>> I think that the only alternative to making those symbols public<br>
>> in oslo_policy is for Neutron to perform attribute and<br>
>> sub-attribute authZ checks in a different fashion (perhaps an<br>
>> additional engine). This will be a backward incompatible<br>
>> configuration change as deployers will have to replace their<br>
>> policy.json file. Probably scripts might be provided to ease the<br>
>> transition, but it's not going to be simple.<br>
><br>
<br>
</span>Not sure why you think that it would require policy.json conversion.<br>
We could just validate actions against their basic rules (like<br>
'create_network: ...') via oslo.policy, and add a separate<br>
neutron-specific matching just for attribute and sub-attribute matching.<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
If we get to that point of despair, we are maybe better off just<br>
dropping oslo.policy engine completely, now that we would need to<br>
maintain our own anyway.<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
That said, I really hope we don't consider this option outside this<br>
thread, and instead claim everyone proposing things like that being a<br>
war criminal and a traitor (again, neutron-troika would be of help).<br></blockquote><div><br></div><div>Are you suggesting I am a criminal and a traitor? I'm off to Mexico then ;) <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
/Ihar<br>
-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v2<br>
<br>
</span>iQEcBAEBAgAGBQJVNLLnAAoJEC5aWaUY1u57RhAH/2vzlaojHk/89NE+t92IETOx<br>
OcRdheOyuwzCNxQaYip+BqWVLgjuLo7OHuirtPdDjhpBptNxEy+EwpPY8VG65bEm<br>
YQhCIegTZbB0RqrosUMP6RRvBraxJhyPS+Uvona2HVOaqiFoh7TBT2JO2mZY/Ohu<br>
dJ3TNqXDF5o0tez60XfWSJ58SGklqoFdypy703ZPaODJ8pWlCpb9oo79r8+Dgqzk<br>
tI7Zp3RDZ8ZuHsVSoaIF/eS30+UZV2riBZ9AFiJ80mON7mDuVD1rOz5CuWaM6WPD<br>
tkPKlPFKN0zmcswb1NZRtR17U3Ao7ajysj/D7VNARQX7HFNntjRjPzGL3OiyExI=<br>
=7hmF<br>
<div class=""><div class="h5">-----END PGP SIGNATURE-----<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div></div>