Skip to content

Commit

Permalink
Add denylist ip config for datasource endpoint (#573)
Browse files Browse the repository at this point in the history
Signed-off-by: Heemin Kim <[email protected]>
(cherry picked from commit 35edec1)
  • Loading branch information
heemin32 authored and github-actions[bot] committed Oct 27, 2023
1 parent 2a7f1b0 commit 3b8ee71
Show file tree
Hide file tree
Showing 17 changed files with 309 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ See the [CONTRIBUTING guide](./CONTRIBUTING.md#Changelog) for instructions on ho

## [Unreleased 2.x](https://github.com/opensearch-project/geospatial/compare/2.11...2.x)
### Features
* Add denylist ip config for datasource endpoint ([#573](https://github.com/opensearch-project/geospatial/pull/573))
### Enhancements
### Bug Fixes
### Infrastructure
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ dependencies {
implementation "org.apache.commons:commons-csv:1.10.0"
zipArchive group: 'org.opensearch.plugin', name:'opensearch-job-scheduler', version: "${opensearch_build}"
compileOnly "org.opensearch:opensearch-job-scheduler-spi:${opensearch_build}"
implementation "com.github.seancfoley:ipaddress:5.4.0"
}

licenseHeaders.enabled = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestToXContentListener;
Expand All @@ -38,9 +39,11 @@
public class RestPutDatasourceHandler extends BaseRestHandler {
private static final String ACTION_NAME = "ip2geo_datasource_put";
private final ClusterSettings clusterSettings;
private final URLDenyListChecker urlDenyListChecker;

public RestPutDatasourceHandler(final ClusterSettings clusterSettings) {
public RestPutDatasourceHandler(final ClusterSettings clusterSettings, final URLDenyListChecker urlDenyListChecker) {
this.clusterSettings = clusterSettings;
this.urlDenyListChecker = urlDenyListChecker;
}

@Override
Expand All @@ -62,6 +65,9 @@ protected RestChannelConsumer prepareRequest(final RestRequest request, final No
if (putDatasourceRequest.getUpdateInterval() == null) {
putDatasourceRequest.setUpdateInterval(TimeValue.timeValueDays(clusterSettings.get(Ip2GeoSettings.DATASOURCE_UPDATE_INTERVAL)));
}

// Call to validate if URL is in a deny-list or not.
urlDenyListChecker.toUrlIfNotInDenyList(putDatasourceRequest.getEndpoint());
return channel -> client.executeLocally(PutDatasourceAction.INSTANCE, putDatasourceRequest, new RestToXContentListener<>(channel));
}

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

import org.opensearch.client.node.NodeClient;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestToXContentListener;
Expand All @@ -24,6 +25,12 @@
public class RestUpdateDatasourceHandler extends BaseRestHandler {
private static final String ACTION_NAME = "ip2geo_datasource_update";

private final URLDenyListChecker urlDenyListChecker;

public RestUpdateDatasourceHandler(final URLDenyListChecker urlDenyListChecker) {
this.urlDenyListChecker = urlDenyListChecker;
}

@Override
public String getName() {
return ACTION_NAME;
Expand All @@ -37,6 +44,10 @@ protected RestChannelConsumer prepareRequest(final RestRequest request, final No
UpdateDatasourceRequest.PARSER.parse(parser, updateDatasourceRequest, null);
}
}
if (updateDatasourceRequest.getEndpoint() != null) {
// Call to validate if URL is in a deny-list or not.
urlDenyListChecker.toUrlIfNotInDenyList(updateDatasourceRequest.getEndpoint());
}
return channel -> client.executeLocally(
UpdateDatasourceAction.INSTANCE,
updateDatasourceRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Collections;
import java.util.List;
import java.util.function.Function;

import org.opensearch.common.settings.Setting;
import org.opensearch.common.unit.TimeValue;
Expand Down Expand Up @@ -73,12 +75,23 @@ public class Ip2GeoSettings {
Setting.Property.Dynamic
);

/**
* A list of CIDR which will be blocked to be used as datasource endpoint
*/
public static final Setting<List<String>> DATASOURCE_ENDPOINT_DENYLIST = Setting.listSetting(
"plugins.geospatial.ip2geo.datasource.endpoint.denylist",
Collections.emptyList(),
Function.identity(),
Setting.Property.NodeScope,
Setting.Property.Dynamic
);

/**
* Return all settings of Ip2Geo feature
* @return a list of all settings for Ip2Geo feature
*/
public static final List<Setting<?>> settings() {
return List.of(DATASOURCE_ENDPOINT, DATASOURCE_UPDATE_INTERVAL, BATCH_SIZE, TIMEOUT, CACHE_SIZE);
return List.of(DATASOURCE_ENDPOINT, DATASOURCE_UPDATE_INTERVAL, BATCH_SIZE, TIMEOUT, CACHE_SIZE, DATASOURCE_ENDPOINT_DENYLIST);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.geospatial.ip2geo.common;

import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.UnknownHostException;
import java.util.List;

import lombok.extern.log4j.Log4j2;

import org.opensearch.common.SuppressForbidden;
import org.opensearch.common.settings.ClusterSettings;

import inet.ipaddr.IPAddressString;

/**
* A class to check url against a deny-list
*/
@Log4j2
public class URLDenyListChecker {
private final ClusterSettings clusterSettings;

public URLDenyListChecker(final ClusterSettings clusterSettings) {
this.clusterSettings = clusterSettings;
}

/**
* Convert String to URL after verifying the url is not on a deny-list
*
* @param url value to validate and convert to URL
* @return value in URL type
*/
public URL toUrlIfNotInDenyList(final String url) {
try {
return toUrlIfNotInDenyList(url, clusterSettings.get(Ip2GeoSettings.DATASOURCE_ENDPOINT_DENYLIST));
} catch (UnknownHostException e) {
log.error("Unknown host", e);
throw new IllegalArgumentException("host provided in the datasource endpoint is unknown");
} catch (MalformedURLException e) {
log.error("Malformed URL", e);
throw new IllegalArgumentException("URL provided in the datasource endpoint is malformed");

Check warning on line 46 in src/main/java/org/opensearch/geospatial/ip2geo/common/URLDenyListChecker.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/geospatial/ip2geo/common/URLDenyListChecker.java#L41-L46

Added lines #L41 - L46 were not covered by tests
}
}

@SuppressForbidden(reason = "Need to connect to http endpoint to read GeoIP database file")
private URL toUrlIfNotInDenyList(final String url, final List<String> denyList) throws UnknownHostException, MalformedURLException {
URL urlToReturn = new URL(url);
if (isInDenyList(new IPAddressString(InetAddress.getByName(urlToReturn.getHost()).getHostAddress()), denyList)) {
throw new IllegalArgumentException(
"given endpoint is blocked by deny list in cluster setting " + Ip2GeoSettings.DATASOURCE_ENDPOINT_DENYLIST.getKey()
);
}
return urlToReturn;
}

private boolean isInDenyList(final IPAddressString url, final List<String> denyList) {
return denyList.stream().map(cidr -> new IPAddressString(cidr)).anyMatch(cidr -> cidr.contains(url));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import org.opensearch.geospatial.constants.IndexSetting;
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
import org.opensearch.geospatial.shared.Constants;
import org.opensearch.geospatial.shared.StashedThreadContext;
import org.opensearch.index.query.QueryBuilders;
Expand Down Expand Up @@ -91,11 +92,13 @@ public class GeoIpDataDao {
private final ClusterService clusterService;
private final ClusterSettings clusterSettings;
private final Client client;
private final URLDenyListChecker urlDenyListChecker;

public GeoIpDataDao(final ClusterService clusterService, final Client client) {
public GeoIpDataDao(final ClusterService clusterService, final Client client, final URLDenyListChecker urlDenyListChecker) {
this.clusterService = clusterService;
this.clusterSettings = clusterService.getClusterSettings();
this.client = client;
this.urlDenyListChecker = urlDenyListChecker;
}

/**
Expand Down Expand Up @@ -172,7 +175,7 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
SpecialPermission.check();
return AccessController.doPrivileged((PrivilegedAction<CSVParser>) () -> {
try {
URL zipUrl = new URL(manifest.getUrl());
URL zipUrl = urlDenyListChecker.toUrlIfNotInDenyList(manifest.getUrl());
return internalGetDatabaseReader(manifest, zipUrl.openConnection());
} catch (IOException e) {
throw new OpenSearchException("failed to read geoip data from {}", manifest.getUrl(), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.opensearch.geospatial.annotation.VisibleForTesting;
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
import org.opensearch.geospatial.ip2geo.common.DatasourceState;
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
import org.opensearch.geospatial.ip2geo.dao.DatasourceDao;
import org.opensearch.geospatial.ip2geo.dao.GeoIpDataDao;
import org.opensearch.jobscheduler.spi.schedule.IntervalSchedule;
Expand All @@ -37,16 +38,19 @@ public class DatasourceUpdateService {
private final ClusterSettings clusterSettings;
private final DatasourceDao datasourceDao;
private final GeoIpDataDao geoIpDataDao;
private final URLDenyListChecker urlDenyListChecker;

public DatasourceUpdateService(
final ClusterService clusterService,
final DatasourceDao datasourceDao,
final GeoIpDataDao geoIpDataDao
final GeoIpDataDao geoIpDataDao,
final URLDenyListChecker urlDenyListChecker
) {
this.clusterService = clusterService;
this.clusterSettings = clusterService.getClusterSettings();
this.datasourceDao = datasourceDao;
this.geoIpDataDao = geoIpDataDao;
this.urlDenyListChecker = urlDenyListChecker;
}

/**
Expand All @@ -61,7 +65,7 @@ public DatasourceUpdateService(
* @throws IOException
*/
public void updateOrCreateGeoIpData(final Datasource datasource, final Runnable renewLock) throws IOException {
URL url = new URL(datasource.getEndpoint());
URL url = urlDenyListChecker.toUrlIfNotInDenyList(datasource.getEndpoint());
DatasourceManifest manifest = DatasourceManifest.Builder.build(url);

if (shouldUpdate(datasource, manifest) == false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.opensearch.geospatial.ip2geo.common.Ip2GeoExecutor;
import org.opensearch.geospatial.ip2geo.common.Ip2GeoLockService;
import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
import org.opensearch.geospatial.ip2geo.dao.DatasourceDao;
import org.opensearch.geospatial.ip2geo.dao.GeoIpDataDao;
import org.opensearch.geospatial.ip2geo.dao.Ip2GeoCachedDao;
Expand Down Expand Up @@ -97,6 +98,7 @@ public class GeospatialPlugin extends Plugin implements IngestPlugin, ActionPlug
private Ip2GeoCachedDao ip2GeoCachedDao;
private DatasourceDao datasourceDao;
private GeoIpDataDao geoIpDataDao;
private URLDenyListChecker urlDenyListChecker;

@Override
public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
Expand All @@ -105,8 +107,9 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett

@Override
public Map<String, Processor.Factory> getProcessors(Processor.Parameters parameters) {
this.urlDenyListChecker = new URLDenyListChecker(parameters.ingestService.getClusterService().getClusterSettings());
this.datasourceDao = new DatasourceDao(parameters.client, parameters.ingestService.getClusterService());
this.geoIpDataDao = new GeoIpDataDao(parameters.ingestService.getClusterService(), parameters.client);
this.geoIpDataDao = new GeoIpDataDao(parameters.ingestService.getClusterService(), parameters.client, urlDenyListChecker);
this.ip2GeoCachedDao = new Ip2GeoCachedDao(parameters.ingestService.getClusterService(), datasourceDao, geoIpDataDao);
return MapBuilder.<String, Processor.Factory>newMapBuilder()
.put(FeatureProcessor.TYPE, new FeatureProcessor.Factory())
Expand Down Expand Up @@ -153,7 +156,12 @@ public Collection<Object> createComponents(
IndexNameExpressionResolver indexNameExpressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier
) {
DatasourceUpdateService datasourceUpdateService = new DatasourceUpdateService(clusterService, datasourceDao, geoIpDataDao);
DatasourceUpdateService datasourceUpdateService = new DatasourceUpdateService(
clusterService,
datasourceDao,
geoIpDataDao,
urlDenyListChecker
);
Ip2GeoExecutor ip2GeoExecutor = new Ip2GeoExecutor(threadPool);
Ip2GeoLockService ip2GeoLockService = new Ip2GeoLockService(clusterService, client);
/**
Expand Down Expand Up @@ -187,9 +195,9 @@ public List<RestHandler> getRestHandlers(
List<RestHandler> geoJsonHandlers = List.of(new RestUploadStatsAction(), new RestUploadGeoJSONAction());

List<RestHandler> ip2geoHandlers = List.of(
new RestPutDatasourceHandler(clusterSettings),
new RestPutDatasourceHandler(clusterSettings, urlDenyListChecker),
new RestGetDatasourceHandler(),
new RestUpdateDatasourceHandler(),
new RestUpdateDatasourceHandler(urlDenyListChecker),
new RestDeleteDatasourceHandler()
);

Expand Down
64 changes: 64 additions & 0 deletions src/test/java/org/opensearch/geospatial/ClusterSettingHelper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.geospatial;

import static org.apache.lucene.tests.util.LuceneTestCase.createTempDir;
import static org.opensearch.test.NodeRoles.dataNode;
import static org.opensearch.test.OpenSearchTestCase.getTestTransportPlugin;
import static org.opensearch.test.OpenSearchTestCase.getTestTransportType;
import static org.opensearch.test.OpenSearchTestCase.randomLong;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import org.opensearch.cluster.ClusterName;
import org.opensearch.common.SuppressForbidden;
import org.opensearch.common.network.NetworkModule;
import org.opensearch.common.settings.Settings;
import org.opensearch.env.Environment;
import org.opensearch.geospatial.plugin.GeospatialPlugin;
import org.opensearch.node.MockNode;
import org.opensearch.node.Node;
import org.opensearch.plugins.Plugin;
import org.opensearch.test.InternalTestCluster;
import org.opensearch.test.MockHttpTransport;

@SuppressForbidden(reason = "used only for testing")
public class ClusterSettingHelper {
public Node createMockNode(Map<String, Object> configSettings) throws IOException {
Path configDir = createTempDir();
File configFile = configDir.resolve("opensearch.yml").toFile();
FileWriter configFileWriter = new FileWriter(configFile);

for (Map.Entry<String, Object> setting : configSettings.entrySet()) {
configFileWriter.write("\"" + setting.getKey() + "\": " + setting.getValue());
}
configFileWriter.close();
return new MockNode(baseSettings().build(), basePlugins(), configDir, true);
}

private List<Class<? extends Plugin>> basePlugins() {
List<Class<? extends Plugin>> plugins = new ArrayList<>();
plugins.add(getTestTransportPlugin());
plugins.add(MockHttpTransport.TestPlugin.class);
plugins.add(GeospatialPlugin.class);
return plugins;
}

private static Settings.Builder baseSettings() {
final Path tempDir = createTempDir();
return Settings.builder()
.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", randomLong()))
.put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
.put(NetworkModule.TRANSPORT_TYPE_KEY, getTestTransportType())
.put(dataNode());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.opensearch.geospatial.ip2geo.common.Ip2GeoExecutor;
import org.opensearch.geospatial.ip2geo.common.Ip2GeoLockService;
import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
import org.opensearch.geospatial.ip2geo.dao.DatasourceDao;
import org.opensearch.geospatial.ip2geo.dao.GeoIpDataDao;
import org.opensearch.geospatial.ip2geo.dao.Ip2GeoCachedDao;
Expand Down Expand Up @@ -98,6 +99,7 @@ public abstract class Ip2GeoTestCase extends RestActionTestCase {
protected Ip2GeoProcessorDao ip2GeoProcessorDao;
@Mock
protected RoutingTable routingTable;
protected URLDenyListChecker urlDenyListChecker;
protected IngestMetadata ingestMetadata;
protected NoOpNodeClient client;
protected VerifyingClient verifyingClient;
Expand All @@ -115,6 +117,7 @@ public void prepareIp2GeoTestCase() {
clusterSettings = new ClusterSettings(settings, new HashSet<>(Ip2GeoSettings.settings()));
lockService = new LockService(client, clusterService);
ingestMetadata = new IngestMetadata(Collections.emptyMap());
urlDenyListChecker = spy(new URLDenyListChecker(clusterSettings));
when(metadata.custom(IngestMetadata.TYPE)).thenReturn(ingestMetadata);
when(clusterService.getSettings()).thenReturn(Settings.EMPTY);
when(clusterService.getClusterSettings()).thenReturn(clusterSettings);
Expand Down
Loading

0 comments on commit 3b8ee71

Please sign in to comment.