From 518c9d27fc454118cf65f6df9a7c6cb694ddc88c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 29 Jul 2024 22:21:42 -0400 Subject: [PATCH 1/9] Add RuntimePermission for markAsSystemContext and allow core to perform the method Signed-off-by: Craig Perkins --- .../opensearch/common/util/concurrent/ThreadContext.java | 7 +++++++ .../resources/org/opensearch/bootstrap/security.policy | 1 + 2 files changed, 8 insertions(+) diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java index 906a27e9f398c..733966741e653 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java @@ -50,6 +50,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.security.Permission; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -119,6 +120,8 @@ public final class ThreadContext implements Writeable { private final long maxWarningHeaderSize; private final List propagators; + public static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new RuntimePermission("markAsSystemContext"); + /** * Creates a new ThreadContext instance * @param settings the settings to read the default request headers from @@ -556,6 +559,10 @@ boolean isDefaultContext() { * by the system itself rather than by a user action. */ public void markAsSystemContext() { + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + sm.checkPermission(ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION); + } threadLocal.set(threadLocal.get().setSystemContext(propagators)); } diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index 55e8db0d9c6a3..f3d658fbbe1cb 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -48,6 +48,7 @@ grant codeBase "${codebase.opensearch}" { permission java.lang.RuntimePermission "setContextClassLoader"; // needed for SPI class loading permission java.lang.RuntimePermission "accessDeclaredMembers"; + permission java.lang.RuntimePermission "markAsSystemContext"; }; //// Very special jar permissions: From 0360a5e30e46325614352161dc1657115d198002 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 29 Jul 2024 22:22:40 -0400 Subject: [PATCH 2/9] private Signed-off-by: Craig Perkins --- .../org/opensearch/common/util/concurrent/ThreadContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java index 733966741e653..9b0a89ea323c7 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java @@ -120,7 +120,7 @@ public final class ThreadContext implements Writeable { private final long maxWarningHeaderSize; private final List propagators; - public static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new RuntimePermission("markAsSystemContext"); + private static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new RuntimePermission("markAsSystemContext"); /** * Creates a new ThreadContext instance From e3a6adf043020a505a29edddb6eabdc654860476 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 08:39:15 -0400 Subject: [PATCH 3/9] Surround with doPrivileged Signed-off-by: Craig Perkins --- .../service/ClusterApplierService.java | 7 +++++- .../cluster/service/MasterService.java | 7 +++++- .../seqno/GlobalCheckpointSyncAction.java | 7 +++++- .../RetentionLeaseBackgroundSyncAction.java | 7 +++++- .../index/seqno/RetentionLeaseSyncAction.java | 7 +++++- .../checkpoint/PublishCheckpointAction.java | 7 +++++- .../transport/RemoteClusterConnection.java | 7 +++++- .../transport/SniffConnectionStrategy.java | 7 +++++- .../metadata/TemplateUpgradeServiceTests.java | 7 +++++- .../util/concurrent/ThreadContextTests.java | 22 +++++++++++++++---- ...ContextBasedTracerContextStorageTests.java | 7 +++++- .../org/opensearch/bootstrap/test.policy | 3 ++- .../FakeThreadPoolClusterManagerService.java | 7 +++++- 13 files changed, 86 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java index 6234427445754..e8b1316709404 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java @@ -67,6 +67,8 @@ import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collection; import java.util.Map; @@ -396,7 +398,10 @@ private void submitStateUpdateTask( final ThreadContext threadContext = threadPool.getThreadContext(); final Supplier supplier = threadContext.newRestorableContext(true); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); final UpdateTask updateTask = new UpdateTask( config.priority(), source, diff --git a/server/src/main/java/org/opensearch/cluster/service/MasterService.java b/server/src/main/java/org/opensearch/cluster/service/MasterService.java index 4ab8255df7658..e1c6288846c90 100644 --- a/server/src/main/java/org/opensearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/MasterService.java @@ -76,6 +76,8 @@ import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -1022,7 +1024,10 @@ public void submitStateUpdateTasks( final ThreadContext threadContext = threadPool.getThreadContext(); final Supplier supplier = threadContext.newRestorableContext(true); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); List safeTasks = tasks.entrySet() .stream() diff --git a/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java index c6a1f5f27a875..167a30b902276 100644 --- a/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java +++ b/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java @@ -55,6 +55,8 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; /** * Background global checkpoint sync action initiated when a shard goes inactive. This is needed because while we send the global checkpoint @@ -98,7 +100,10 @@ public GlobalCheckpointSyncAction( public void updateGlobalCheckpointForShard(final ShardId shardId) { final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); execute(new Request(shardId), ActionListener.wrap(r -> {}, e -> { if (ExceptionsHelper.unwrap(e, AlreadyClosedException.class, IndexShardClosedException.class) == null) { logger.info(new ParameterizedMessage("{} global checkpoint sync failed", shardId), e); diff --git a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java index 5fa0a1a6459e7..d9182b597acee 100644 --- a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java +++ b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java @@ -65,6 +65,8 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Map; import java.util.Objects; @@ -122,7 +124,10 @@ final void backgroundSync(ShardId shardId, String primaryAllocationId, long prim final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we have to execute under the system context so that if security is enabled the sync is authorized - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); final Request request = new Request(shardId, retentionLeases); final ReplicationTask task = (ReplicationTask) taskManager.register("transport", "retention_lease_background_sync", request); transportService.sendChildRequest( diff --git a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java index ca3c7e1d49700..fd0475c10e946 100644 --- a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java +++ b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java @@ -69,6 +69,8 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Map; import java.util.Objects; @@ -137,7 +139,10 @@ final void sync( final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we have to execute under the system context so that if security is enabled the sync is authorized - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); final Request request = new Request(shardId, retentionLeases); final ReplicationTask task = (ReplicationTask) taskManager.register("transport", "retention_lease_sync", request); transportService.sendChildRequest( diff --git a/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java b/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java index 8f39aa194b06c..0f7e65670c51b 100644 --- a/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java +++ b/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java @@ -41,6 +41,8 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Objects; /** @@ -113,7 +115,10 @@ final void publish(IndexShard indexShard, ReplicationCheckpoint checkpoint) { final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we have to execute under the system context so that if security is enabled the sync is authorized - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); PublishCheckpointRequest request = new PublishCheckpointRequest(checkpoint); final ReplicationTask task = (ReplicationTask) taskManager.register("transport", "segrep_publish_checkpoint", request); final ReplicationTimer timer = new ReplicationTimer(); diff --git a/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java b/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java index 8a5f6dfffb036..09a862923d118 100644 --- a/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java +++ b/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java @@ -47,6 +47,8 @@ import java.io.Closeable; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.function.Function; /** @@ -136,7 +138,10 @@ void collectNodes(ActionListener> listener) { new ContextPreservingActionListener<>(threadContext.newRestorableContext(false), listener); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we stash any context here since this is an internal execution and should not leak any existing context information - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); final ClusterStateRequest request = new ClusterStateRequest(); request.clear(); diff --git a/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java b/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java index 07ba96b135189..a5918ee859f63 100644 --- a/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java +++ b/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java @@ -59,6 +59,8 @@ import java.io.IOException; import java.net.InetSocketAddress; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -349,7 +351,10 @@ private void collectRemoteNodes(Iterator> seedNodes, Act try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we stash any context here since this is an internal execution and should not leak any // existing context information. - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); transportService.sendRequest( connection, ClusterStateAction.NAME, diff --git a/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java index 36d984b7eb99b..4a5fa2e178312 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java @@ -56,6 +56,8 @@ import org.junit.After; import org.junit.Before; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -225,7 +227,10 @@ public void testUpdateTemplates() { service.upgradesInProgress.set(additionsCount + deletionsCount + 2); // +2 to skip tryFinishUpgrade final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); service.upgradeTemplates(additions, deletions); } diff --git a/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java b/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java index 4e66575711046..beb275040eaf2 100644 --- a/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java +++ b/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java @@ -37,6 +37,8 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -565,7 +567,10 @@ public void testPreservesThreadsOriginalContextOnRunException() throws IOExcepti threadContext.putHeader("foo", "bar"); boolean systemContext = randomBoolean(); if (systemContext) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); } threadContext.putTransient("foo", "bar_transient"); withContext = threadContext.preserveContext(new AbstractRunnable() { @@ -736,7 +741,10 @@ public void testMarkAsSystemContext() throws IOException { assertFalse(threadContext.isSystemContext()); try (ThreadContext.StoredContext context = threadContext.stashContext()) { assertFalse(threadContext.isSystemContext()); - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); assertTrue(threadContext.isSystemContext()); } assertFalse(threadContext.isSystemContext()); @@ -761,7 +769,10 @@ public void testSystemContextWithPropagator() { assertEquals(Integer.valueOf(1), threadContext.getTransient("test_transient_propagation_key")); assertEquals("bar", threadContext.getHeader("foo")); try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); assertNull(threadContext.getHeader("foo")); assertNull(threadContext.getTransient("test_transient_propagation_key")); assertEquals("1", threadContext.getHeader("default")); @@ -793,7 +804,10 @@ public void testSerializeSystemContext() throws IOException { threadContext.writeTo(out); try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { assertEquals("test", threadContext.getTransient("test_transient_propagation_key")); - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); threadContext.writeTo(outFromSystemContext); assertNull(threadContext.getHeader("foo")); assertNull(threadContext.getTransient("test_transient_propagation_key")); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java index bf11bcaf39a96..40cca48e36041 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java @@ -21,6 +21,8 @@ import org.junit.After; import org.junit.Before; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -260,7 +262,10 @@ public void testSpanNotPropagatedToChildSystemThreadContext() { try (StoredContext ignored = threadContext.stashContext()) { assertThat(threadContext.getTransient(ThreadContextBasedTracerContextStorage.CURRENT_SPAN), is(not(nullValue()))); assertThat(threadContextStorage.get(ThreadContextBasedTracerContextStorage.CURRENT_SPAN), is(span)); - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); assertThat(threadContext.getTransient(ThreadContextBasedTracerContextStorage.CURRENT_SPAN), is(nullValue())); } } diff --git a/server/src/test/resources/org/opensearch/bootstrap/test.policy b/server/src/test/resources/org/opensearch/bootstrap/test.policy index 7b0a9b3d5d709..7557400942808 100644 --- a/server/src/test/resources/org/opensearch/bootstrap/test.policy +++ b/server/src/test/resources/org/opensearch/bootstrap/test.policy @@ -7,7 +7,8 @@ */ grant { - // allow to test Security policy and codebases + // allow to test Security policy and codebases permission java.util.PropertyPermission "*", "read,write"; permission java.security.SecurityPermission "createPolicy.JavaPolicy"; + permission java.lang.RuntimePermission "markAsSystemContext"; }; diff --git a/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java b/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java index 53ef595c7931e..2d70236ab3a7b 100644 --- a/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java +++ b/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java @@ -49,6 +49,8 @@ import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.threadpool.ThreadPool; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -134,7 +136,10 @@ public void run() { scheduledNextTask = false; final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignored = threadContext.stashContext()) { - threadContext.markAsSystemContext(); + AccessController.doPrivileged((PrivilegedAction) () -> { + threadContext.markAsSystemContext(); + return null; + }); task.run(); } if (waitForPublish == false) { From 67fc028d758938e2e7dcc8c983faef49766bee44 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 09:18:06 -0400 Subject: [PATCH 4/9] Create ThreadContextAccess Signed-off-by: Craig Perkins --- .../service/ClusterApplierService.java | 8 +--- .../cluster/service/MasterService.java | 8 +--- .../util/concurrent/ThreadContextAccess.java | 37 +++++++++++++++++++ .../seqno/GlobalCheckpointSyncAction.java | 8 +--- .../RetentionLeaseBackgroundSyncAction.java | 8 +--- .../index/seqno/RetentionLeaseSyncAction.java | 8 +--- .../checkpoint/PublishCheckpointAction.java | 8 +--- .../transport/RemoteClusterConnection.java | 8 +--- .../transport/SniffConnectionStrategy.java | 8 +--- .../metadata/TemplateUpgradeServiceTests.java | 8 +--- .../util/concurrent/ThreadContextTests.java | 22 ++--------- ...ContextBasedTracerContextStorageTests.java | 8 +--- .../FakeThreadPoolClusterManagerService.java | 8 +--- 13 files changed, 63 insertions(+), 84 deletions(-) create mode 100644 server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java index e8b1316709404..b2548a8976c73 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java @@ -61,14 +61,13 @@ import org.opensearch.common.util.concurrent.OpenSearchExecutors; import org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collection; import java.util.Map; @@ -398,10 +397,7 @@ private void submitStateUpdateTask( final ThreadContext threadContext = threadPool.getThreadContext(); final Supplier supplier = threadContext.newRestorableContext(true); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); final UpdateTask updateTask = new UpdateTask( config.priority(), source, diff --git a/server/src/main/java/org/opensearch/cluster/service/MasterService.java b/server/src/main/java/org/opensearch/cluster/service/MasterService.java index e1c6288846c90..713de8cdd0fda 100644 --- a/server/src/main/java/org/opensearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/MasterService.java @@ -66,6 +66,7 @@ import org.opensearch.common.util.concurrent.OpenSearchExecutors; import org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.Assertions; import org.opensearch.core.common.text.Text; import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException; @@ -76,8 +77,6 @@ import org.opensearch.threadpool.Scheduler; import org.opensearch.threadpool.ThreadPool; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -1024,10 +1023,7 @@ public void submitStateUpdateTasks( final ThreadContext threadContext = threadPool.getThreadContext(); final Supplier supplier = threadContext.newRestorableContext(true); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); List safeTasks = tasks.entrySet() .stream() diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java new file mode 100644 index 0000000000000..2bafe3ad32b07 --- /dev/null +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java @@ -0,0 +1,37 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.util.concurrent; + +import org.opensearch.SpecialPermission; + +import java.security.AccessController; +import java.security.PrivilegedAction; + +/** + * This class wraps the {@link ThreadContext} operations requiring access in + * {@link AccessController#doPrivileged(PrivilegedAction)} blocks. + */ +@SuppressWarnings("removal") +public final class ThreadContextAccess { + + private ThreadContextAccess() {} + + public static T doPrivileged(PrivilegedAction operation) { + SpecialPermission.check(); + return AccessController.doPrivileged(operation); + } + + public static void doPrivilegedVoid(Runnable action) { + SpecialPermission.check(); + AccessController.doPrivileged((PrivilegedAction) () -> { + action.run(); + return null; + }); + } +} diff --git a/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java index 167a30b902276..fedf239871368 100644 --- a/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java +++ b/server/src/main/java/org/opensearch/index/seqno/GlobalCheckpointSyncAction.java @@ -44,6 +44,7 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.index.shard.ShardId; @@ -55,8 +56,6 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; /** * Background global checkpoint sync action initiated when a shard goes inactive. This is needed because while we send the global checkpoint @@ -100,10 +99,7 @@ public GlobalCheckpointSyncAction( public void updateGlobalCheckpointForShard(final ShardId shardId) { final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); execute(new Request(shardId), ActionListener.wrap(r -> {}, e -> { if (ExceptionsHelper.unwrap(e, AlreadyClosedException.class, IndexShardClosedException.class) == null) { logger.info(new ParameterizedMessage("{} global checkpoint sync failed", shardId), e); diff --git a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java index d9182b597acee..e8ebf11ef0e5c 100644 --- a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java +++ b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java @@ -48,6 +48,7 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -65,8 +66,6 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Map; import java.util.Objects; @@ -124,10 +123,7 @@ final void backgroundSync(ShardId shardId, String primaryAllocationId, long prim final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we have to execute under the system context so that if security is enabled the sync is authorized - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); final Request request = new Request(shardId, retentionLeases); final ReplicationTask task = (ReplicationTask) taskManager.register("transport", "retention_lease_background_sync", request); transportService.sendChildRequest( diff --git a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java index fd0475c10e946..9e8437ca78879 100644 --- a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java +++ b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java @@ -50,6 +50,7 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -69,8 +70,6 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Map; import java.util.Objects; @@ -139,10 +138,7 @@ final void sync( final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we have to execute under the system context so that if security is enabled the sync is authorized - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); final Request request = new Request(shardId, retentionLeases); final ReplicationTask task = (ReplicationTask) taskManager.register("transport", "retention_lease_sync", request); transportService.sendChildRequest( diff --git a/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java b/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java index 0f7e65670c51b..d1e2884956f5c 100644 --- a/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java +++ b/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java @@ -24,6 +24,7 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.index.IndexNotFoundException; @@ -41,8 +42,6 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Objects; /** @@ -115,10 +114,7 @@ final void publish(IndexShard indexShard, ReplicationCheckpoint checkpoint) { final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we have to execute under the system context so that if security is enabled the sync is authorized - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); PublishCheckpointRequest request = new PublishCheckpointRequest(checkpoint); final ReplicationTask task = (ReplicationTask) taskManager.register("transport", "segrep_publish_checkpoint", request); final ReplicationTimer timer = new ReplicationTimer(); diff --git a/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java b/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java index 09a862923d118..8f0ee52ac3acd 100644 --- a/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java +++ b/server/src/main/java/org/opensearch/transport/RemoteClusterConnection.java @@ -40,6 +40,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; @@ -47,8 +48,6 @@ import java.io.Closeable; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.function.Function; /** @@ -138,10 +137,7 @@ void collectNodes(ActionListener> listener) { new ContextPreservingActionListener<>(threadContext.newRestorableContext(false), listener); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we stash any context here since this is an internal execution and should not leak any existing context information - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); final ClusterStateRequest request = new ClusterStateRequest(); request.clear(); diff --git a/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java b/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java index a5918ee859f63..1d94228218fd0 100644 --- a/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java +++ b/server/src/main/java/org/opensearch/transport/SniffConnectionStrategy.java @@ -47,6 +47,7 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.Strings; @@ -59,8 +60,6 @@ import java.io.IOException; import java.net.InetSocketAddress; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -351,10 +350,7 @@ private void collectRemoteNodes(Iterator> seedNodes, Act try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { // we stash any context here since this is an internal execution and should not leak any // existing context information. - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); transportService.sendRequest( connection, ClusterStateAction.NAME, diff --git a/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java index 4a5fa2e178312..562e293083633 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java @@ -47,6 +47,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.collect.Tuple; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; @@ -56,8 +57,6 @@ import org.junit.After; import org.junit.Before; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -227,10 +226,7 @@ public void testUpdateTemplates() { service.upgradesInProgress.set(additionsCount + deletionsCount + 2); // +2 to skip tryFinishUpgrade final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); service.upgradeTemplates(additions, deletions); } diff --git a/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java b/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java index beb275040eaf2..4c7cd4513412d 100644 --- a/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java +++ b/server/src/test/java/org/opensearch/common/util/concurrent/ThreadContextTests.java @@ -37,8 +37,6 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -567,10 +565,7 @@ public void testPreservesThreadsOriginalContextOnRunException() throws IOExcepti threadContext.putHeader("foo", "bar"); boolean systemContext = randomBoolean(); if (systemContext) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); } threadContext.putTransient("foo", "bar_transient"); withContext = threadContext.preserveContext(new AbstractRunnable() { @@ -741,10 +736,7 @@ public void testMarkAsSystemContext() throws IOException { assertFalse(threadContext.isSystemContext()); try (ThreadContext.StoredContext context = threadContext.stashContext()) { assertFalse(threadContext.isSystemContext()); - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); assertTrue(threadContext.isSystemContext()); } assertFalse(threadContext.isSystemContext()); @@ -769,10 +761,7 @@ public void testSystemContextWithPropagator() { assertEquals(Integer.valueOf(1), threadContext.getTransient("test_transient_propagation_key")); assertEquals("bar", threadContext.getHeader("foo")); try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); assertNull(threadContext.getHeader("foo")); assertNull(threadContext.getTransient("test_transient_propagation_key")); assertEquals("1", threadContext.getHeader("default")); @@ -804,10 +793,7 @@ public void testSerializeSystemContext() throws IOException { threadContext.writeTo(out); try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { assertEquals("test", threadContext.getTransient("test_transient_propagation_key")); - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); threadContext.writeTo(outFromSystemContext); assertNull(threadContext.getHeader("foo")); assertNull(threadContext.getTransient("test_transient_propagation_key")); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java index 40cca48e36041..98dfc367c20f5 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/ThreadContextBasedTracerContextStorageTests.java @@ -12,6 +12,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.util.concurrent.ThreadContext.StoredContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.telemetry.Telemetry; import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.metrics.MetricsTelemetry; @@ -21,8 +22,6 @@ import org.junit.After; import org.junit.Before; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -262,10 +261,7 @@ public void testSpanNotPropagatedToChildSystemThreadContext() { try (StoredContext ignored = threadContext.stashContext()) { assertThat(threadContext.getTransient(ThreadContextBasedTracerContextStorage.CURRENT_SPAN), is(not(nullValue()))); assertThat(threadContextStorage.get(ThreadContextBasedTracerContextStorage.CURRENT_SPAN), is(span)); - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); assertThat(threadContext.getTransient(ThreadContextBasedTracerContextStorage.CURRENT_SPAN), is(nullValue())); } } diff --git a/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java b/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java index 2d70236ab3a7b..64f3dbc4fd967 100644 --- a/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java +++ b/test/framework/src/main/java/org/opensearch/cluster/service/FakeThreadPoolClusterManagerService.java @@ -44,13 +44,12 @@ import org.opensearch.common.util.concurrent.OpenSearchExecutors; import org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.util.concurrent.ThreadContextAccess; import org.opensearch.core.action.ActionListener; import org.opensearch.node.Node; import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.threadpool.ThreadPool; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -136,10 +135,7 @@ public void run() { scheduledNextTask = false; final ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignored = threadContext.stashContext()) { - AccessController.doPrivileged((PrivilegedAction) () -> { - threadContext.markAsSystemContext(); - return null; - }); + ThreadContextAccess.doPrivilegedVoid(threadContext::markAsSystemContext); task.run(); } if (waitForPublish == false) { From ea5f2470a6c3efc5880ec7e7e6dbc9f59edc02b0 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 09:26:22 -0400 Subject: [PATCH 5/9] Create notion of ThreadContextPermission Signed-off-by: Craig Perkins --- .../org/opensearch/secure_sm/SecureSM.java | 4 ++ .../secure_sm/ThreadContextPermission.java | 40 +++++++++++++++++++ .../common/util/concurrent/ThreadContext.java | 4 +- .../org/opensearch/bootstrap/security.policy | 2 +- .../org/opensearch/bootstrap/test.policy | 2 +- 5 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 libs/secure-sm/src/main/java/org/opensearch/secure_sm/ThreadContextPermission.java diff --git a/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java index a2531f4a9156e..ae771b656603d 100644 --- a/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java +++ b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java @@ -208,6 +208,10 @@ private void debugThreadGroups(final ThreadGroup caller, final ThreadGroup targe System.err.println("access: target group=" + target); } + // thread context permissions + + public static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new ThreadContextPermission("markAsSystemContext"); + // thread permission logic private static final Permission MODIFY_THREAD_PERMISSION = new RuntimePermission("modifyThread"); diff --git a/libs/secure-sm/src/main/java/org/opensearch/secure_sm/ThreadContextPermission.java b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/ThreadContextPermission.java new file mode 100644 index 0000000000000..2f33eb513c165 --- /dev/null +++ b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/ThreadContextPermission.java @@ -0,0 +1,40 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.secure_sm; + +import java.security.BasicPermission; + +/** + * Permission to utilize methods in the ThreadContext class that are normally not accessible + * + * @see ThreadGroup + * @see SecureSM + */ +public final class ThreadContextPermission extends BasicPermission { + + /** + * Creates a new ThreadContextPermission object. + * + * @param name target name + */ + public ThreadContextPermission(String name) { + super(name); + } + + /** + * Creates a new ThreadContextPermission object. + * This constructor exists for use by the {@code Policy} object to instantiate new Permission objects. + * + * @param name target name + * @param actions ignored + */ + public ThreadContextPermission(String name, String actions) { + super(name, actions); + } +} diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java index 9b0a89ea323c7..b4db882de70ac 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java @@ -50,7 +50,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.security.Permission; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -71,6 +70,7 @@ import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_MAX_WARNING_HEADER_COUNT; import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_MAX_WARNING_HEADER_SIZE; +import static org.opensearch.secure_sm.SecureSM.ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION; /** * A ThreadContext is a map of string headers and a transient map of keyed objects that are associated with @@ -120,8 +120,6 @@ public final class ThreadContext implements Writeable { private final long maxWarningHeaderSize; private final List propagators; - private static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new RuntimePermission("markAsSystemContext"); - /** * Creates a new ThreadContext instance * @param settings the settings to read the default request headers from diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index f3d658fbbe1cb..b7aaa2e3eec48 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -48,7 +48,7 @@ grant codeBase "${codebase.opensearch}" { permission java.lang.RuntimePermission "setContextClassLoader"; // needed for SPI class loading permission java.lang.RuntimePermission "accessDeclaredMembers"; - permission java.lang.RuntimePermission "markAsSystemContext"; + permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext"; }; //// Very special jar permissions: diff --git a/server/src/test/resources/org/opensearch/bootstrap/test.policy b/server/src/test/resources/org/opensearch/bootstrap/test.policy index 7557400942808..e88afa1a9fa23 100644 --- a/server/src/test/resources/org/opensearch/bootstrap/test.policy +++ b/server/src/test/resources/org/opensearch/bootstrap/test.policy @@ -10,5 +10,5 @@ grant { // allow to test Security policy and codebases permission java.util.PropertyPermission "*", "read,write"; permission java.security.SecurityPermission "createPolicy.JavaPolicy"; - permission java.lang.RuntimePermission "markAsSystemContext"; + permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext"; }; From 8b1ad2918cf99cad16e489f205c8320b65fa721f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 09:35:34 -0400 Subject: [PATCH 6/9] Add to CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e88a084f7d7f6..f8f1077c4db79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added +- Add ThreadContextPermission for markAsSystemContext and allow core to perform the method ([#15016](https://github.com/opensearch-project/OpenSearch/pull/15016)) ### Dependencies - Bump `org.apache.commons:commons-lang3` from 3.14.0 to 3.15.0 ([#14861](https://github.com/opensearch-project/OpenSearch/pull/14861)) From 58b26743374bca1360ddf7ed5f798501c9e8cc77 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 14:03:44 -0400 Subject: [PATCH 7/9] Add javadoc Signed-off-by: Craig Perkins --- .../java/org/opensearch/secure_sm/SecureSM.java | 4 ---- .../common/util/concurrent/ThreadContext.java | 14 +++++++++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java index ae771b656603d..a2531f4a9156e 100644 --- a/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java +++ b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java @@ -208,10 +208,6 @@ private void debugThreadGroups(final ThreadGroup caller, final ThreadGroup targe System.err.println("access: target group=" + target); } - // thread context permissions - - public static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new ThreadContextPermission("markAsSystemContext"); - // thread permission logic private static final Permission MODIFY_THREAD_PERMISSION = new RuntimePermission("modifyThread"); diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java index b4db882de70ac..b955934c4f547 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java @@ -45,11 +45,13 @@ import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; import org.opensearch.http.HttpTransportSettings; +import org.opensearch.secure_sm.ThreadContextPermission; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskThreadContextStatePropagator; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.security.Permission; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -70,7 +72,6 @@ import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_MAX_WARNING_HEADER_COUNT; import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_MAX_WARNING_HEADER_SIZE; -import static org.opensearch.secure_sm.SecureSM.ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION; /** * A ThreadContext is a map of string headers and a transient map of keyed objects that are associated with @@ -112,6 +113,10 @@ public final class ThreadContext implements Writeable { */ public static final String ACTION_ORIGIN_TRANSIENT_NAME = "action.origin"; + // thread context permissions + + private static final Permission ACCESS_SYSTEM_THREAD_CONTEXT_PERMISSION = new ThreadContextPermission("markAsSystemContext"); + private static final Logger logger = LogManager.getLogger(ThreadContext.class); private static final ThreadContextStruct DEFAULT_CONTEXT = new ThreadContextStruct(); private final Map defaultHeader; @@ -555,6 +560,13 @@ boolean isDefaultContext() { /** * Marks this thread context as an internal system context. This signals that actions in this context are issued * by the system itself rather than by a user action. + * + * Usage of markAsSystemContext is guarded by a ThreadContextPermission. In order to use + * markAsSystemContext, the codebase needs to explicitly be granted permission in the JSM policy file. + * + * Add an entry in the grant portion of the policy file like this: + * + * permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext"; */ public void markAsSystemContext() { SecurityManager sm = System.getSecurityManager(); From 1803dbac0b74369721cf31695e3519ff76ac101d Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 14:42:19 -0400 Subject: [PATCH 8/9] Add to test-framework.policy file Signed-off-by: Craig Perkins --- .../resources/org/opensearch/bootstrap/test-framework.policy | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy index 0abfd7ef22ae7..f674c90c45a0e 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy @@ -157,4 +157,5 @@ grant { permission java.lang.RuntimePermission "reflectionFactoryAccess"; permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect"; permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; + permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext"; }; From 1ff09c2ed025ca7376afa1db6c69fdd059758c2f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 30 Jul 2024 16:30:04 -0400 Subject: [PATCH 9/9] Mark as internal Signed-off-by: Craig Perkins --- .../common/util/concurrent/ThreadContextAccess.java | 4 ++++ .../src/test/resources/org/opensearch/bootstrap/test.policy | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java index 2bafe3ad32b07..14f8b8d79bf4d 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContextAccess.java @@ -9,6 +9,7 @@ package org.opensearch.common.util.concurrent; import org.opensearch.SpecialPermission; +import org.opensearch.common.annotation.InternalApi; import java.security.AccessController; import java.security.PrivilegedAction; @@ -16,8 +17,11 @@ /** * This class wraps the {@link ThreadContext} operations requiring access in * {@link AccessController#doPrivileged(PrivilegedAction)} blocks. + * + * @opensearch.internal */ @SuppressWarnings("removal") +@InternalApi public final class ThreadContextAccess { private ThreadContextAccess() {} diff --git a/server/src/test/resources/org/opensearch/bootstrap/test.policy b/server/src/test/resources/org/opensearch/bootstrap/test.policy index e88afa1a9fa23..c2b5a8e9c0a4e 100644 --- a/server/src/test/resources/org/opensearch/bootstrap/test.policy +++ b/server/src/test/resources/org/opensearch/bootstrap/test.policy @@ -10,5 +10,4 @@ grant { // allow to test Security policy and codebases permission java.util.PropertyPermission "*", "read,write"; permission java.security.SecurityPermission "createPolicy.JavaPolicy"; - permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext"; };