Skip to content

Commit 8ad2723

Browse files
rjernstrjernst
authored
Remove doPrivileged from ES modules (#127848)
Continuing the cleanup of SecurityManager related code, this commit removes uses of doPrivileged in all Elasticsearch modules.
1 parent da553b1 commit 8ad2723

File tree

36 files changed

+424
-937
lines changed

36 files changed

+424
-937
lines changed

‎modules/apm/src/main/java/org/elasticsearch/telemetry/apm/AbstractInstrument.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
import org.elasticsearch.telemetry.apm.internal.MetricNameValidator;
1616
import org.elasticsearch.telemetry.metric.Instrument;
1717

18-
import java.security.AccessController;
19-
import java.security.PrivilegedAction;
2018
import java.util.Objects;
2119
import java.util.concurrent.atomic.AtomicReference;
2220
import java.util.function.Function;
@@ -35,7 +33,7 @@ public abstract class AbstractInstrument<T> implements Instrument {
3533

3634
public AbstractInstrument(Meter meter, Builder<T> builder) {
3735
this.name = builder.getName();
38-
this.instrumentBuilder = m -> AccessController.doPrivileged((PrivilegedAction<T>) () -> builder.build(m));
36+
this.instrumentBuilder = m -> builder.build(m);
3937
this.delegate.set(this.instrumentBuilder.apply(meter));
4038
}
4139

‎modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java

+7-12
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
import org.elasticsearch.core.SuppressForbidden;
2121
import org.elasticsearch.telemetry.apm.internal.tracing.APMTracer;
2222

23-
import java.security.AccessController;
24-
import java.security.PrivilegedAction;
2523
import java.util.List;
2624
import java.util.Objects;
2725
import java.util.Set;
@@ -94,16 +92,13 @@ public void setAgentSetting(String key, String value) {
9492
return;
9593
}
9694
final String completeKey = "elastic.apm." + Objects.requireNonNull(key);
97-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
98-
if (value == null || value.isEmpty()) {
99-
LOGGER.trace("Clearing system property [{}]", completeKey);
100-
System.clearProperty(completeKey);
101-
} else {
102-
LOGGER.trace("Setting setting property [{}] to [{}]", completeKey, value);
103-
System.setProperty(completeKey, value);
104-
}
105-
return null;
106-
});
95+
if (value == null || value.isEmpty()) {
96+
LOGGER.trace("Clearing system property [{}]", completeKey);
97+
System.clearProperty(completeKey);
98+
} else {
99+
LOGGER.trace("Setting setting property [{}] to [{}]", completeKey, value);
100+
System.setProperty(completeKey, value);
101+
}
107102
}
108103

109104
private static final String TELEMETRY_SETTING_PREFIX = "telemetry.";

‎modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMMeterService.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
import org.elasticsearch.common.settings.Settings;
1919
import org.elasticsearch.telemetry.apm.APMMeterRegistry;
2020

21-
import java.security.AccessController;
22-
import java.security.PrivilegedAction;
2321
import java.util.function.Supplier;
2422

2523
public class APMMeterService extends AbstractLifecycleComponent {
@@ -74,7 +72,7 @@ protected void doClose() {}
7472

7573
protected Meter createOtelMeter() {
7674
assert this.enabled;
77-
return AccessController.doPrivileged((PrivilegedAction<Meter>) otelMeterSupplier::get);
75+
return otelMeterSupplier.get();
7876
}
7977

8078
protected Meter createNoopMeter() {

‎modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/tracing/APMTracer.java

+8-19
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@
3939
import org.elasticsearch.telemetry.tracing.TraceContext;
4040
import org.elasticsearch.telemetry.tracing.Traceable;
4141

42-
import java.security.AccessController;
43-
import java.security.PrivilegedAction;
4442
import java.time.Instant;
4543
import java.util.List;
4644
import java.util.Map;
@@ -145,11 +143,9 @@ APMServices createApmServices() {
145143
assert this.enabled;
146144
assert this.services == null;
147145

148-
return AccessController.doPrivileged((PrivilegedAction<APMServices>) () -> {
149-
var openTelemetry = GlobalOpenTelemetry.get();
150-
var tracer = openTelemetry.getTracer("elasticsearch", Build.current().version());
151-
return new APMServices(tracer, openTelemetry);
152-
});
146+
var openTelemetry = GlobalOpenTelemetry.get();
147+
var tracer = openTelemetry.getTracer("elasticsearch", Build.current().version());
148+
return new APMServices(tracer, openTelemetry);
153149
}
154150

155151
private void destroyApmServices() {
@@ -175,7 +171,7 @@ public void startTrace(TraceContext traceContext, Traceable traceable, String sp
175171
return;
176172
}
177173

178-
spans.computeIfAbsent(spanId, _spanId -> AccessController.doPrivileged((PrivilegedAction<Context>) () -> {
174+
spans.computeIfAbsent(spanId, _spanId -> {
179175
logger.trace("Tracing [{}] [{}]", spanId, spanName);
180176
final SpanBuilder spanBuilder = services.tracer.spanBuilder(spanName);
181177

@@ -198,7 +194,7 @@ public void startTrace(TraceContext traceContext, Traceable traceable, String sp
198194
updateThreadContext(traceContext, services, contextForNewSpan);
199195

200196
return contextForNewSpan;
201-
}));
197+
});
202198
}
203199

204200
/**
@@ -282,8 +278,7 @@ private Context getParentContext(TraceContext traceContext) {
282278
public Releasable withScope(Traceable traceable) {
283279
final Context context = spans.get(traceable.getSpanId());
284280
if (context != null) {
285-
var scope = AccessController.doPrivileged((PrivilegedAction<Scope>) context::makeCurrent);
286-
return scope::close;
281+
return context.makeCurrent()::close;
287282
}
288283
return () -> {};
289284
}
@@ -380,10 +375,7 @@ public void stopTrace(Traceable traceable) {
380375
final var span = Span.fromContextOrNull(spans.remove(traceable.getSpanId()));
381376
if (span != null) {
382377
logger.trace("Finishing trace [{}]", traceable);
383-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
384-
span.end();
385-
return null;
386-
});
378+
span.end();
387379
}
388380
}
389381

@@ -392,10 +384,7 @@ public void stopTrace(Traceable traceable) {
392384
*/
393385
@Override
394386
public void stopTrace() {
395-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
396-
Span.current().end();
397-
return null;
398-
});
387+
Span.current().end();
399388
}
400389

401390
@Override

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/HttpClient.java

+37-53
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
import org.elasticsearch.ElasticsearchStatusException;
1313
import org.elasticsearch.ResourceNotFoundException;
14-
import org.elasticsearch.SpecialPermission;
15-
import org.elasticsearch.common.CheckedSupplier;
1614
import org.elasticsearch.core.SuppressForbidden;
1715
import org.elasticsearch.rest.RestStatus;
1816

@@ -22,9 +20,6 @@
2220
import java.net.HttpURLConnection;
2321
import java.net.PasswordAuthentication;
2422
import java.net.URL;
25-
import java.security.AccessController;
26-
import java.security.PrivilegedActionException;
27-
import java.security.PrivilegedExceptionAction;
2823
import java.util.Arrays;
2924
import java.util.Objects;
3025

@@ -88,46 +83,44 @@ InputStream get(final PasswordAuthentication auth, final String url) throws IOEx
8883

8984
final String originalAuthority = new URL(url).getAuthority();
9085

91-
return doPrivileged(() -> {
92-
String innerUrl = url;
93-
HttpURLConnection conn = createConnection(auth, innerUrl);
94-
95-
int redirectsCount = 0;
96-
while (true) {
97-
switch (conn.getResponseCode()) {
98-
case HTTP_OK:
99-
return getInputStream(conn);
100-
case HTTP_MOVED_PERM:
101-
case HTTP_MOVED_TEMP:
102-
case HTTP_SEE_OTHER:
103-
if (redirectsCount++ > 50) {
104-
throw new IllegalStateException("too many redirects connection to [" + url + "]");
105-
}
106-
107-
// deal with redirections (including relative urls)
108-
final String location = conn.getHeaderField("Location");
109-
final URL base = new URL(innerUrl);
110-
final URL next = new URL(base, location);
111-
innerUrl = next.toExternalForm();
112-
113-
// compare the *original* authority and the next authority to determine whether to include auth details.
114-
// this means that the host and port (if it is provided explicitly) are considered. it also means that if we
115-
// were to ping-pong back to the original authority, then we'd start including the auth details again.
116-
final String nextAuthority = next.getAuthority();
117-
if (originalAuthority.equals(nextAuthority)) {
118-
conn = createConnection(auth, innerUrl);
119-
} else {
120-
conn = createConnection(NO_AUTH, innerUrl);
121-
}
122-
break;
123-
case HTTP_NOT_FOUND:
124-
throw new ResourceNotFoundException("{} not found", url);
125-
default:
126-
int responseCode = conn.getResponseCode();
127-
throw new ElasticsearchStatusException("error during downloading {}", RestStatus.fromCode(responseCode), url);
128-
}
86+
String innerUrl = url;
87+
HttpURLConnection conn = createConnection(auth, innerUrl);
88+
89+
int redirectsCount = 0;
90+
while (true) {
91+
switch (conn.getResponseCode()) {
92+
case HTTP_OK:
93+
return getInputStream(conn);
94+
case HTTP_MOVED_PERM:
95+
case HTTP_MOVED_TEMP:
96+
case HTTP_SEE_OTHER:
97+
if (redirectsCount++ > 50) {
98+
throw new IllegalStateException("too many redirects connection to [" + url + "]");
99+
}
100+
101+
// deal with redirections (including relative urls)
102+
final String location = conn.getHeaderField("Location");
103+
final URL base = new URL(innerUrl);
104+
final URL next = new URL(base, location);
105+
innerUrl = next.toExternalForm();
106+
107+
// compare the *original* authority and the next authority to determine whether to include auth details.
108+
// this means that the host and port (if it is provided explicitly) are considered. it also means that if we
109+
// were to ping-pong back to the original authority, then we'd start including the auth details again.
110+
final String nextAuthority = next.getAuthority();
111+
if (originalAuthority.equals(nextAuthority)) {
112+
conn = createConnection(auth, innerUrl);
113+
} else {
114+
conn = createConnection(NO_AUTH, innerUrl);
115+
}
116+
break;
117+
case HTTP_NOT_FOUND:
118+
throw new ResourceNotFoundException("{} not found", url);
119+
default:
120+
int responseCode = conn.getResponseCode();
121+
throw new ElasticsearchStatusException("error during downloading {}", RestStatus.fromCode(responseCode), url);
129122
}
130-
});
123+
}
131124
}
132125

133126
@SuppressForbidden(reason = "we need socket connection to download data from internet")
@@ -150,13 +143,4 @@ protected PasswordAuthentication getPasswordAuthentication() {
150143
conn.setInstanceFollowRedirects(false);
151144
return conn;
152145
}
153-
154-
private static <R> R doPrivileged(final CheckedSupplier<R, IOException> supplier) throws IOException {
155-
SpecialPermission.check();
156-
try {
157-
return AccessController.doPrivileged((PrivilegedExceptionAction<R>) supplier::get);
158-
} catch (PrivilegedActionException e) {
159-
throw (IOException) e.getCause();
160-
}
161-
}
162146
}

‎modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import java.lang.reflect.Field;
2020
import java.lang.reflect.Method;
2121
import java.nio.charset.StandardCharsets;
22-
import java.security.AccessController;
23-
import java.security.PrivilegedAction;
2422
import java.util.ArrayList;
2523
import java.util.Arrays;
2624
import java.util.Collections;
@@ -492,7 +490,7 @@ public static Whitelist loadFromResourceFiles(Class<?> owner, Map<String, Whitel
492490
}
493491
}
494492

495-
ClassLoader loader = AccessController.doPrivileged((PrivilegedAction<ClassLoader>) owner::getClassLoader);
493+
ClassLoader loader = owner.getClassLoader();
496494

497495
return new Whitelist(loader, whitelistClasses, whitelistStatics, whitelistClassBindings, Collections.emptyList());
498496
}

‎modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import java.lang.invoke.MethodHandle;
2323
import java.lang.invoke.MethodHandles;
2424
import java.lang.invoke.MethodType;
25-
import java.security.AccessController;
26-
import java.security.PrivilegedAction;
2725
import java.util.List;
2826

2927
import static java.lang.invoke.MethodHandles.Lookup;
@@ -504,9 +502,7 @@ private static Class<?> createLambdaClass(Compiler.Loader loader, ClassWriter cw
504502
byte[] classBytes = cw.toByteArray();
505503
// DEBUG:
506504
// new ClassReader(classBytes).accept(new TraceClassVisitor(new PrintWriter(System.out)), ClassReader.SKIP_DEBUG);
507-
return AccessController.doPrivileged(
508-
(PrivilegedAction<Class<?>>) () -> loader.defineLambda(lambdaClassType.getClassName(), classBytes)
509-
);
505+
return loader.defineLambda(lambdaClassType.getClassName(), classBytes);
510506
}
511507

512508
/**

‎modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java

+3-26
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@
2727

2828
import java.lang.invoke.MethodType;
2929
import java.lang.reflect.Method;
30-
import java.security.AccessControlContext;
31-
import java.security.AccessController;
3230
import java.security.Permissions;
33-
import java.security.PrivilegedAction;
34-
import java.security.ProtectionDomain;
3531
import java.util.ArrayList;
3632
import java.util.Arrays;
3733
import java.util.Collections;
@@ -52,18 +48,12 @@ public final class PainlessScriptEngine implements ScriptEngine {
5248
*/
5349
public static final String NAME = "painless";
5450

55-
/**
56-
* Permissions context used during compilation.
57-
*/
58-
private static final AccessControlContext COMPILATION_CONTEXT;
59-
6051
/*
6152
* Setup the allowed permissions.
6253
*/
6354
static {
6455
final Permissions none = new Permissions();
6556
none.setReadOnly();
66-
COMPILATION_CONTEXT = new AccessControlContext(new ProtectionDomain[] { new ProtectionDomain(null, none) });
6757
}
6858

6959
/**
@@ -123,12 +113,7 @@ public <T> T compile(String scriptName, String scriptSource, ScriptContext<T> co
123113
SpecialPermission.check();
124114

125115
// Create our loader (which loads compiled code with no permissions).
126-
final Loader loader = AccessController.doPrivileged(new PrivilegedAction<Loader>() {
127-
@Override
128-
public Loader run() {
129-
return compiler.createLoader(getClass().getClassLoader());
130-
}
131-
});
116+
final Loader loader = compiler.createLoader(getClass().getClassLoader());
132117

133118
ScriptScope scriptScope = compile(contextsToCompilers.get(context), loader, scriptName, scriptSource, params);
134119

@@ -398,17 +383,9 @@ ScriptScope compile(Compiler compiler, Loader loader, String scriptName, String
398383

399384
try {
400385
// Drop all permissions to actually compile the code itself.
401-
return AccessController.doPrivileged(new PrivilegedAction<ScriptScope>() {
402-
@Override
403-
public ScriptScope run() {
404-
String name = scriptName == null ? source : scriptName;
405-
return compiler.compile(loader, name, source, compilerSettings);
406-
}
407-
}, COMPILATION_CONTEXT);
386+
String name = scriptName == null ? source : scriptName;
387+
return compiler.compile(loader, name, source, compilerSettings);
408388
// Note that it is safe to catch any of the following errors since Painless is stateless.
409-
} catch (SecurityException e) {
410-
// security exceptions are rethrown so that they can propagate to the ES log, they are not user errors
411-
throw e;
412389
} catch (OutOfMemoryError | StackOverflowError | LinkageError | Exception e) {
413390
throw convertToScriptException(source, e);
414391
}

‎modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureStorageCleanupThirdPartyTests.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,8 @@ private void ensureSasTokenPermissions() {
137137
.client("default", LocationMode.PRIMARY_ONLY, randomFrom(OperationPurpose.values()));
138138
final BlobServiceClient client = azureBlobServiceClient.getSyncClient();
139139
try {
140-
SocketAccess.doPrivilegedException(() -> {
141-
final BlobContainerClient blobContainer = client.getBlobContainerClient(blobStore.toString());
142-
return blobContainer.exists();
143-
});
140+
final BlobContainerClient blobContainer = client.getBlobContainerClient(blobStore.toString());
141+
blobContainer.exists();
144142
future.onFailure(
145143
new RuntimeException(
146144
"The SAS token used in this test allowed for checking container existence. This test only supports tokens "

0 commit comments

Comments
 (0)