Added implicit executor authorization to the agent operator API. This patch updates the agent handlers for the LAUNCH_, WAIT_, and KILL_NESTED_CONTAINER calls of the operator API to set the `container_id` field within the authorization object, facilitating implicit executor authorization.
Review: https://reviews.apache.org/r/58254/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ecab5ff3 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ecab5ff3 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ecab5ff3 Branch: refs/heads/master Commit: ecab5ff3f8c6d50dd6551f552eeb381f49eb3949 Parents: c401190 Author: Greg Mann <[email protected]> Authored: Fri Apr 21 10:45:19 2017 -0700 Committer: Vinod Kone <[email protected]> Committed: Fri Apr 21 10:45:19 2017 -0700 ---------------------------------------------------------------------- include/mesos/authorizer/authorizer.proto | 17 ++-- src/authorizer/local/authorizer.cpp | 109 ++++++++++++++++++++++--- src/slave/http.cpp | 5 ++ 3 files changed, 113 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/ecab5ff3/include/mesos/authorizer/authorizer.proto ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto index 0951be9..3ae5df5 100644 --- a/include/mesos/authorizer/authorizer.proto +++ b/include/mesos/authorizer/authorizer.proto @@ -161,18 +161,20 @@ enum Action { // is the entire set of flags. VIEW_FLAGS = 18; - // This action will always set the `ExecutorInfo`, `FrameworkInfo` fields - // and optionally a `CommandInfo` if available. + // This action will always set the `ExecutorInfo`, `FrameworkInfo`, and + // `ContainerID` fields and optionally a `CommandInfo` if available. LAUNCH_NESTED_CONTAINER = 19; - // This action will set objects of type `ExecutorInfo` and `FrameworkInfo`. + // This action will set objects of type `ExecutorInfo`, `FrameworkInfo`, and + // `ContainerID`. KILL_NESTED_CONTAINER = 20; - // This action will set objects of type `ExecutorInfo` and `FrameworkInfo`. + // This action will set objects of type `ExecutorInfo`, `FrameworkInfo`, and + // `ContainerID`. WAIT_NESTED_CONTAINER = 21; - // This action will always set the `ExecutorInfo`, `FrameworkInfo` fields - // and optionally a `CommandInfo` if available. + // This action will always set the `ExecutorInfo`, `FrameworkInfo`, and + // `ContainerID` fields and optionally a `CommandInfo` if available. LAUNCH_NESTED_CONTAINER_SESSION = 22; // This action will set objects of type `ExecutorInfo` and `FrameworkInfo`. @@ -188,7 +190,8 @@ enum Action { // either allowed to change the log level or he is unauthorized. SET_LOG_LEVEL = 26; - // This action will set objects of type `ExecutorInfo` and `FrameworkInfo`. + // This action will set objects of type `ExecutorInfo`, `FrameworkInfo`, and + // `ContainerID`. REMOVE_NESTED_CONTAINER = 27; } http://git-wip-us.apache.org/repos/asf/mesos/blob/ecab5ff3/src/authorizer/local/authorizer.cpp ---------------------------------------------------------------------- diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp index 8c3178a..3eb59d3 100644 --- a/src/authorizer/local/authorizer.cpp +++ b/src/authorizer/local/authorizer.cpp @@ -335,6 +335,9 @@ public: object->framework_info->has_user()) { aclObject.add_values(object->framework_info->user()); aclObject.set_type(mesos::ACL::Entity::SOME); + } else if (object->container_id != nullptr) { + aclObject.add_values(object->container_id->value()); + aclObject.set_type(mesos::ACL::Entity::SOME); } break; @@ -538,6 +541,40 @@ private: }; +class LocalImplicitExecutorObjectApprover : public ObjectApprover +{ +public: + LocalImplicitExecutorObjectApprover(const ContainerID& subject) + : subject_(subject) {} + + // Executors are permitted to perform an action when the root ContainerID in + // the object is equal to the ContainerID that was extracted from the + // subject's claims. + virtual Try<bool> approved( + const Option<ObjectApprover::Object>& object) const noexcept override + { + return object.isSome() && + object->container_id != nullptr && + subject_ == protobuf::getRootContainerId(*object->container_id); + } + +private: + const ContainerID subject_; +}; + + +// Implementation of the ObjectApprover interface denying all objects. +class RejectingObjectApprover : public ObjectApprover +{ +public: + virtual Try<bool> approved( + const Option<ObjectApprover::Object>& object) const noexcept override + { + return false; + } +}; + + class LocalAuthorizerProcess : public ProtobufProcess<LocalAuthorizerProcess> { public: @@ -573,7 +610,12 @@ public: Future<bool> authorized(const authorization::Request& request) { - return getObjectApprover(request.subject(), request.action()) + Option<authorization::Subject> subject; + if (request.has_subject()) { + subject = request.subject(); + } + + return getObjectApprover(subject, request.action()) .then([=](const Owned<ObjectApprover>& objectApprover) -> Future<bool> { Option<ObjectApprover::Object> object = None(); if (request.has_object()) { @@ -645,20 +687,65 @@ public: acls.permissive())); } - Future<Owned<ObjectApprover>> getObjectApprover( + Future<Owned<ObjectApprover>> getImplicitExecutorObjectApprover( const Option<authorization::Subject>& subject, const authorization::Action& action) { - // Implementation of the ObjectApprover interface denying all objects. - class RejectingObjectApprover : public ObjectApprover - { - public: - virtual Try<bool> approved( - const Option<ObjectApprover::Object>& object) const noexcept override - { - return false; + CHECK(subject.isSome() && + subject->has_claims() && + !subject->has_value() && + (action == authorization::LAUNCH_NESTED_CONTAINER || + action == authorization::WAIT_NESTED_CONTAINER || + action == authorization::KILL_NESTED_CONTAINER || + action == authorization::LAUNCH_NESTED_CONTAINER_SESSION || + action == authorization::REMOVE_NESTED_CONTAINER || + action == authorization::ATTACH_CONTAINER_OUTPUT)); + + Option<ContainerID> subjectContainerId; + foreach (const Label& claim, subject->claims().labels()) { + if (claim.key() == "cid" && claim.has_value()) { + subjectContainerId = ContainerID(); + subjectContainerId->set_value(claim.value()); + break; } - }; + } + + if (subjectContainerId.isNone()) { + // If the subject's claims do not include a ContainerID, + // we deny all objects. + return Owned<ObjectApprover>(new RejectingObjectApprover()); + } + + return Owned<ObjectApprover>(new LocalImplicitExecutorObjectApprover( + subjectContainerId.get())); + } + + Future<Owned<ObjectApprover>> getObjectApprover( + const Option<authorization::Subject>& subject, + const authorization::Action& action) + { + // We return the `LocalImplicitExecutorObjectApprover` only for subjects and + // actions which it knows how to handle. This means the subject should have + // claims but no value, and the action should be one of the actions used by + // the default executor. + if (subject.isSome() && + subject->has_claims() && + !subject->has_value() && + (action == authorization::LAUNCH_NESTED_CONTAINER || + action == authorization::WAIT_NESTED_CONTAINER || + action == authorization::KILL_NESTED_CONTAINER || + action == authorization::LAUNCH_NESTED_CONTAINER_SESSION || + action == authorization::REMOVE_NESTED_CONTAINER || + action == authorization::ATTACH_CONTAINER_OUTPUT)) { + return getImplicitExecutorObjectApprover(subject, action); + } + + // Currently, implicit executor authorization is the only case which handles + // subjects that do not have the `value` field set. If the previous case was + // not true and `value` is not set, then we should fail all requests. + if (subject.isSome() && !subject->has_value()) { + return Owned<ObjectApprover>(new RejectingObjectApprover()); + } if (action == authorization::LAUNCH_NESTED_CONTAINER || action == authorization::LAUNCH_NESTED_CONTAINER_SESSION) { http://git-wip-us.apache.org/repos/asf/mesos/blob/ecab5ff3/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 468cf33..93825fa 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -2252,6 +2252,7 @@ Future<Response> Slave::Http::_launchNestedContainer( object.executor_info = &(executor->info); object.framework_info = &(framework->info); object.command_info = &(commandInfo); + object.container_id = &(containerId); Try<bool> approved = approver.get()->approved(object); @@ -2340,6 +2341,7 @@ Future<Response> Slave::Http::waitNestedContainer( ObjectApprover::Object object; object.executor_info = &(executor->info); object.framework_info = &(framework->info); + object.container_id = &(containerId); Try<bool> approved = waitApprover.get()->approved(object); @@ -2414,6 +2416,7 @@ Future<Response> Slave::Http::killNestedContainer( ObjectApprover::Object object; object.executor_info = &(executor->info); object.framework_info = &(framework->info); + object.container_id = &(containerId); Try<bool> approved = killApprover.get()->approved(object); @@ -2473,6 +2476,7 @@ Future<Response> Slave::Http::removeNestedContainer( ObjectApprover::Object object; object.executor_info = &(executor->info); object.framework_info = &(framework->info); + object.container_id = &(containerId); Try<bool> approved = removeApprover.get()->approved(object); @@ -2928,6 +2932,7 @@ Future<Response> Slave::Http::attachContainerOutput( ObjectApprover::Object object; object.executor_info = &(executor->info); object.framework_info = &(framework->info); + object.container_id = &(containerId); Try<bool> approved = attachOutputApprover.get()->approved(object);
