Skip to content

Commit

Permalink
Changed allowlist config to denylist ip config for datasource uri hos…
Browse files Browse the repository at this point in the history
…ts (#2042) (#2059)

Signed-off-by: Vamsi Manohar <[email protected]>
(cherry picked from commit 9082a46)
  • Loading branch information
vmmusings authored Sep 7, 2023
1 parent 05ca844 commit 533e54d
Show file tree
Hide file tree
Showing 18 changed files with 295 additions and 126 deletions.
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
3 changes: 2 additions & 1 deletion datasources/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ dependencies {
implementation group: 'org.opensearch', name: 'opensearch-x-content', version: "${opensearch_version}"
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.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

0 comments on commit 533e54d

Please sign in to comment.