Improve permission control of executor service proxies [API-2008] [5.…

…2.z] (#24451) We had a feature to perform permission control for invocations sent over the client. These permissions were missing from the message tasks of ExecutorService and DurableExecutor service. In this PR, similar to the permissions used in the ScheduledExecutorService, I have introduced two new permissions for `READ` and `MODIFY` operations and updated the message tasks with those. Also, I saw that we were not enforcing permissions for the invocations sent over the client-side ScheduledExecutorService proxy. I have fixed that and added tests for it. backport of #24271
hazelcast · May 11, 2023 · 5a452df · 5a452df
1 parent bf066a9
commit 5a452df
Show file tree

Hide file tree

Showing 17 changed files with 94 additions and 18 deletions.
diff --git a/.../client/impl/protocol/task/executorservice/ExecutorServiceCancelOnAddressMessageTask.java b/.../client/impl/protocol/task/executorservice/ExecutorServiceCancelOnAddressMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.executor.impl.operations.CancellationOperation;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.ExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -57,7 +59,8 @@ protected ClientMessage encodeResponse(Object response) {
 
     @Override
     public String getDistributedObjectName() {
-        return null;
+        DistributedExecutorService service = getService(getServiceName());
+        return service.getName(parameters.uuid);
     }
 
     @Override
@@ -67,7 +70,13 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        String name = getDistributedObjectName();
+        if (name == null) {
+            // The permission constructor expects a non-null name.
+            name = ExecutorServicePermission.EMPTY_EXECUTOR_NAME;
+        }
+
+        return new ExecutorServicePermission(name, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...lient/impl/protocol/task/executorservice/ExecutorServiceCancelOnPartitionMessageTask.java b/...lient/impl/protocol/task/executorservice/ExecutorServiceCancelOnPartitionMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.executor.impl.operations.CancellationOperation;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.ExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -50,7 +52,8 @@ protected ClientMessage encodeResponse(Object response) {
 
     @Override
     public String getDistributedObjectName() {
-        return null;
+        DistributedExecutorService service = getService(getServiceName());
+        return service.getName(parameters.uuid);
     }
 
     @Override
@@ -60,7 +63,13 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        String name = getDistributedObjectName();
+        if (name == null) {
+            // The permission constructor expects a non-null name.
+            name = ExecutorServicePermission.EMPTY_EXECUTOR_NAME;
+        }
+
+        return new ExecutorServicePermission(name, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...lcast/client/impl/protocol/task/executorservice/ExecutorServiceIsShutdownMessageTask.java b/...lcast/client/impl/protocol/task/executorservice/ExecutorServiceIsShutdownMessageTask.java
@@ -22,6 +22,8 @@
 import com.hazelcast.executor.impl.DistributedExecutorService;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.ExecutorServicePermission;
 
 import java.security.Permission;
 
@@ -55,12 +57,12 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new ExecutorServicePermission(parameters, ActionConstants.ACTION_READ);
     }
 
     @Override
     public String getDistributedObjectName() {
-        return null;
+        return parameters;
     }
 
     @Override

diff --git a/...zelcast/client/impl/protocol/task/executorservice/ExecutorServiceShutdownMessageTask.java b/...zelcast/client/impl/protocol/task/executorservice/ExecutorServiceShutdownMessageTask.java
@@ -22,6 +22,8 @@
 import com.hazelcast.executor.impl.DistributedExecutorService;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.ExecutorServicePermission;
 
 import java.security.Permission;
 
@@ -56,12 +58,12 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new ExecutorServicePermission(parameters, ActionConstants.ACTION_MODIFY);
     }
 
     @Override
     public String getDistributedObjectName() {
-        return null;
+        return parameters;
     }
 
     @Override

diff --git a/.../client/impl/protocol/task/executorservice/ExecutorServiceSubmitToAddressMessageTask.java b/.../client/impl/protocol/task/executorservice/ExecutorServiceSubmitToAddressMessageTask.java
@@ -25,6 +25,8 @@
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
 import com.hazelcast.security.SecurityContext;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.ExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import javax.security.auth.Subject;
@@ -83,7 +85,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new ExecutorServicePermission(parameters.name, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...lient/impl/protocol/task/executorservice/ExecutorServiceSubmitToPartitionMessageTask.java b/...lient/impl/protocol/task/executorservice/ExecutorServiceSubmitToPartitionMessageTask.java
@@ -25,6 +25,8 @@
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
 import com.hazelcast.security.SecurityContext;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.ExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import javax.security.auth.Subject;
@@ -75,7 +77,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new ExecutorServicePermission(parameters.name, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...t/impl/protocol/task/executorservice/durable/DurableExecutorDisposeResultMessageTask.java b/...t/impl/protocol/task/executorservice/durable/DurableExecutorDisposeResultMessageTask.java
@@ -22,6 +22,8 @@
 import com.hazelcast.durableexecutor.impl.operations.DisposeResultOperation;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.DurableExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -57,7 +59,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new DurableExecutorServicePermission(parameters.name, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...ient/impl/protocol/task/executorservice/durable/DurableExecutorIsShutdownMessageTask.java b/...ient/impl/protocol/task/executorservice/durable/DurableExecutorIsShutdownMessageTask.java
@@ -22,6 +22,8 @@
 import com.hazelcast.durableexecutor.impl.DistributedDurableExecutorService;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.DurableExecutorServicePermission;
 
 import java.security.Permission;
 
@@ -57,7 +59,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new DurableExecutorServicePermission(parameters, ActionConstants.ACTION_READ);
     }
 
     @Override

diff --git a/...ocol/task/executorservice/durable/DurableExecutorRetrieveAndDisposeResultMessageTask.java b/...ocol/task/executorservice/durable/DurableExecutorRetrieveAndDisposeResultMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.DurableExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -59,7 +61,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new DurableExecutorServicePermission(parameters.name, ActionConstants.ACTION_READ, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/.../impl/protocol/task/executorservice/durable/DurableExecutorRetrieveResultMessageTask.java b/.../impl/protocol/task/executorservice/durable/DurableExecutorRetrieveResultMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.DurableExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -59,7 +61,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new DurableExecutorServicePermission(parameters.name, ActionConstants.ACTION_READ);
     }
 
     @Override

diff --git a/...client/impl/protocol/task/executorservice/durable/DurableExecutorShutdownMessageTask.java b/...client/impl/protocol/task/executorservice/durable/DurableExecutorShutdownMessageTask.java
@@ -22,6 +22,8 @@
 import com.hazelcast.durableexecutor.impl.DistributedDurableExecutorService;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.DurableExecutorServicePermission;
 
 import java.security.Permission;
 
@@ -58,7 +60,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new DurableExecutorServicePermission(parameters, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...pl/protocol/task/executorservice/durable/DurableExecutorSubmitToPartitionMessageTask.java b/...pl/protocol/task/executorservice/durable/DurableExecutorSubmitToPartitionMessageTask.java
@@ -24,6 +24,8 @@
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
 import com.hazelcast.security.SecurityContext;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.DurableExecutorServicePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import javax.security.auth.Subject;
@@ -75,7 +77,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new DurableExecutorServicePermission(parameters.name, ActionConstants.ACTION_MODIFY);
     }
 
     @Override

diff --git a/...t/impl/protocol/task/scheduledexecutor/ScheduledExecutorSubmitToPartitionMessageTask.java b/...t/impl/protocol/task/scheduledexecutor/ScheduledExecutorSubmitToPartitionMessageTask.java
@@ -24,10 +24,12 @@
 import com.hazelcast.scheduledexecutor.impl.DistributedScheduledExecutorService;
 import com.hazelcast.scheduledexecutor.impl.TaskDefinition;
 import com.hazelcast.scheduledexecutor.impl.operations.ScheduleTaskOperation;
+import com.hazelcast.security.SecurityContext;
 import com.hazelcast.security.permission.ActionConstants;
 import com.hazelcast.security.permission.ScheduledExecutorPermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
+import javax.security.auth.Subject;
 import java.security.Permission;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
@@ -42,6 +44,13 @@ public ScheduledExecutorSubmitToPartitionMessageTask(ClientMessage clientMessage
     @Override
     protected Operation prepareOperation() {
         Callable callable = serializationService.toObject(parameters.task);
+        SecurityContext securityContext = clientEngine.getSecurityContext();
+        if (securityContext != null) {
+            Subject subject = endpoint.getSubject();
+            callable = securityContext.createSecureCallable(subject, callable);
+            serializationService.getManagedContext().initialize(callable);
+        }
+
         TaskDefinition def = new TaskDefinition(TaskDefinition.Type.getById(parameters.type),
                 parameters.taskName, callable, parameters.initialDelayInMillis, parameters.periodInMillis,
                 TimeUnit.MILLISECONDS, isAutoDisposable());

diff --git a/...ient/impl/protocol/task/scheduledexecutor/ScheduledExecutorSubmitToTargetMessageTask.java b/...ient/impl/protocol/task/scheduledexecutor/ScheduledExecutorSubmitToTargetMessageTask.java
@@ -25,10 +25,12 @@
 import com.hazelcast.scheduledexecutor.impl.DistributedScheduledExecutorService;
 import com.hazelcast.scheduledexecutor.impl.TaskDefinition;
 import com.hazelcast.scheduledexecutor.impl.operations.ScheduleTaskOperation;
+import com.hazelcast.security.SecurityContext;
 import com.hazelcast.security.permission.ActionConstants;
 import com.hazelcast.security.permission.ScheduledExecutorPermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
+import javax.security.auth.Subject;
 import java.security.Permission;
 import java.util.UUID;
 import java.util.concurrent.Callable;
@@ -44,6 +46,13 @@ public ScheduledExecutorSubmitToTargetMessageTask(ClientMessage clientMessage, N
     @Override
     protected Operation prepareOperation() {
         Callable callable = serializationService.toObject(parameters.task);
+        SecurityContext securityContext = clientEngine.getSecurityContext();
+        if (securityContext != null) {
+            Subject subject = endpoint.getSubject();
+            callable = securityContext.createSecureCallable(subject, callable);
+            serializationService.getManagedContext().initialize(callable);
+        }
+
         TaskDefinition def = new TaskDefinition(TaskDefinition.Type.getById(parameters.type),
                 parameters.taskName, callable, parameters.initialDelayInMillis, parameters.periodInMillis,
                 TimeUnit.MILLISECONDS, isAutoDisposable());

diff --git a/hazelcast/src/main/java/com/hazelcast/security/permission/ActionConstants.java b/hazelcast/src/main/java/com/hazelcast/security/permission/ActionConstants.java
@@ -37,6 +37,7 @@
 import com.hazelcast.multimap.impl.MultiMapService;
 import com.hazelcast.replicatedmap.impl.ReplicatedMapService;
 import com.hazelcast.ringbuffer.impl.RingbufferService;
+import com.hazelcast.scheduledexecutor.impl.DistributedScheduledExecutorService;
 import com.hazelcast.sql.impl.SqlInternalService;
 import com.hazelcast.topic.impl.TopicService;
 import com.hazelcast.topic.impl.reliable.ReliableTopicService;
@@ -112,6 +113,7 @@ public final class ActionConstants {
         PERMISSION_FACTORY_MAP.put(ReliableTopicService.SERVICE_NAME, ReliableTopicPermission::new);
         PERMISSION_FACTORY_MAP.put(JetServiceBackend.SERVICE_NAME, (name, actions) -> new JobPermission(actions));
         PERMISSION_FACTORY_MAP.put(SqlInternalService.SERVICE_NAME, SqlPermission::new);
+        PERMISSION_FACTORY_MAP.put(DistributedScheduledExecutorService.SERVICE_NAME, ScheduledExecutorPermission::new);
     }
 
     private ActionConstants() {

diff --git a/...ast/src/main/java/com/hazelcast/security/permission/DurableExecutorServicePermission.java b/...ast/src/main/java/com/hazelcast/security/permission/DurableExecutorServicePermission.java
@@ -18,7 +18,9 @@
 
 public class DurableExecutorServicePermission extends InstancePermission {
 
-    private static final int ALL = CREATE | DESTROY;
+    private static final int READ = 4;
+    private static final int MODIFY = 8;
+    private static final int ALL = CREATE | DESTROY | READ | MODIFY;
 
     public DurableExecutorServicePermission(String name, String... actions) {
         super(name, actions);
@@ -36,6 +38,10 @@ protected int initMask(String[] actions) {
                 mask |= CREATE;
             } else if (ActionConstants.ACTION_DESTROY.equals(action)) {
                 mask |= DESTROY;
+            } else if (ActionConstants.ACTION_READ.equals(action)) {
+                mask |= READ;
+            } else if (ActionConstants.ACTION_MODIFY.equals(action)) {
+                mask |= MODIFY;
             }
         }
         return mask;

diff --git a/hazelcast/src/main/java/com/hazelcast/security/permission/ExecutorServicePermission.java b/hazelcast/src/main/java/com/hazelcast/security/permission/ExecutorServicePermission.java
@@ -18,7 +18,15 @@
 
 public class ExecutorServicePermission extends InstancePermission {
 
-    private static final int ALL = CREATE | DESTROY;
+    /**
+     * The name of the executor used when no such executor
+     * is found for the client invocations.
+     */
+    public static final String EMPTY_EXECUTOR_NAME = "<EMPTY>";
+
+    private static final int READ = 4;
+    private static final int MODIFY = 8;
+    private static final int ALL = CREATE | DESTROY | READ | MODIFY;
 
     public ExecutorServicePermission(String name, String... actions) {
         super(name, actions);
@@ -36,6 +44,10 @@ protected int initMask(String[] actions) {
                 mask |= CREATE;
             } else if (ActionConstants.ACTION_DESTROY.equals(action)) {
                 mask |= DESTROY;
+            } else if (ActionConstants.ACTION_READ.equals(action)) {
+                mask |= READ;
+            } else if (ActionConstants.ACTION_MODIFY.equals(action)) {
+                mask |= MODIFY;
             }
         }
         return mask;