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

[Backport 2.x] Changed allowlist config to denylist ip config for datasource uri hosts #2058

Merged
merged 1 commit into from
Sep 7, 2023
Merged
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
1 change: 1 addition & 0 deletions common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ dependencies {
implementation 'com.github.babbel:okhttp-aws-signer:1.0.2'
api group: 'com.amazonaws', name: 'aws-java-sdk-core', version: '1.12.1'
api group: 'com.amazonaws', name: 'aws-java-sdk-sts', version: '1.12.1'
implementation "com.github.seancfoley:ipaddress:5.4.0"

testImplementation group: 'junit', name: 'junit', version: '4.13.2'
testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.9.1'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

package org.opensearch.sql.common.authinterceptors;
package org.opensearch.sql.common.interceptors;

import com.amazonaws.auth.AWSCredentials;
import com.amazonaws.auth.AWSCredentialsProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

package org.opensearch.sql.common.authinterceptors;
package org.opensearch.sql.common.interceptors;

import java.io.IOException;
import lombok.NonNull;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
*
* * Copyright OpenSearch Contributors
* * SPDX-License-Identifier: Apache-2.0
*
*/

package org.opensearch.sql.common.interceptors;

import java.io.IOException;
import java.util.List;
import lombok.NonNull;
import okhttp3.Interceptor;
import okhttp3.Request;
import okhttp3.Response;
import org.jetbrains.annotations.NotNull;
import org.opensearch.sql.common.setting.Settings;
import org.opensearch.sql.common.utils.URIValidationUtils;

public class URIValidatorInterceptor implements Interceptor {

private final List<String> denyHostList;

public URIValidatorInterceptor(@NonNull List<String> denyHostList) {
this.denyHostList = denyHostList;
}

@NotNull
@Override
public Response intercept(Interceptor.Chain chain) throws IOException {
Request request = chain.request();
String host = request.url().host();
boolean isValidHost = URIValidationUtils.validateURIHost(host, denyHostList);
if (isValidHost) {
return chain.proceed(request);
} else {
throw new IllegalArgumentException(
String.format(
"Disallowed hostname in the uri. Validate with %s config",
Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST.getKeyValue()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public enum Key {
QUERY_MEMORY_LIMIT("plugins.query.memory_limit"),
QUERY_SIZE_LIMIT("plugins.query.size_limit"),
ENCYRPTION_MASTER_KEY("plugins.query.datasources.encryption.masterkey"),
DATASOURCES_URI_ALLOWHOSTS("plugins.query.datasources.uri.allowhosts"),
DATASOURCES_URI_HOSTS_DENY_LIST("plugins.query.datasources.uri.hosts.denylist"),

METRICS_ROLLING_WINDOW("plugins.query.metrics.rolling_window"),
METRICS_ROLLING_INTERVAL("plugins.query.metrics.rolling_interval"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.opensearch.sql.common.utils;

import inet.ipaddr.IPAddressString;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.List;

/** Utility Class for URI host validation. */
public class URIValidationUtils {

public static boolean validateURIHost(String host, List<String> denyHostList)
throws UnknownHostException {
IPAddressString ipStr = new IPAddressString(InetAddress.getByName(host).getHostAddress());
for (String denyHost : denyHostList) {
IPAddressString denyHostStr = new IPAddressString(denyHost);
if (denyHostStr.contains(ipStr)) {
return false;
}
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

package org.opensearch.sql.common.authinterceptors;
package org.opensearch.sql.common.interceptors;

import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.auth.AWSSessionCredentials;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

package org.opensearch.sql.common.authinterceptors;
package org.opensearch.sql.common.interceptors;

import java.util.Collections;
import lombok.SneakyThrows;
Expand Down
1 change: 1 addition & 0 deletions datasources/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ dependencies {
implementation group: 'org.opensearch', name: 'common-utils', version: "${opensearch_build}"
implementation group: 'commons-io', name: 'commons-io', version: '2.8.0'
implementation 'com.amazonaws:aws-encryption-sdk-java:2.4.1'
implementation group: 'commons-validator', name: 'commons-validator', version: '1.7'

testImplementation group: 'junit', name: 'junit', version: '4.13.2'
testImplementation('org.junit.jupiter:junit-jupiter:5.6.2')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package org.opensearch.sql.datasources.utils;

import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import lombok.experimental.UtilityClass;
import org.apache.commons.validator.routines.DomainValidator;
import org.opensearch.sql.common.utils.URIValidationUtils;

/** Common Validation methods for all datasource connectors. */
@UtilityClass
public class DatasourceValidationUtils {

public static final int MAX_LENGTH_FOR_CONFIG_PROPERTY = 1000;

public static void validateHost(String uriString, List<String> denyHostList)
throws URISyntaxException, UnknownHostException {
validateDomain(uriString);
if (!URIValidationUtils.validateURIHost(new URI(uriString).getHost(), denyHostList)) {
throw new IllegalArgumentException(
"Disallowed hostname in the uri. "
+ "Validate with plugins.query.datasources.uri.hosts.denylist config");
}
;
}

public static void validateLengthAndRequiredFields(
Map<String, String> config, Set<String> fields) {
Set<String> missingFields = new HashSet<>();
Set<String> invalidLengthFields = new HashSet<>();
for (String field : fields) {
if (!config.containsKey(field)) {
missingFields.add(field);
} else if (config.get(field).length() > MAX_LENGTH_FOR_CONFIG_PROPERTY) {
invalidLengthFields.add(field);
}
}
StringBuilder errorStringBuilder = new StringBuilder();
if (missingFields.size() > 0) {
errorStringBuilder.append(
String.format(
"Missing %s fields in the Prometheus connector properties.", missingFields));
}

if (invalidLengthFields.size() > 0) {
errorStringBuilder.append(
String.format("Fields %s exceeds more than 1000 characters.", invalidLengthFields));
}
if (errorStringBuilder.length() > 0) {
throw new IllegalArgumentException(errorStringBuilder.toString());
}
}

private static void validateDomain(String uriString) throws URISyntaxException {
URI uri = new URI(uriString);
String host = uri.getHost();
if (host == null
|| (!(DomainValidator.getInstance().isValid(host)
|| DomainValidator.getInstance().isValidLocalTld(host)))) {
throw new IllegalArgumentException(
String.format("Invalid hostname in the uri: %s", uriString));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package org.opensearch.sql.datasources.utils;

import java.util.Collections;
import java.util.HashMap;
import java.util.Set;
import lombok.SneakyThrows;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
public class DatasourceValidationUtilsTest {

@SneakyThrows
@Test
public void testValidateHost() {
IllegalArgumentException illegalArgumentException =
Assertions.assertThrows(
IllegalArgumentException.class,
() ->
DatasourceValidationUtils.validateHost(
"http://localhost:9090", Collections.singletonList("127.0.0.0/8")));
Assertions.assertEquals(
"Disallowed hostname in the uri. Validate with plugins.query.datasources.uri.hosts.denylist"
+ " config",
illegalArgumentException.getMessage());
}

@SneakyThrows
@Test
public void testValidateHostWithSuccess() {
Assertions.assertDoesNotThrow(
() ->
DatasourceValidationUtils.validateHost(
"http://localhost:9090", Collections.singletonList("192.168.0.0/8")));
}

@SneakyThrows
@Test
public void testValidateHostWithInvalidDomain() {
IllegalArgumentException illegalArgumentException =
Assertions.assertThrows(
IllegalArgumentException.class,
() ->
DatasourceValidationUtils.validateHost(
"http:://prometheus:9090", Collections.singletonList("127.0.0.0/8")));
Assertions.assertEquals(
"Invalid hostname in the uri: http:://prometheus:9090",
illegalArgumentException.getMessage());
}

@Test
public void testValidateLengthAndRequiredFieldsWithAbsentField() {
HashMap<String, String> config = new HashMap<>();
config.put("s3.uri", "test");
IllegalArgumentException illegalArgumentException =
Assertions.assertThrows(
IllegalArgumentException.class,
() ->
DatasourceValidationUtils.validateLengthAndRequiredFields(
config, Set.of("s3.uri", "s3.auth.type")));
Assertions.assertEquals(
"Missing [s3.auth.type] fields in the Prometheus connector properties.",
illegalArgumentException.getMessage());
}

@Test
public void testValidateLengthAndRequiredFieldsWithInvalidLength() {
HashMap<String, String> config = new HashMap<>();
config.put("s3.uri", RandomStringUtils.random(1001));
IllegalArgumentException illegalArgumentException =
Assertions.assertThrows(
IllegalArgumentException.class,
() ->
DatasourceValidationUtils.validateLengthAndRequiredFields(
config, Set.of("s3.uri", "s3.auth.type")));
Assertions.assertEquals(
"Missing [s3.auth.type] fields in the Prometheus connector properties.Fields "
+ "[s3.uri] exceeds more than 1000 characters.",
illegalArgumentException.getMessage());
}

@Test
public void testValidateLengthAndRequiredFieldsWithSuccess() {
HashMap<String, String> config = new HashMap<>();
config.put("s3.uri", "test");
Assertions.assertDoesNotThrow(
() ->
DatasourceValidationUtils.validateLengthAndRequiredFields(
config, Collections.emptySet()));
}
}
12 changes: 4 additions & 8 deletions docs/user/ppl/admin/datasources.rst
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,11 @@ Master Key config for encrypting credential information
# Print the master key
print("Generated master key:", master_key)

Datasource Allow Hosts Config
Datasource URI Hosts Deny Lists Config
========================================================
* In the OpenSearch configuration file (opensearch.yml), the parameter "plugins.query.datasources.uri.allowhosts" can be utilized to control the permitted hosts within the datasource URI configuration.
* By default, the value is set to `.*`, which allows any domain to be accepted.
* For instance, if you set the value to `dummy.*.com`, the following URIs are some examples that would be allowed in the datasource configuration:
- https://dummy.prometheus.com:9080
- http://dummy.prometheus.com

Note: The mentioned URIs are just examples to illustrate the concept.
* In the OpenSearch configuration file (opensearch.yml), the parameter "plugins.query.datasources.uri.hosts.denylist" can be utilized to control the permitted host ips within the datasource URI configuration.
* By default, the value is set to empty list, which allows any domain to be accepted.
* For instance, if you set the value to `127.0.0.0/8`, ppl plugins will deny all the query requests where the datasource URI resolves to the ip range from `127.0.0.0` to `127.255.255.255`


Using a datasource in PPL command
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.io.InputStream;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.opensearch.cluster.ClusterName;
Expand Down Expand Up @@ -119,10 +121,11 @@ public class OpenSearchSettings extends Settings {
Setting.Property.Final,
Setting.Property.Filtered);

public static final Setting<String> DATASOURCE_URI_ALLOW_HOSTS =
Setting.simpleString(
Key.DATASOURCES_URI_ALLOWHOSTS.getKeyValue(),
".*",
public static final Setting<List<String>> DATASOURCE_URI_HOSTS_DENY_LIST =
Setting.listSetting(
Key.DATASOURCES_URI_HOSTS_DENY_LIST.getKeyValue(),
Collections.emptyList(),
Function.identity(),
Setting.Property.NodeScope,
Setting.Property.Dynamic);

Expand Down Expand Up @@ -187,9 +190,9 @@ public OpenSearchSettings(ClusterSettings clusterSettings) {
register(
settingBuilder,
clusterSettings,
Key.DATASOURCES_URI_ALLOWHOSTS,
DATASOURCE_URI_ALLOW_HOSTS,
new Updater(Key.DATASOURCES_URI_ALLOWHOSTS));
Key.DATASOURCES_URI_HOSTS_DENY_LIST,
DATASOURCE_URI_HOSTS_DENY_LIST,
new Updater(Key.DATASOURCES_URI_HOSTS_DENY_LIST));
registerNonDynamicSettings(
settingBuilder, clusterSettings, Key.CLUSTER_NAME, ClusterName.CLUSTER_NAME_SETTING);
defaultSettings = settingBuilder.build();
Expand Down Expand Up @@ -253,7 +256,7 @@ public static List<Setting<?>> pluginSettings() {
.add(QUERY_SIZE_LIMIT_SETTING)
.add(METRICS_ROLLING_WINDOW_SETTING)
.add(METRICS_ROLLING_INTERVAL_SETTING)
.add(DATASOURCE_URI_ALLOW_HOSTS)
.add(DATASOURCE_URI_HOSTS_DENY_LIST)
.build();
}

Expand Down
2 changes: 0 additions & 2 deletions prometheus/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ dependencies {
implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "${versions.jackson_databind}"
implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: "${versions.jackson}"
implementation group: 'org.json', name: 'json', version: '20230227'
// https://mvnrepository.com/artifact/commons-validator/commons-validator
implementation group: 'commons-validator', name: 'commons-validator', version: '1.7'

testImplementation('org.junit.jupiter:junit-jupiter:5.6.2')
testImplementation group: 'org.hamcrest', name: 'hamcrest-library', version: '2.1'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ private JSONObject readResponse(Response response) throws IOException {
}
} else {
throw new PrometheusClientException(
String.format(
"Request to Prometheus is Unsuccessful with : %s",
Objects.requireNonNull(response.body(), "Response body can't be null").string()));
String.format("Request to Prometheus is Unsuccessful with code : %s", response.code()));
}
}
}
Loading
Loading