Skip to content

Commit efbb482

Browse files
rjernstrjernst
authored
Remove doPrivileged uses from server (elastic#127781) (elastic#128929)
Now that SecurityManager is no longer used, doPrivileged is no longer necessary. This commit removes uses of it from core and server
1 parent d59e267 commit efbb482

File tree

16 files changed

+96
-234
lines changed

16 files changed

+96
-234
lines changed

‎libs/core/src/main/java/org/elasticsearch/core/internal/provider/EmbeddedImplClassLoader.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,8 @@
2323
import java.nio.file.FileSystems;
2424
import java.nio.file.Files;
2525
import java.nio.file.Path;
26-
import java.security.AccessController;
2726
import java.security.CodeSigner;
2827
import java.security.CodeSource;
29-
import java.security.PrivilegedAction;
3028
import java.security.SecureClassLoader;
3129
import java.util.ArrayList;
3230
import java.util.Collections;
@@ -96,8 +94,7 @@ record JarMeta(String prefix, boolean isMultiRelease, Set<String> packages, Map<
9694
private final ClassLoader parent;
9795

9896
static EmbeddedImplClassLoader getInstance(ClassLoader parent, String providerName) {
99-
PrivilegedAction<EmbeddedImplClassLoader> pa = () -> new EmbeddedImplClassLoader(parent, getProviderPrefixes(parent, providerName));
100-
return AccessController.doPrivileged(pa);
97+
return new EmbeddedImplClassLoader(parent, getProviderPrefixes(parent, providerName));
10198
}
10299

103100
private EmbeddedImplClassLoader(ClassLoader parent, Map<JarMeta, CodeSource> prefixToCodeBase) {
@@ -120,14 +117,12 @@ private EmbeddedImplClassLoader(ClassLoader parent, Map<JarMeta, CodeSource> pre
120117
record Resource(InputStream inputStream, CodeSource codeSource) {}
121118

122119
/** Searches for the named resource. Iterates over all prefixes. */
123-
private Resource privilegedGetResourceOrNull(JarMeta jarMeta, String pkg, String filepath) {
124-
return AccessController.doPrivileged((PrivilegedAction<Resource>) () -> {
125-
InputStream is = findResourceInLoaderPkgOrNull(jarMeta, pkg, filepath, parent::getResourceAsStream);
126-
if (is != null) {
127-
return new Resource(is, prefixToCodeBase.get(jarMeta.prefix()));
128-
}
129-
return null;
130-
});
120+
private Resource getResourceOrNull(JarMeta jarMeta, String pkg, String filepath) {
121+
InputStream is = findResourceInLoaderPkgOrNull(jarMeta, pkg, filepath, parent::getResourceAsStream);
122+
if (is != null) {
123+
return new Resource(is, prefixToCodeBase.get(jarMeta.prefix()));
124+
}
125+
return null;
131126
}
132127

133128
@Override
@@ -148,7 +143,7 @@ public Class<?> findClass(String name) throws ClassNotFoundException {
148143
String pkg = toPackageName(filepath);
149144
JarMeta jarMeta = packageToJarMeta.get(pkg);
150145
if (jarMeta != null) {
151-
Resource res = privilegedGetResourceOrNull(jarMeta, pkg, filepath);
146+
Resource res = getResourceOrNull(jarMeta, pkg, filepath);
152147
if (res != null) {
153148
try (InputStream in = res.inputStream()) {
154149
byte[] bytes = in.readAllBytes();

‎libs/core/src/main/java/org/elasticsearch/core/internal/provider/ProviderLocator.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
import java.io.UncheckedIOException;
1616
import java.lang.module.Configuration;
1717
import java.lang.module.ModuleFinder;
18-
import java.security.AccessController;
19-
import java.security.PrivilegedActionException;
20-
import java.security.PrivilegedExceptionAction;
2118
import java.util.Locale;
2219
import java.util.Objects;
2320
import java.util.ServiceConfigurationError;
@@ -97,10 +94,9 @@ public ProviderLocator(String providerName, Class<T> providerType, String provid
9794
@Override
9895
public T get() {
9996
try {
100-
PrivilegedExceptionAction<T> pa = this::load;
101-
return AccessController.doPrivileged(pa);
102-
} catch (PrivilegedActionException e) {
103-
throw new UncheckedIOException((IOException) e.getCause());
97+
return load();
98+
} catch (IOException e) {
99+
throw new UncheckedIOException(e);
104100
}
105101
}
106102

‎qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public void testConcurrentDeprecationLogger() throws IOException, BrokenBarrierE
174174
assertLogLine(
175175
deprecationEvents.get(i),
176176
DeprecationLogger.CRITICAL,
177-
"org.elasticsearch.common.logging.DeprecationLogger.lambda\\$doPrivilegedLog\\$0",
177+
"org.elasticsearch.common.logging.DeprecationLogger.logDeprecation",
178178
".*This is a maybe logged deprecation message" + i + ".*"
179179
);
180180
}
@@ -207,7 +207,7 @@ public void testDeprecatedSettings() throws IOException {
207207
assertLogLine(
208208
deprecationEvents.get(0),
209209
DeprecationLogger.CRITICAL,
210-
"org.elasticsearch.common.logging.DeprecationLogger.lambda\\$doPrivilegedLog\\$0",
210+
"org.elasticsearch.common.logging.DeprecationLogger.logDeprecation",
211211
".*\\[deprecated.foo\\] setting was deprecated in Elasticsearch and will be removed in a future release..*"
212212
);
213213
}

‎server/src/main/java/org/elasticsearch/bootstrap/ElasticsearchUncaughtExceptionHandler.java‎

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
import org.elasticsearch.core.SuppressForbidden;
1515

1616
import java.io.IOError;
17-
import java.security.AccessController;
18-
import java.security.PrivilegedAction;
1917

2018
class ElasticsearchUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler {
2119
private static final Logger logger = LogManager.getLogger(ElasticsearchUncaughtExceptionHandler.class);
@@ -53,41 +51,17 @@ static boolean isFatalUncaught(Throwable e) {
5351

5452
void onFatalUncaught(final String threadName, final Throwable t) {
5553
final String message = "fatal error in thread [" + threadName + "], exiting";
56-
logErrorMessage(t, message);
54+
logger.error(message, t);
5755
}
5856

5957
void onNonFatalUncaught(final String threadName, final Throwable t) {
6058
final String message = "uncaught exception in thread [" + threadName + "]";
61-
logErrorMessage(t, message);
62-
}
63-
64-
private static void logErrorMessage(Throwable t, String message) {
65-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
66-
logger.error(message, t);
67-
return null;
68-
});
59+
logger.error(message, t);
6960
}
7061

62+
@SuppressForbidden(reason = "intentionally halting")
7163
void halt(int status) {
72-
AccessController.doPrivileged(new PrivilegedHaltAction(status));
64+
// we halt to prevent shutdown hooks from running
65+
Runtime.getRuntime().halt(status);
7366
}
74-
75-
static class PrivilegedHaltAction implements PrivilegedAction<Void> {
76-
77-
private final int status;
78-
79-
private PrivilegedHaltAction(final int status) {
80-
this.status = status;
81-
}
82-
83-
@SuppressForbidden(reason = "halt")
84-
@Override
85-
public Void run() {
86-
// we halt to prevent shutdown hooks from running
87-
Runtime.getRuntime().halt(status);
88-
return null;
89-
}
90-
91-
}
92-
9367
}

‎server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobStore.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import java.io.IOException;
1919
import java.nio.file.Files;
2020
import java.nio.file.Path;
21-
import java.security.AccessController;
22-
import java.security.PrivilegedAction;
2321
import java.util.Iterator;
2422
import java.util.List;
2523

@@ -57,14 +55,11 @@ public int bufferSizeInBytes() {
5755
public BlobContainer blobContainer(BlobPath path) {
5856
Path f = buildPath(path);
5957
if (readOnly == false) {
60-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
61-
try {
62-
Files.createDirectories(f);
63-
} catch (IOException ex) {
64-
throw new ElasticsearchException("failed to create blob container", ex);
65-
}
66-
return null;
67-
});
58+
try {
59+
Files.createDirectories(f);
60+
} catch (IOException ex) {
61+
throw new ElasticsearchException("failed to create blob container", ex);
62+
}
6863
}
6964
return new FsBlobContainer(this, path, f);
7065
}

‎server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
import org.elasticsearch.common.regex.Regex;
1616
import org.elasticsearch.common.settings.Settings;
1717

18-
import java.security.AccessController;
19-
import java.security.PrivilegedAction;
2018
import java.util.Collections;
2119
import java.util.List;
2220

@@ -119,18 +117,11 @@ private DeprecationLogger logDeprecation(Level level, DeprecationCategory catego
119117
String opaqueId = HeaderWarning.getXOpaqueId();
120118
String productOrigin = HeaderWarning.getProductOrigin();
121119
ESLogMessage deprecationMessage = DeprecatedMessage.of(category, key, opaqueId, productOrigin, msg, params);
122-
doPrivilegedLog(level, deprecationMessage);
120+
logger.log(level, deprecationMessage);
123121
}
124122
return this;
125123
}
126124

127-
private void doPrivilegedLog(Level level, ESLogMessage deprecationMessage) {
128-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
129-
logger.log(level, deprecationMessage);
130-
return null;
131-
});
132-
}
133-
134125
/**
135126
* Used for handling previous version RestApiCompatible logic.
136127
* Logs a message at the {@link DeprecationLogger#CRITICAL} level

‎server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
import org.elasticsearch.core.SuppressForbidden;
1818
import org.elasticsearch.node.Node;
1919

20-
import java.security.AccessController;
21-
import java.security.PrivilegedAction;
2220
import java.util.List;
2321
import java.util.Optional;
2422
import java.util.concurrent.AbstractExecutorService;
@@ -393,11 +391,9 @@ static class EsThreadFactory implements ThreadFactory {
393391

394392
@Override
395393
public Thread newThread(Runnable r) {
396-
return AccessController.doPrivileged((PrivilegedAction<Thread>) () -> {
397-
Thread t = new EsThread(group, r, namePrefix + "[T#" + threadNumber.getAndIncrement() + "]", 0, isSystem);
398-
t.setDaemon(true);
399-
return t;
400-
});
394+
Thread t = new EsThread(group, r, namePrefix + "[T#" + threadNumber.getAndIncrement() + "]", 0, isSystem);
395+
t.setDaemon(true);
396+
return t;
401397
}
402398
}
403399

‎server/src/main/java/org/elasticsearch/plugins/ExtendedPluginsClassLoader.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99

1010
package org.elasticsearch.plugins;
1111

12-
import java.security.AccessController;
13-
import java.security.PrivilegedAction;
1412
import java.util.Collections;
1513
import java.util.List;
1614

@@ -43,8 +41,6 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
4341
* Return a new classloader across the parent and extended loaders.
4442
*/
4543
public static ExtendedPluginsClassLoader create(ClassLoader parent, List<ClassLoader> extendedLoaders) {
46-
return AccessController.doPrivileged(
47-
(PrivilegedAction<ExtendedPluginsClassLoader>) () -> new ExtendedPluginsClassLoader(parent, extendedLoaders)
48-
);
44+
return new ExtendedPluginsClassLoader(parent, extendedLoaders);
4945
}
5046
}

‎server/src/main/java/org/elasticsearch/plugins/PluginsLoader.java‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727
import java.net.URL;
2828
import java.net.URLClassLoader;
2929
import java.nio.file.Path;
30-
import java.security.AccessController;
31-
import java.security.PrivilegedAction;
3230
import java.util.ArrayList;
3331
import java.util.Collections;
3432
import java.util.HashMap;
@@ -423,7 +421,7 @@ static LayerAndLoader createModuleLayer(
423421
finder,
424422
Set.of(moduleName)
425423
);
426-
var controller = privilegedDefineModulesWithOneLoader(configuration, parentLayersOrBoot(parentLayers), parentLoader);
424+
var controller = ModuleLayer.defineModulesWithOneLoader(configuration, parentLayersOrBoot(parentLayers), parentLoader);
427425
var pluginModule = controller.layer().findModule(moduleName).get();
428426
ensureEntryPointAccessible(controller, pluginModule, className);
429427
// export/open upstream modules to this plugin module
@@ -432,7 +430,7 @@ static LayerAndLoader createModuleLayer(
432430
addPluginExportsServices(qualifiedExports, controller);
433431
enableNativeAccess(moduleName, modulesWithNativeAccess, controller);
434432
logger.debug(() -> "Loading bundle: created module layer and loader for module " + moduleName);
435-
return new LayerAndLoader(controller.layer(), privilegedFindLoader(controller.layer(), moduleName));
433+
return new LayerAndLoader(controller.layer(), controller.layer().findLoader(moduleName));
436434
}
437435

438436
/** Determines the module name of the SPI module, given its URL. */
@@ -490,18 +488,6 @@ private static void ensureEntryPointAccessible(Controller controller, Module plu
490488
}
491489
}
492490

493-
@SuppressWarnings("removal")
494-
static Controller privilegedDefineModulesWithOneLoader(Configuration cf, List<ModuleLayer> parentLayers, ClassLoader parentLoader) {
495-
return AccessController.doPrivileged(
496-
(PrivilegedAction<Controller>) () -> ModuleLayer.defineModulesWithOneLoader(cf, parentLayers, parentLoader)
497-
);
498-
}
499-
500-
@SuppressWarnings("removal")
501-
static ClassLoader privilegedFindLoader(ModuleLayer layer, String name) {
502-
return AccessController.doPrivileged((PrivilegedAction<ClassLoader>) () -> layer.findLoader(name));
503-
}
504-
505491
private static List<ModuleLayer> parentLayersOrBoot(List<ModuleLayer> parentLayers) {
506492
if (parentLayers == null || parentLayers.isEmpty()) {
507493
return List.of(ModuleLayer.boot());

‎server/src/main/java/org/elasticsearch/plugins/PluginsService.java‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@
3232
import java.io.IOException;
3333
import java.lang.reflect.Constructor;
3434
import java.nio.file.Path;
35-
import java.security.AccessController;
36-
import java.security.PrivilegedAction;
3735
import java.util.ArrayList;
3836
import java.util.Collection;
3937
import java.util.Collections;
@@ -395,7 +393,7 @@ private void loadBundle(PluginLayer pluginLayer, Map<String, LoadedPlugin> loade
395393
// Set context class loader to plugin's class loader so that plugins
396394
// that have dependencies with their own SPI endpoints have a chance to load
397395
// and initialize them appropriately.
398-
privilegedSetContextClassLoader(pluginLayer.pluginClassLoader());
396+
Thread.currentThread().setContextClassLoader(pluginLayer.pluginClassLoader());
399397

400398
Plugin plugin;
401399
if (pluginBundle.pluginDescriptor().isStable()) {
@@ -428,7 +426,7 @@ We need to pass a name though so that we can show that a plugin was loaded (via
428426
}
429427
loadedPlugins.put(name, new LoadedPlugin(pluginBundle.plugin, plugin, pluginLayer.pluginClassLoader()));
430428
} finally {
431-
privilegedSetContextClassLoader(cl);
429+
Thread.currentThread().setContextClassLoader(cl);
432430
}
433431
}
434432

@@ -537,12 +535,4 @@ private static String signatureMessage(final Class<? extends Plugin> clazz) {
537535
public final <T> Stream<T> filterPlugins(Class<T> type) {
538536
return plugins().stream().filter(x -> type.isAssignableFrom(x.instance().getClass())).map(p -> ((T) p.instance()));
539537
}
540-
541-
@SuppressWarnings("removal")
542-
private static void privilegedSetContextClassLoader(ClassLoader loader) {
543-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
544-
Thread.currentThread().setContextClassLoader(loader);
545-
return null;
546-
});
547-
}
548538
}

0 commit comments

Comments
 (0)