Skip to content
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

Add doPrivilege blocks for socket connect operations in plugins #22534

Merged
merged 9 commits into from
Jan 18, 2017
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,32 @@
package org.elasticsearch.cloud.azure.classic.management;

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

import com.microsoft.windowsazure.Configuration;
import com.microsoft.windowsazure.core.Builder;
import com.microsoft.windowsazure.core.DefaultBuilder;
import com.microsoft.windowsazure.core.utils.KeyStoreType;
import com.microsoft.windowsazure.exception.ServiceException;
import com.microsoft.windowsazure.management.compute.ComputeManagementClient;
import com.microsoft.windowsazure.management.compute.ComputeManagementService;
import com.microsoft.windowsazure.management.compute.models.HostedServiceGetDetailedResponse;
import com.microsoft.windowsazure.management.configuration.ManagementConfiguration;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.SpecialPermission;
import org.elasticsearch.cloud.azure.classic.AzureServiceRemoteException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.xml.sax.SAXException;

import javax.xml.parsers.ParserConfigurationException;

public class AzureComputeServiceImpl extends AbstractLifecycleComponent
implements AzureComputeService {
Expand Down Expand Up @@ -89,10 +99,15 @@ private static String getRequiredSetting(Settings settings, Setting<String> sett

@Override
public HostedServiceGetDetailedResponse getServiceDetails() {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new SpecialPermission());
}
try {
return client.getHostedServicesOperations().getDetailed(serviceName);
} catch (Exception e) {
throw new AzureServiceRemoteException("can not get list of azure nodes", e);
return AccessController.doPrivileged((PrivilegedExceptionAction<HostedServiceGetDetailedResponse>)
() -> client.getHostedServicesOperations().getDetailed(serviceName));
} catch (PrivilegedActionException 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 @@ -21,6 +21,7 @@

import org.apache.lucene.util.IOUtils;
import org.elasticsearch.cloud.aws.AwsEc2ServiceImpl;
import org.elasticsearch.cloud.aws.util.SocketAccess;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.network.NetworkService.CustomNameResolver;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -97,9 +98,9 @@ public InetAddress[] resolve(Ec2HostnameType type) throws IOException {
try {
URL url = new URL(metadataUrl);
logger.debug("obtaining ec2 hostname from ec2 meta-data url {}", url);
URLConnection urlConnection = url.openConnection();
URLConnection urlConnection = SocketAccess.doPrivilegedIOException(url::openConnection);
urlConnection.setConnectTimeout(2000);
in = urlConnection.getInputStream();
in = SocketAccess.doPrivilegedIOException(urlConnection::getInputStream);
BufferedReader urlReader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8));

String metadataResult = urlReader.readLine();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* 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.
*/

package org.elasticsearch.cloud.aws.util;

import org.elasticsearch.SpecialPermission;

import java.io.IOException;
import java.net.SocketPermission;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;

/**
* This plugin uses aws libraries to connect to aws services. For these remote calls the plugin needs
* {@link SocketPermission} 'connect' to establish connections. This class wraps the operations requiring access in
* {@link AccessController#doPrivileged(PrivilegedAction)} blocks.
*/
public final class SocketAccess {

private SocketAccess() {}

public static <T> T doPrivileged(PrivilegedAction<T> operation) {
checkSpecialPermission();
return AccessController.doPrivileged(operation);
}

public static <T> T doPrivilegedIOException(PrivilegedExceptionAction<T> operation) throws IOException {
checkSpecialPermission();
try {
return AccessController.doPrivileged(operation);
} catch (PrivilegedActionException e) {
throw (IOException) e.getCause();
}
}

private static void checkSpecialPermission() {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new SpecialPermission());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use a singleton instance?

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@

package org.elasticsearch.discovery.ec2;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import com.amazonaws.AmazonClientException;
import com.amazonaws.services.ec2.AmazonEC2;
import com.amazonaws.services.ec2.model.DescribeInstancesRequest;
Expand All @@ -38,6 +32,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.cloud.aws.AwsEc2Service;
import org.elasticsearch.cloud.aws.AwsEc2Service.DISCOVERY_EC2;
import org.elasticsearch.cloud.aws.util.SocketAccess;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -114,7 +109,7 @@ protected List<DiscoveryNode> 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 = client.describeInstances(buildDescribeInstancesRequest());
descInstances = SocketAccess.doPrivileged(() -> client.describeInstances(buildDescribeInstancesRequest()));
} catch (AmazonClientException 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 @@ -44,6 +44,7 @@
import org.elasticsearch.cloud.aws.AwsEc2Service;
import org.elasticsearch.cloud.aws.AwsEc2ServiceImpl;
import org.elasticsearch.cloud.aws.network.Ec2NameResolver;
import org.elasticsearch.cloud.aws.util.SocketAccess;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.logging.DeprecationLogger;
Expand All @@ -56,7 +57,6 @@
import org.elasticsearch.discovery.ec2.AwsEc2UnicastHostsProvider;
import org.elasticsearch.discovery.zen.UnicastHostsProvider;
import org.elasticsearch.discovery.zen.ZenDiscovery;
import org.elasticsearch.discovery.zen.ZenPing;
import org.elasticsearch.node.Node;
import org.elasticsearch.plugins.DiscoveryPlugin;
import org.elasticsearch.plugins.Plugin;
Expand All @@ -75,20 +75,19 @@ public class Ec2DiscoveryPlugin extends Plugin implements DiscoveryPlugin, Close
if (sm != null) {
sm.checkPermission(new SpecialPermission());
}
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
try {
// kick jackson to do some static caching of declared members info
Jackson.jsonNodeOf("{}");
// ClientConfiguration clinit has some classloader problems
// TODO: fix that
Class.forName("com.amazonaws.ClientConfiguration");
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
return null;
// Initializing Jackson requires RuntimePermission accessDeclaredMembers
// The ClientConfiguration class requires RuntimePermission getClassLoader
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you leave a comment why this is needed in this particular place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - I can add a comment.

try {
// kick jackson to do some static caching of declared members info
Jackson.jsonNodeOf("{}");
// ClientConfiguration clinit has some classloader problems
// TODO: fix that
Class.forName("com.amazonaws.ClientConfiguration");
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
return null;
});
}

Expand Down Expand Up @@ -194,14 +193,14 @@ static Settings getAvailabilityZoneNodeAttributes(Settings settings, String azMe
try {
url = new URL(azMetadataUrl);
logger.debug("obtaining ec2 [placement/availability-zone] from ec2 meta-data url {}", url);
urlConnection = url.openConnection();
urlConnection = SocketAccess.doPrivilegedIOException(url::openConnection);
urlConnection.setConnectTimeout(2000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can leverage forbidden APIs to restrict the use of openConnection and add a dedicated method that does it with suppressforbidden on it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we spin off anotehr issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that would be good. On #22116 I left a comment a few days ago that maybe we should add another task on that issue to mark relevant APIs as forbidden?

} catch (IOException e) {
// should not happen, we know the url is not malformed, and openConnection does not actually hit network
throw new UncheckedIOException(e);
}

try (InputStream in = urlConnection.getInputStream();
try (InputStream in = SocketAccess.doPrivilegedIOException(urlConnection::getInputStream);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here...

BufferedReader urlReader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) {

String metadataResult = urlReader.readLine();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@

import java.io.Closeable;
import java.io.IOException;
import java.security.AccessController;
import java.security.GeneralSecurityException;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -43,6 +40,7 @@
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.SpecialPermission;
import org.elasticsearch.cloud.gce.util.Access;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
Expand All @@ -68,21 +66,14 @@ public Collection<Instance> instances() {
try {
// hack around code messiness in GCE code
// TODO: get this fixed
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new SpecialPermission());
}
InstanceList instanceList = AccessController.doPrivileged(new PrivilegedExceptionAction<InstanceList>() {
@Override
public InstanceList run() throws Exception {
Compute.Instances.List list = client().instances().list(project, zoneId);
return list.execute();
}
InstanceList instanceList = Access.doPrivilegedIOException(() -> {
Compute.Instances.List list = client().instances().list(project, zoneId);
return list.execute();
});
// assist type inference
return instanceList.isEmpty() || instanceList.getItems() == null ?
return instanceList.isEmpty() || instanceList.getItems() == null ?
Collections.<Instance>emptyList() : instanceList.getItems();
} catch (PrivilegedActionException e) {
} catch (IOException e) {
logger.warn((Supplier<?>) () -> new ParameterizedMessage("Problem fetching instance list for zone {}", zoneId), e);
logger.debug("Full exception:", e);
// assist type inference
Expand Down Expand Up @@ -134,7 +125,7 @@ protected synchronized HttpTransport getGceHttpTransport() throws GeneralSecurit
public synchronized Compute client() {
if (refreshInterval != null && refreshInterval.millis() != 0) {
if (client != null &&
(refreshInterval.millis() < 0 || (System.currentTimeMillis() - lastRefresh) < refreshInterval.millis())) {
(refreshInterval.millis() < 0 || (System.currentTimeMillis() - lastRefresh) < refreshInterval.millis())) {
if (logger.isTraceEnabled()) logger.trace("using cache to retrieve client");
return client;
}
Expand All @@ -151,22 +142,12 @@ public synchronized Compute client() {
String tokenServerEncodedUrl = GceMetadataService.GCE_HOST.get(settings) +
"/computeMetadata/v1/instance/service-accounts/default/token";
ComputeCredential credential = new ComputeCredential.Builder(getGceHttpTransport(), gceJsonFactory)
.setTokenServerEncodedUrl(tokenServerEncodedUrl)
.build();
.setTokenServerEncodedUrl(tokenServerEncodedUrl)
.build();

// hack around code messiness in GCE code
// TODO: get this fixed
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new SpecialPermission());
}
AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
@Override
public Void run() throws IOException {
credential.refreshToken();
return null;
}
});
Access.doPrivilegedIOException(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 @@ -34,6 +34,7 @@
import com.google.api.client.http.HttpResponse;
import com.google.api.client.http.HttpTransport;
import org.elasticsearch.SpecialPermission;
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 All @@ -56,8 +57,8 @@ public GceMetadataService(Settings settings) {

protected synchronized HttpTransport getGceHttpTransport() throws GeneralSecurityException, IOException {
if (gceHttpTransport == null) {
gceHttpTransport = GoogleNetHttpTransport.newTrustedTransport();
}
gceHttpTransport = GoogleNetHttpTransport.newTrustedTransport();
}
return gceHttpTransport;
}

Expand All @@ -71,30 +72,16 @@ public String metadata(String metadataPath) throws IOException, URISyntaxExcepti
try {
// hack around code messiness in GCE code
// TODO: get this fixed
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(new SpecialPermission());
}
headers = AccessController.doPrivileged(new PrivilegedExceptionAction<HttpHeaders>() {
@Override
public HttpHeaders run() throws IOException {
return new HttpHeaders();
}
});
GenericUrl genericUrl = AccessController.doPrivileged(new PrivilegedAction<GenericUrl>() {
@Override
public GenericUrl run() {
return new GenericUrl(urlMetadataNetwork);
}
});
headers = Access.doPrivileged(HttpHeaders::new);
GenericUrl genericUrl = Access.doPrivileged(() -> new GenericUrl(urlMetadataNetwork));

// This is needed to query meta data: https://cloud.google.com/compute/docs/metadata
headers.put("Metadata-Flavor", "Google");
HttpResponse response;
response = getGceHttpTransport().createRequestFactory()
.buildGetRequest(genericUrl)
.setHeaders(headers)
.execute();
HttpResponse response = Access.doPrivilegedIOException(() ->
getGceHttpTransport().createRequestFactory()
.buildGetRequest(genericUrl)
.setHeaders(headers)
.execute());
String metadata = response.parseAsString();
logger.debug("metadata found [{}]", metadata);
return metadata;
Expand Down
Loading