From 81f988d6a388be287284c458778668cc9aaed63a Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 26 Sep 2025 12:35:46 +1000 Subject: [PATCH] Change reindex to use ::es-redacted:: filtering (#135414) In audit logs we redact certain fields from the body of rest requests. This commit changes the way we redact fields in the reindex request. Previously the only form of redaction we supported was total removal of fields, however that can be problematic when an admin wants to know whether a field was supplied or not. Here we change the way we redact requests for reindexing to replace fields with `::es-redacted::` instead of removing them. --- docs/changelog/135414.yaml | 5 + .../reindex/RestReindexAction.java | 42 ++++- .../reindex/RestReindexActionTests.java | 156 ++++++++++++++++++ .../rest/FilteredRestRequest.java | 65 ++++++++ .../elasticsearch/rest/RestRequestFilter.java | 46 +----- 5 files changed, 267 insertions(+), 47 deletions(-) create mode 100644 docs/changelog/135414.yaml create mode 100644 server/src/main/java/org/elasticsearch/rest/FilteredRestRequest.java 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(); + }