You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/08/01 18:38:16 UTC

[1/2] mesos git commit: Added an authorizer action for viewing of resource provider information.

Repository: mesos
Updated Branches:
  refs/heads/master 7fb8c3475 -> 58cd54459


Added an authorizer action for viewing of resource provider information.

Review: https://reviews.apache.org/r/68103/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d9a6c55e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d9a6c55e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d9a6c55e

Branch: refs/heads/master
Commit: d9a6c55e67bdaa07c7d5712997969b180654021c
Parents: 7fb8c34
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Wed Aug 1 11:30:54 2018 -0700
Committer: Chun-Hung Hsiao <ch...@mesosphere.io>
Committed: Wed Aug 1 11:30:54 2018 -0700

----------------------------------------------------------------------
 include/mesos/authorizer/acls.proto       | 11 ++++++
 include/mesos/authorizer/authorizer.proto |  4 ++
 src/authorizer/local/authorizer.cpp       | 23 +++++++++++
 src/tests/authorization_tests.cpp         | 54 ++++++++++++++++++++++++++
 4 files changed, 92 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d9a6c55e/include/mesos/authorizer/acls.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto
index 1777c04..f5d2580 100644
--- a/include/mesos/authorizer/acls.proto
+++ b/include/mesos/authorizer/acls.proto
@@ -556,6 +556,16 @@ message ACL {
     // 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.
+    required Entity principals = 1;
+
+    // Objects: Given implicitly. Use Entity type ANY or NONE to allow or deny
+    // access.
+    required Entity resource_providers = 2;
+  }
 }
 
 
@@ -639,4 +649,5 @@ message ACLs {
   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;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/d9a6c55e/include/mesos/authorizer/authorizer.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto
index 8b5fa09..7330416 100644
--- a/include/mesos/authorizer/authorizer.proto
+++ b/include/mesos/authorizer/authorizer.proto
@@ -269,6 +269,10 @@ enum Action {
 
   // `DESTROY_MOUNT_DISK` will have an object with `Resource` set.
   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;
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/d9a6c55e/src/authorizer/local/authorizer.cpp
----------------------------------------------------------------------
diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp
index abf5b46..f99b88e 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -413,6 +413,7 @@ public:
         case authorization::UPDATE_MAINTENANCE_SCHEDULE:
         case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG:
         case authorization::PRUNE_IMAGES:
+        case authorization::VIEW_RESOURCE_PROVIDER:
           aclObject.set_type(ACL::Entity::ANY);
 
           break;
@@ -731,6 +732,7 @@ public:
         case authorization::WAIT_NESTED_CONTAINER:
         case authorization::WAIT_STANDALONE_CONTAINER:
         case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG:
+        case authorization::VIEW_RESOURCE_PROVIDER:
         case authorization::UNKNOWN:
           UNREACHABLE();
       }
@@ -975,6 +977,7 @@ public:
       case authorization::WAIT_NESTED_CONTAINER:
       case authorization::WAIT_STANDALONE_CONTAINER:
       case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG:
+      case authorization::VIEW_RESOURCE_PROVIDER:
         UNREACHABLE();
     }
 
@@ -1193,6 +1196,7 @@ public:
       case authorization::WAIT_NESTED_CONTAINER:
       case authorization::WAIT_STANDALONE_CONTAINER:
       case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG:
+      case authorization::VIEW_RESOURCE_PROVIDER:
       case authorization::UNKNOWN: {
         Result<vector<GenericACL>> genericACLs =
           createGenericACLs(action, acls);
@@ -1558,6 +1562,18 @@ private:
         }
 
         return acls_;
+      case authorization::VIEW_RESOURCE_PROVIDER:
+        foreach (
+            const ACL::ViewResourceProvider& acl,
+            acls.view_resource_providers()) {
+          GenericACL acl_;
+          acl_.subjects = acl.principals();
+          acl_.objects = acl.resource_providers();
+
+          acls_.push_back(acl_);
+        }
+
+        return acls_;
       case authorization::REGISTER_FRAMEWORK:
       case authorization::CREATE_VOLUME:
       case authorization::RESIZE_VOLUME:
@@ -1747,6 +1763,13 @@ Option<Error> LocalAuthorizer::validate(const ACLs& acls)
     }
   }
 
+  foreach (const ACL::ViewResourceProvider& acl,
+           acls.view_resource_providers()) {
+    if (acl.resource_providers().type() == ACL::Entity::SOME) {
+      return Error("ACL.ViewResourceProvider 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/d9a6c55e/src/tests/authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/authorization_tests.cpp b/src/tests/authorization_tests.cpp
index 41ecac2..0c3e59e 100644
--- a/src/tests/authorization_tests.cpp
+++ b/src/tests/authorization_tests.cpp
@@ -4712,6 +4712,60 @@ TYPED_TEST(AuthorizationTest, ViewFlags)
 }
 
 
+TYPED_TEST(AuthorizationTest, ViewResourceProvider)
+{
+  // Setup ACLs.
+  ACLs acls;
+
+  {
+    // "foo" principal can view resource provider information.
+    mesos::ACL::ViewResourceProvider* acl = acls.add_view_resource_providers();
+    acl->mutable_principals()->add_values("foo");
+    acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::ANY);
+  }
+
+  {
+    // Nobody else can view resource provider information.
+    mesos::ACL::ViewResourceProvider* acl = acls.add_view_resource_providers();
+    acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+    acl->mutable_resource_providers()->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());
+
+  {
+    authorization::Request request;
+    request.set_action(authorization::VIEW_RESOURCE_PROVIDER);
+    request.mutable_subject()->set_value("foo");
+
+    AWAIT_EXPECT_TRUE(authorizer->authorized(request));
+  }
+
+  {
+    authorization::Request request;
+    request.set_action(authorization::VIEW_RESOURCE_PROVIDER);
+    request.mutable_subject()->set_value("bar");
+    AWAIT_EXPECT_FALSE(authorizer->authorized(request));
+  }
+
+  // Test that no authorizer is created with invalid ACLs.
+  {
+    ACLs invalid;
+
+    mesos::ACL::ViewResourceProvider* acl =
+      invalid.add_view_resource_providers();
+    acl->mutable_principals()->add_values("foo");
+    acl->mutable_resource_providers()->add_values("yoda");
+
+    Try<Authorizer*> create = TypeParam::create(parameterize(invalid));
+    EXPECT_ERROR(create);
+  }
+}
+
+
 TYPED_TEST(AuthorizationTest, SetLogLevel)
 {
   // Setup ACLs.


[2/2] mesos git commit: Implemented authorization for agent `GET_RESOURCE_PROVIDER` calls.

Posted by ch...@apache.org.
Implemented authorization for agent `GET_RESOURCE_PROVIDER` calls.

Review: https://reviews.apache.org/r/68104/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/58cd5445
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/58cd5445
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/58cd5445

Branch: refs/heads/master
Commit: 58cd544596fa499d632da060e36d1ab6987b2dbd
Parents: d9a6c55
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Wed Aug 1 11:31:01 2018 -0700
Committer: Chun-Hung Hsiao <ch...@mesosphere.io>
Committed: Wed Aug 1 11:31:01 2018 -0700

----------------------------------------------------------------------
 src/slave/http.cpp      | 42 ++++++++++++++++++++++++++----------------
 src/tests/api_tests.cpp | 19 ++++++++++++++++++-
 2 files changed, 44 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/58cd5445/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index ab5864d..1b6d266 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -85,6 +85,7 @@ using mesos::authorization::VIEW_FLAGS;
 using mesos::authorization::VIEW_FRAMEWORK;
 using mesos::authorization::VIEW_TASK;
 using mesos::authorization::VIEW_EXECUTOR;
+using mesos::authorization::VIEW_RESOURCE_PROVIDER;
 using mesos::authorization::VIEW_ROLE;
 using mesos::authorization::SET_LOG_LEVEL;
 using mesos::authorization::ATTACH_CONTAINER_INPUT;
@@ -1813,27 +1814,36 @@ Future<Response> Http::getResourceProviders(
 
   LOG(INFO) << "Processing GET_RESOURCE_PROVIDERS call";
 
-  // TODO(nfnt): Authorize this call (MESOS-8314).
-
-  agent::Response response;
-  response.set_type(mesos::agent::Response::GET_RESOURCE_PROVIDERS);
+  return ObjectApprovers::create(
+      slave->authorizer, principal, {VIEW_RESOURCE_PROVIDER})
+    .then(defer(
+        slave->self(),
+        [this, acceptType](
+            const Owned<ObjectApprovers>& approvers) -> Response {
+          agent::Response response;
+          response.set_type(mesos::agent::Response::GET_RESOURCE_PROVIDERS);
 
-  agent::Response::GetResourceProviders* resourceProviders =
-    response.mutable_get_resource_providers();
+          agent::Response::GetResourceProviders* resourceProviders =
+            response.mutable_get_resource_providers();
 
-  foreachvalue (ResourceProvider* resourceProvider,
-                slave->resourceProviders) {
-    agent::Response::GetResourceProviders::ResourceProvider* provider =
-      resourceProviders->add_resource_providers();
+          foreachvalue (
+              ResourceProvider* resourceProvider, slave->resourceProviders) {
+            if (!approvers->approved<VIEW_RESOURCE_PROVIDER>()) {
+              continue;
+            }
+            agent::Response::GetResourceProviders::ResourceProvider* provider =
+              resourceProviders->add_resource_providers();
 
-    provider->mutable_resource_provider_info()
-      ->CopyFrom(resourceProvider->info);
+            provider->mutable_resource_provider_info()->CopyFrom(
+                resourceProvider->info);
 
-    provider->mutable_total_resources()->CopyFrom(
-        resourceProvider->totalResources);
-  }
+            provider->mutable_total_resources()->CopyFrom(
+                resourceProvider->totalResources);
+          }
 
-  return OK(serialize(acceptType, evolve(response)), stringify(acceptType));
+          return OK(
+              serialize(acceptType, evolve(response)), stringify(acceptType));
+        }));
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cd5445/src/tests/api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index 182622a..5cf35e6 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -7000,6 +7000,15 @@ TEST_P(AgentAPITest, GetResourceProviders)
     FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
 
   slave::Flags slaveFlags = CreateSlaveFlags();
+  slaveFlags.authenticate_http_readwrite = true;
+
+  {
+    // `DEFAULT_CREDENTIAL_2` is not allowed to view any resource providers.
+    mesos::ACL::ViewResourceProvider* acl =
+      slaveFlags.acls->add_view_resource_providers();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
+    acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::NONE);
+  }
 
   Try<Owned<cluster::Slave>> slave = StartSlave(&detector, slaveFlags);
   ASSERT_SOME(slave);
@@ -7047,7 +7056,6 @@ TEST_P(AgentAPITest, GetResourceProviders)
   AWAIT_READY(v1Response);
   ASSERT_TRUE(v1Response->IsInitialized());
   ASSERT_EQ(v1::agent::Response::GET_RESOURCE_PROVIDERS, v1Response->type());
-
   EXPECT_EQ(1, v1Response->get_resource_providers().resource_providers_size());
 
   const mesos::v1::ResourceProviderInfo& responseInfo =
@@ -7067,6 +7075,15 @@ TEST_P(AgentAPITest, GetResourceProviders)
       .total_resources();
 
   EXPECT_EQ(v1::Resources(resource), responseResources);
+
+  // `DEFAULT_CREDENTIAL_2` is not allow to view any resource provider
+  // information.
+  v1Response =
+    post(slave.get()->pid, v1Call, contentType, DEFAULT_CREDENTIAL_2);
+  AWAIT_READY(v1Response);
+  ASSERT_TRUE(v1Response->IsInitialized());
+  ASSERT_EQ(v1::agent::Response::GET_RESOURCE_PROVIDERS, v1Response->type());
+  EXPECT_EQ(0, v1Response->get_resource_providers().resource_providers_size());
 }