Extend permission checks in MessageTasks and add a test coverage [HZ-…

…2090] [5.2.z] (#25540) Backport of #25509 This PR extends permission checks in client messages and adds basic test coverage.
hazelcast · Oct 16, 2023 · 2560c33 · 2560c33
1 parent 1425b64
commit 2560c33
Show file tree

Hide file tree

Showing 24 changed files with 298 additions and 25 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -7,3 +7,4 @@
 **/client/**           @hazelcast/apis
 **/hazelcast-client**  @hazelcast/apis
 **/serialization**     @hazelcast/apis
+**/MessageTaskSecurityTest* @hazelcast/security-working-group
diff --git a/...lcast/src/main/java/com/hazelcast/client/impl/protocol/task/CreateProxiesMessageTask.java b/...lcast/src/main/java/com/hazelcast/client/impl/protocol/task/CreateProxiesMessageTask.java
@@ -22,10 +22,14 @@
 import com.hazelcast.core.MemberLeftException;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.SecurityContext;
+import com.hazelcast.security.permission.ActionConstants;
 import com.hazelcast.spi.impl.operationservice.Operation;
+import com.hazelcast.spi.impl.proxyservice.ProxyService;
 import com.hazelcast.spi.impl.proxyservice.impl.ProxyInfo;
 import com.hazelcast.spi.impl.proxyservice.impl.operations.PostJoinProxyOperation;
 
+import java.security.AccessControlException;
 import java.security.Permission;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -37,6 +41,8 @@
 public class CreateProxiesMessageTask extends AbstractMultiTargetMessageTask<List<Map.Entry<String, String>>>
         implements Supplier<Operation> {
 
+    private List<Map.Entry<String, String>> filteredProxies;
+
     public CreateProxiesMessageTask(ClientMessage clientMessage, Node node, Connection connection) {
         super(clientMessage, node, connection);
     }
@@ -47,8 +53,8 @@ protected Supplier<Operation> createOperationSupplier() {
 
     @Override
     public Operation get() {
-        List<ProxyInfo> proxyInfos = new ArrayList<ProxyInfo>(parameters.size());
-        for (Map.Entry<String, String> proxy : parameters) {
+        List<ProxyInfo> proxyInfos = new ArrayList<ProxyInfo>(filteredProxies.size());
+        for (Map.Entry<String, String> proxy : filteredProxies) {
             proxyInfos.add(new ProxyInfo(proxy.getValue(), proxy.getKey(), endpoint.getUuid()));
         }
         return new PostJoinProxyOperation(proxyInfos);
@@ -79,11 +85,49 @@ protected ClientMessage encodeResponse(Object response) {
         return ClientCreateProxiesCodec.encodeResponse();
     }
 
+    /**
+     *@see #beforeProcess()
+     */
     @Override
     public Permission getRequiredPermission() {
         return null;
     }
 
+    @Override
+    protected void beforeProcess() {
+        // replacement for getRequiredPermission-based checks, we have to check multiple permission
+        SecurityContext securityContext = clientEngine.getSecurityContext();
+        if (securityContext != null) {
+            filteredProxies = new ArrayList<>(parameters.size());
+            ProxyService proxyService = clientEngine.getProxyService();
+            for (Map.Entry<String, String> proxy : parameters) {
+                String objectName = proxy.getKey();
+                String serviceName = proxy.getValue();
+                if (proxyService.existsDistributedObject(serviceName, objectName)) {
+                    continue;
+                }
+                try {
+                    Permission permission = ActionConstants.getPermission(objectName, serviceName,
+                            ActionConstants.ACTION_CREATE);
+                    securityContext.checkPermission(endpoint.getSubject(), permission);
+                    filteredProxies.add(proxy);
+                } catch (AccessControlException ace) {
+                    logger.info("Insufficient client permissions. Proxy won't be created for type '" + serviceName + "': "
+                            + objectName);
+                    if (logger.isFineEnabled()) {
+                        logger.fine("Skipping proxy creation due to AccessControlException", ace);
+                    }
+                } catch (Exception e) {
+                    // unknown serviceName or another unexpected issue
+                    logger.warning("Proxy won't be created for type '" + serviceName + "': " + objectName, e);
+                }
+            }
+        } else {
+            filteredProxies = parameters;
+        }
+        super.beforeProcess();
+    }
+
     @Override
     public String getServiceName() {
         return null;

diff --git a/.../hazelcast/client/impl/protocol/task/cache/CacheAddNearCacheInvalidationListenerTask.java b/.../hazelcast/client/impl/protocol/task/cache/CacheAddNearCacheInvalidationListenerTask.java
@@ -26,6 +26,8 @@
 import com.hazelcast.internal.nearcache.impl.invalidation.Invalidation;
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 
 import java.security.Permission;
 import java.util.List;
@@ -118,7 +120,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_LISTEN);
     }
 
 }
diff --git a/...m/hazelcast/client/impl/protocol/task/cache/CacheAddPartitionLostListenerMessageTask.java b/...m/hazelcast/client/impl/protocol/task/cache/CacheAddPartitionLostListenerMessageTask.java
@@ -26,6 +26,8 @@
 import com.hazelcast.client.impl.protocol.task.AbstractAddListenerMessageTask;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 import com.hazelcast.spi.impl.eventservice.EventFilter;
 import com.hazelcast.spi.impl.eventservice.EventRegistration;
 import com.hazelcast.spi.impl.eventservice.EventService;
@@ -96,7 +98,7 @@ public Object[] getParameters() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_LISTEN);
     }
 
     @Override

diff --git a/...main/java/com/hazelcast/client/impl/protocol/task/cache/CacheCreateConfigMessageTask.java b/...main/java/com/hazelcast/client/impl/protocol/task/cache/CacheCreateConfigMessageTask.java
@@ -16,6 +16,7 @@
 
 package com.hazelcast.client.impl.protocol.task.cache;
 
+import com.hazelcast.cache.CacheUtil;
 import com.hazelcast.cache.impl.CacheService;
 import com.hazelcast.cache.impl.ICacheService;
 import com.hazelcast.cache.impl.PreJoinCacheConfig;
@@ -26,6 +27,8 @@
 import com.hazelcast.config.CacheConfig;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 import com.hazelcast.spi.impl.InternalCompletableFuture;
 import com.hazelcast.spi.merge.SplitBrainMergePolicyProvider;
 
@@ -79,7 +82,8 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        CacheConfig cacheConfig = parameters.cacheConfig.asCacheConfig(serializationService);
+        return new CachePermission(CacheUtil.getDistributedObjectName(cacheConfig.getName()), ActionConstants.ACTION_CREATE);
     }
 
     @Override

diff --git a/.../src/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheDestroyMessageTask.java b/.../src/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheDestroyMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.client.impl.protocol.task.AbstractInvocationMessageTask;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 import com.hazelcast.spi.impl.operationservice.InvocationBuilder;
 import com.hazelcast.spi.impl.operationservice.Operation;
 import com.hazelcast.spi.impl.operationservice.impl.OperationServiceImpl;
@@ -66,12 +68,12 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters, ActionConstants.ACTION_DESTROY);
     }
 
     @Override
     public String getDistributedObjectName() {
-        return null;
+        return parameters;
     }
 
     @Override

diff --git a/...rc/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheGetConfigMessageTask.java b/...rc/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheGetConfigMessageTask.java
@@ -24,6 +24,8 @@
 import com.hazelcast.config.CacheConfig;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -70,7 +72,7 @@ public Object[] getParameters() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_READ);
     }
 
     @Override

diff --git a/.../src/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheIterateMessageTask.java b/.../src/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheIterateMessageTask.java
@@ -24,8 +24,11 @@
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.iteration.IterationPointer;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
+import java.security.Permission;
 import java.util.Collections;
 
 import static com.hazelcast.internal.iteration.IterationPointer.decodePointers;
@@ -79,4 +82,9 @@ public Object[] getParameters() {
     public String getMethodName() {
         return "iterator";
     }
+
+    @Override
+    public Permission getRequiredPermission() {
+        return new CachePermission(parameters.name, ActionConstants.ACTION_READ);
+    }
 }
diff --git a/...a/com/hazelcast/client/impl/protocol/task/cache/CacheListenerRegistrationMessageTask.java b/...a/com/hazelcast/client/impl/protocol/task/cache/CacheListenerRegistrationMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.client.impl.protocol.task.AbstractTargetMessageTask;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import javax.cache.configuration.CacheEntryListenerConfiguration;
@@ -69,7 +71,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_LISTEN);
     }
 
     @Override

diff --git a/.../src/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheLoadAllMessageTask.java b/.../src/main/java/com/hazelcast/client/impl/protocol/task/cache/CacheLoadAllMessageTask.java
@@ -25,6 +25,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.CachePermission;
 import com.hazelcast.spi.impl.operationservice.OperationFactory;
 
 import javax.cache.CacheException;
@@ -84,7 +86,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_READ);
     }
 
     @Override

diff --git a/.../java/com/hazelcast/client/impl/protocol/task/cache/CacheManagementConfigMessageTask.java b/.../java/com/hazelcast/client/impl/protocol/task/cache/CacheManagementConfigMessageTask.java
@@ -23,6 +23,7 @@
 import com.hazelcast.client.impl.protocol.task.AbstractTargetMessageTask;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ConfigPermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -67,7 +68,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new ConfigPermission();
     }
 
     @Override

diff --git a/...hazelcast/client/impl/protocol/task/cache/CacheRemoveInvalidationListenerMessageTask.java b/...hazelcast/client/impl/protocol/task/cache/CacheRemoveInvalidationListenerMessageTask.java
@@ -22,6 +22,8 @@
 import com.hazelcast.client.impl.protocol.task.AbstractRemoveListenerMessageTask;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 
 import java.security.Permission;
 import java.util.UUID;
@@ -72,7 +74,7 @@ public String getDistributedObjectName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_LISTEN);
     }
 
     @Override

diff --git a/...azelcast/client/impl/protocol/task/cache/CacheRemovePartitionLostListenerMessageTask.java b/...azelcast/client/impl/protocol/task/cache/CacheRemovePartitionLostListenerMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.client.impl.protocol.task.AbstractRemoveListenerMessageTask;
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.CachePermission;
 
 import java.security.Permission;
 import java.util.UUID;
@@ -70,7 +72,7 @@ public String getDistributedObjectName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new CachePermission(parameters.name, ActionConstants.ACTION_LISTEN);
     }
 
     @Override

diff --git a/...src/main/java/com/hazelcast/client/impl/protocol/task/map/MapDestroyCacheMessageTask.java b/...src/main/java/com/hazelcast/client/impl/protocol/task/map/MapDestroyCacheMessageTask.java
@@ -23,6 +23,8 @@
 import com.hazelcast.instance.impl.Node;
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.map.impl.querycache.subscriber.operation.DestroyQueryCacheOperation;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.MapPermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -76,7 +78,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new MapPermission(parameters.mapName, ActionConstants.ACTION_DESTROY);
     }
 
     @Override

diff --git a/...src/main/java/com/hazelcast/client/impl/protocol/task/map/MapFetchEntriesMessageTask.java b/...src/main/java/com/hazelcast/client/impl/protocol/task/map/MapFetchEntriesMessageTask.java
@@ -24,6 +24,8 @@
 import com.hazelcast.map.impl.MapService;
 import com.hazelcast.map.impl.iterator.MapEntriesWithCursor;
 import com.hazelcast.map.impl.operation.MapOperationProvider;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.MapPermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -67,7 +69,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new MapPermission(parameters.name, ActionConstants.ACTION_READ);
     }
 
     @Override

diff --git a/...st/src/main/java/com/hazelcast/client/impl/protocol/task/map/MapFetchKeysMessageTask.java b/...st/src/main/java/com/hazelcast/client/impl/protocol/task/map/MapFetchKeysMessageTask.java
@@ -24,6 +24,8 @@
 import com.hazelcast.map.impl.MapService;
 import com.hazelcast.map.impl.iterator.MapKeysWithCursor;
 import com.hazelcast.map.impl.operation.MapOperationProvider;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.MapPermission;
 import com.hazelcast.spi.impl.operationservice.Operation;
 
 import java.security.Permission;
@@ -67,7 +69,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new MapPermission(parameters.name, ActionConstants.ACTION_READ);
     }
 
     @Override

diff --git a/.../main/java/com/hazelcast/client/impl/protocol/task/map/MapPublisherCreateMessageTask.java b/.../main/java/com/hazelcast/client/impl/protocol/task/map/MapPublisherCreateMessageTask.java
@@ -33,6 +33,8 @@
 import com.hazelcast.internal.nio.Connection;
 import com.hazelcast.internal.serialization.Data;
 import com.hazelcast.query.Predicate;
+import com.hazelcast.security.permission.ActionConstants;
+import com.hazelcast.security.permission.MapPermission;
 import com.hazelcast.spi.impl.operationservice.InvocationBuilder;
 import com.hazelcast.spi.impl.operationservice.impl.OperationServiceImpl;
 import com.hazelcast.internal.util.ExceptionUtil;
@@ -136,7 +138,7 @@ public String getServiceName() {
 
     @Override
     public Permission getRequiredPermission() {
-        return null;
+        return new MapPermission(parameters.mapName, ActionConstants.ACTION_LISTEN);
     }
 
     @Override