This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 93f19a4 IGNITE-12232 NPE while authenticating joined node - Fixes
#6911.
93f19a4 is described below
commit 93f19a464f70e630e71ac53a48c63d66058674ce
Author: Dmitrii Ryabov <[email protected]>
AuthorDate: Fri Oct 18 11:22:54 2019 +0300
IGNITE-12232 NPE while authenticating joined node - Fixes #6911.
Signed-off-by: Aleksey Plekhanov <[email protected]>
---
.../ignite/spi/discovery/tcp/ServerImpl.java | 22 ++++++--
.../processors/security/AbstractSecurityTest.java | 5 +-
.../processors/security/InvalidServerTest.java | 65 ++++++++++++++++++++++
.../client/ThinClientPermissionCheckTest.java | 2 +-
.../security/impl/TestSecurityPluginProvider.java | 9 ++-
.../security/impl/TestSecurityProcessor.java | 16 +++++-
.../ignite/testsuites/SecurityTestSuite.java | 2 +
7 files changed, 110 insertions(+), 11 deletions(-)
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 490eb9c..4a4b15a 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
@@ -2044,7 +2044,11 @@ class ServerImpl extends TcpDiscoveryImpl {
* @param rmtPerms The second set of permissions.
* @return {@code True} if given parameters contain the same permissions,
{@code False} otherwise.
*/
- private boolean permissionsEqual(SecurityPermissionSet locPerms,
SecurityPermissionSet rmtPerms) {
+ private boolean permissionsEqual(@Nullable SecurityPermissionSet locPerms,
+ @Nullable SecurityPermissionSet rmtPerms) {
+ if (locPerms == null || rmtPerms == null)
+ return false;
+
boolean dfltAllowMatch = locPerms.defaultAllowAll() ==
rmtPerms.defaultAllowAll();
boolean bothHaveSamePerms =
F.eqNotOrdered(rmtPerms.systemPermissions(), locPerms.systemPermissions()) &&
@@ -4737,7 +4741,7 @@ class ServerImpl extends TcpDiscoveryImpl {
spi.marshaller(),
U.resolveClassLoader(spi.ignite().configuration()), node
);
- if
(!permissionsEqual(coordSubj.subject().permissions(),
subj.subject().permissions())) {
+ if (!permissionsEqual(getPermissions(coordSubj),
getPermissions(subj))) {
// Node has not pass authentication.
LT.warn(log, "Authentication failed [nodeId="
+ node.id() +
", addrs=" + U.addressesAsString(node) +
']');
@@ -4838,8 +4842,7 @@ class ServerImpl extends TcpDiscoveryImpl {
spi.marshaller(), ldr, locNode
);
- if
(!permissionsEqual(locCrd.subject().permissions(),
- rmCrd.subject().permissions())) {
+ if
(!permissionsEqual(getPermissions(locCrd), getPermissions(rmCrd))) {
// Node has not pass authentication.
LT.warn(log,
"Failed to authenticate local node
" +
@@ -4938,6 +4941,17 @@ class ServerImpl extends TcpDiscoveryImpl {
}
/**
+ * @param secCtx Security context.
+ * @return Security permission set.
+ */
+ private @Nullable SecurityPermissionSet getPermissions(SecurityContext
secCtx) {
+ if (secCtx == null || secCtx.subject() == null)
+ return null;
+
+ return secCtx.subject().permissions();
+ }
+
+ /**
* Processes node add finished message.
*
* @param msg Node add finished message.
diff --git
a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/AbstractSecurityTest.java
b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/AbstractSecurityTest.java
index cc02d67..13688dc 100644
---
a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/AbstractSecurityTest.java
+++
b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/AbstractSecurityTest.java
@@ -35,6 +35,9 @@ public class AbstractSecurityTest extends
GridCommonAbstractTest {
/** Empty array of permissions. */
protected static final SecurityPermission[] EMPTY_PERMS = new
SecurityPermission[0];
+ /** Global authentication flag. */
+ protected boolean globalAuth;
+
/** {@inheritDoc} */
@Override protected void afterTestsStopped() throws Exception {
stopAllGrids();
@@ -76,7 +79,7 @@ public class AbstractSecurityTest extends
GridCommonAbstractTest {
* @param isClient Is client.
*/
protected IgniteEx startGrid(String login, SecurityPermissionSet prmSet,
boolean isClient) throws Exception {
- return startGrid(getConfiguration(login, new
TestSecurityPluginProvider(login, "", prmSet))
+ return startGrid(getConfiguration(login, new
TestSecurityPluginProvider(login, "", prmSet, globalAuth))
.setClientMode(isClient));
}
}
diff --git
a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/InvalidServerTest.java
b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/InvalidServerTest.java
new file mode 100644
index 0000000..7e2e6dc
--- /dev/null
+++
b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/InvalidServerTest.java
@@ -0,0 +1,65 @@
+/*
+ * 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;
+
+import org.apache.ignite.IgniteAuthenticationException;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import
org.apache.ignite.internal.processors.security.impl.TestSecurityProcessor;
+import org.apache.ignite.plugin.security.SecurityCredentials;
+import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
+import
org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractMessage;
+import
org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryJoinRequestMessage;
+import
org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryNodeAddedMessage;
+import org.junit.Test;
+
+import static
org.apache.ignite.testframework.GridTestUtils.assertThrowsWithCause;
+
+/**
+ * Test server connection when it's permissions are removed after
+ * {@link TcpDiscoveryJoinRequestMessage} processed.
+ */
+public class InvalidServerTest extends AbstractSecurityTest {
+ /** Test server name. */
+ private static final String TEST_SERVER_NAME = "test_server";
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String
instanceName,
+ AbstractTestSecurityPluginProvider pluginProv) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(instanceName,
pluginProv);
+
+ cfg.setDiscoverySpi(new TcpDiscoverySpi() {
+ @Override protected void
startMessageProcess(TcpDiscoveryAbstractMessage msg) {
+ if (msg instanceof TcpDiscoveryNodeAddedMessage &&
msg.verified())
+ TestSecurityProcessor.PERMS.remove(new
SecurityCredentials(TEST_SERVER_NAME, ""));
+ }
+ });
+
+ return cfg;
+ }
+
+ /** */
+ @Test
+ public void testInvalidServer() throws Exception {
+ globalAuth = true;
+
+ startGridAllowAll("server1");
+ startGridAllowAll("server2");
+
+ assertThrowsWithCause(() -> startGridAllowAll(TEST_SERVER_NAME),
IgniteAuthenticationException.class);
+ }
+}
diff --git
a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/ThinClientPermissionCheckTest.java
b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/ThinClientPermissionCheckTest.java
index 3259a94..6f5354c 100644
---
a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/ThinClientPermissionCheckTest.java
+++
b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/ThinClientPermissionCheckTest.java
@@ -99,7 +99,7 @@ public class ThinClientPermissionCheckTest extends
AbstractSecurityTest {
return getConfiguration(
instanceName,
- new TestSecurityPluginProvider("srv_" + instanceName, null,
ALLOW_ALL, clientData)
+ new TestSecurityPluginProvider("srv_" + instanceName, null,
ALLOW_ALL, false, clientData)
).setCacheConfiguration(
new CacheConfiguration().setName(CACHE),
new CacheConfiguration().setName(FORBIDDEN_CACHE)
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 9c3e871..5ee18ff 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
@@ -34,15 +34,19 @@ public class TestSecurityPluginProvider extends
AbstractTestSecurityPluginProvid
/** Permissions. */
private final SecurityPermissionSet perms;
+ /** Global authentication. */
+ private final boolean globalAuth;
+
/** Users security data. */
private final TestSecurityData[] clientData;
/** */
- public TestSecurityPluginProvider(String login, String pwd,
SecurityPermissionSet perms,
+ public TestSecurityPluginProvider(String login, String pwd,
SecurityPermissionSet perms, boolean globalAuth,
TestSecurityData... clientData) {
this.login = login;
this.pwd = pwd;
this.perms = perms;
+ this.globalAuth = globalAuth;
this.clientData = clientData.clone();
}
@@ -50,6 +54,7 @@ public class TestSecurityPluginProvider extends
AbstractTestSecurityPluginProvid
@Override protected GridSecurityProcessor
securityProcessor(GridKernalContext ctx) {
return new TestSecurityProcessor(ctx,
new TestSecurityData(login, pwd, perms),
- Arrays.asList(clientData));
+ Arrays.asList(clientData),
+ 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 0bda5b7..b62f97b 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
@@ -46,7 +46,7 @@ import static
org.apache.ignite.plugin.security.SecuritySubjectType.REMOTE_NODE;
*/
public class TestSecurityProcessor extends GridProcessorAdapter implements
GridSecurityProcessor {
/** Permissions. */
- private static final Map<SecurityCredentials, SecurityPermissionSet> PERMS
= new ConcurrentHashMap<>();
+ public static final Map<SecurityCredentials, SecurityPermissionSet> PERMS
= new ConcurrentHashMap<>();
/** Node security data. */
private final TestSecurityData nodeSecData;
@@ -54,21 +54,28 @@ public class TestSecurityProcessor extends
GridProcessorAdapter implements GridS
/** Users security data. */
private final Collection<TestSecurityData> predefinedAuthData;
+ /** Global authentication. */
+ private final boolean globalAuth;
+
/**
* Constructor.
*/
public TestSecurityProcessor(GridKernalContext ctx, TestSecurityData
nodeSecData,
- Collection<TestSecurityData> predefinedAuthData) {
+ Collection<TestSecurityData> predefinedAuthData, boolean globalAuth) {
super(ctx);
this.nodeSecData = nodeSecData;
this.predefinedAuthData = predefinedAuthData.isEmpty()
? Collections.emptyList()
: new ArrayList<>(predefinedAuthData);
+ this.globalAuth = globalAuth;
}
/** {@inheritDoc} */
@Override public SecurityContext authenticateNode(ClusterNode node,
SecurityCredentials cred) {
+ if (!PERMS.containsKey(cred))
+ return null;
+
return new TestSecurityContext(
new TestSecuritySubject()
.setType(REMOTE_NODE)
@@ -81,11 +88,14 @@ public class TestSecurityProcessor extends
GridProcessorAdapter implements GridS
/** {@inheritDoc} */
@Override public boolean isGlobalNodeAuthentication() {
- return false;
+ return globalAuth;
}
/** {@inheritDoc} */
@Override public SecurityContext authenticate(AuthenticationContext ctx) {
+ if (!PERMS.containsKey(ctx.credentials()))
+ return null;
+
return new TestSecurityContext(
new TestSecuritySubject()
.setType(ctx.subjectType())
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 f48ee48..5b7725e 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
@@ -17,6 +17,7 @@
package org.apache.ignite.testsuites;
+import org.apache.ignite.internal.processors.security.InvalidServerTest;
import
org.apache.ignite.internal.processors.security.cache.CacheOperationPermissionCheckTest;
import
org.apache.ignite.internal.processors.security.cache.EntryProcessorPermissionCheckTest;
import
org.apache.ignite.internal.processors.security.cache.ScanQueryPermissionCheckTest;
@@ -52,6 +53,7 @@ import org.junit.runners.Suite;
DataStreamerRemoteSecurityContextCheckTest.class,
CacheLoadRemoteSecurityContextCheckTest.class,
ThinClientPermissionCheckTest.class,
+ InvalidServerTest.class,
})
public class SecurityTestSuite {
}
