Block redirect in IP2Geo and move validation to transport action

release-notes/opensearch-geospatial.release-notes-3.2.0.0.md

@@ -2,6 +2,9 @@
 
 Compatible with OpenSearch and OpenSearch Dashboards version 3.2.0
 
+### Bug Fixes
+* Block redirect in IP2Geo and move validation to transport action ([#782](https://github.com/opensearch-project/geospatial/pull/782))
+
 ### Maintenance
 * Upgrade gradle to 8.14.3 and run CI checks with JDK24 ([#776](https://github.com/opensearch-project/geospatial/pull/776))
 

src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java

@@ -10,7 +10,6 @@
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.List;
-import java.util.Locale;
 
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.ActionRequestValidationException;
@@ -19,7 +18,6 @@
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.xcontent.ObjectParser;
-import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
 import org.opensearch.geospatial.ip2geo.common.ParameterValidator;
 
 import lombok.Getter;
@@ -106,60 +104,19 @@ public ActionRequestValidationException validate() {
     /**
      * Conduct following validation on endpoint
      * 1. endpoint format complies with RFC-2396
-     * 2. validate manifest file from the endpoint
      *
      * @param errors the errors to add error messages
      */
     private void validateEndpoint(final ActionRequestValidationException errors) {
         try {
             URL url = new URL(endpoint);
             url.toURI(); // Validate URL complies with RFC-2396
-            validateManifestFile(url, errors);
         } catch (MalformedURLException | URISyntaxException e) {
             log.info("Invalid URL[{}] is provided", endpoint, e);
             errors.addValidationError("Invalid URL format is provided");
         }
     }
 
-    /**
-     * Conduct following validation on url
-     * 1. can read manifest file from the endpoint
-     * 2. the url in the manifest file complies with RFC-2396
-     * 3. updateInterval is less than validForInDays value in the manifest file
-     *
-     * @param url the url to validate
-     * @param errors the errors to add error messages
-     */
-    private void validateManifestFile(final URL url, final ActionRequestValidationException errors) {
-        DatasourceManifest manifest;
-        try {
-            manifest = DatasourceManifest.Builder.build(url);
-        } catch (Exception e) {
-            log.info("Error occurred while reading a file from {}", url, e);
-            errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", url, e.getMessage()));
-            return;
-        }
-
-        try {
-            new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
-        } catch (MalformedURLException | URISyntaxException e) {
-            log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
-            errors.addValidationError("Invalid URL format is provided for url field in the manifest file");
-            return;
-        }
-
-        if (manifest.getValidForInDays() != null && updateInterval.days() >= manifest.getValidForInDays()) {
-            errors.addValidationError(
-                String.format(
-                    Locale.ROOT,
-                    "updateInterval %d should be smaller than %d",
-                    updateInterval.days(),
-                    manifest.getValidForInDays()
-                )
-            );
-        }
-    }
-
     /**
      * Validate updateInterval is equal or larger than 1
      *

src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceTransportAction.java

@@ -7,7 +7,11 @@
 
 import static org.opensearch.geospatial.ip2geo.common.Ip2GeoLockService.LOCK_DURATION_IN_SECONDS;
 
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
+import java.net.URL;
 import java.time.Instant;
+import java.util.Locale;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.opensearch.ResourceAlreadyExistsException;
@@ -20,6 +24,7 @@
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.geospatial.annotation.VisibleForTesting;
 import org.opensearch.geospatial.exceptions.ConcurrentModificationException;
+import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
 import org.opensearch.geospatial.ip2geo.common.DatasourceState;
 import org.opensearch.geospatial.ip2geo.common.Ip2GeoLockService;
 import org.opensearch.geospatial.ip2geo.dao.DatasourceDao;
@@ -70,6 +75,7 @@ public PutDatasourceTransportAction(
 
     @Override
     protected void doExecute(final Task task, final PutDatasourceRequest request, final ActionListener<AcknowledgedResponse> listener) {
+        validateManifestFile(request);
         lockService.acquireLock(request.getName(), LOCK_DURATION_IN_SECONDS, ActionListener.wrap(lock -> {
             if (lock == null) {
                 listener.onFailure(
@@ -170,4 +176,43 @@ private void markDatasourceAsCreateFailed(final Datasource datasource) {
             log.error("Failed to mark datasource state as CREATE_FAILED for {}", datasource.getName(), e);
         }
     }
+
+    /**
+     * Conduct the following validation on request
+     * 1. can read the manifest file from the endpoint
+     * 2. the url in the manifest file complies with RFC-2396
+     * 3. updateInterval is less than validForInDays value in the manifest file
+     *
+     * @param request the request to validate
+     */
+    private void validateManifestFile(final PutDatasourceRequest request) {
+        DatasourceManifest manifest;
+        try {
+            URL url = new URL(request.getEndpoint());
+            manifest = DatasourceManifest.Builder.build(url);
+        } catch (Exception e) {
+            log.info("Error occurred while reading a file from {}", request.getEndpoint(), e);
+            throw new IllegalArgumentException(
+                String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", request.getEndpoint(), e.getMessage())
+            );
+        }
+
+        try {
+            new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
+        } catch (MalformedURLException | URISyntaxException e) {
+            log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
+            throw new IllegalArgumentException("Invalid URL format is provided for url field in the manifest file");
+        }
+
+        if (manifest.getValidForInDays() != null && request.getUpdateInterval().days() >= manifest.getValidForInDays()) {
+            throw new IllegalArgumentException(
+                String.format(
+                    Locale.ROOT,
+                    "updateInterval %d should be smaller than %d",
+                    request.getUpdateInterval().days(),
+                    manifest.getValidForInDays()
+                )
+            );
+        }
+    }
 }

src/main/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequest.java

@@ -9,7 +9,6 @@
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.util.Locale;
 
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.ActionRequestValidationException;
@@ -18,7 +17,6 @@
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.xcontent.ObjectParser;
-import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
 import org.opensearch.geospatial.ip2geo.common.ParameterValidator;
 
 import lombok.EqualsAndHashCode;
@@ -124,39 +122,12 @@ private void validateEndpoint(final ActionRequestValidationException errors) {
         try {
             URL url = new URL(endpoint);
             url.toURI(); // Validate URL complies with RFC-2396
-            validateManifestFile(url, errors);
         } catch (MalformedURLException | URISyntaxException e) {
             log.info("Invalid URL[{}] is provided", endpoint, e);
             errors.addValidationError("Invalid URL format is provided");
         }
     }
 
-    /**
-     * Conduct following validation on url
-     * 1. can read manifest file from the endpoint
-     * 2. the url in the manifest file complies with RFC-2396
-     *
-     * @param url the url to validate
-     * @param errors the errors to add error messages
-     */
-    private void validateManifestFile(final URL url, final ActionRequestValidationException errors) {
-        DatasourceManifest manifest;
-        try {
-            manifest = DatasourceManifest.Builder.build(url);
-        } catch (Exception e) {
-            log.info("Error occurred while reading a file from {}", url, e);
-            errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", url, e.getMessage()));
-            return;
-        }
-
-        try {
-            new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
-        } catch (MalformedURLException | URISyntaxException e) {
-            log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
-            errors.addValidationError("Invalid URL format is provided for url field in the manifest file");
-        }
-    }
-
     /**
      * Validate updateInterval is equal or larger than 1
      *

src/main/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceTransportAction.java

@@ -6,6 +6,8 @@
 package org.opensearch.geospatial.ip2geo.action;
 
 import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.security.InvalidParameterException;
 import java.time.Instant;
@@ -80,6 +82,7 @@ public UpdateDatasourceTransportAction(
      */
     @Override
     protected void doExecute(final Task task, final UpdateDatasourceRequest request, final ActionListener<AcknowledgedResponse> listener) {
+        validateManifestFile(request);
         lockService.acquireLock(request.getName(), LOCK_DURATION_IN_SECONDS, ActionListener.wrap(lock -> {
             if (lock == null) {
                 listener.onFailure(
@@ -219,4 +222,35 @@ private boolean isEndpointChanged(final UpdateDatasourceRequest request, final D
     private boolean isUpdateIntervalChanged(final UpdateDatasourceRequest request) {
         return request.getUpdateInterval() != null;
     }
+
+    /**
+     * Conduct the following validation on url
+     * 1. can read the manifest file from the endpoint
+     * 2. the url in the manifest file complies with RFC-2396
+     *
+     * @param request the request to validate
+     */
+    private void validateManifestFile(final UpdateDatasourceRequest request) {
+        if (request.getEndpoint() == null) {
+            return;
+        }
+
+        DatasourceManifest manifest;
+        try {
+            URL url = new URL(request.getEndpoint());
+            manifest = DatasourceManifest.Builder.build(url);
+        } catch (Exception e) {
+            log.info("Error occurred while reading a file from {}", request.getEndpoint(), e);
+            throw new IllegalArgumentException(
+                String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", request.getEndpoint(), e.getMessage())
+            );
+        }
+
+        try {
+            new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
+        } catch (MalformedURLException | URISyntaxException e) {
+            log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
+            throw new IllegalArgumentException("Invalid URL format is provided for url field in the manifest file");
+        }
+    }
 }

src/main/java/org/opensearch/geospatial/ip2geo/common/DatasourceManifest.java

@@ -8,6 +8,7 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.CharBuffer;
@@ -129,6 +130,9 @@ public static DatasourceManifest build(final URL url) {
         @SuppressForbidden(reason = "Need to connect to http endpoint to read manifest file")
         protected static DatasourceManifest internalBuild(final URLConnection connection) throws IOException {
             connection.addRequestProperty(Constants.USER_AGENT_KEY, Constants.USER_AGENT_VALUE);
+            if (connection instanceof HttpURLConnection) {
+                HttpRedirectValidator.validateNoRedirects((HttpURLConnection) connection);
+            }
             InputStreamReader inputStreamReader = new InputStreamReader(connection.getInputStream());
             try (BufferedReader reader = new BufferedReader(inputStreamReader)) {
                 CharBuffer charBuffer = CharBuffer.allocate(MANIFEST_FILE_MAX_BYTES);

src/main/java/org/opensearch/geospatial/ip2geo/common/HttpRedirectValidator.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.geospatial.ip2geo.common;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.util.Locale;
+
+import lombok.extern.log4j.Log4j2;
+
+/**
+ * Utility class for validating HTTP connections no redirects
+ */
+@Log4j2
+public class HttpRedirectValidator {
+    private static final int HTTP_REDIRECT_STATUS_MIN = 300;
+    private static final int HTTP_REDIRECT_STATUS_MAX = 400;
+    private static final String LOCATION_HEADER = "Location";
+
+    // Private constructor to prevent instantiation
+    private HttpRedirectValidator() {}
+
+    /**
+     * Validates that an HTTP connection does not attempt to redirect
+     *
+     * @param httpConnection the HTTP connection to validate
+     * @throws IOException if an I/O error occurs
+     * @throws IllegalArgumentException if a redirect attempt is detected
+     */
+    public static void validateNoRedirects(final HttpURLConnection httpConnection) throws IOException {
+        httpConnection.setInstanceFollowRedirects(false);
+
+        final int responseCode = httpConnection.getResponseCode();
+        if (responseCode >= HTTP_REDIRECT_STATUS_MIN && responseCode < HTTP_REDIRECT_STATUS_MAX) {
+            final String redirectLocation = httpConnection.getHeaderField(LOCATION_HEADER);
+            throw new IllegalArgumentException(
+                String.format(
+                    Locale.ROOT,
+                    "HTTP redirects are not allowed. URL [%s] attempted to redirect to [%s] with status code [%d]",
+                    httpConnection.getURL().toString(),
+                    redirectLocation != null ? redirectLocation : "unknown",
+                    responseCode
+                )
+            );
+        }
+    }
+}

src/main/java/org/opensearch/geospatial/ip2geo/dao/GeoIpDataDao.java

@@ -11,6 +11,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.charset.StandardCharsets;
@@ -53,6 +54,7 @@
 import org.opensearch.geospatial.annotation.VisibleForTesting;
 import org.opensearch.geospatial.constants.IndexSetting;
 import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
+import org.opensearch.geospatial.ip2geo.common.HttpRedirectValidator;
 import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
 import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
 import org.opensearch.geospatial.shared.Constants;
@@ -169,7 +171,8 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
         return AccessController.doPrivileged(() -> {
             try {
                 URL zipUrl = urlDenyListChecker.toUrlIfNotInDenyList(manifest.getUrl());
-                return internalGetDatabaseReader(manifest, zipUrl.openConnection());
+                URLConnection connection = zipUrl.openConnection();
+                return internalGetDatabaseReader(manifest, connection);
             } catch (IOException e) {
                 throw new OpenSearchException("failed to read geoip data from {}", manifest.getUrl(), e);
             }
@@ -180,6 +183,9 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
     @SuppressForbidden(reason = "Need to connect to http endpoint to read GeoIP database file")
     protected CSVParser internalGetDatabaseReader(final DatasourceManifest manifest, final URLConnection connection) throws IOException {
         connection.addRequestProperty(Constants.USER_AGENT_KEY, Constants.USER_AGENT_VALUE);
+        if (connection instanceof HttpURLConnection) {
+            HttpRedirectValidator.validateNoRedirects((HttpURLConnection) connection);
+        }
         ZipInputStream zipIn = new ZipInputStream(connection.getInputStream());
         ZipEntry zipEntry = zipIn.getNextEntry();
         while (zipEntry != null) {