[ignite] branch master updated: IGNITE-12621 Node leave may cause NullPointerException during IO message processing if security is enabled - Fixes #7366.

[ir...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

irakov 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 14dd160  IGNITE-12621 Node leave may cause NullPointerException during IO message processing if security is enabled - Fixes #7366.
14dd160 is described below

commit 14dd160f906d46ddd51563b817cc573dfc16b945
Author: Slava Koptilin <sl...@gmail.com>
AuthorDate: Fri Feb 7 19:00:55 2020 +0300

    IGNITE-12621 Node leave may cause NullPointerException during IO message processing if security is enabled - Fixes #7366.
    
    Signed-off-by: Ivan Rakov <ir...@apache.org>
---
 .../managers/communication/GridIoManager.java      |  9 ++--
 .../managers/discovery/GridDiscoveryManager.java   | 17 ++++++++
 .../security/IgniteSecurityProcessor.java          | 19 ++++++++-
 .../processors/security/SecurityUtils.java         |  5 ++-
 .../security/IgniteSecurityProcessorTest.java      | 48 ++++++++++++++++++++++
 .../ignite/testsuites/SecurityTestSuite.java       |  2 +
 6 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java b/modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java
index e8aeb53..898245e 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java
@@ -2378,15 +2378,18 @@ public class GridIoManager extends GridManagerAdapter<CommunicationSpi<Serializa
     }
 
     public void addUserMessageListener(final @Nullable Object topic, final @Nullable IgniteBiPredicate<UUID, ?> p) {
-        addUserMessageListener(topic, p, null);
+        addUserMessageListener(topic, p, ctx.localNodeId());
     }
 
     /**
      * @param topic Topic to subscribe to.
      * @param p Message predicate.
      */
-    public void addUserMessageListener(final @Nullable Object topic,
-        final @Nullable IgniteBiPredicate<UUID, ?> p, final @Nullable UUID nodeId) {
+    public void addUserMessageListener(
+        final @Nullable Object topic,
+        final @Nullable IgniteBiPredicate<UUID, ?> p,
+        final UUID nodeId
+    ) {
         if (p != null) {
             try {
                 if (p instanceof PlatformMessageFilter)
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java b/modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java
index 8cc317a..1ed95de 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java
@@ -2483,6 +2483,23 @@ public class GridDiscoveryManager extends GridManagerAdapter<DiscoverySpi> {
         ((IgniteDiscoverySpi)spi).resolveCommunicationFailure(node, err);
     }
 
+    /**
+     * Resolves by ID cluster node which is alive or has recently left the cluster.
+     *
+     * @param nodeId Node id.
+     * @return resolved node, or <code>null</code> if node not found.
+     */
+    public ClusterNode historicalNode(UUID nodeId) {
+        for (DiscoCache discoCache : discoCacheHist.descendingValues()) {
+            ClusterNode node = discoCache.node(nodeId);
+
+            if (node != null)
+                return node;
+        }
+
+        return null;
+    }
+
     /** Worker for network segment checks. */
     private class SegmentCheckWorker extends GridWorker {
         /** */
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 c8f8868..c6cd587 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
@@ -20,6 +20,7 @@ package org.apache.ignite.internal.processors.security;
 import java.security.Security;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -110,12 +111,28 @@ public class IgniteSecurityProcessor implements IgniteSecurity, GridProcessor {
         return withContext(
             secCtxs.computeIfAbsent(nodeId,
                 uuid -> nodeSecurityContext(
-                    marsh, U.resolveClassLoader(ctx.config()), ctx.discovery().node(uuid)
+                    marsh, U.resolveClassLoader(ctx.config()), findNode(uuid)
                 )
             )
         );
     }
 
+    /**
+     * Resolves cluster node by its ID.
+     *
+     * @param nodeId Node id.
+     * @throws IllegalStateException If node with provided ID doesn't exist.
+     */
+    private ClusterNode findNode(UUID nodeId) {
+        ClusterNode node = Optional.ofNullable(ctx.discovery().node(nodeId))
+            .orElseGet(() -> ctx.discovery().historicalNode(nodeId));
+
+        if (node == null)
+            throw new IllegalStateException("Failed to find node with given ID for security context setup: " + nodeId);
+
+        return node;
+    }
+
     /** {@inheritDoc} */
     @Override public SecurityContext securityContext() {
         SecurityContext res = curSecCtx.get();
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java
index b7e84d7..c83b1d3 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/SecurityUtils.java
@@ -38,6 +38,7 @@ import org.apache.ignite.internal.GridInternalWrapper;
 import org.apache.ignite.internal.GridKernalContext;
 import org.apache.ignite.internal.IgniteNodeAttributes;
 import org.apache.ignite.internal.processors.security.sandbox.IgniteSandbox;
+import org.apache.ignite.internal.util.typedef.internal.A;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.marshaller.Marshaller;
 import org.apache.ignite.plugin.security.SecurityException;
@@ -59,7 +60,7 @@ public class SecurityUtils {
     private static final int DFLT_SERIALIZE_VERSION = isSecurityCompatibilityMode() ? 1 : 2;
 
     /** Current serialization version. */
-    private static final ThreadLocal<Integer> SERIALIZE_VERSION = new ThreadLocal<Integer>(){
+    private static final ThreadLocal<Integer> SERIALIZE_VERSION = new ThreadLocal<Integer>() {
         @Override protected Integer initialValue() {
             return DFLT_SERIALIZE_VERSION;
         }
@@ -132,6 +133,8 @@ public class SecurityUtils {
      * @return Node's security context.
      */
     public static SecurityContext nodeSecurityContext(Marshaller marsh, ClassLoader ldr, ClusterNode node) {
+        A.notNull(node, "Cluster node");
+
         byte[] subjBytes = node.attribute(IgniteNodeAttributes.ATTR_SECURITY_SUBJECT_V2);
 
         if (subjBytes == null)
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessorTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessorTest.java
new file mode 100644
index 0000000..b0441e0
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessorTest.java
@@ -0,0 +1,48 @@
+/*
+ * 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 java.util.UUID;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.GridKernalContext;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.junit.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit test for {@link IgniteSecurityProcessor}.
+ */
+public class IgniteSecurityProcessorTest {
+    /**
+     * Checks that {@link IgniteSecurityProcessor#withContext(UUID)} throws exception in case a node ID is unknown.
+     */
+    @Test(expected = IllegalStateException.class)
+    public void testThrowIllegalStateExceptionIfNodeNotFoundInDiscoCache() {
+        GridKernalContext ctx = mock(GridKernalContext.class);
+        when(ctx.config()).thenReturn(new IgniteConfiguration());
+        when(ctx.discovery()).thenReturn(mock(GridDiscoveryManager.class));
+
+        GridSecurityProcessor secPrc = mock(GridSecurityProcessor.class);
+
+        IgniteSecurityProcessor ignSecPrc = new IgniteSecurityProcessor(ctx, secPrc);
+
+        ignSecPrc.withContext(UUID.randomUUID());
+    }
+}
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 270c533..3329d3b 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.IgniteSecurityProcessorTest;
 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.ContinuousQueryPermissionCheckTest;
@@ -78,6 +79,7 @@ import org.junit.runners.Suite;
     IgniteOperationsInsideSandboxTest.class,
     SecuritySubjectPermissionsTest.class,
     AccessToClassesInsideInternalPackageTest.class,
+    IgniteSecurityProcessorTest.class
 })
 public class SecurityTestSuite {
 }