This is an automated email from the ASF dual-hosted git repository. chhsiao pushed a commit to branch 1.7.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit ab51668e06dc998e43b82be5dcda4ea05be259ea Author: Chun-Hung Hsiao <[email protected]> AuthorDate: Tue Mar 26 21:02:08 2019 -0700 Added authorization for applying `DESTROY_DISK` on `RAW` disks. Review: https://reviews.apache.org/r/70315/ --- docs/examples/acls_template.json | 50 ++++++ include/mesos/authorizer/acls.proto | 12 +- include/mesos/authorizer/authorizer.proto | 12 +- src/authorizer/local/authorizer.cpp | 13 +- src/master/master.cpp | 7 +- src/tests/authorization_tests.cpp | 281 +++++++++++++++++++++++++++--- 6 files changed, 339 insertions(+), 36 deletions(-) diff --git a/docs/examples/acls_template.json b/docs/examples/acls_template.json index ad28cb8..35cef0c 100644 --- a/docs/examples/acls_template.json +++ b/docs/examples/acls_template.json @@ -169,5 +169,55 @@ "type": "ANY" } } + ], + "create_block_disks": [ + { + "principals": { + "type": "ANY" + }, + "roles": { + "type": "ANY" + } + } + ], + "destroy_block_disks": [ + { + "principals": { + "type": "ANY" + }, + "roles": { + "type": "ANY" + } + } + ], + "create_mount_disks": [ + { + "principals": { + "type": "ANY" + }, + "roles": { + "type": "ANY" + } + } + ], + "destroy_mount_disks": [ + { + "principals": { + "type": "ANY" + }, + "roles": { + "type": "ANY" + } + } + ], + "destroy_raw_disks": [ + { + "principals": { + "type": "ANY" + }, + "roles": { + "type": "ANY" + } + } ] } diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto index f5d2580..1af83f1 100644 --- a/include/mesos/authorizer/acls.proto +++ b/include/mesos/authorizer/acls.proto @@ -557,6 +557,15 @@ message ACL { required Entity roles = 2; } + // Which principals are authorized to destroy raw disks. + message DestroyRawDisk { + // Subjects: Framework principal. + required Entity principals = 1; + + // Objects: The list of roles for which volume disks can be destroyed. + required Entity roles = 2; + } + // Which principals are authorized to access resource provider information. message ViewResourceProvider { // Subjects: HTTP Username. @@ -643,11 +652,12 @@ message ACLs { repeated ACL.RemoveStandaloneContainer remove_standalone_containers = 44; repeated ACL.ViewStandaloneContainer view_standalone_containers = 46; repeated ACL.ModifyResourceProviderConfig modify_resource_provider_configs = 45; + repeated ACL.ViewResourceProvider view_resource_providers = 53; repeated ACL.PruneImages prune_images = 47; repeated ACL.ResizeVolume resize_volumes = 48; repeated ACL.CreateBlockDisk create_block_disks = 49; repeated ACL.DestroyBlockDisk destroy_block_disks = 50; repeated ACL.CreateMountDisk create_mount_disks = 51; repeated ACL.DestroyMountDisk destroy_mount_disks = 52; - repeated ACL.ViewResourceProvider view_resource_providers = 53; + repeated ACL.DestroyRawDisk destroy_raw_disks = 55; } diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto index ab30c20..f548921 100644 --- a/include/mesos/authorizer/authorizer.proto +++ b/include/mesos/authorizer/authorizer.proto @@ -252,6 +252,10 @@ enum Action { MODIFY_RESOURCE_PROVIDER_CONFIG = 39; // This action will not fill in any object fields. A principal is either + // allowed to view resource provider information or is unauthorized. + VIEW_RESOURCE_PROVIDER = 47; + + // 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; @@ -285,9 +289,11 @@ enum Action { // role until all `*_WITH_ROLE` aliases are removed. DESTROY_MOUNT_DISK = 46; - // This action will not fill in any object fields. A principal is either - // allowed to view resource provider information or is unauthorized. - VIEW_RESOURCE_PROVIDER = 47; + // `DESTROY_RAW_DISK` will have an object with `Resource` set. + // + // NOTE: For consistency, the `value` field will be set with the most refined + // role until all `*_WITH_ROLE` aliases are removed. + DESTROY_RAW_DISK = 49; } diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp index baf0e8d..8ca0101 100644 --- a/src/authorizer/local/authorizer.cpp +++ b/src/authorizer/local/authorizer.cpp @@ -429,6 +429,7 @@ public: case authorization::DESTROY_BLOCK_DISK: case authorization::CREATE_MOUNT_DISK: case authorization::DESTROY_MOUNT_DISK: + case authorization::DESTROY_RAW_DISK: return Error("Authorization for action " + stringify(action_) + " requires a specialized approver object."); case authorization::UNKNOWN: @@ -605,7 +606,8 @@ public: case authorization::CREATE_BLOCK_DISK: case authorization::DESTROY_BLOCK_DISK: case authorization::CREATE_MOUNT_DISK: - case authorization::DESTROY_MOUNT_DISK: { + case authorization::DESTROY_MOUNT_DISK: + case authorization::DESTROY_RAW_DISK: { entityObject.set_type(ACL::Entity::SOME); if (object->resource) { if (object->resource->reservations_size() > 0) { @@ -942,6 +944,11 @@ public: createHierarchicalRoleACLs(acls.destroy_mount_disks()); break; } + case authorization::DESTROY_RAW_DISK: { + hierarchicalRoleACLs = + createHierarchicalRoleACLs(acls.destroy_raw_disks()); + break; + } case authorization::ACCESS_MESOS_LOG: case authorization::ACCESS_SANDBOX: case authorization::ATTACH_CONTAINER_INPUT: @@ -1163,7 +1170,8 @@ public: case authorization::CREATE_BLOCK_DISK: case authorization::DESTROY_BLOCK_DISK: case authorization::CREATE_MOUNT_DISK: - case authorization::DESTROY_MOUNT_DISK: { + case authorization::DESTROY_MOUNT_DISK: + case authorization::DESTROY_RAW_DISK: { return getHierarchicalRoleApprover(subject, action); } case authorization::ACCESS_MESOS_LOG: @@ -1590,6 +1598,7 @@ private: case authorization::DESTROY_BLOCK_DISK: case authorization::CREATE_MOUNT_DISK: case authorization::DESTROY_MOUNT_DISK: + case authorization::DESTROY_RAW_DISK: return Error("Extracting ACLs for " + stringify(action) + " requires " "a specialized function"); case authorization::UNKNOWN: diff --git a/src/master/master.cpp b/src/master/master.cpp index 3092608..ed072d3 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -3939,9 +3939,12 @@ Future<bool> Master::authorizeDestroyDisk( action = authorization::DESTROY_BLOCK_DISK; break; } - case Resource::DiskInfo::Source::UNKNOWN: - case Resource::DiskInfo::Source::PATH: case Resource::DiskInfo::Source::RAW: { + action = authorization::DESTROY_RAW_DISK; + break; + } + case Resource::DiskInfo::Source::UNKNOWN: + case Resource::DiskInfo::Source::PATH: { return Failure( "Failed to authorize principal '" + (principal.isSome() ? stringify(principal.get()) : "ANY") + diff --git a/src/tests/authorization_tests.cpp b/src/tests/authorization_tests.cpp index de57fc9..2104dd9 100644 --- a/src/tests/authorization_tests.cpp +++ b/src/tests/authorization_tests.cpp @@ -5956,13 +5956,6 @@ TYPED_TEST(AuthorizationTest, CreateBlockDisk) acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); } - { - // No other principals can create block disks. - mesos::ACL::CreateBlockDisk* acl = acls.add_create_block_disks(); - acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY); - acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); - } - // Create an `Authorizer` with the ACLs. Try<Authorizer*> create = TypeParam::create(parameterize(acls)); ASSERT_SOME(create); @@ -6216,13 +6209,6 @@ TYPED_TEST(AuthorizationTest, DestroyBlockDisk) acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); } - { - // No other principals can destroy block disks. - mesos::ACL::DestroyBlockDisk* acl = acls.add_destroy_block_disks(); - acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY); - acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); - } - // Create an `Authorizer` with the ACLs. Try<Authorizer*> create = TypeParam::create(parameterize(acls)); ASSERT_SOME(create); @@ -6476,13 +6462,6 @@ TYPED_TEST(AuthorizationTest, CreateMountDisk) acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); } - { - // No other principals can create mount disks. - mesos::ACL::CreateMountDisk* acl = acls.add_create_mount_disks(); - acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY); - acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); - } - // Create an `Authorizer` with the ACLs. Try<Authorizer*> create = TypeParam::create(parameterize(acls)); ASSERT_SOME(create); @@ -6736,13 +6715,6 @@ TYPED_TEST(AuthorizationTest, DestroyMountDisk) acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); } - { - // No other principals can destroy mount disks. - mesos::ACL::DestroyMountDisk* acl = acls.add_destroy_mount_disks(); - acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY); - acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); - } - // Create an `Authorizer` with the ACLs. Try<Authorizer*> create = TypeParam::create(parameterize(acls)); ASSERT_SOME(create); @@ -6937,6 +6909,259 @@ TYPED_TEST(AuthorizationTest, DestroyMountDisk) } } + +// This tests the authorization to destroy raw disks. +TYPED_TEST(AuthorizationTest, DestroyRawDisk) +{ + ACLs acls; + + { + // Principal "foo" can destroy raw disks for any role. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->add_values("foo"); + acl->mutable_roles()->set_type(mesos::ACL::Entity::ANY); + } + + { + // Principal "bar" can only destroy raw disks for the "panda" role. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->add_values("bar"); + acl->mutable_roles()->add_values("panda"); + } + + { + // Principal "baz" cannot destroy raw disks. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->add_values("baz"); + acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); + } + + { + // Principal "elizabeth-ii" can destroy raw disks for the "king" role + // and its nested ones. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->add_values("elizabeth-ii"); + acl->mutable_roles()->add_values("king/%"); + acl->mutable_roles()->add_values("king"); + } + + { + // Principal "charles" can destroy raw disks for any role below the + // "king/" role. Not in "king" itself. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->add_values("charles"); + acl->mutable_roles()->add_values("king/%"); + } + + { + // Principal "j-welby" can destroy raw disks only for the "king" + // role but not in any nested one. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->add_values("j-welby"); + acl->mutable_roles()->add_values("king"); + } + + { + // No other principals can destroy raw disks. + mesos::ACL::DestroyRawDisk* acl = acls.add_destroy_raw_disks(); + acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY); + acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE); + } + + // Create an `Authorizer` with the ACLs. + Try<Authorizer*> create = TypeParam::create(parameterize(acls)); + ASSERT_SOME(create); + Owned<Authorizer> authorizer(create.get()); + + // Principal "foo" can destroy raw disks for any role, so this + // request will pass. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("foo"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("awesome_role"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + // Principal "bar" can destroy raw disks for the "panda" role, + // so this request will pass. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("bar"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("panda"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + // Principal "bar" cannot destroy raw disks for the "giraffe" role, + // so this request will fail. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("bar"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("giraffe"); + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } + + // Principal "baz" cannot destroy raw disks for any role, + // so this request will fail. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("baz"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("panda"); + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } + + // Principal "zelda" is not mentioned in the ACLs of the Authorizer, so it + // will be caught by the final ACL, which provides a default case that denies + // access for all other principals. This request will fail. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("zelda"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("panda"); + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } + + // `elizabeth-ii` has full permissions for the `king` role as well as all + // its nested roles. She should be able to destroy raw disks in the next + // three blocks. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("elizabeth-ii"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("elizabeth-ii"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king/prince"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("elizabeth-ii"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king/prince/duke"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + // `charles` doesn't have permissions for the `king` role, so the first + // test should fail. However he has permissions for `king`'s nested roles + // so the next two tests should succeed. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("charles"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king"); + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } + + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("charles"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king/prince"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("charles"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king/prince/duke"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + // `j-welby` only has permissions for the role `king` itself, but not + // for its nested roles, therefore only the first of the following three + // tests should succeed. + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("j-welby"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king"); + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("j-welby"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king/prince"); + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } + + { + authorization::Request request; + request.set_action(authorization::DESTROY_RAW_DISK); + request.mutable_subject()->set_value("j-welby"); + request.mutable_object() + ->mutable_resource() + ->mutable_reservations() + ->Add() + ->set_role("king/prince/duke"); + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } +} + } // namespace tests { } // namespace internal { } // namespace mesos {
