> 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.
>
> Benjamin Hindman wrote:
> 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.
message ACL {
message GET_WWW {
message Subject {
enum Type {
IP = 1;
HOST = 2;
ANYONE = 3;
NONE = 4;
}
required Type type = 1;
optional string id = 2; // Required for IP/HOST.
}
required Subject subject = 1;
repeated string urls = 2;
}
message PUT_WWW {
message Subject {
enum Type {
IP = 1;
HOST = 2;
ANYONE = 3;
NONE = 4;
}
required Type type = 1;
optional string id = 2; // Required for IP/HOST.
}
required Subject subject = 1;
repeated string urls = 2;
}
message ALLOCATE_ROLE {
message Subject {
enum Type {
PRINCIPAL = 1;
ANYONE = 2;
NONE = 3;
}
required Type type = 1;
optional string id = 2; // Required for PRINCIPAL.
}
required Subject subject = 1;
repeated string roles = 2;
}
message LAUNCH_USER {
message Subject {
enum Type {
PRINCIPAL = 1;
ANYONE = 2;
NONE = 3;
}
required Type type = 1;
optional string id = 2; // Required for PRINCIPAL.
}
required Subject subject = 1;
repeated string users = 2;
}
repeated GET_WWW get_www = 1;
repeated PUT_WWW put_www = 2;
repeated ALLOCATE_ROLE allocate_role = 3;
repeated LAUNCH_USER launch_user = 4;
}
This is what I came up with after feedback. Let me know what you think.
I imagine we would have different overloads for Authorizer::authorize() as
follows:
Authorizer::authorize(const ACL::GET_WWW& get_www);
Authorizer::authorize(const ACL::PUT_WWW& put_www);
Authorizer::authorize(const ACL::ALLOCATE_ROLE& allocate_role);
Authorizer::authorize(const ACL::LAUNCH_USER& launch_user);
Any suggestions for naming are also welcome. I particularly don't like the
repeated fields at the bottom to have singular names. One option is:
repeated GET_WWW gets = 1;
repeated PUT_WWW puts = 2;
repeated ALLOCATE_ROLE allocates = 3;
repeated LAUNCH_USER launches = 4;
- Vinod
-----------------------------------------------------------
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
>
>
