Added authorization for prune images API call. Review: https://reviews.apache.org/r/64864/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4beb46db Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4beb46db Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4beb46db Branch: refs/heads/1.5.x Commit: 4beb46db5a54e495212754a672daf81853a57e06 Parents: f023091 Author: Zhitao Li <[email protected]> Authored: Sun Dec 31 18:27:44 2017 +0800 Committer: Gilbert Song <[email protected]> Committed: Sun Dec 31 20:24:29 2017 +0800 ---------------------------------------------------------------------- include/mesos/authorizer/acls.proto | 14 ++++++ include/mesos/authorizer/authorizer.proto | 4 ++ src/authorizer/local/authorizer.cpp | 20 +++++++++ src/slave/http.cpp | 60 +++++++++++++++++--------- 4 files changed, 78 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/4beb46db/include/mesos/authorizer/acls.proto ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto index 5c7ed34..152410f 100644 --- a/include/mesos/authorizer/acls.proto +++ b/include/mesos/authorizer/acls.proto @@ -495,6 +495,19 @@ message ACL { // SOME resource provider types and names. required Entity resource_providers = 2; } + + // Which principals are authorized to prune unused container images. + message PruneImages { + // Subjects: HTTP Username. + required Entity principals = 1; + + // Objects: Given implicitly. + // Use Entity type ANY or NONE to allow or deny access. + // + // TODO(zhitao): Consider allowing granular permission to act upon + // SOME image reference. + required Entity images = 2; + } } @@ -572,4 +585,5 @@ message ACLs { repeated ACL.RemoveStandaloneContainer remove_standalone_container = 44; repeated ACL.ViewStandaloneContainer view_standalone_containers = 46; repeated ACL.ModifyResourceProviderConfig modify_resource_provider_configs = 45; + repeated ACL.PruneImages prune_images = 47; } http://git-wip-us.apache.org/repos/asf/mesos/blob/4beb46db/include/mesos/authorizer/authorizer.proto ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto index 90ffdfa..1508c01 100644 --- a/include/mesos/authorizer/authorizer.proto +++ b/include/mesos/authorizer/authorizer.proto @@ -250,6 +250,10 @@ enum Action { // allowed to add, update and remove resource provider config files or is // unauthorized. MODIFY_RESOURCE_PROVIDER_CONFIG = 39; + + // This action will not fill in any object fields. A principal is either + // allowed to prune unused container images or is unauthorized. + PRUNE_IMAGES = 41; } http://git-wip-us.apache.org/repos/asf/mesos/blob/4beb46db/src/authorizer/local/authorizer.cpp ---------------------------------------------------------------------- diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp index 0722ac9..b3ec601 100644 --- a/src/authorizer/local/authorizer.cpp +++ b/src/authorizer/local/authorizer.cpp @@ -412,6 +412,7 @@ public: case authorization::STOP_MAINTENANCE: case authorization::UPDATE_MAINTENANCE_SCHEDULE: case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG: + case authorization::PRUNE_IMAGES: aclObject.set_type(ACL::Entity::ANY); break; @@ -700,6 +701,7 @@ public: case authorization::LAUNCH_NESTED_CONTAINER_SESSION: case authorization::LAUNCH_STANDALONE_CONTAINER: case authorization::MARK_AGENT_GONE: + case authorization::PRUNE_IMAGES: case authorization::REGISTER_AGENT: case authorization::REMOVE_NESTED_CONTAINER: case authorization::REMOVE_STANDALONE_CONTAINER: @@ -917,6 +919,7 @@ public: case authorization::LAUNCH_NESTED_CONTAINER_SESSION: case authorization::LAUNCH_STANDALONE_CONTAINER: case authorization::MARK_AGENT_GONE: + case authorization::PRUNE_IMAGES: case authorization::REGISTER_AGENT: case authorization::REMOVE_NESTED_CONTAINER: case authorization::REMOVE_STANDALONE_CONTAINER: @@ -1130,6 +1133,7 @@ public: case authorization::KILL_STANDALONE_CONTAINER: case authorization::LAUNCH_STANDALONE_CONTAINER: case authorization::MARK_AGENT_GONE: + case authorization::PRUNE_IMAGES: case authorization::REGISTER_AGENT: case authorization::REMOVE_NESTED_CONTAINER: case authorization::REMOVE_STANDALONE_CONTAINER: @@ -1504,6 +1508,16 @@ private: } return acls_; + case authorization::PRUNE_IMAGES: + foreach (const ACL::PruneImages& acl, acls.prune_images()) { + GenericACL acl_; + acl_.subjects = acl.principals(); + acl_.objects = acl.images(); + + acls_.push_back(acl_); + } + + return acls_; case authorization::REGISTER_FRAMEWORK: case authorization::CREATE_VOLUME: case authorization::RESERVE_RESOURCES: @@ -1684,6 +1698,12 @@ Option<Error> LocalAuthorizer::validate(const ACLs& acls) } } + foreach (const ACL::PruneImages& acl, acls.prune_images()) { + if (acl.images().type() == ACL::Entity::SOME) { + return Error("acls.prune_images type must be either NONE or ANY"); + } + } + // TODO(alexr): Consider validating not only protobuf, but also the original // JSON in order to spot misspelled names. A misspelled action may affect // authorization result and hence lead to a security issue (e.g. when there http://git-wip-us.apache.org/repos/asf/mesos/blob/4beb46db/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 446be55..71e0bbb 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -2444,8 +2444,6 @@ Future<Response> Http::pruneImages( { CHECK_EQ(agent::Call::PRUNE_IMAGES, call.type()); - // TODO(zhitao): Add AuthN/AuthZ. - LOG(INFO) << "Processing PRUNE_IMAGES call"; vector<Image> excludedImages(call.prune_images().excluded_images().begin(), call.prune_images().excluded_images().end()); @@ -2458,25 +2456,47 @@ Future<Response> Http::pruneImages( std::back_inserter(excludedImages)); } - return slave->containerizer->pruneImages(excludedImages) - .then([acceptType](const Future<Nothing>& result) - ->Future<Response> { - if (!result.isReady()) { - // TODO(zhitao): Because `containerizer::pruneImages` returns - // a `Nothing` now, we cannot distinguish between actual - // failure or the case that operator should drain the agent. - // Consider returning more information. - LOG(WARNING) - << "Failed to prune images: " - << (result.isFailed() ? result.failure() : "discarded"); - - return result.isFailed() - ? InternalServerError(result.failure()) - : InternalServerError(); - } + Future<Owned<ObjectApprover>> approver; - return OK(); - }); + if (slave->authorizer.isSome()) { + Option<authorization::Subject> subject = createSubject(principal); + + approver = slave->authorizer.get()->getObjectApprover( + subject, + authorization::PRUNE_IMAGES); + } else { + approver = Owned<ObjectApprover>(new AcceptingObjectApprover()); + } + + return approver + .then(defer(slave->self(), [this, excludedImages]( + const Owned<ObjectApprover>& approver) -> Future<Response> { + Try<bool> approved = approver->approved(ObjectApprover::Object()); + if (approved.isError()) { + return InternalServerError("Authorization error: " + approved.error()); + } else if (!approved.get()) { + return Forbidden(); + } + + return slave->containerizer->pruneImages(excludedImages) + .then([](const Future<Nothing>& result) -> Future<Response> { + if (!result.isReady()) { + // TODO(zhitao): Because `containerizer::pruneImages` returns + // a `Nothing` now, we cannot distinguish between actual + // failure or the case that operator should drain the agent. + // Consider returning more information. + LOG(WARNING) + << "Failed to prune images: " + << (result.isFailed() ? result.failure() : "discarded"); + + return result.isFailed() + ? InternalServerError(result.failure()) + : InternalServerError(); + } + + return OK(); + }); + })); }
