> On March 4, 2014, 7:29 p.m., Benjamin Hindman wrote:
> > include/mesos/mesos.proto, lines 481-485
> > <https://reviews.apache.org/r/18730/diff/2/?file=509427#file509427line481>
> >
> > In line with Niklas' and Adam's comments, I agree we should be making
> > ACLs more explicit. How about defining a message for each action? For
> > example:
> >
> > message ACL {
> > message RunAsUser {
> > required string user = 1;
> > }
> >
> > message AllocateFromRole {
> > required string role = 1;
> > }
> >
> > enum Action {
> > RUN_AS_USER = 1,
> > ALLOCATE_FROM_ROLE = 2
> > };
> >
> > required string subject = 1;
> > required Action action = 2;
> >
> > optional RunAsUser run_as_user = 3;
> > optional AllocateFromRole allocate_from_role = 4;
> > }
> >
> > This should still give us lots of flexibility while also being explicit
> > (and type safe!).
> >
> > I can also imagine doing the same thing for 'subject', e.g.:
> >
> > message ACL {
> > message Subject {
> > optional string principal = 1;
> > optional string user = 2;
> > // Add more subjects as necessary.
> > }
> > required Subject subject = 1;
> > ...
> > }
> >
> > We could accomplish the same with another enum as well, but it's
> > probably cleaner to use a nested message.
> >
> > Also, feel free to suggest different naming!
>
> Vinod Kone wrote:
> I played around with the ACL format a bit and came up with the below.
> This is inspire by how ZooKeeper implements ACLs
> (https://zookeeper.apache.org/doc/r3.1.2/zookeeperProgrammers.html#sc_ZooKeeperAccessControl).
>
> message ACL {
>
> message Object {
> enum Type {
> USER = 0; // For launching tasks as.
> ROLE = 1; // For getting allocations for.
> WWW = 2; // For accessing a web endpoint.
> }
>
> required Type type = 1;
> required string id = 2;
> }
>
> message Subject {
> enum Type {
> PRINCIPAL = 0;
> IP = 1;
> HOST = 2;
> ANYONE = 3;
> }
>
> required Type type = 1;
> optional string id = 2; // Can be optional iff type is ANYONE.
> }
>
> enum Action {
> LAUNCH_USER = 0;
> ALLOCATE_ROLE = 1;
> GET_WWW = 2;
> PUT_WWW = 3;
> }
>
> required Object object = 1;
> required Subject subject = 2;
> required Action action = 3;
> }
>
>
> I am assuming we can set up ACLs on Objects as follows:
>
> Object Subject Action
> ---------------------------------------------------------------------
> WWW:/index.html ANYONE GET_WWW
> WWW:/stats.json IP:"10.0.0.0/24" GET_WWW
> WWW:/framework/<foo>/tasks.json PRINCIPAL:"foo-user" GET_WWW
> WWW:/drain.json PRINCIPAL:"admin" PUT_WWW
>
> ROLE:ads PRINCIPAL:"marathon"
> ALLOCATE_ROLE
> ROLE:* ANYONE
> ALLOCATE_ROLE
>
> USER:root PRINCIPAL:"aurora" LAUNCH_USER
>
>
> It's not yet clear how we validate that ACLs that are being set up don't
> conflict with each other.
>
> Also, authorization itself might be a bit involved depending on the
> formats we allow for "Subject.id".
>
> Authorizer->authorize(WWW:/stats.json, "10.0.0.12", "GET_WWW") -->
> returns true
>
> Let me know what you think?
>
>
>
>
> Niklas Nielsen wrote:
> +1, Looks good. I don't think you can avoid conflicts all together; a
> well known order in which the rules are applied should be enough? Conflict
> detection will probably be a complicated piece of code.
I like the explicitness Vinod, but I think we should "merge" the action and
object. I'm concerned that some combinations won't "make sense", and it would
be nice to enforce that with types. For example, right now we can do subject
PRINCIPAL:"aurora" action GET_WWW object ROLE:"aurora", which is an error by
construction. By merging the action and object we can get type safety, i.e.,
message ACL {
message GetWWW {
repeated string url = 1;
}
enum Action {
GET_WWW = 1;
...
}
}
Moreover, the ACLs should be easier to construct (setting one less field). In
fact, extending this all the way to avoid bad subject action/object
combinations would be beneficial too. As of now all subjects might be
candidates for all actions but I doubt that will be the case in the future.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18730/#review36150
-----------------------------------------------------------
On March 4, 2014, 11:27 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18730/
> -----------------------------------------------------------
>
> (Updated March 4, 2014, 11:27 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
>
>
> Bugs: MESOS-911
> https://issues.apache.org/jira/browse/MESOS-911
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 37f8a7fcd23d467b1274c46c405b836510afbd49
> src/Makefile.am 61d832b89132be2cc5b8ae9bbf743685464f78a4
> src/authorizer/authorizer.hpp PRE-CREATION
> src/tests/authorization_tests.cpp PRE-CREATION
> src/tests/master_contender_detector_tests.cpp
> 8da7420e18c7a960b566fae13a5975857eb777ee
>
> Diff: https://reviews.apache.org/r/18730/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
