From aba0aaa5b8dd36ac7ff1329bd99c7753cea1dcde Mon Sep 17 00:00:00 2001 From: Mikhail Petrov <32207922+petrov-mg@users.noreply.github.com> Date: Fri, 10 Nov 2023 21:19:43 +0300 Subject: [PATCH] IGNITE-19807 Deprecated legacy authorization approach via Security Context. (#10800) --- .../security/IgniteSecurityProcessor.java | 35 +- .../processors/security/SecurityContext.java | 12 + .../plugin/security/SecuritySubject.java | 8 + .../ignite/spi/discovery/tcp/ServerImpl.java | 12 +- .../cluster/NodeJoinPermissionsTest.java | 329 ++++++++++++++++++ .../impl/TestSecurityPluginProvider.java | 2 +- .../security/impl/TestSecurityProcessor.java | 2 +- .../ignite/testsuites/SecurityTestSuite.java | 4 +- .../zk/ZookeeperDiscoverySpiTestSuite4.java | 4 +- 9 files changed, 392 insertions(+), 16 deletions(-) create mode 100644 modules/core/src/test/java/org/apache/ignite/internal/processors/security/cluster/NodeJoinPermissionsTest.java diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessor.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessor.java index 0840426667f7c..65c983f843532 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessor.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessor.java @@ -53,6 +53,7 @@ import static org.apache.ignite.internal.processors.security.SecurityUtils.hasSecurityManager; import static org.apache.ignite.internal.processors.security.SecurityUtils.nodeSecurityContext; import static org.apache.ignite.plugin.security.SecurityPermission.ADMIN_USER_ACCESS; +import static org.apache.ignite.plugin.security.SecurityPermission.JOIN_AS_SERVER; /** * Default {@code IgniteSecurity} implementation. @@ -363,7 +364,14 @@ else if (packAccess.contains(',' + IGNITE_INTERNAL_PACKAGE)) @Override public @Nullable IgniteNodeValidationResult validateNode(ClusterNode node) { IgniteNodeValidationResult res = validateSecProcClass(node); - return res != null ? res : secPrc.validateNode(node); + if (res == null) { + res = validateNodeJoinPermission(node); + + if (res == null) + res = secPrc.validateNode(node); + } + + return res; } /** {@inheritDoc} */ @@ -443,6 +451,31 @@ private IgniteNodeValidationResult validateSecProcClass(ClusterNode node) { return null; } + /** */ + private IgniteNodeValidationResult validateNodeJoinPermission(ClusterNode node) { + if (node.isClient()) + return null; + + SecurityContext secCtx = nodeSecurityContext( + marsh, + U.resolveClassLoader(ctx.config()), + node + ); + + try { + if (!secCtx.systemOperationAllowed(JOIN_AS_SERVER)) + secPrc.authorize(null, JOIN_AS_SERVER, secCtx); + + return null; + } + catch (SecurityException e) { + String msg = "Node is not authorized to join as a server node [joiningNodeId=" + node.id() + + ", addrs=" + U.addressesAsString(node) + ']'; + + return new IgniteNodeValidationResult(node.id(), msg, msg); + } + } + /** @return Security processor implementation to which current security facade delegates operations. */ public GridSecurityProcessor securityProcessor() { return secPrc; diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityContext.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityContext.java index 623b866d13c68..90ba76328c71d 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityContext.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityContext.java @@ -35,7 +35,10 @@ public interface SecurityContext { * @param taskClsName Task class name. * @param perm Permission to check. * @return {@code True} if task operation is allowed. + * @deprecated Use {@link IgniteSecurity#authorize(String, SecurityPermission)} instead. + * This method will be removed in the future releases. */ + @Deprecated public boolean taskOperationAllowed(String taskClsName, SecurityPermission perm); /** @@ -44,7 +47,10 @@ public interface SecurityContext { * @param cacheName Cache name. * @param perm Permission to check. * @return {@code True} if cache operation is allowed. + * @deprecated Use {@link IgniteSecurity#authorize(String, SecurityPermission)} instead. + * This method will be removed in the future releases. */ + @Deprecated public boolean cacheOperationAllowed(String cacheName, SecurityPermission perm); /** @@ -53,7 +59,10 @@ public interface SecurityContext { * @param srvcName Service name. * @param perm Permission to check. * @return {@code True} if task operation is allowed. + * @deprecated Use {@link IgniteSecurity#authorize(String, SecurityPermission)} instead. + * This method will be removed in the future releases. */ + @Deprecated public boolean serviceOperationAllowed(String srvcName, SecurityPermission perm); /** @@ -61,6 +70,9 @@ public interface SecurityContext { * * @param perm Permission to check. * @return {@code True} if system operation is allowed. + * @deprecated Use {@link IgniteSecurity#authorize(SecurityPermission)} instead. + * This method will be removed in the future releases. */ + @Deprecated public boolean systemOperationAllowed(SecurityPermission perm); } diff --git a/modules/core/src/main/java/org/apache/ignite/plugin/security/SecuritySubject.java b/modules/core/src/main/java/org/apache/ignite/plugin/security/SecuritySubject.java index 5d4e624f236c4..f83fe3d9643d9 100644 --- a/modules/core/src/main/java/org/apache/ignite/plugin/security/SecuritySubject.java +++ b/modules/core/src/main/java/org/apache/ignite/plugin/security/SecuritySubject.java @@ -70,12 +70,20 @@ public default Certificate[] certificates() { * Authorized permission set for the subject. * * @return Authorized permission set for the subject. + * @deprecated {@link SecuritySubject} must contain only immutable set of + * information that represents a security principal. Security permissions are part of authorization process + * and have nothing to do with {@link SecuritySubject}. This method will be removed in the future releases. */ + @Deprecated public SecurityPermissionSet permissions(); /** * @return Permissions for SecurityManager checks. + * @deprecated {@link SecuritySubject} must contain only immutable set of + * information that represents a security principal. Security permissions are part of authorization process + * and have nothing to do with {@link SecuritySubject}. This method will be removed in the future releases. */ + @Deprecated public default PermissionCollection sandboxPermissions() { ProtectionDomain pd = SecurityUtils.doPrivileged(() -> getClass().getProtectionDomain()); diff --git a/modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java b/modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java index 8c85ee1819b75..c28c88b851f7c 100644 --- a/modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java @@ -114,7 +114,6 @@ import org.apache.ignite.lang.IgniteProductVersion; import org.apache.ignite.lang.IgniteUuid; import org.apache.ignite.plugin.security.SecurityCredentials; -import org.apache.ignite.plugin.security.SecurityPermission; import org.apache.ignite.plugin.security.SecurityPermissionSet; import org.apache.ignite.spi.IgniteNodeValidationResult; import org.apache.ignite.spi.IgniteSpiContext; @@ -4317,23 +4316,14 @@ else if (log.isDebugEnabled()) return; } else { - String authFailedMsg = null; - if (!(subj instanceof Serializable)) { // Node has not pass authentication. LT.warn(log, "Authentication subject is not Serializable [nodeId=" + node.id() + ", addrs=" + U.addressesAsString(node) + ']'); - authFailedMsg = "Authentication subject is not serializable"; - } - else if (node.clientRouterNodeId() == null && - !subj.systemOperationAllowed(SecurityPermission.JOIN_AS_SERVER)) - authFailedMsg = "Node is not authorised to join as a server node"; - - if (authFailedMsg != null) { // Always output in debug. if (log.isDebugEnabled()) - log.debug(authFailedMsg + " [nodeId=" + node.id() + + log.debug("Authentication subject is not serializable [nodeId=" + node.id() + ", addrs=" + U.addressesAsString(node)); try { diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/cluster/NodeJoinPermissionsTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/cluster/NodeJoinPermissionsTest.java new file mode 100644 index 0000000000000..b641cd7b634eb --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/cluster/NodeJoinPermissionsTest.java @@ -0,0 +1,329 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.security.cluster; + +import java.io.Serializable; +import java.net.InetSocketAddress; +import java.util.Arrays; +import java.util.Collection; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.apache.ignite.cluster.ClusterNode; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.internal.GridKernalContext; +import org.apache.ignite.internal.processors.security.AbstractSecurityTest; +import org.apache.ignite.internal.processors.security.AbstractTestSecurityPluginProvider; +import org.apache.ignite.internal.processors.security.GridSecurityProcessor; +import org.apache.ignite.internal.processors.security.SecurityContext; +import org.apache.ignite.internal.processors.security.impl.TestSecurityData; +import org.apache.ignite.internal.processors.security.impl.TestSecurityPluginProvider; +import org.apache.ignite.internal.processors.security.impl.TestSecurityProcessor; +import org.apache.ignite.internal.processors.security.impl.TestSecuritySubject; +import org.apache.ignite.internal.util.typedef.F; +import org.apache.ignite.plugin.security.AuthenticationContext; +import org.apache.ignite.plugin.security.SecurityCredentials; +import org.apache.ignite.plugin.security.SecurityException; +import org.apache.ignite.plugin.security.SecurityPermission; +import org.apache.ignite.plugin.security.SecurityPermissionSet; +import org.apache.ignite.plugin.security.SecuritySubject; +import org.apache.ignite.spi.IgniteSpiException; +import org.apache.ignite.testframework.GridTestUtils; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.apache.ignite.events.EventType.EVT_CLIENT_NODE_RECONNECTED; +import static org.apache.ignite.plugin.security.SecurityPermission.JOIN_AS_SERVER; +import static org.apache.ignite.plugin.security.SecurityPermissionSetBuilder.systemPermissions; +import static org.apache.ignite.plugin.security.SecuritySubjectType.REMOTE_NODE; + +/** */ +@RunWith(Parameterized.class) +public class NodeJoinPermissionsTest extends AbstractSecurityTest { + /** */ + @Parameterized.Parameter + public boolean isLegacyAuthApproach; + + /** */ + @Parameterized.Parameters(name = "isLegacyAuthorizationApproach={0}") + public static Object[] parameters() { + return new Object[] { false, true }; + } + + /** {@inheritDoc} */ + @Override protected void beforeTest() throws Exception { + super.beforeTest(); + + stopAllGrids(); + } + + /** */ + private IgniteConfiguration configuration(int idx, SecurityPermission... sysPermissions) throws Exception { + String login = getTestIgniteInstanceName(idx); + + AbstractTestSecurityPluginProvider secPuginProv = isLegacyAuthApproach + ? new TestSecurityPluginProvider( + login, + "", + systemPermissions(sysPermissions), + false) + : new SecurityPluginProvider( + login, + "", + systemPermissions(sysPermissions), + false); + + return getConfiguration(login, secPuginProv); + } + + /** */ + @Test + public void testNodeJoinPermissions() throws Exception { + // The first node can start successfully without any permissions granted. + startGrid(configuration(0)); + + GridTestUtils.assertThrowsAnyCause( + log, + () -> startGrid(configuration(1)), + IgniteSpiException.class, + "Node is not authorized to join as a server node" + ); + + assertEquals(1, grid(0).cluster().nodes().size()); + + startGrid(configuration(1, JOIN_AS_SERVER)); + + assertEquals(2, grid(0).cluster().nodes().size()); + + // Client node can join cluster without any permissions granted. + startClientGrid(configuration(2)); + + assertEquals(3, grid(0).cluster().nodes().size()); + + // Client node can reconnect without any permissions granted. + CountDownLatch clientNodeReconnectedLatch = new CountDownLatch(1); + + grid(2).events().localListen(evt -> { + clientNodeReconnectedLatch.countDown(); + + return true; + }, EVT_CLIENT_NODE_RECONNECTED); + + ignite(0).context().discovery().failNode(nodeId(2), null); + + assertTrue(clientNodeReconnectedLatch.await(getTestTimeout(), TimeUnit.MILLISECONDS)); + + assertEquals(3, grid(0).cluster().nodes().size()); + } + + /** */ + private static class SecurityPluginProvider extends TestSecurityPluginProvider { + /** */ + public SecurityPluginProvider( + String login, + String pwd, + SecurityPermissionSet perms, + boolean globalAuth, + TestSecurityData... clientData + ) { + super(login, pwd, perms, globalAuth, clientData); + } + + /** {@inheritDoc} */ + @Override protected GridSecurityProcessor securityProcessor(GridKernalContext ctx) { + return new SecurityProcessor( + ctx, + new TestSecurityData(login, pwd, perms, sandboxPerms), + Arrays.asList(clientData), + globalAuth + ); + } + } + + /** + * Security Processor implementaiton that does not pass user security permissions to the Security Context and + * expects all authorization checks to be delegated exclusively to {@link GridSecurityProcessor#authorize}. + */ + private static class SecurityProcessor extends TestSecurityProcessor { + /** */ + public SecurityProcessor( + GridKernalContext ctx, + TestSecurityData nodeSecData, + Collection predefinedAuthData, + boolean globalAuth + ) { + super(ctx, nodeSecData, predefinedAuthData, globalAuth); + } + + /** {@inheritDoc} */ + @Override public SecurityContext authenticateNode(ClusterNode node, SecurityCredentials cred) { + TestSecurityData data = USERS.get(cred.getLogin()); + + if (data == null || !Objects.equals(cred, data.credentials())) + return null; + + SecurityContext res = new TestSecurityContext( + new TestSecuritySubject() + .setType(REMOTE_NODE) + .setId(node.id()) + .setAddr(new InetSocketAddress(F.first(node.addresses()), 0)) + .setLogin(cred.getLogin()) + .sandboxPermissions(data.sandboxPermissions()) + ); + + SECURITY_CONTEXTS.put(res.subject().id(), res); + + return res; + } + + /** {@inheritDoc} */ + @Override public SecurityContext authenticate(AuthenticationContext ctx) { + TestSecurityData data = USERS.get(ctx.credentials().getLogin()); + + if (data == null || !Objects.equals(ctx.credentials(), data.credentials())) + return null; + + SecurityContext res = new TestSecurityContext( + new TestSecuritySubject() + .setType(ctx.subjectType()) + .setId(ctx.subjectId()) + .setAddr(ctx.address()) + .setLogin(ctx.credentials().getLogin()) + .setCerts(ctx.certificates()) + .sandboxPermissions(data.sandboxPermissions()) + ); + + SECURITY_CONTEXTS.put(res.subject().id(), res); + + return res; + } + + /** {@inheritDoc} */ + @Override public void authorize( + String name, + SecurityPermission perm, + SecurityContext securityCtx + ) throws SecurityException { + TestSecurityData userData = USERS.get(securityCtx.subject().login()); + + if (userData == null || !contains(userData.permissions(), name, perm)) { + throw new SecurityException("Authorization failed [perm=" + perm + + ", name=" + name + + ", subject=" + securityCtx.subject() + ']'); + } + } + + /** */ + public static boolean contains(SecurityPermissionSet userPerms, String name, SecurityPermission perm) { + boolean dfltAllowAll = userPerms.defaultAllowAll(); + + switch (perm) { + case CACHE_PUT: + case CACHE_READ: + case CACHE_REMOVE: + return contains(userPerms.cachePermissions(), dfltAllowAll, name, perm); + + case CACHE_CREATE: + case CACHE_DESTROY: + return (name != null && contains(userPerms.cachePermissions(), dfltAllowAll, name, perm)) + || containsSystemPermission(userPerms, perm); + + case TASK_CANCEL: + case TASK_EXECUTE: + return contains(userPerms.taskPermissions(), dfltAllowAll, name, perm); + + case SERVICE_DEPLOY: + case SERVICE_INVOKE: + case SERVICE_CANCEL: + return contains(userPerms.servicePermissions(), dfltAllowAll, name, perm); + + default: + return containsSystemPermission(userPerms, perm); + } + } + + /** */ + private static boolean contains( + Map> userPerms, + boolean dfltAllowAll, + String name, + SecurityPermission perm + ) { + Collection perms = userPerms.get(name); + + if (perms == null) + return dfltAllowAll; + + return perms.stream().anyMatch(perm::equals); + } + + /** */ + private static boolean containsSystemPermission( + SecurityPermissionSet userPerms, + SecurityPermission perm + ) { + Collection sysPerms = userPerms.systemPermissions(); + + if (F.isEmpty(sysPerms)) + return userPerms.defaultAllowAll(); + + return sysPerms.stream().anyMatch(perm::equals); + } + } + + /** */ + private static class TestSecurityContext implements SecurityContext, Serializable { + /** */ + private static final long serialVersionUID = 0L; + + /** */ + private final SecuritySubject subj; + + /** */ + public TestSecurityContext(SecuritySubject subj) { + this.subj = subj; + } + + /** {@inheritDoc} */ + @Override public SecuritySubject subject() { + return subj; + } + + /** {@inheritDoc} */ + @Override public boolean taskOperationAllowed(String taskClsName, SecurityPermission perm) { + return false; + } + + /** {@inheritDoc} */ + @Override public boolean cacheOperationAllowed(String cacheName, SecurityPermission perm) { + return false; + } + + /** {@inheritDoc} */ + @Override public boolean serviceOperationAllowed(String srvcName, SecurityPermission perm) { + return false; + } + + /** {@inheritDoc} */ + @Override public boolean systemOperationAllowed(SecurityPermission perm) { + return false; + } + } +} diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityPluginProvider.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityPluginProvider.java index 1302bdcad8a54..89ed5f7c6f5f7 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityPluginProvider.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityPluginProvider.java @@ -36,7 +36,7 @@ public class TestSecurityPluginProvider extends AbstractTestSecurityPluginProvid protected final SecurityPermissionSet perms; /** */ - private final Permissions sandboxPerms; + protected final Permissions sandboxPerms; /** Global authentication. */ protected final boolean globalAuth; diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityProcessor.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityProcessor.java index e0b2ce60b89e1..308e1d6c9cdb7 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityProcessor.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/impl/TestSecurityProcessor.java @@ -53,7 +53,7 @@ public class TestSecurityProcessor extends GridProcessorAdapter implements GridS public static final Map USERS = new ConcurrentHashMap<>(); /** */ - private static final Map SECURITY_CONTEXTS = new ConcurrentHashMap<>(); + protected static final Map SECURITY_CONTEXTS = new ConcurrentHashMap<>(); /** */ private static final Collection> EXT_SYS_CLASSES = ConcurrentHashMap.newKeySet(); diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/SecurityTestSuite.java b/modules/core/src/test/java/org/apache/ignite/testsuites/SecurityTestSuite.java index b754c1dbfe998..64dc1d9e3a93a 100644 --- a/modules/core/src/test/java/org/apache/ignite/testsuites/SecurityTestSuite.java +++ b/modules/core/src/test/java/org/apache/ignite/testsuites/SecurityTestSuite.java @@ -41,6 +41,7 @@ import org.apache.ignite.internal.processors.security.client.ThinClientSslPermissionCheckTest; import org.apache.ignite.internal.processors.security.cluster.ClusterNodeOperationPermissionTest; import org.apache.ignite.internal.processors.security.cluster.ClusterStatePermissionTest; +import org.apache.ignite.internal.processors.security.cluster.NodeJoinPermissionsTest; import org.apache.ignite.internal.processors.security.compute.ComputePermissionCheckTest; import org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskCancelRemoteSecurityContextCheckTest; import org.apache.ignite.internal.processors.security.compute.closure.ComputeTaskRemoteSecurityContextCheckTest; @@ -138,7 +139,8 @@ ServiceAuthorizationTest.class, ServiceStaticConfigTest.class, ClusterNodeOperationPermissionTest.class, - NodeSecurityContextPropagationTest.class + NodeSecurityContextPropagationTest.class, + NodeJoinPermissionsTest.class }) public class SecurityTestSuite { /** */ diff --git a/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/ZookeeperDiscoverySpiTestSuite4.java b/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/ZookeeperDiscoverySpiTestSuite4.java index e42adaf25c478..6a6d1f9a7012a 100644 --- a/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/ZookeeperDiscoverySpiTestSuite4.java +++ b/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/ZookeeperDiscoverySpiTestSuite4.java @@ -27,6 +27,7 @@ import org.apache.ignite.internal.processors.cache.distributed.replicated.IgniteCacheReplicatedQuerySelfTest; import org.apache.ignite.internal.processors.metastorage.DistributedMetaStoragePersistentTest; import org.apache.ignite.internal.processors.metastorage.DistributedMetaStorageTest; +import org.apache.ignite.internal.processors.security.cluster.NodeJoinPermissionsTest; import org.apache.ignite.spi.discovery.DiscoverySpiDataExchangeTest; import org.junit.BeforeClass; import org.junit.runner.RunWith; @@ -48,7 +49,8 @@ DistributedMetaStoragePersistentTest.class, IgniteNodeValidationFailedEventTest.class, DiscoverySpiDataExchangeTest.class, - CacheCreateDestroyEventSecurityContextTest.class + CacheCreateDestroyEventSecurityContextTest.class, + NodeJoinPermissionsTest.class }) public class ZookeeperDiscoverySpiTestSuite4 { /** */