Skip to content

Commit 6488ce4

Browse files
heemin32heemin32
committed
Changes on IP2Geo processors
1. Block redirect request 2. Move manifest validation check inside transport action Signed-off-by: Heemin Kim <heemin@amazon.com>
1 parent d34fd1d commit 6488ce4

17 files changed

+330
-154
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
Compatible with OpenSearch and OpenSearch Dashboards version 3.2.0
44

5+
### Bug Fixes
6+
* Block redirect in IP2Geo and move validation to transport action ([#782](https://github.com/opensearch-project/geospatial/pull/782))
7+
58
### Maintenance
69
* Upgrade gradle to 8.14.3 and run CI checks with JDK24 ([#776](https://github.com/opensearch-project/geospatial/pull/776))
710

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

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import java.net.URISyntaxException;
1111
import java.net.URL;
1212
import java.util.List;
13-
import java.util.Locale;
1413

1514
import org.opensearch.action.ActionRequest;
1615
import org.opensearch.action.ActionRequestValidationException;
@@ -19,7 +18,6 @@
1918
import org.opensearch.core.common.io.stream.StreamInput;
2019
import org.opensearch.core.common.io.stream.StreamOutput;
2120
import org.opensearch.core.xcontent.ObjectParser;
22-
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
2321
import org.opensearch.geospatial.ip2geo.common.ParameterValidator;
2422

2523
import lombok.Getter;
@@ -106,60 +104,19 @@ public ActionRequestValidationException validate() {
106104
/**
107105
* Conduct following validation on endpoint
108106
* 1. endpoint format complies with RFC-2396
109-
* 2. validate manifest file from the endpoint
110107
*
111108
* @param errors the errors to add error messages
112109
*/
113110
private void validateEndpoint(final ActionRequestValidationException errors) {
114111
try {
115112
URL url = new URL(endpoint);
116113
url.toURI(); // Validate URL complies with RFC-2396
117-
validateManifestFile(url, errors);
118114
} catch (MalformedURLException | URISyntaxException e) {
119115
log.info("Invalid URL[{}] is provided", endpoint, e);
120116
errors.addValidationError("Invalid URL format is provided");
121117
}
122118
}
123119

124-
/**
125-
* Conduct following validation on url
126-
* 1. can read manifest file from the endpoint
127-
* 2. the url in the manifest file complies with RFC-2396
128-
* 3. updateInterval is less than validForInDays value in the manifest file
129-
*
130-
* @param url the url to validate
131-
* @param errors the errors to add error messages
132-
*/
133-
private void validateManifestFile(final URL url, final ActionRequestValidationException errors) {
134-
DatasourceManifest manifest;
135-
try {
136-
manifest = DatasourceManifest.Builder.build(url);
137-
} catch (Exception e) {
138-
log.info("Error occurred while reading a file from {}", url, e);
139-
errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", url, e.getMessage()));
140-
return;
141-
}
142-
143-
try {
144-
new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
145-
} catch (MalformedURLException | URISyntaxException e) {
146-
log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
147-
errors.addValidationError("Invalid URL format is provided for url field in the manifest file");
148-
return;
149-
}
150-
151-
if (manifest.getValidForInDays() != null && updateInterval.days() >= manifest.getValidForInDays()) {
152-
errors.addValidationError(
153-
String.format(
154-
Locale.ROOT,
155-
"updateInterval %d should be smaller than %d",
156-
updateInterval.days(),
157-
manifest.getValidForInDays()
158-
)
159-
);
160-
}
161-
}
162-
163120
/**
164121
* Validate updateInterval is equal or larger than 1
165122
*

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77

88
import static org.opensearch.geospatial.ip2geo.common.Ip2GeoLockService.LOCK_DURATION_IN_SECONDS;
99

10+
import java.net.MalformedURLException;
11+
import java.net.URISyntaxException;
12+
import java.net.URL;
1013
import java.time.Instant;
14+
import java.util.Locale;
1115
import java.util.concurrent.atomic.AtomicReference;
1216

1317
import org.opensearch.ResourceAlreadyExistsException;
@@ -20,6 +24,7 @@
2024
import org.opensearch.core.action.ActionListener;
2125
import org.opensearch.geospatial.annotation.VisibleForTesting;
2226
import org.opensearch.geospatial.exceptions.ConcurrentModificationException;
27+
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
2328
import org.opensearch.geospatial.ip2geo.common.DatasourceState;
2429
import org.opensearch.geospatial.ip2geo.common.Ip2GeoLockService;
2530
import org.opensearch.geospatial.ip2geo.dao.DatasourceDao;
@@ -70,6 +75,7 @@ public PutDatasourceTransportAction(
7075

7176
@Override
7277
protected void doExecute(final Task task, final PutDatasourceRequest request, final ActionListener<AcknowledgedResponse> listener) {
78+
validateManifestFile(request);
7379
lockService.acquireLock(request.getName(), LOCK_DURATION_IN_SECONDS, ActionListener.wrap(lock -> {
7480
if (lock == null) {
7581
listener.onFailure(
@@ -170,4 +176,43 @@ private void markDatasourceAsCreateFailed(final Datasource datasource) {
170176
log.error("Failed to mark datasource state as CREATE_FAILED for {}", datasource.getName(), e);
171177
}
172178
}
179+
180+
/**
181+
* Conduct the following validation on request
182+
* 1. can read the manifest file from the endpoint
183+
* 2. the url in the manifest file complies with RFC-2396
184+
* 3. updateInterval is less than validForInDays value in the manifest file
185+
*
186+
* @param request the request to validate
187+
*/
188+
private void validateManifestFile(final PutDatasourceRequest request) {
189+
DatasourceManifest manifest;
190+
try {
191+
URL url = new URL(request.getEndpoint());
192+
manifest = DatasourceManifest.Builder.build(url);
193+
} catch (Exception e) {
194+
log.info("Error occurred while reading a file from {}", request.getEndpoint(), e);
195+
throw new IllegalArgumentException(
196+
String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", request.getEndpoint(), e.getMessage())
197+
);
198+
}
199+
200+
try {
201+
new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
202+
} catch (MalformedURLException | URISyntaxException e) {
203+
log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
204+
throw new IllegalArgumentException("Invalid URL format is provided for url field in the manifest file");
205+
}
206+
207+
if (manifest.getValidForInDays() != null && request.getUpdateInterval().days() >= manifest.getValidForInDays()) {
208+
throw new IllegalArgumentException(
209+
String.format(
210+
Locale.ROOT,
211+
"updateInterval %d should be smaller than %d",
212+
request.getUpdateInterval().days(),
213+
manifest.getValidForInDays()
214+
)
215+
);
216+
}
217+
}
173218
}

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

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import java.net.MalformedURLException;
1010
import java.net.URISyntaxException;
1111
import java.net.URL;
12-
import java.util.Locale;
1312

1413
import org.opensearch.action.ActionRequest;
1514
import org.opensearch.action.ActionRequestValidationException;
@@ -18,7 +17,6 @@
1817
import org.opensearch.core.common.io.stream.StreamInput;
1918
import org.opensearch.core.common.io.stream.StreamOutput;
2019
import org.opensearch.core.xcontent.ObjectParser;
21-
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
2220
import org.opensearch.geospatial.ip2geo.common.ParameterValidator;
2321

2422
import lombok.EqualsAndHashCode;
@@ -124,39 +122,12 @@ private void validateEndpoint(final ActionRequestValidationException errors) {
124122
try {
125123
URL url = new URL(endpoint);
126124
url.toURI(); // Validate URL complies with RFC-2396
127-
validateManifestFile(url, errors);
128125
} catch (MalformedURLException | URISyntaxException e) {
129126
log.info("Invalid URL[{}] is provided", endpoint, e);
130127
errors.addValidationError("Invalid URL format is provided");
131128
}
132129
}
133130

134-
/**
135-
* Conduct following validation on url
136-
* 1. can read manifest file from the endpoint
137-
* 2. the url in the manifest file complies with RFC-2396
138-
*
139-
* @param url the url to validate
140-
* @param errors the errors to add error messages
141-
*/
142-
private void validateManifestFile(final URL url, final ActionRequestValidationException errors) {
143-
DatasourceManifest manifest;
144-
try {
145-
manifest = DatasourceManifest.Builder.build(url);
146-
} catch (Exception e) {
147-
log.info("Error occurred while reading a file from {}", url, e);
148-
errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", url, e.getMessage()));
149-
return;
150-
}
151-
152-
try {
153-
new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
154-
} catch (MalformedURLException | URISyntaxException e) {
155-
log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
156-
errors.addValidationError("Invalid URL format is provided for url field in the manifest file");
157-
}
158-
}
159-
160131
/**
161132
* Validate updateInterval is equal or larger than 1
162133
*

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package org.opensearch.geospatial.ip2geo.action;
77

88
import java.io.IOException;
9+
import java.net.MalformedURLException;
10+
import java.net.URISyntaxException;
911
import java.net.URL;
1012
import java.security.InvalidParameterException;
1113
import java.time.Instant;
@@ -80,6 +82,7 @@ public UpdateDatasourceTransportAction(
8082
*/
8183
@Override
8284
protected void doExecute(final Task task, final UpdateDatasourceRequest request, final ActionListener<AcknowledgedResponse> listener) {
85+
validateManifestFile(request);
8386
lockService.acquireLock(request.getName(), LOCK_DURATION_IN_SECONDS, ActionListener.wrap(lock -> {
8487
if (lock == null) {
8588
listener.onFailure(
@@ -219,4 +222,35 @@ private boolean isEndpointChanged(final UpdateDatasourceRequest request, final D
219222
private boolean isUpdateIntervalChanged(final UpdateDatasourceRequest request) {
220223
return request.getUpdateInterval() != null;
221224
}
225+
226+
/**
227+
* Conduct the following validation on url
228+
* 1. can read the manifest file from the endpoint
229+
* 2. the url in the manifest file complies with RFC-2396
230+
*
231+
* @param request the request to validate
232+
*/
233+
private void validateManifestFile(final UpdateDatasourceRequest request) {
234+
if (request.getEndpoint() == null) {
235+
return;
236+
}
237+
238+
DatasourceManifest manifest;
239+
try {
240+
URL url = new URL(request.getEndpoint());
241+
manifest = DatasourceManifest.Builder.build(url);
242+
} catch (Exception e) {
243+
log.info("Error occurred while reading a file from {}", request.getEndpoint(), e);
244+
throw new IllegalArgumentException(
245+
String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", request.getEndpoint(), e.getMessage())
246+
);
247+
}
248+
249+
try {
250+
new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
251+
} catch (MalformedURLException | URISyntaxException e) {
252+
log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
253+
throw new IllegalArgumentException("Invalid URL format is provided for url field in the manifest file");
254+
}
255+
}
222256
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.io.BufferedReader;
99
import java.io.IOException;
1010
import java.io.InputStreamReader;
11+
import java.net.HttpURLConnection;
1112
import java.net.URL;
1213
import java.net.URLConnection;
1314
import java.nio.CharBuffer;
@@ -129,6 +130,9 @@ public static DatasourceManifest build(final URL url) {
129130
@SuppressForbidden(reason = "Need to connect to http endpoint to read manifest file")
130131
protected static DatasourceManifest internalBuild(final URLConnection connection) throws IOException {
131132
connection.addRequestProperty(Constants.USER_AGENT_KEY, Constants.USER_AGENT_VALUE);
133+
if (connection instanceof HttpURLConnection) {
134+
HttpRedirectValidator.validateNoRedirects((HttpURLConnection) connection);
135+
}
132136
InputStreamReader inputStreamReader = new InputStreamReader(connection.getInputStream());
133137
try (BufferedReader reader = new BufferedReader(inputStreamReader)) {
134138
CharBuffer charBuffer = CharBuffer.allocate(MANIFEST_FILE_MAX_BYTES);

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.geospatial.ip2geo.common;
7+
8+
import java.io.IOException;
9+
import java.net.HttpURLConnection;
10+
import java.util.Locale;
11+
12+
import lombok.extern.log4j.Log4j2;
13+
14+
/**
15+
* Utility class for validating HTTP connections no redirects
16+
*/
17+
@Log4j2
18+
public class HttpRedirectValidator {
19+
private static final int HTTP_REDIRECT_STATUS_MIN = 300;
20+
private static final int HTTP_REDIRECT_STATUS_MAX = 400;
21+
private static final String LOCATION_HEADER = "Location";
22+
23+
// Private constructor to prevent instantiation
24+
private HttpRedirectValidator() {}
25+
26+
/**
27+
* Validates that an HTTP connection does not attempt to redirect
28+
*
29+
* @param httpConnection the HTTP connection to validate
30+
* @throws IOException if an I/O error occurs
31+
* @throws IllegalArgumentException if a redirect attempt is detected
32+
*/
33+
public static void validateNoRedirects(final HttpURLConnection httpConnection) throws IOException {
34+
httpConnection.setInstanceFollowRedirects(false);
35+
36+
final int responseCode = httpConnection.getResponseCode();
37+
if (responseCode >= HTTP_REDIRECT_STATUS_MIN && responseCode < HTTP_REDIRECT_STATUS_MAX) {
38+
final String redirectLocation = httpConnection.getHeaderField(LOCATION_HEADER);
39+
throw new IllegalArgumentException(
40+
String.format(
41+
Locale.ROOT,
42+
"HTTP redirects are not allowed. URL [%s] attempted to redirect to [%s] with status code [%d]",
43+
httpConnection.getURL().toString(),
44+
redirectLocation != null ? redirectLocation : "unknown",
45+
responseCode
46+
)
47+
);
48+
}
49+
}
50+
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.io.IOException;
1212
import java.io.InputStream;
1313
import java.io.InputStreamReader;
14+
import java.net.HttpURLConnection;
1415
import java.net.URL;
1516
import java.net.URLConnection;
1617
import java.nio.charset.StandardCharsets;
@@ -53,6 +54,7 @@
5354
import org.opensearch.geospatial.annotation.VisibleForTesting;
5455
import org.opensearch.geospatial.constants.IndexSetting;
5556
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
57+
import org.opensearch.geospatial.ip2geo.common.HttpRedirectValidator;
5658
import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
5759
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
5860
import org.opensearch.geospatial.shared.Constants;
@@ -169,7 +171,8 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
169171
return AccessController.doPrivileged(() -> {
170172
try {
171173
URL zipUrl = urlDenyListChecker.toUrlIfNotInDenyList(manifest.getUrl());
172-
return internalGetDatabaseReader(manifest, zipUrl.openConnection());
174+
URLConnection connection = zipUrl.openConnection();
175+
return internalGetDatabaseReader(manifest, connection);
173176
} catch (IOException e) {
174177
throw new OpenSearchException("failed to read geoip data from {}", manifest.getUrl(), e);
175178
}
@@ -180,6 +183,9 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
180183
@SuppressForbidden(reason = "Need to connect to http endpoint to read GeoIP database file")
181184
protected CSVParser internalGetDatabaseReader(final DatasourceManifest manifest, final URLConnection connection) throws IOException {
182185
connection.addRequestProperty(Constants.USER_AGENT_KEY, Constants.USER_AGENT_VALUE);
186+
if (connection instanceof HttpURLConnection) {
187+
HttpRedirectValidator.validateNoRedirects((HttpURLConnection) connection);
188+
}
183189
ZipInputStream zipIn = new ZipInputStream(connection.getInputStream());
184190
ZipEntry zipEntry = zipIn.getNextEntry();
185191
while (zipEntry != null) {

0 commit comments

Comments
 (0)