Skip to content

Remove doPrivileged from plugins #127996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@
import org.elasticsearch.logging.Logger;

import java.io.IOException;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.ServiceLoader;

public class AzureComputeServiceImpl extends AbstractLifecycleComponent implements AzureComputeService {
Expand Down Expand Up @@ -94,11 +91,8 @@ private static String getRequiredSetting(Settings settings, Setting<String> sett
public HostedServiceGetDetailedResponse getServiceDetails() {
SpecialPermission.check();
try {
return AccessController.doPrivileged(
(PrivilegedExceptionAction<HostedServiceGetDetailedResponse>) () -> client.getHostedServicesOperations()
.getDetailed(serviceName)
);
} catch (PrivilegedActionException e) {
return client.getHostedServicesOperations().getDetailed(serviceName);
} catch (Exception e) {
throw new AzureServiceRemoteException("can not get list of azure nodes", e.getCause());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ protected List<TransportAddress> fetchDynamicNodes() {
// NOTE: we don't filter by security group during the describe instances request for two reasons:
// 1. differences in VPCs require different parameters during query (ID vs Name)
// 2. We want to use two different strategies: (all security groups vs. any security groups)
descInstances = SocketAccess.doPrivileged(() -> clientReference.client().describeInstances(buildDescribeInstancesRequest()));
descInstances = clientReference.client().describeInstances(buildDescribeInstancesRequest());
} catch (final Exception e) {
logger.info("Exception while retrieving instance list from AWS API: {}", e.getMessage());
logger.debug("Full exception:", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private Ec2Client buildClient(Ec2ClientSettings clientSettings) {
final var endpoint = Endpoint.builder().url(URI.create(clientSettings.endpoint)).build();
ec2ClientBuilder.endpointProvider(endpointParams -> CompletableFuture.completedFuture(endpoint));
}
return SocketAccess.doPrivileged(ec2ClientBuilder::build);
return ec2ClientBuilder.build();
}

private static void applyProxyConfiguration(Ec2ClientSettings clientSettings, ApacheHttpClient.Builder httpClientBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ class AwsEc2Utils {
static String getInstanceMetadata(String metadataPath) {
final var httpClientBuilder = ApacheHttpClient.builder();
httpClientBuilder.connectionTimeout(IMDS_CONNECTION_TIMEOUT);
try (var ec2Client = SocketAccess.doPrivileged(Ec2MetadataClient.builder().httpClient(httpClientBuilder)::build)) {
final var metadataValue = SocketAccess.doPrivileged(() -> ec2Client.get(metadataPath)).asString();
try (var ec2Client = Ec2MetadataClient.builder().httpClient(httpClientBuilder).build()) {
final var metadataValue = ec2Client.get(metadataPath).asString();
if (Strings.hasText(metadataValue) == false) {
throw new IllegalStateException("no ec2 metadata returned from " + metadataPath);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
import org.elasticsearch.cloud.gce.GceInstancesService;
import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.TimeValue;
Expand Down Expand Up @@ -138,23 +137,21 @@ protected GceInstancesService createGceInstancesService() {
return new GceInstancesService() {
@Override
public Collection<Instance> instances() {
return Access.doPrivileged(() -> {
final List<Instance> instances = new ArrayList<>();
final List<Instance> instances = new ArrayList<>();

for (DiscoveryNode discoveryNode : nodes.values()) {
Instance instance = new Instance();
instance.setName(discoveryNode.getName());
instance.setStatus("STARTED");
for (DiscoveryNode discoveryNode : nodes.values()) {
Instance instance = new Instance();
instance.setName(discoveryNode.getName());
instance.setStatus("STARTED");

NetworkInterface networkInterface = new NetworkInterface();
networkInterface.setNetworkIP(discoveryNode.getAddress().toString());
instance.setNetworkInterfaces(singletonList(networkInterface));
NetworkInterface networkInterface = new NetworkInterface();
networkInterface.setNetworkIP(discoveryNode.getAddress().toString());
instance.setNetworkInterfaces(singletonList(networkInterface));

instances.add(instance);
}
instances.add(instance);
}

return instances;
});
return instances;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.api.services.compute.model.Instance;
import com.google.api.services.compute.model.InstanceList;

import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -68,19 +67,17 @@ public Collection<Instance> instances() {
try {
// hack around code messiness in GCE code
// TODO: get this fixed
return Access.doPrivilegedIOException(() -> {
String nextPageToken = null;
List<Instance> zoneInstances = new ArrayList<>();
do {
Compute.Instances.List list = client().instances().list(project, zoneId).setPageToken(nextPageToken);
InstanceList instanceList = list.execute();
nextPageToken = instanceList.getNextPageToken();
if (instanceList.isEmpty() == false && instanceList.getItems() != null) {
zoneInstances.addAll(instanceList.getItems());
}
} while (nextPageToken != null);
return zoneInstances;
});
String nextPageToken = null;
List<Instance> zoneInstances = new ArrayList<>();
do {
Compute.Instances.List list = client().instances().list(project, zoneId).setPageToken(nextPageToken);
InstanceList instanceList = list.execute();
nextPageToken = instanceList.getNextPageToken();
if (instanceList.isEmpty() == false && instanceList.getItems() != null) {
zoneInstances.addAll(instanceList.getItems());
}
} while (nextPageToken != null);
return zoneInstances;
} catch (IOException e) {
logger.warn(() -> "Problem fetching instance list for zone " + zoneId, e);
logger.debug("Full exception:", e);
Expand Down Expand Up @@ -152,15 +149,15 @@ private List<String> resolveZones() {

String getAppEngineValueFromMetadataServer(String serviceURL) throws GeneralSecurityException, IOException {
String metadata = GceMetadataService.GCE_HOST.get(settings);
GenericUrl url = Access.doPrivileged(() -> new GenericUrl(metadata + serviceURL));
GenericUrl url = new GenericUrl(metadata + serviceURL);

HttpTransport httpTransport = getGceHttpTransport();
HttpRequestFactory requestFactory = httpTransport.createRequestFactory();
HttpRequest request = requestFactory.buildGetRequest(url)
.setConnectTimeout(500)
.setReadTimeout(500)
.setHeaders(new HttpHeaders().set("Metadata-Flavor", "Google"));
HttpResponse response = Access.doPrivilegedIOException(() -> request.execute());
HttpResponse response = request.execute();
return headerContainsMetadataFlavor(response) ? response.parseAsString() : null;
}

Expand Down Expand Up @@ -211,7 +208,7 @@ public synchronized Compute client() {

// hack around code messiness in GCE code
// TODO: get this fixed
Access.doPrivilegedIOException(credential::refreshToken);
credential.refreshToken();

logger.debug("token [{}] will expire in [{}] s", credential.getAccessToken(), credential.getExpiresInSeconds());
if (credential.getExpiresInSeconds() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import com.google.api.client.http.HttpResponse;
import com.google.api.client.http.HttpTransport;

import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -66,14 +65,12 @@ public String metadata(String metadataPath) throws IOException, URISyntaxExcepti
try {
// hack around code messiness in GCE code
// TODO: get this fixed
headers = Access.doPrivileged(HttpHeaders::new);
GenericUrl genericUrl = Access.doPrivileged(() -> new GenericUrl(urlMetadataNetwork));
headers = new HttpHeaders();
GenericUrl genericUrl = new GenericUrl(urlMetadataNetwork);

// This is needed to query meta data: https://cloud.google.com/compute/docs/metadata
headers.put("Metadata-Flavor", "Google");
HttpResponse response = Access.doPrivilegedIOException(
() -> getGceHttpTransport().createRequestFactory().buildGetRequest(genericUrl).setHeaders(headers).execute()
);
HttpResponse response = getGceHttpTransport().createRequestFactory().buildGetRequest(genericUrl).setHeaders(headers).execute();
String metadata = response.parseAsString();
logger.debug("metadata found [{}]", metadata);
return metadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
package org.elasticsearch.cloud.gce.network;

import org.elasticsearch.cloud.gce.GceMetadataService;
import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.network.NetworkService.CustomNameResolver;

import java.io.IOException;
import java.net.InetAddress;
import java.net.URISyntaxException;

/**
* <p>Resolves certain GCE related 'meta' hostnames into an actual hostname
Expand Down Expand Up @@ -97,13 +97,13 @@ private InetAddress[] resolve(String value) throws IOException {
}

try {
String metadataResult = Access.doPrivilegedIOException(() -> gceMetadataService.metadata(gceMetadataPath));
String metadataResult = gceMetadataService.metadata(gceMetadataPath);
if (metadataResult == null || metadataResult.length() == 0) {
throw new IOException("no gce metadata returned from [" + gceMetadataPath + "] for [" + value + "]");
}
// only one address: because we explicitly ask for only one via the GceHostnameType
return new InetAddress[] { InetAddress.getByName(metadataResult) };
} catch (IOException e) {
} catch (URISyntaxException | IOException e) {
throw new IOException("IOException caught when fetching InetAddress from [" + gceMetadataPath + "]", e);
}
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.api.client.util.ExponentialBackOff;
import com.google.api.client.util.Sleeper;

import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
Expand Down Expand Up @@ -60,7 +59,7 @@ public RetryHttpInitializerWrapper(Credential wrappedCredential, TimeValue maxWa
// Use only for testing
static MockGoogleCredential.Builder newMockCredentialBuilder() {
// TODO: figure out why GCE is so bad like this
return Access.doPrivileged(MockGoogleCredential.Builder::new);
return new MockGoogleCredential.Builder();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,11 @@

package org.elasticsearch.plugin.discovery.gce;

import com.google.api.client.http.HttpHeaders;
import com.google.api.client.util.ClassInfo;

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.cloud.gce.GceInstancesService;
import org.elasticsearch.cloud.gce.GceInstancesServiceImpl;
import org.elasticsearch.cloud.gce.GceMetadataService;
import org.elasticsearch.cloud.gce.network.GceNameResolver;
import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -53,18 +49,6 @@ public class GceDiscoveryPlugin extends Plugin implements DiscoveryPlugin, Close
// stashed when created in order to properly close
private final SetOnce<GceInstancesService> gceInstancesService = new SetOnce<>();

static {
/*
* GCE's http client changes access levels because its silly and we
* can't allow that on any old stack so we pull it here, up front,
* so we can cleanly check the permissions for it. Without this changing
* the permission can fail if any part of core is on the stack because
* our plugin permissions don't allow core to "reach through" plugins to
* change the permission. Because that'd be silly.
*/
Access.doPrivilegedVoid(() -> ClassInfo.of(HttpHeaders.class, true));
}

public GceDiscoveryPlugin(Settings settings) {
this.settings = settings;
logger.trace("starting gce discovery plugin...");
Expand Down
Loading