Skip to content

Commit 3ab8e8b

Browse files
rjernstelasticsearchmachine
rjernst
and
elasticsearchmachine
authored
Remove doPrivileged from ES modules (#127848) (#128995)
* Remove doPrivileged from ES modules (#127848) Continuing the cleanup of SecurityManager related code, this commit removes uses of doPrivileged in all Elasticsearch modules. * [CI] Auto commit changes from spotless * fix checkstyle --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent c833893 commit 3ab8e8b

File tree

35 files changed

+420
-939
lines changed

35 files changed

+420
-939
lines changed

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

Lines changed: 1 addition & 3 deletions
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‎

Lines changed: 7 additions & 12 deletions
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;
@@ -96,16 +94,13 @@ public void setAgentSetting(String key, String value) {
9694
return;
9795
}
9896
final String completeKey = "elastic.apm." + Objects.requireNonNull(key);
99-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
100-
if (value == null || value.isEmpty()) {
101-
LOGGER.trace("Clearing system property [{}]", completeKey);
102-
System.clearProperty(completeKey);
103-
} else {
104-
LOGGER.trace("Setting setting property [{}] to [{}]", completeKey, value);
105-
System.setProperty(completeKey, value);
106-
}
107-
return null;
108-
});
97+
if (value == null || value.isEmpty()) {
98+
LOGGER.trace("Clearing system property [{}]", completeKey);
99+
System.clearProperty(completeKey);
100+
} else {
101+
LOGGER.trace("Setting setting property [{}] to [{}]", completeKey, value);
102+
System.setProperty(completeKey, value);
103+
}
109104
}
110105

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

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

Lines changed: 1 addition & 3 deletions
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‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@
3838
import org.elasticsearch.telemetry.tracing.TraceContext;
3939
import org.elasticsearch.telemetry.tracing.Traceable;
4040

41-
import java.security.AccessController;
42-
import java.security.PrivilegedAction;
4341
import java.time.Instant;
4442
import java.util.List;
4543
import java.util.Map;
@@ -144,11 +142,9 @@ APMServices createApmServices() {
144142
assert this.enabled;
145143
assert this.services == null;
146144

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

154150
private void destroyApmServices() {
@@ -174,7 +170,7 @@ public void startTrace(TraceContext traceContext, Traceable traceable, String sp
174170
return;
175171
}
176172

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

@@ -197,7 +193,7 @@ public void startTrace(TraceContext traceContext, Traceable traceable, String sp
197193
updateThreadContext(traceContext, services, contextForNewSpan);
198194

199195
return contextForNewSpan;
200-
}));
196+
});
201197
}
202198

203199
/**
@@ -281,8 +277,7 @@ private Context getParentContext(TraceContext traceContext) {
281277
public Releasable withScope(Traceable traceable) {
282278
final Context context = spans.get(traceable.getSpanId());
283279
if (context != null) {
284-
var scope = AccessController.doPrivileged((PrivilegedAction<Scope>) context::makeCurrent);
285-
return scope::close;
280+
return context.makeCurrent()::close;
286281
}
287282
return () -> {};
288283
}
@@ -381,10 +376,7 @@ public void stopTrace(Traceable traceable) {
381376
final var span = Span.fromContextOrNull(spans.remove(traceable.getSpanId()));
382377
if (span != null) {
383378
logger.trace("Finishing trace [{}]", traceable);
384-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
385-
span.end();
386-
return null;
387-
});
379+
span.end();
388380
}
389381
}
390382

@@ -393,10 +385,7 @@ public void stopTrace(Traceable traceable) {
393385
*/
394386
@Override
395387
public void stopTrace() {
396-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
397-
Span.current().end();
398-
return null;
399-
});
388+
Span.current().end();
400389
}
401390

402391
@Override

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

Lines changed: 37 additions & 53 deletions
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‎

Lines changed: 1 addition & 3 deletions
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‎

Lines changed: 1 addition & 5 deletions
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‎

Lines changed: 3 additions & 26 deletions
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‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,8 @@ private void ensureSasTokenPermissions() {
124124
final AzureBlobServiceClient azureBlobServiceClient = blobStore.getService().client("default", LocationMode.PRIMARY_ONLY);
125125
final BlobServiceClient client = azureBlobServiceClient.getSyncClient();
126126
try {
127-
SocketAccess.doPrivilegedException(() -> {
128-
final BlobContainerClient blobContainer = client.getBlobContainerClient(blobStore.toString());
129-
return blobContainer.exists();
130-
});
127+
final BlobContainerClient blobContainer = client.getBlobContainerClient(blobStore.toString());
128+
blobContainer.exists();
131129
future.onFailure(
132130
new RuntimeException(
133131
"The SAS token used in this test allowed for checking container existence. This test only supports tokens "

0 commit comments

Comments
 (0)