> On March 4, 2014, 11:29 a.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? > > >
+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. - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18730/#review36150 ----------------------------------------------------------- On March 4, 2014, 3: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, 3: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 > >
