Changed allowlist config to denylist ip config for datasource uri hosts

Signed-off-by: Vamsi Manohar <[email protected]>
opensearch-project · Aug 31, 2023 · 6693567 · 6693567
1 parent 56bc7d5
commit 6693567
Show file tree

Hide file tree

Showing 14 changed files with 130 additions and 76 deletions.
diff --git 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.3.3"
 
     testImplementation group: 'junit', name: 'junit', version: '4.13.2'
     testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.9.1'

diff --git a/...thinterceptors/AwsSigningInterceptor.java → ...n/interceptors/AwsSigningInterceptor.java b/...thinterceptors/AwsSigningInterceptor.java → ...n/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/...ptors/BasicAuthenticationInterceptor.java → ...ptors/BasicAuthenticationInterceptor.java b/...ptors/BasicAuthenticationInterceptor.java → ...ptors/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
@@ -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()));
+    }
+  }
+}
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
@@ -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
@@ -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;
+  }
+}
diff --git a/...terceptors/AwsSigningInterceptorTest.java → ...terceptors/AwsSigningInterceptorTest.java b/...terceptors/AwsSigningInterceptorTest.java → ...terceptors/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/...s/BasicAuthenticationInterceptorTest.java → ...s/BasicAuthenticationInterceptorTest.java b/...s/BasicAuthenticationInterceptorTest.java → ...s/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/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
@@ -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<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);
 
@@ -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<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();
   }
 

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
@@ -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
@@ -11,20 +11,21 @@
 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.common.utils.URIValidationUtils;
 import org.opensearch.sql.datasource.model.DataSource;
 import org.opensearch.sql.datasource.model.DataSourceMetadata;
 import org.opensearch.sql.datasource.model.DataSourceType;
@@ -61,7 +62,7 @@ public DataSource createDataSource(DataSourceMetadata metadata) {
 
   // Need to refactor to a separate Validator class.
   private void validateDataSourceConfigProperties(Map<String, String> dataSourceMetadataConfig)
-      throws URISyntaxException {
+      throws URISyntaxException, UnknownHostException {
     if (dataSourceMetadataConfig.get(AUTH_TYPE) != null) {
       AuthenticationType authenticationType =
           AuthenticationType.get(dataSourceMetadataConfig.get(AUTH_TYPE));
@@ -87,7 +88,7 @@ StorageEngine getStorageEngine(Map<String, String> 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 +101,9 @@ private OkHttpClient getHttpClient(Map<String, String> 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)) {
@@ -148,7 +152,8 @@ private void validateMissingFields(Map<String, String> config, Set<String> field
     }
   }
 
-  private void validateURI(Map<String, String> config) throws URISyntaxException {
+  private void validateURI(Map<String, String> config)
+      throws URISyntaxException, UnknownHostException {
     URI uri = new URI(config.get(URI));
     String host = uri.getHost();
     if (host == null
@@ -157,14 +162,14 @@ private void validateURI(Map<String, String> config) throws URISyntaxException {
       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()) {
+      boolean isAllowedHost =
+          URIValidationUtils.validateURIHost(
+              host, settings.getSettingValue(Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST));
+      if (!isAllowedHost) {
         throw new IllegalArgumentException(
             String.format(
                 "Disallowed hostname in the uri. Validate with %s config",
-                Settings.Key.DATASOURCES_URI_ALLOWHOSTS.getKeyValue()));
+                Settings.Key.DATASOURCES_URI_HOSTS_DENY_LIST.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
@@ -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);
   }