From 4dc5c4f6d6f51692b5dc871b29fa36190fd71d5d Mon Sep 17 00:00:00 2001 From: Andrew Ross Date: Thu, 4 Sep 2025 12:51:48 -0700 Subject: [PATCH] Replace deprecated AccessController in :server Use the drop-in replacement for the deprecated AccessController where appropriate. Also remove tests that were assuming a security manager was present as that can never be true post JDK 21. Signed-off-by: Andrew Ross --- .../OpenSearchUncaughtExceptionHandler.java | 11 +-- .../util/concurrent/ThreadContextAccess.java | 14 ++-- .../store/remote/utils/TransferManager.java | 19 ++--- .../opensearch/plugins/PluginsService.java | 12 +-- .../search/lookup/LeafDocLookup.java | 10 +-- .../bootstrap/OpenSearchPolicyTests.java | 77 ------------------- .../opensearch/bootstrap/SecurityTests.java | 17 +--- 7 files changed, 21 insertions(+), 139 deletions(-) delete mode 100644 server/src/test/java/org/opensearch/bootstrap/OpenSearchPolicyTests.java diff --git a/server/src/main/java/org/opensearch/bootstrap/OpenSearchUncaughtExceptionHandler.java b/server/src/main/java/org/opensearch/bootstrap/OpenSearchUncaughtExceptionHandler.java index 5f9a01436b4cb..750920e58cd12 100644 --- a/server/src/main/java/org/opensearch/bootstrap/OpenSearchUncaughtExceptionHandler.java +++ b/server/src/main/java/org/opensearch/bootstrap/OpenSearchUncaughtExceptionHandler.java @@ -36,10 +36,9 @@ import org.apache.logging.log4j.Logger; import org.opensearch.cli.Terminal; import org.opensearch.common.SuppressForbidden; +import org.opensearch.secure_sm.AccessController; import java.io.IOError; -import java.security.AccessController; -import java.security.PrivilegedAction; /** * UncaughtException Handler used during bootstrapping @@ -98,12 +97,11 @@ void onNonFatalUncaught(final String threadName, final Throwable t) { Terminal.DEFAULT.flush(); } - @SuppressWarnings("removal") void halt(int status) { AccessController.doPrivileged(new PrivilegedHaltAction(status)); } - static class PrivilegedHaltAction implements PrivilegedAction { + static class PrivilegedHaltAction implements Runnable { private final int status; @@ -113,12 +111,9 @@ private PrivilegedHaltAction(final int status) { @SuppressForbidden(reason = "halt") @Override - public Void run() { + public void run() { // we halt to prevent shutdown hooks from running Runtime.getRuntime().halt(status); - return null; } - } - } 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 14f8b8d79bf4d..b47dfe5fe56a0 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 @@ -10,32 +10,28 @@ import org.opensearch.SpecialPermission; import org.opensearch.common.annotation.InternalApi; +import org.opensearch.secure_sm.AccessController; -import java.security.AccessController; -import java.security.PrivilegedAction; +import java.util.function.Supplier; /** * This class wraps the {@link ThreadContext} operations requiring access in - * {@link AccessController#doPrivileged(PrivilegedAction)} blocks. + * {@link AccessController#doPrivileged} blocks. * * @opensearch.internal */ -@SuppressWarnings("removal") @InternalApi public final class ThreadContextAccess { private ThreadContextAccess() {} - public static T doPrivileged(PrivilegedAction operation) { + public static T doPrivileged(Supplier operation) { SpecialPermission.check(); return AccessController.doPrivileged(operation); } public static void doPrivilegedVoid(Runnable action) { SpecialPermission.check(); - AccessController.doPrivileged((PrivilegedAction) () -> { - action.run(); - return null; - }); + AccessController.doPrivileged(action); } } diff --git a/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java b/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java index 007d715fb4391..4b6964ed02f60 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java +++ b/server/src/main/java/org/opensearch/index/store/remote/utils/TransferManager.java @@ -16,6 +16,7 @@ import org.opensearch.index.store.remote.filecache.CachedIndexInput; import org.opensearch.index.store.remote.filecache.FileCache; import org.opensearch.index.store.remote.filecache.FileCachedIndexInput; +import org.opensearch.secure_sm.AccessController; import org.opensearch.threadpool.ThreadPool; import java.io.BufferedOutputStream; @@ -25,9 +26,6 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; -import java.security.AccessController; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.Executor; @@ -78,7 +76,7 @@ public IndexInput fetchBlob(BlobFetchRequest blobFetchRequest) throws IOExceptio logger.trace("fetchBlob called for {}", key.toString()); try { - return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + return AccessController.doPrivilegedChecked(() -> { CachedIndexInput cacheEntry = fileCache.compute(key, (path, cachedIndexInput) -> { if (cachedIndexInput == null || cachedIndexInput.isClosed()) { logger.trace("Transfer Manager - IndexInput closed or not in cache"); @@ -100,14 +98,13 @@ public IndexInput fetchBlob(BlobFetchRequest blobFetchRequest) throws IOExceptio fileCache.decRef(key); } }); - } catch (PrivilegedActionException e) { - final Exception cause = e.getException(); - if (cause instanceof IOException) { - throw (IOException) cause; - } else if (cause instanceof RuntimeException) { - throw (RuntimeException) cause; + } catch (Exception e) { + if (e instanceof IOException) { + throw (IOException) e; + } else if (e instanceof RuntimeException) { + throw (RuntimeException) e; } else { - throw new IOException(cause); + throw new IOException(e); } } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 4736b761d4b04..5e382584dbe0e 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -64,8 +64,6 @@ import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -799,10 +797,7 @@ private Plugin loadBundle(Bundle bundle, Map loaded) { // Set context class loader to plugin's class loader so that plugins // that have dependencies with their own SPI endpoints have a chance to load // and initialize them appropriately. - AccessController.doPrivileged((PrivilegedAction) () -> { - Thread.currentThread().setContextClassLoader(loader); - return null; - }); + Thread.currentThread().setContextClassLoader(loader); logger.debug("Loading plugin [" + name + "]..."); Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), loader); @@ -821,10 +816,7 @@ private Plugin loadBundle(Bundle bundle, Map loaded) { loaded.put(name, plugin); return plugin; } finally { - AccessController.doPrivileged((PrivilegedAction) () -> { - Thread.currentThread().setContextClassLoader(cl); - return null; - }); + Thread.currentThread().setContextClassLoader(cl); } } diff --git a/server/src/main/java/org/opensearch/search/lookup/LeafDocLookup.java b/server/src/main/java/org/opensearch/search/lookup/LeafDocLookup.java index 70cbd8d7ad6c3..53ef5d5c1540a 100644 --- a/server/src/main/java/org/opensearch/search/lookup/LeafDocLookup.java +++ b/server/src/main/java/org/opensearch/search/lookup/LeafDocLookup.java @@ -38,10 +38,9 @@ import org.opensearch.index.fielddata.ScriptDocValues; import org.opensearch.index.mapper.MappedFieldType; import org.opensearch.index.mapper.MapperService; +import org.opensearch.secure_sm.AccessController; import java.io.IOException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -91,12 +90,7 @@ public ScriptDocValues get(Object key) { } // load fielddata on behalf of the script: otherwise it would need additional permissions // to deal with pagedbytes/ramusagestimator/etc - scriptValues = AccessController.doPrivileged(new PrivilegedAction>() { - @Override - public ScriptDocValues run() { - return fieldDataLookup.apply(fieldType).load(reader).getScriptValues(); - } - }); + scriptValues = AccessController.doPrivileged(() -> fieldDataLookup.apply(fieldType).load(reader).getScriptValues()); localCacheFieldData.put(fieldName, scriptValues); } try { diff --git a/server/src/test/java/org/opensearch/bootstrap/OpenSearchPolicyTests.java b/server/src/test/java/org/opensearch/bootstrap/OpenSearchPolicyTests.java deleted file mode 100644 index 2b4d2a755f543..0000000000000 --- a/server/src/test/java/org/opensearch/bootstrap/OpenSearchPolicyTests.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * 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. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch 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. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.bootstrap; - -import org.opensearch.test.OpenSearchTestCase; - -import java.security.AccessControlContext; -import java.security.AccessController; -import java.security.PermissionCollection; -import java.security.Permissions; -import java.security.PrivilegedAction; -import java.security.ProtectionDomain; - -/** - * Tests for OpenSearchPolicy - */ -public class OpenSearchPolicyTests extends OpenSearchTestCase { - - /** - * test restricting privileges to no permissions actually works - */ - @SuppressWarnings("removal") - public void testRestrictPrivileges() { - assumeTrue("test requires security manager", System.getSecurityManager() != null); - try { - System.getProperty("user.home"); - } catch (SecurityException e) { - fail("this test needs to be fixed: user.home not available by policy"); - } - - PermissionCollection noPermissions = new Permissions(); - AccessControlContext noPermissionsAcc = new AccessControlContext( - new ProtectionDomain[] { new ProtectionDomain(null, noPermissions) } - ); - try { - AccessController.doPrivileged(new PrivilegedAction() { - public Void run() { - System.getProperty("user.home"); - fail("access should have been denied"); - return null; - } - }, noPermissionsAcc); - } catch (SecurityException expected) { - // expected exception - } - } -} diff --git a/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java b/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java index 738fc80c34ce6..00e641ee86b57 100644 --- a/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java +++ b/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java @@ -39,8 +39,6 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Map; public class SecurityTests extends OpenSearchTestCase { @@ -76,17 +74,6 @@ public void testEnsureRegularFile() throws IOException { } catch (IOException expected) {} } - /** can't execute processes */ - @SuppressWarnings("removal") - public void testProcessExecution() throws Exception { - assumeTrue("test requires security manager", System.getSecurityManager() != null); - try { - Runtime.getRuntime().exec(new String[] { "ls" }); - fail("didn't get expected exception"); - } catch (SecurityException expected) {} - } - - @SuppressWarnings("removal") public void testReadPolicyWithCodebases() throws IOException { final Map codebases = Map.of( "test-netty-tcnative-boringssl-static-2.0.61.Final-linux-x86_64.jar", @@ -101,8 +88,6 @@ public void testReadPolicyWithCodebases() throws IOException { URI.create("file://test-zstd-jni-1.5.6-1.jar").toURL() ); - AccessController.doPrivileged( - (PrivilegedAction) () -> Security.readPolicy(SecurityTests.class.getResource("test-codebases.policy"), codebases) - ); + Security.readPolicy(SecurityTests.class.getResource("test-codebases.policy"), codebases); } }