Skip to content

Commit 9fd0e7e

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 0d9d4f5 commit 9fd0e7e

14 files changed

+323
-150
lines changed

‎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;
@@ -130,6 +131,9 @@ public static DatasourceManifest build(final URL url) {
130131
@SuppressForbidden(reason = "Need to connect to http endpoint to read manifest file")
131132
protected static DatasourceManifest internalBuild(final URLConnection connection) throws IOException {
132133
connection.addRequestProperty(Constants.USER_AGENT_KEY, Constants.USER_AGENT_VALUE);
134+
if (connection instanceof HttpURLConnection) {
135+
HttpRedirectValidator.validateNoRedirects((HttpURLConnection) connection);
136+
}
133137
InputStreamReader inputStreamReader = new InputStreamReader(connection.getInputStream());
134138
try (BufferedReader reader = new BufferedReader(inputStreamReader)) {
135139
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;
@@ -55,6 +56,7 @@
5556
import org.opensearch.geospatial.annotation.VisibleForTesting;
5657
import org.opensearch.geospatial.constants.IndexSetting;
5758
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;
59+
import org.opensearch.geospatial.ip2geo.common.HttpRedirectValidator;
5860
import org.opensearch.geospatial.ip2geo.common.Ip2GeoSettings;
5961
import org.opensearch.geospatial.ip2geo.common.URLDenyListChecker;
6062
import org.opensearch.geospatial.shared.Constants;
@@ -176,7 +178,8 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
176178
return AccessController.doPrivileged((PrivilegedAction<CSVParser>) () -> {
177179
try {
178180
URL zipUrl = urlDenyListChecker.toUrlIfNotInDenyList(manifest.getUrl());
179-
return internalGetDatabaseReader(manifest, zipUrl.openConnection());
181+
URLConnection connection = zipUrl.openConnection();
182+
return internalGetDatabaseReader(manifest, connection);
180183
} catch (IOException e) {
181184
throw new OpenSearchException("failed to read geoip data from {}", manifest.getUrl(), e);
182185
}
@@ -187,6 +190,9 @@ public CSVParser getDatabaseReader(final DatasourceManifest manifest) {
187190
@SuppressForbidden(reason = "Need to connect to http endpoint to read GeoIP database file")
188191
protected CSVParser internalGetDatabaseReader(final DatasourceManifest manifest, final URLConnection connection) throws IOException {
189192
connection.addRequestProperty(Constants.USER_AGENT_KEY, Constants.USER_AGENT_VALUE);
193+
if (connection instanceof HttpURLConnection) {
194+
HttpRedirectValidator.validateNoRedirects((HttpURLConnection) connection);
195+
}
190196
ZipInputStream zipIn = new ZipInputStream(connection.getInputStream());
191197
ZipEntry zipEntry = zipIn.getNextEntry();
192198
while (zipEntry != null) {

‎src/test/java/org/opensearch/geospatial/ip2geo/Ip2GeoTestCase.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,18 @@ protected String sampleManifestUrl() {
180180
return Paths.get(this.getClass().getClassLoader().getResource("ip2geo/manifest.json").toURI()).toUri().toURL().toExternalForm();
181181
}
182182

183+
@SneakyThrows
184+
@SuppressForbidden(reason = "unit test")
185+
protected String anotherSampleManifestUrl() {
186+
return Paths.get(this.getClass().getClassLoader().getResource("ip2geo/another_manifest.json").toURI())
187+
.toUri()
188+
.toURL()
189+
.toExternalForm();
190+
}
191+
192+
@SneakyThrows
183193
@SuppressForbidden(reason = "unit test")
184-
protected String sampleManifestUrlWithInvalidUrl() throws Exception {
194+
protected String sampleManifestUrlWithInvalidUrl() {
185195
return Paths.get(this.getClass().getClassLoader().getResource("ip2geo/manifest_invalid_url.json").toURI())
186196
.toUri()
187197
.toURL()
@@ -222,7 +232,7 @@ protected Datasource randomDatasource(final Instant updateStartTime) {
222232
datasource.setState(randomState());
223233
datasource.setCurrentIndex(datasource.newIndexName(UUID.randomUUID().toString()));
224234
datasource.setIndices(Arrays.asList(GeospatialTestHelper.randomLowerCaseString(), GeospatialTestHelper.randomLowerCaseString()));
225-
datasource.setEndpoint(String.format(Locale.ROOT, "https://%s.com/manifest.json", GeospatialTestHelper.randomLowerCaseString()));
235+
datasource.setEndpoint(sampleManifestUrl());
226236
datasource.getDatabase()
227237
.setFields(Arrays.asList(GeospatialTestHelper.randomLowerCaseString(), GeospatialTestHelper.randomLowerCaseString()));
228238
datasource.getDatabase().setProvider(GeospatialTestHelper.randomLowerCaseString());

0 commit comments

Comments
 (0)