mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 18730: Implemented a basic Authorizer interface.
Date Thu, 06 Mar 2014 02:52:25 GMT


> 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
> 
>


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