diff --git a/docs/changelog/135414.yaml b/docs/changelog/135414.yaml new file mode 100644 index 0000000000000..432df429e72b6 --- /dev/null +++ b/docs/changelog/135414.yaml @@ -0,0 +1,5 @@ +pr: 135414 +summary: "Change reindex to use ::es-redacted:: filtering" +area: Audit +type: enhancement +issues: [] diff --git a/modules/reindex/src/main/java/org/elasticsearch/reindex/RestReindexAction.java b/modules/reindex/src/main/java/org/elasticsearch/reindex/RestReindexAction.java index 925d84c1d03eb..c7a1f925b97c7 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/reindex/RestReindexAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/reindex/RestReindexAction.java @@ -14,6 +14,7 @@ import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.reindex.ReindexAction; import org.elasticsearch.index.reindex.ReindexRequest; +import org.elasticsearch.rest.FilteredRestRequest; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequestFilter; import org.elasticsearch.rest.Scope; @@ -22,8 +23,10 @@ import java.io.IOException; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Collectors; import static org.elasticsearch.core.TimeValue.parseTimeValue; import static org.elasticsearch.rest.RestRequest.Method.POST; @@ -79,10 +82,43 @@ protected ReindexRequest buildRequest(RestRequest request) throws IOException { return internal; } - private static final Set FILTERED_FIELDS = Set.of("source.remote.host.password"); - + /** + * This method isn't used because we implement {@link #getFilteredRequest(RestRequest)} instead + */ @Override public Set getFilteredFields() { - return FILTERED_FIELDS; + assert false : "This method should never be called"; + throw new UnsupportedOperationException(); + } + + @Override + public RestRequest getFilteredRequest(RestRequest restRequest) { + if (restRequest.hasContent()) { + return new FilteredRestRequest(restRequest, Set.of()) { + @Override + @SuppressWarnings({ "rawtypes", "unchecked" }) + protected Map transformBody(Map map) { + final var source = map.get("source"); + if (source instanceof Map sourceMap) { + final var remote = sourceMap.get("remote"); + if (remote instanceof Map remoteMap) { + remoteMap.computeIfPresent("password", (key, value) -> "::es-redacted::"); + remoteMap.computeIfPresent("headers", (key, value) -> { + if (value instanceof Map headers) { + return headers.entrySet() + .stream() + .collect(Collectors.toMap(Map.Entry::getKey, ignore -> "::es-redacted::")); + } else { + return null; + } + }); + } + } + return map; + } + }; + } else { + return restRequest; + } } } diff --git a/modules/reindex/src/test/java/org/elasticsearch/reindex/RestReindexActionTests.java b/modules/reindex/src/test/java/org/elasticsearch/reindex/RestReindexActionTests.java index 61c4bd5ad3aea..3f734ea5264fc 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/reindex/RestReindexActionTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/reindex/RestReindexActionTests.java @@ -11,8 +11,10 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.reindex.AbstractBulkByScrollRequest; import org.elasticsearch.index.reindex.ReindexRequest; +import org.elasticsearch.rest.RestRequest; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.RestActionTestCase; import org.elasticsearch.xcontent.XContentBuilder; @@ -21,8 +23,16 @@ import org.junit.Before; import java.io.IOException; +import java.util.List; +import java.util.Map; import static java.util.Collections.singletonMap; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; +import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.notNullValue; public class RestReindexActionTests extends RestActionTestCase { @@ -74,4 +84,150 @@ public void testSetScrollTimeout() throws IOException { assertEquals("10m", request.getScrollTime().toString()); } } + + public void testFilterSource() throws IOException { + final FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry()); + final var body = """ + { + "source" : { + "index": "photos", + "remote" : { + "host": "https://bugle.example.net:2400/", + "username": "peter.parker", + "password": "mj4ever!", + "headers": { + "X-Hero-Name": "spiderman" + } + } + }, + "dest": { + "index": "webshots" + } + } + """; + requestBuilder.withContent(new BytesArray(body), XContentType.JSON); + + final FakeRestRequest restRequest = requestBuilder.build(); + ReindexRequest request = action.buildRequest(restRequest); + + // Check that the request parsed correctly + assertThat(request.getRemoteInfo().getScheme(), equalTo("https")); + assertThat(request.getRemoteInfo().getHost(), equalTo("bugle.example.net")); + assertThat(request.getRemoteInfo().getPort(), equalTo(2400)); + assertThat(request.getRemoteInfo().getUsername(), equalTo("peter.parker")); + assertThat(request.getRemoteInfo().getPassword(), equalTo("mj4ever!")); + assertThat(request.getRemoteInfo().getHeaders(), hasEntry("X-Hero-Name", "spiderman")); + assertThat(request.getRemoteInfo().getHeaders(), aMapWithSize(1)); + + final RestRequest filtered = action.getFilteredRequest(restRequest); + assertToXContentEquivalent(new BytesArray(""" + { + "source" : { + "index": "photos", + "remote" : { + "host": "https://bugle.example.net:2400/", + "username": "peter.parker", + "password": "::es-redacted::", + "headers": { + "X-Hero-Name": "::es-redacted::" + } + } + }, + "dest": { + "index": "webshots" + } + } + """), filtered.content(), XContentType.JSON); + } + + public void testUnfilteredSource() throws IOException { + final FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry()); + final var empty1 = ""; + final var empty2 = "{}"; + final var nonRemote = """ + { + "source" : { "index": "your-index" }, + "dest" : { "index": "my-index" } + } + """; + final var noCredentials = """ + { + "source" : { + "index": "remote-index", + "remote" : { + "host": "https://es.example.net:12345/", + "headers": {} + } + }, + "dest": { + "index": "my-index" + } + } + """; + for (String body : List.of(empty1, empty2, nonRemote, noCredentials)) { + final BytesArray bodyAsBytes = new BytesArray(body); + requestBuilder.withContent(bodyAsBytes, XContentType.JSON); + final FakeRestRequest restRequest = requestBuilder.build(); + final RestRequest filtered = action.getFilteredRequest(restRequest); + assertToXContentEquivalent(bodyAsBytes, filtered.content(), XContentType.JSON); + } + } + + public void testFilteringBadlyStructureSourceIsSafe() throws IOException { + final FakeRestRequest.Builder requestBuilder = new FakeRestRequest.Builder(xContentRegistry()); + final var remoteAsString = """ + { + "source" : { + "index": "remote-index", + "remote" : "https://es.example.net:12345/" + }, + "dest": { + "index": "my-index" + } + } + """; + final var passwordAsNumber = """ + { + "source" : { + "index": "remote-index", + "remote" : { + "host": "https://es.example.net:12345/", + "username": "skroob", + "password": 12345 + } + }, + "dest": { + "index": "my-index" + } + } + """; + final var headersAsList = """ + { + "source" : { + "index": "remote-index", + "remote" : { + "host": "https://es.example.net:12345/", + "headers": [ "bogus" ] + } + }, + "dest": { + "index": "my-index" + } + } + """; + for (String body : List.of(remoteAsString, passwordAsNumber, headersAsList)) { + final BytesArray bodyAsBytes = new BytesArray(body); + requestBuilder.withContent(bodyAsBytes, XContentType.JSON); + final FakeRestRequest restRequest = requestBuilder.build(); + + final RestRequest filtered = action.getFilteredRequest(restRequest); + assertThat(filtered, notNullValue()); + + // We will redacted some parts of these bodies, so just check that they end up as valid JSON with the right top level fields + final Map filteredMap = XContentHelper.convertToMap(filtered.content(), false, XContentType.JSON).v2(); + assertThat(filteredMap, notNullValue()); + assertThat(filteredMap, hasKey("source")); + assertThat(filteredMap, hasKey("dest")); + } + } } diff --git a/server/src/main/java/org/elasticsearch/rest/FilteredRestRequest.java b/server/src/main/java/org/elasticsearch/rest/FilteredRestRequest.java new file mode 100644 index 0000000000000..8db45d4a2dcd6 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/FilteredRestRequest.java @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.rest; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.bytes.ReleasableBytesReference; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.core.Tuple; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentType; + +import java.io.IOException; +import java.util.Map; +import java.util.Set; + +public class FilteredRestRequest extends RestRequest { + + private final RestRequest restRequest; + private final String[] excludeFields; + private BytesReference filteredBytes; + + public FilteredRestRequest(RestRequest restRequest, Set excludeFields) { + super(restRequest); + this.restRequest = restRequest; + this.excludeFields = excludeFields.toArray(String[]::new); + this.filteredBytes = null; + } + + @Override + public boolean hasContent() { + return true; + } + + @Override + public ReleasableBytesReference content() { + if (filteredBytes == null) { + Tuple> result = XContentHelper.convertToMap( + restRequest.requiredContent(), + true, + restRequest.getXContentType() + ); + final Map transformedSource = transformBody(result.v2()); + try { + XContentBuilder xContentBuilder = XContentBuilder.builder(result.v1().xContent()).map(transformedSource); + filteredBytes = BytesReference.bytes(xContentBuilder); + } catch (IOException e) { + throw new ElasticsearchException("failed to parse request", e); + } + } + return ReleasableBytesReference.wrap(filteredBytes); + } + + protected Map transformBody(Map map) { + return XContentMapValues.filter(map, null, excludeFields); + } +} diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequestFilter.java b/server/src/main/java/org/elasticsearch/rest/RestRequestFilter.java index 7c90d9168e6c8..37b6b80326f10 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequestFilter.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequestFilter.java @@ -9,18 +9,6 @@ package org.elasticsearch.rest; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.bytes.ReleasableBytesReference; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.common.xcontent.support.XContentMapValues; -import org.elasticsearch.core.Tuple; -import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentType; - -import java.io.IOException; -import java.util.Map; import java.util.Set; /** @@ -35,38 +23,7 @@ public interface RestRequestFilter { default RestRequest getFilteredRequest(RestRequest restRequest) { Set fields = getFilteredFields(); if (restRequest.hasContent() && fields.isEmpty() == false) { - return new RestRequest(restRequest) { - - private BytesReference filteredBytes = null; - - @Override - public boolean hasContent() { - return true; - } - - @Override - public ReleasableBytesReference content() { - if (filteredBytes == null) { - Tuple> result = XContentHelper.convertToMap( - restRequest.requiredContent(), - true, - restRequest.getXContentType() - ); - Map transformedSource = XContentMapValues.filter( - result.v2(), - null, - fields.toArray(Strings.EMPTY_ARRAY) - ); - try { - XContentBuilder xContentBuilder = XContentBuilder.builder(result.v1().xContent()).map(transformedSource); - filteredBytes = BytesReference.bytes(xContentBuilder); - } catch (IOException e) { - throw new ElasticsearchException("failed to parse request", e); - } - } - return ReleasableBytesReference.wrap(filteredBytes); - } - }; + return new FilteredRestRequest(restRequest, fields); } else { return restRequest; } @@ -76,4 +33,5 @@ public ReleasableBytesReference content() { * The list of fields that should be filtered. This can be a dot separated pattern to match sub objects and also supports wildcards */ Set getFilteredFields(); + }