diff --git a/common/build.gradle b/common/build.gradle index d27e213db1..5cf219fbae 100644 --- a/common/build.gradle +++ b/common/build.gradle @@ -41,6 +41,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' diff --git a/common/src/main/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptor.java b/common/src/main/java/org/opensearch/sql/common/interceptors/AwsSigningInterceptor.java similarity index 97% rename from common/src/main/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptor.java rename to common/src/main/java/org/opensearch/sql/common/interceptors/AwsSigningInterceptor.java index e2d33dca8b..16196544b5 100644 --- a/common/src/main/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptor.java +++ b/common/src/main/java/org/opensearch/sql/common/interceptors/AwsSigningInterceptor.java @@ -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; diff --git a/common/src/main/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptor.java b/common/src/main/java/org/opensearch/sql/common/interceptors/BasicAuthenticationInterceptor.java similarity index 93% rename from common/src/main/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptor.java rename to common/src/main/java/org/opensearch/sql/common/interceptors/BasicAuthenticationInterceptor.java index 2275482e30..15e9a0fc12 100644 --- a/common/src/main/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptor.java +++ b/common/src/main/java/org/opensearch/sql/common/interceptors/BasicAuthenticationInterceptor.java @@ -5,7 +5,7 @@ * */ -package org.opensearch.sql.common.authinterceptors; +package org.opensearch.sql.common.interceptors; import java.io.IOException; import lombok.NonNull; diff --git a/common/src/main/java/org/opensearch/sql/common/interceptors/URIValidatorInterceptor.java b/common/src/main/java/org/opensearch/sql/common/interceptors/URIValidatorInterceptor.java new file mode 100644 index 0000000000..68e7339beb --- /dev/null +++ b/common/src/main/java/org/opensearch/sql/common/interceptors/URIValidatorInterceptor.java @@ -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 denyHostList; + + public URIValidatorInterceptor(@NonNull List 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())); + } + } +} diff --git a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java index 1e5243f91f..be780e8d80 100644 --- a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java +++ b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java @@ -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"), diff --git a/common/src/main/java/org/opensearch/sql/common/utils/URIValidationUtils.java b/common/src/main/java/org/opensearch/sql/common/utils/URIValidationUtils.java new file mode 100644 index 0000000000..c7893fc053 --- /dev/null +++ b/common/src/main/java/org/opensearch/sql/common/utils/URIValidationUtils.java @@ -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 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; + } +} diff --git a/common/src/test/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptorTest.java b/common/src/test/java/org/opensearch/sql/common/interceptors/AwsSigningInterceptorTest.java similarity index 98% rename from common/src/test/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptorTest.java rename to common/src/test/java/org/opensearch/sql/common/interceptors/AwsSigningInterceptorTest.java index 435ac9dc93..6c5d1bac89 100644 --- a/common/src/test/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptorTest.java +++ b/common/src/test/java/org/opensearch/sql/common/interceptors/AwsSigningInterceptorTest.java @@ -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; diff --git a/common/src/test/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptorTest.java b/common/src/test/java/org/opensearch/sql/common/interceptors/BasicAuthenticationInterceptorTest.java similarity index 96% rename from common/src/test/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptorTest.java rename to common/src/test/java/org/opensearch/sql/common/interceptors/BasicAuthenticationInterceptorTest.java index d59928d2ef..af93060fab 100644 --- a/common/src/test/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptorTest.java +++ b/common/src/test/java/org/opensearch/sql/common/interceptors/BasicAuthenticationInterceptorTest.java @@ -5,7 +5,7 @@ * */ -package org.opensearch.sql.common.authinterceptors; +package org.opensearch.sql.common.interceptors; import java.util.Collections; import lombok.SneakyThrows; diff --git a/datasources/build.gradle b/datasources/build.gradle index 5c0d582c7e..cdd2ee813b 100644 --- a/datasources/build.gradle +++ b/datasources/build.gradle @@ -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') diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/DatasourceValidationUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/DatasourceValidationUtils.java new file mode 100644 index 0000000000..ba7458d0b4 --- /dev/null +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/DatasourceValidationUtils.java @@ -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 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 config, Set fields) { + Set missingFields = new HashSet<>(); + Set 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)); + } + } +} diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/utils/DatasourceValidationUtilsTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/utils/DatasourceValidationUtilsTest.java new file mode 100644 index 0000000000..836c61f647 --- /dev/null +++ b/datasources/src/test/java/org/opensearch/sql/datasources/utils/DatasourceValidationUtilsTest.java @@ -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 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 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 config = new HashMap<>(); + config.put("s3.uri", "test"); + Assertions.assertDoesNotThrow( + () -> + DatasourceValidationUtils.validateLengthAndRequiredFields( + config, Collections.emptySet())); + } +} diff --git a/docs/user/ppl/admin/datasources.rst b/docs/user/ppl/admin/datasources.rst index 0f026f72cf..37d2fdfcee 100644 --- a/docs/user/ppl/admin/datasources.rst +++ b/docs/user/ppl/admin/datasources.rst @@ -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 diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java index 133903dabe..48ceacaf10 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java @@ -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; @@ -119,10 +121,11 @@ public class OpenSearchSettings extends Settings { Setting.Property.Final, Setting.Property.Filtered); - public static final Setting DATASOURCE_URI_ALLOW_HOSTS = - Setting.simpleString( - Key.DATASOURCES_URI_ALLOWHOSTS.getKeyValue(), - ".*", + public static final Setting> DATASOURCE_URI_HOSTS_DENY_LIST = + Setting.listSetting( + Key.DATASOURCES_URI_HOSTS_DENY_LIST.getKeyValue(), + Collections.emptyList(), + Function.identity(), Setting.Property.NodeScope, Setting.Property.Dynamic); @@ -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(); @@ -253,7 +256,7 @@ public static List> 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(); } diff --git a/prometheus/build.gradle b/prometheus/build.gradle index e98dfd83e4..f8c10c7f6b 100644 --- a/prometheus/build.gradle +++ b/prometheus/build.gradle @@ -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' diff --git a/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java b/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java index 46525fd58c..48154964eb 100644 --- a/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java +++ b/prometheus/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java @@ -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())); } } } diff --git a/prometheus/src/main/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactory.java b/prometheus/src/main/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactory.java index 76d89aad47..dd74f9d550 100644 --- a/prometheus/src/main/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactory.java +++ b/prometheus/src/main/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactory.java @@ -11,24 +11,23 @@ import com.amazonaws.auth.BasicAWSCredentials; import java.net.URI; import java.net.URISyntaxException; +import java.net.UnknownHostException; import java.security.AccessController; import java.security.PrivilegedAction; -import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import lombok.RequiredArgsConstructor; import okhttp3.OkHttpClient; -import org.apache.commons.validator.routines.DomainValidator; -import org.opensearch.sql.common.authinterceptors.AwsSigningInterceptor; -import org.opensearch.sql.common.authinterceptors.BasicAuthenticationInterceptor; +import org.opensearch.sql.common.interceptors.AwsSigningInterceptor; +import org.opensearch.sql.common.interceptors.BasicAuthenticationInterceptor; +import org.opensearch.sql.common.interceptors.URIValidatorInterceptor; import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.datasource.model.DataSource; import org.opensearch.sql.datasource.model.DataSourceMetadata; import org.opensearch.sql.datasource.model.DataSourceType; import org.opensearch.sql.datasources.auth.AuthenticationType; +import org.opensearch.sql.datasources.utils.DatasourceValidationUtils; import org.opensearch.sql.prometheus.client.PrometheusClient; import org.opensearch.sql.prometheus.client.PrometheusClientImpl; import org.opensearch.sql.storage.DataSourceFactory; @@ -61,20 +60,24 @@ public DataSource createDataSource(DataSourceMetadata metadata) { // Need to refactor to a separate Validator class. private void validateDataSourceConfigProperties(Map dataSourceMetadataConfig) - throws URISyntaxException { + throws URISyntaxException, UnknownHostException { if (dataSourceMetadataConfig.get(AUTH_TYPE) != null) { AuthenticationType authenticationType = AuthenticationType.get(dataSourceMetadataConfig.get(AUTH_TYPE)); if (AuthenticationType.BASICAUTH.equals(authenticationType)) { - validateMissingFields(dataSourceMetadataConfig, Set.of(URI, USERNAME, PASSWORD)); + DatasourceValidationUtils.validateLengthAndRequiredFields( + dataSourceMetadataConfig, Set.of(URI, USERNAME, PASSWORD)); } else if (AuthenticationType.AWSSIGV4AUTH.equals(authenticationType)) { - validateMissingFields( + DatasourceValidationUtils.validateLengthAndRequiredFields( dataSourceMetadataConfig, Set.of(URI, ACCESS_KEY, SECRET_KEY, REGION)); } } else { - validateMissingFields(dataSourceMetadataConfig, Set.of(URI)); + DatasourceValidationUtils.validateLengthAndRequiredFields( + dataSourceMetadataConfig, Set.of(URI)); } - validateURI(dataSourceMetadataConfig); + DatasourceValidationUtils.validateHost( + dataSourceMetadataConfig.get(URI), + settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)); } StorageEngine getStorageEngine(Map requiredConfig) { @@ -87,7 +90,7 @@ StorageEngine getStorageEngine(Map requiredConfig) { validateDataSourceConfigProperties(requiredConfig); return new PrometheusClientImpl( getHttpClient(requiredConfig), new URI(requiredConfig.get(URI))); - } catch (URISyntaxException e) { + } catch (URISyntaxException | UnknownHostException e) { throw new IllegalArgumentException( String.format("Invalid URI in prometheus properties: %s", e.getMessage())); } @@ -100,6 +103,9 @@ private OkHttpClient getHttpClient(Map config) { okHttpClient.callTimeout(1, TimeUnit.MINUTES); okHttpClient.connectTimeout(30, TimeUnit.SECONDS); okHttpClient.followRedirects(false); + okHttpClient.addInterceptor( + new URIValidatorInterceptor( + settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST))); if (config.get(AUTH_TYPE) != null) { AuthenticationType authenticationType = AuthenticationType.get(config.get(AUTH_TYPE)); if (AuthenticationType.BASICAUTH.equals(authenticationType)) { @@ -121,51 +127,4 @@ private OkHttpClient getHttpClient(Map config) { } return okHttpClient.build(); } - - private void validateMissingFields(Map config, Set fields) { - Set missingFields = new HashSet<>(); - Set 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 void validateURI(Map config) throws URISyntaxException { - URI uri = new URI(config.get(URI)); - 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", config.get(URI))); - } else { - Pattern allowHostsPattern = - Pattern.compile(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)); - Matcher matcher = allowHostsPattern.matcher(host); - if (!matcher.matches()) { - throw new IllegalArgumentException( - String.format( - "Disallowed hostname in the uri. Validate with %s config", - Settings.Key.DATASOURCES_URI_ALLOWHOSTS.getKeyValue())); - } - } - } } diff --git a/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java b/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java index 81cd2e3860..d6ca25b206 100644 --- a/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java +++ b/prometheus/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java @@ -114,10 +114,9 @@ void testQueryRangeWithNon2xxError() { assertThrows( PrometheusClientException.class, () -> prometheusClient.queryRange(QUERY, STARTTIME, ENDTIME, STEP)); - assertTrue( - prometheusClientException - .getMessage() - .contains("Request to Prometheus is Unsuccessful with :")); + assertEquals( + "Request to Prometheus is Unsuccessful with code : 400", + prometheusClientException.getMessage()); RecordedRequest recordedRequest = mockWebServer.takeRequest(); verifyQueryRangeCall(recordedRequest); } diff --git a/prometheus/src/test/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactoryTest.java b/prometheus/src/test/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactoryTest.java index 49cdc42f4e..46658699ca 100644 --- a/prometheus/src/test/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactoryTest.java +++ b/prometheus/src/test/java/org/opensearch/sql/prometheus/storage/PrometheusStorageFactoryTest.java @@ -9,6 +9,7 @@ import static org.mockito.Mockito.when; +import java.util.Collections; import java.util.HashMap; import lombok.SneakyThrows; import org.apache.commons.lang3.RandomStringUtils; @@ -38,10 +39,11 @@ void testGetConnectorType() { @Test @SneakyThrows void testGetStorageEngineWithBasicAuth() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)).thenReturn(".*"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.emptyList()); PrometheusStorageFactory prometheusStorageFactory = new PrometheusStorageFactory(settings); HashMap properties = new HashMap<>(); - properties.put("prometheus.uri", "http://dummyprometheus.com:9090"); + properties.put("prometheus.uri", "http://localhost:9090"); properties.put("prometheus.auth.type", "basicauth"); properties.put("prometheus.auth.username", "admin"); properties.put("prometheus.auth.password", "admin"); @@ -52,10 +54,11 @@ void testGetStorageEngineWithBasicAuth() { @Test @SneakyThrows void testGetStorageEngineWithAWSSigV4Auth() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)).thenReturn(".*"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.emptyList()); PrometheusStorageFactory prometheusStorageFactory = new PrometheusStorageFactory(settings); HashMap properties = new HashMap<>(); - properties.put("prometheus.uri", "http://dummyprometheus.com:9090"); + properties.put("prometheus.uri", "http://localhost:9090"); properties.put("prometheus.auth.type", "awssigv4"); properties.put("prometheus.auth.region", "us-east-1"); properties.put("prometheus.auth.secret_key", "accessKey"); @@ -123,7 +126,8 @@ void testGetStorageEngineWithLongConfigProperties() { @Test @SneakyThrows void testGetStorageEngineWithWrongAuthType() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)).thenReturn(".*"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.emptyList()); PrometheusStorageFactory prometheusStorageFactory = new PrometheusStorageFactory(settings); HashMap properties = new HashMap<>(); properties.put("prometheus.uri", "https://test.com"); @@ -142,7 +146,8 @@ void testGetStorageEngineWithWrongAuthType() { @Test @SneakyThrows void testGetStorageEngineWithNONEAuthType() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)).thenReturn(".*"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.emptyList()); PrometheusStorageFactory prometheusStorageFactory = new PrometheusStorageFactory(settings); HashMap properties = new HashMap<>(); properties.put("prometheus.uri", "https://test.com"); @@ -168,9 +173,10 @@ void testGetStorageEngineWithInvalidURISyntax() { @Test void createDataSourceSuccess() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)).thenReturn(".*"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.emptyList()); HashMap properties = new HashMap<>(); - properties.put("prometheus.uri", "http://dummyprometheus.com:9090"); + properties.put("prometheus.uri", "http://localhost:9090"); properties.put("prometheus.auth.type", "basicauth"); properties.put("prometheus.auth.username", "admin"); properties.put("prometheus.auth.password", "admin"); @@ -186,7 +192,8 @@ void createDataSourceSuccess() { @Test void createDataSourceSuccessWithLocalhost() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)).thenReturn(".*"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.emptyList()); HashMap properties = new HashMap<>(); properties.put("prometheus.uri", "http://localhost:9090"); properties.put("prometheus.auth.type", "basicauth"); @@ -248,10 +255,10 @@ void createDataSourceWithInvalidIp() { @Test void createDataSourceWithHostnameNotMatchingWithAllowHostsConfig() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)) - .thenReturn("^dummy.*.com$"); + when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST)) + .thenReturn(Collections.singletonList("127.0.0.0/8")); HashMap properties = new HashMap<>(); - properties.put("prometheus.uri", "http://localhost.com:9090"); + properties.put("prometheus.uri", "http://localhost:9090"); properties.put("prometheus.auth.type", "basicauth"); properties.put("prometheus.auth.username", "admin"); properties.put("prometheus.auth.password", "admin"); @@ -265,29 +272,9 @@ void createDataSourceWithHostnameNotMatchingWithAllowHostsConfig() { RuntimeException exception = Assertions.assertThrows( RuntimeException.class, () -> prometheusStorageFactory.createDataSource(metadata)); - Assertions.assertTrue( - exception - .getMessage() - .contains( - "Disallowed hostname in the uri. " - + "Validate with plugins.query.datasources.uri.allowhosts config")); - } - - @Test - void createDataSourceSuccessWithHostnameRestrictions() { - when(settings.getSettingValue(Settings.Key.DATASOURCES_URI_ALLOWHOSTS)) - .thenReturn("^dummy.*.com$"); - HashMap properties = new HashMap<>(); - properties.put("prometheus.uri", "http://dummy.prometheus.com:9090"); - properties.put("prometheus.auth.type", "basicauth"); - properties.put("prometheus.auth.username", "admin"); - properties.put("prometheus.auth.password", "admin"); - - DataSourceMetadata metadata = new DataSourceMetadata(); - metadata.setName("prometheus"); - metadata.setConnector(DataSourceType.PROMETHEUS); - metadata.setProperties(properties); - DataSource dataSource = new PrometheusStorageFactory(settings).createDataSource(metadata); - Assertions.assertTrue(dataSource.getStorageEngine() instanceof PrometheusStorageEngine); + Assertions.assertEquals( + "Disallowed hostname in the uri. " + + "Validate with plugins.query.datasources.uri.hosts.denylist config", + exception.getMessage()); } }