You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2020/07/28 06:25:57 UTC
[ignite] branch ignite-2.9 updated: IGNITE-13300 Fix Ignite sandbox
vulnerability (user code can be executed in privileged proxy) - Fixes
#8084.
This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch ignite-2.9
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/ignite-2.9 by this push:
new dd9da2b IGNITE-13300 Fix Ignite sandbox vulnerability (user code can be executed in privileged proxy) - Fixes #8084.
dd9da2b is described below
commit dd9da2b5270c70e2f7acf0c25927cc52e7da076e
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Tue Jul 28 11:14:02 2020 +0500
IGNITE-13300 Fix Ignite sandbox vulnerability (user code can be executed in privileged proxy) - Fixes #8084.
Signed-off-by: Aleksey Plekhanov <pl...@gmail.com>
(cherry picked from commit bd3543eadc509e5f9f16a3ccc311ea860a65db44)
---
.../src/main/java/org/apache/ignite/Ignition.java | 2 +-
.../GridResourceProxiedIgniteInjector.java | 4 +-
.../processors/security/SecurityUtils.java | 26 +++++-
.../sandbox/SandboxIgniteComponentProxy.java | 40 ++++++---
.../sandbox/IgniteOperationsInsideSandboxTest.java | 95 ++++++++++++++++++++++
.../security/sandbox/PrivilegedProxyTest.java | 61 ++++++++++++++
.../ignite/testsuites/SecurityTestSuite.java | 2 +
7 files changed, 212 insertions(+), 18 deletions(-)
diff --git a/modules/core/src/main/java/org/apache/ignite/Ignition.java b/modules/core/src/main/java/org/apache/ignite/Ignition.java
index d0ae268..163a2af 100644
--- a/modules/core/src/main/java/org/apache/ignite/Ignition.java
+++ b/modules/core/src/main/java/org/apache/ignite/Ignition.java
@@ -562,7 +562,7 @@ public class Ignition {
*/
private static Ignite wrapToProxy(Ignite ignite) {
return AccessController.doPrivileged((PrivilegedAction<Ignite>)
- () -> SandboxIgniteComponentProxy.proxy(Ignite.class, ignite)
+ () -> SandboxIgniteComponentProxy.igniteProxy(ignite)
);
}
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/resource/GridResourceProxiedIgniteInjector.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/resource/GridResourceProxiedIgniteInjector.java
index 58a6e4b..9d7a378 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/resource/GridResourceProxiedIgniteInjector.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/resource/GridResourceProxiedIgniteInjector.java
@@ -23,7 +23,7 @@ import org.apache.ignite.internal.IgniteEx;
import org.apache.ignite.internal.managers.deployment.GridDeployment;
import static org.apache.ignite.internal.processors.security.SecurityUtils.isSystemType;
-import static org.apache.ignite.internal.processors.security.sandbox.SandboxIgniteComponentProxy.proxy;
+import static org.apache.ignite.internal.processors.security.sandbox.SandboxIgniteComponentProxy.igniteProxy;
/** Ignite instance injector. */
public class GridResourceProxiedIgniteInjector extends GridResourceBasicInjector<Ignite> {
@@ -37,7 +37,7 @@ public class GridResourceProxiedIgniteInjector extends GridResourceBasicInjector
/** */
private Ignite ignite(Object target) {
return isSystemType(((IgniteEx)getResource()).context(), target, false)
- ? getResource() : proxy(Ignite.class, getResource());
+ ? getResource() : igniteProxy(getResource());
}
/** {@inheritDoc} */
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 bc92277..229cd27 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
@@ -24,16 +24,20 @@ import java.lang.reflect.Proxy;
import java.security.AccessControlContext;
import java.security.AccessController;
import java.security.AllPermission;
+import java.security.CodeSource;
import java.security.Permissions;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
+import java.security.ProtectionDomain;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.IgniteException;
import org.apache.ignite.IgniteSystemProperties;
@@ -43,6 +47,7 @@ import org.apache.ignite.internal.GridKernalContext;
import org.apache.ignite.internal.IgniteNodeAttributes;
import org.apache.ignite.internal.processors.security.sandbox.IgniteDomainCombiner;
import org.apache.ignite.internal.processors.security.sandbox.IgniteSandbox;
+import org.apache.ignite.internal.util.typedef.F;
import org.apache.ignite.internal.util.typedef.internal.A;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.marshaller.Marshaller;
@@ -74,6 +79,12 @@ public class SecurityUtils {
/** Permissions that contain {@code AllPermission}. */
public static final Permissions ALL_PERMISSIONS;
+ /** Code source for ignite-core module. */
+ private static final CodeSource CORE_CODE_SOURCE = SecurityUtils.class.getProtectionDomain().getCodeSource();
+
+ /** System types cache. */
+ private static final ConcurrentMap<Class<?>, Boolean> SYSTEM_TYPES = new ConcurrentHashMap<>();
+
static {
ALL_PERMISSIONS = new Permissions();
@@ -182,12 +193,19 @@ public class SecurityUtils {
* @return True if class of {@code target} is a system type.
*/
public static boolean isSystemType(GridKernalContext ctx, Object target, boolean considerWrapperCls) {
- Class cls = considerWrapperCls && target instanceof GridInternalWrapper
- ? ((GridInternalWrapper)target).userObject().getClass()
+ Class<?> cls = considerWrapperCls && target instanceof GridInternalWrapper
+ ? ((GridInternalWrapper<?>)target).userObject().getClass()
: target.getClass();
- return ctx.getClass().getClassLoader() == cls.getClassLoader()
- && ctx.marshallerContext().isSystemType(cls.getName());
+ Boolean isSysType = SYSTEM_TYPES.get(cls);
+
+ if (isSysType == null) {
+ ProtectionDomain pd = doPrivileged(cls::getProtectionDomain);
+
+ SYSTEM_TYPES.put(cls, isSysType = (pd == null) || F.eq(CORE_CODE_SOURCE, pd.getCodeSource()));
+ }
+
+ return isSysType;
}
/**
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/sandbox/SandboxIgniteComponentProxy.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/sandbox/SandboxIgniteComponentProxy.java
index a8d79ef..b5dfcbe 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/security/sandbox/SandboxIgniteComponentProxy.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/security/sandbox/SandboxIgniteComponentProxy.java
@@ -34,12 +34,14 @@ import org.apache.ignite.IgniteCountDownLatch;
import org.apache.ignite.IgniteDataStreamer;
import org.apache.ignite.IgniteLock;
import org.apache.ignite.IgniteQueue;
-import org.apache.ignite.IgniteScheduler;
import org.apache.ignite.IgniteSemaphore;
import org.apache.ignite.IgniteSet;
import org.apache.ignite.IgniteTransactions;
+import org.apache.ignite.binary.BinaryObject;
import org.apache.ignite.cache.affinity.Affinity;
import org.apache.ignite.cache.query.QueryCursor;
+import org.apache.ignite.internal.GridKernalContext;
+import org.apache.ignite.internal.IgniteEx;
import org.apache.ignite.internal.processors.security.SecurityUtils;
import org.apache.ignite.internal.util.lang.GridIterator;
import org.apache.ignite.transactions.Transaction;
@@ -47,12 +49,12 @@ import org.apache.ignite.transactions.Transaction;
/** Create instace of Ignite component proxy to use inside the Ignite Sandbox. */
public final class SandboxIgniteComponentProxy {
/** The array of classes that should be proxied. */
- private static final Class[] PROXIED_CLASSES = new Class[] {
+ private static final Class<?>[] PROXIED_CLASSES = new Class[] {
+ // TODO https://issues.apache.org/jira/browse/IGNITE-13301 IgniteScheduler is not covered by sandbox yet.
Ignite.class,
IgniteCache.class,
IgniteCompute.class,
ExecutorService.class,
- IgniteScheduler.class,
IgniteTransactions.class,
IgniteDataStreamer.class,
IgniteAtomicSequence.class,
@@ -68,19 +70,27 @@ public final class SandboxIgniteComponentProxy {
Affinity.class,
QueryCursor.class,
GridIterator.class,
- Transaction.class
+ Transaction.class,
+ BinaryObject.class
};
/**
+ * @return The proxy of {@code ignite} to use inside the Ignite Sandbox.
+ */
+ public static Ignite igniteProxy(Ignite ignite) {
+ return proxy(((IgniteEx)ignite).context(), Ignite.class, ignite);
+ }
+
+ /**
* @return The proxy of {@code instance} to use inside the Ignite Sandbox.
*/
- public static <T> T proxy(Class cls, T instance) {
+ private static <T> T proxy(GridKernalContext ctx, Class<?> cls, T instance) {
Objects.requireNonNull(cls, "Parameter 'cls' cannot be null.");
Objects.requireNonNull(instance, "Parameter 'instance' cannot be null.");
return SecurityUtils.doPrivileged(
() -> (T)Proxy.newProxyInstance(cls.getClassLoader(), new Class[] {cls},
- new SandboxIgniteComponentProxyHandler(instance))
+ new SandboxIgniteComponentProxyHandler(ctx, instance))
);
}
@@ -89,8 +99,12 @@ public final class SandboxIgniteComponentProxy {
/** */
private final Object original;
+ /** Context. */
+ private final GridKernalContext ctx;
+
/** */
- public SandboxIgniteComponentProxyHandler(Object original) {
+ public SandboxIgniteComponentProxyHandler(GridKernalContext ctx, Object original) {
+ this.ctx = ctx;
this.original = original;
}
@@ -98,14 +112,18 @@ public final class SandboxIgniteComponentProxy {
@Override public Object invoke(Object proxy, Method mtd, Object[] args) throws Throwable {
Object res = SecurityUtils.doPrivileged(() -> mtd.invoke(original, args));
- Class cls = proxiedClass(res);
+ if (res != null && SecurityUtils.isSystemType(ctx, res, true)) {
+ Class<?> cls = proxiedClass(res);
+
+ return cls != null ? proxy(ctx, cls, res) : res;
+ }
- return cls != null ? proxy(cls, res) : res;
+ return res;
}
/** */
- private Class proxiedClass(Object obj) {
- for (Class cls : PROXIED_CLASSES) {
+ private Class<?> proxiedClass(Object obj) {
+ for (Class<?> cls : PROXIED_CLASSES) {
if (cls.isInstance(obj))
return cls;
}
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/IgniteOperationsInsideSandboxTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/IgniteOperationsInsideSandboxTest.java
index f0df33c..a4a6e87 100644
--- a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/IgniteOperationsInsideSandboxTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/IgniteOperationsInsideSandboxTest.java
@@ -23,10 +23,20 @@ import java.util.Map;
import java.util.concurrent.ExecutionException;
import javax.cache.Cache;
import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteAtomicLong;
+import org.apache.ignite.IgniteAtomicReference;
+import org.apache.ignite.IgniteAtomicSequence;
+import org.apache.ignite.IgniteAtomicStamped;
import org.apache.ignite.IgniteCache;
import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteCountDownLatch;
import org.apache.ignite.IgniteDataStreamer;
import org.apache.ignite.IgniteException;
+import org.apache.ignite.IgniteLock;
+import org.apache.ignite.IgniteQueue;
+import org.apache.ignite.IgniteSemaphore;
+import org.apache.ignite.IgniteSet;
+import org.apache.ignite.binary.BinaryObject;
import org.apache.ignite.cache.CacheAtomicityMode;
import org.apache.ignite.cache.CacheEntryProcessor;
import org.apache.ignite.cache.query.ScanQuery;
@@ -36,6 +46,7 @@ import org.apache.ignite.compute.ComputeJobResult;
import org.apache.ignite.compute.ComputeJobResultPolicy;
import org.apache.ignite.compute.ComputeTask;
import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.CollectionConfiguration;
import org.apache.ignite.configuration.IgniteConfiguration;
import org.apache.ignite.internal.util.typedef.T2;
import org.apache.ignite.lang.IgniteCallable;
@@ -225,6 +236,89 @@ public class IgniteOperationsInsideSandboxTest extends AbstractSandboxTest {
}
/** */
+ @Test
+ public void testDataStructures() {
+ compute().broadcast(
+ new TestRunnable() {
+ @SuppressWarnings("LockAcquiredButNotSafelyReleased")
+ @Override public void run() {
+ IgniteQueue<Object> queueTx = ignite.queue("test_queue_tx", 1,
+ new CollectionConfiguration()
+ .setGroupName("test_queue_tx")
+ .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL));
+ queueTx.add(new Object());
+ queueTx.clear();
+
+ IgniteQueue<Object> queueAtomic = ignite.queue("test_queue_atomic", 1,
+ new CollectionConfiguration()
+ .setGroupName("test_queue_atomic")
+ .setAtomicityMode(CacheAtomicityMode.ATOMIC));
+ queueAtomic.add(new Object());
+ queueAtomic.clear();
+
+ IgniteSet<Object> set = ignite.set("test_set", new CollectionConfiguration().setGroupName("test_set"));
+ set.add(new Object());
+ set.clear();
+
+ IgniteAtomicLong atomicLong = ignite.atomicLong("test_atomic_long", 1, true);
+ atomicLong.incrementAndGet();
+
+ IgniteAtomicSequence atomicSeq = ignite.atomicSequence("test_atomic_seq", 1,
+ true);
+ atomicSeq.incrementAndGet();
+
+ IgniteAtomicReference<Object> atomicRef = ignite.atomicReference("test_atomic_ref",
+ null, true);
+ atomicRef.compareAndSet(null, new Object());
+
+ IgniteAtomicStamped<Object, Object> atomicStamped = ignite.atomicStamped("test_atomic_stmp",
+ null, null, true);
+ atomicStamped.compareAndSet(null, new Object(), null, new Object());
+
+ IgniteCountDownLatch cntDownLatch = ignite.countDownLatch("test_cnt_down_latch", 1,
+ true, true);
+ cntDownLatch.countDown();
+
+ IgniteSemaphore semaphore = ignite.semaphore("test_semaphore", 1, true,
+ true);
+ semaphore.acquire();
+ semaphore.release();
+
+ IgniteLock lock = ignite.reentrantLock("test_lock", true, true, true);
+ lock.lock();
+ lock.unlock();
+ }
+ });
+ }
+
+ /** */
+ @Test
+ public void testBinary() {
+ compute().broadcast(
+ new TestRunnable() {
+ @Override public void run() {
+ ignite.binary().toBinary(new Object());
+
+ // Test binary objects.
+ ignite.cache(TEST_CACHE).put(0, new Object());
+ BinaryObject obj = (BinaryObject)ignite.cache(TEST_CACHE).withKeepBinary().get(0);
+ obj.toString();
+ }
+ });
+ }
+
+ /** */
+ @Test
+ public void testAffinity() {
+ compute().broadcast(
+ new TestRunnable() {
+ @Override public void run() {
+ ignite.affinity(TEST_CACHE).partition(new Object());
+ }
+ });
+ }
+
+ /** */
private IgniteCompute compute() {
Ignite clnt = grid(CLNT_ALLOWED_WRITE_PROP);
@@ -249,6 +343,7 @@ public class IgniteOperationsInsideSandboxTest extends AbstractSandboxTest {
/** */
private abstract static class TestRunnable implements IgniteRunnable {
+ /** Ignite. */
@IgniteInstanceResource
protected Ignite ignite;
}
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/PrivilegedProxyTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/PrivilegedProxyTest.java
new file mode 100644
index 0000000..30ea162
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/sandbox/PrivilegedProxyTest.java
@@ -0,0 +1,61 @@
+/*
+ * 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.sandbox;
+
+import java.lang.reflect.Proxy;
+import java.security.AccessControlException;
+import java.util.Collections;
+import org.apache.ignite.Ignition;
+import org.apache.ignite.internal.util.lang.GridIterableAdapter;
+import org.apache.ignite.internal.util.lang.GridIterator;
+import org.junit.Test;
+
+/**
+ * Test that user defined classes can't be wrapped into priveleged proxy.
+ */
+public class PrivilegedProxyTest extends AbstractSandboxTest {
+ /** */
+ @Test
+ public void testPrivelegedUserObject() throws Exception {
+ grid(CLNT_FORBIDDEN_WRITE_PROP).getOrCreateCache(DEFAULT_CACHE_NAME).put(0, new TestIterator<>());
+
+ runForbiddenOperation(() -> grid(CLNT_FORBIDDEN_WRITE_PROP).compute().run(() -> {
+ GridIterator<?> it = (GridIterator<?>)Ignition.localIgnite().cache(DEFAULT_CACHE_NAME).get(0);
+
+ // User object should not be proxied with privileged proxy.
+ assertFalse(Proxy.isProxyClass(it.getClass()));
+
+ it.iterator();
+ }), AccessControlException.class);
+ }
+
+ /** */
+ public static class TestIterator<T> extends GridIterableAdapter<T> {
+ /** */
+ public TestIterator() {
+ super(Collections.emptyIterator());
+ }
+
+ /** {@inheritDoc} */
+ @Override public GridIterator<T> iterator() {
+ controlAction();
+
+ return super.iterator();
+ }
+ }
+}
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 ccddb7e..a78add4 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
@@ -57,6 +57,7 @@ import org.apache.ignite.internal.processors.security.sandbox.EventsSandboxTest;
import org.apache.ignite.internal.processors.security.sandbox.IgniteOperationsInsideSandboxTest;
import org.apache.ignite.internal.processors.security.sandbox.IgnitionComponentProxyTest;
import org.apache.ignite.internal.processors.security.sandbox.MessagingSandboxTest;
+import org.apache.ignite.internal.processors.security.sandbox.PrivilegedProxyTest;
import org.apache.ignite.internal.processors.security.sandbox.SecuritySubjectPermissionsTest;
import org.apache.ignite.ssl.MultipleSSLContextsTest;
import org.junit.runner.RunWith;
@@ -109,6 +110,7 @@ import org.junit.runners.Suite;
ContinuousQuerySandboxTest.class,
ContinuousQueryWithTransformerSandboxTest.class,
EventsSandboxTest.class,
+ PrivilegedProxyTest.class,
IgniteSecurityProcessorTest.class,
GridCommandHandlerSslWithSecurityTest.class,