mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adam B <a...@mesosphere.io>
Subject Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.
Date Thu, 09 Feb 2017 01:48:03 GMT


> On Feb. 7, 2017, 1:16 a.m., Adam B wrote:
> > src/master/master.cpp, line 2178
> > <https://reviews.apache.org/r/56178/diff/5/?file=1626265#file1626265line2178>
> >
> >     What does the framework's capability have to do with whether a custom authorizer
can support multi-role authorization?
> >     If I have an old authorizer module, and multi-role (phase 1) capable frameworks,
I will only be able to authorize frameworks with a single role (regardless of capability).
For a multi-role framework, I could authorize a single role from the list, but that essentially
provides a back-door for those frameworks to use other roles. Or I could call the authorizer
multiple times, one for each role. But we wouldn't want to do that for multi-role capable
authorizers. Maybe we need the concept of "module capabilites" now too?
> >     Am I overthinking this?
> 
> Benjamin Bannier wrote:
>     There is only a weak relation between this framework capability and the capabilities
of authorizers. My thinking here was roughly:
>     
>     * The way authorization works ("send at most a single authorization request per authorizable
action") all information required for authorization needs to be packed into a single authorization
request.
>     * There are authorizers relying only on the `value` field to determine the role of
a framework; this approach is deprecated.
>     * In the current authorization approach there is no meaningful value for `value`
for multirole frameworks.
>     * Multirole frameworks are a new feature so not updating deprecated features to work
with them does not break backwards compatibility.
>     
>     With this change we
>     
>     * maintain the current, single-request authorization approach,
>     * avoid breaking authorizers assuming frameworks can only have a single role,
>     * avoid breaking authorizers using `value` and assuming a single-role world, and
>     * enable authorizers using `framework_info` to authorize multirole frameworks.
>     
>     This is very much in line with how e.g., `FrameworkInfo` has both a (deprecated)
`role` and a `roles` field, and the correct action is taken depending on framework capabilities.
>     
>     Would we instead e.g., send an authorization request for each role, we wouldn't just
introduce new code to use a deprecated field (`value`), but we would also introduce a lot
of potential overhead, all while ignoring the existence of `framework_info` which already
now bundles all this information and can be sent as part of an authorization request.

Thanks for the explanation.
I agree that sending an authz request for each role is overkill just to support a deprecated
field. Forget about that.
My only concern is that authz decisions generally prefer to deny access when they don't know
what to do, and an old authorizer that only looks at the `value` field will try to authorize
a Framework registering with no role. Is that the same as registering with `*`? If not, they
probably already handle that as an error. If so, they would probably authorize based on `*`
(allow), when they should be authorizing based on `foo` and `bar` (deny). If that's a serious
concern, I'd rather remove the `value` field or otherwise break their compilation so they
are forced to review the API changes.


- Adam


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56178/#review164477
-----------------------------------------------------------


On Feb. 7, 2017, 2:26 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 2:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
>     https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -----
> 
>   src/authorizer/local/authorizer.cpp b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> -------
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message