[9.0] Change reindex to use ::es-redacted:: filtering (#135414)

docs/changelog/135414.yaml

@@ -0,0 +1,5 @@
+pr: 135414
+summary: "Change reindex to use ::es-redacted:: filtering"
+area: Audit
+type: enhancement
+issues: []

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<String> FILTERED_FIELDS = Set.of("source.remote.host.password");
-
+    /**
+     * This method isn't used because we implement {@link #getFilteredRequest(RestRequest)} instead
+     */
     @Override
     public Set<String> 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<String, Object> transformBody(Map<String, Object> 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;
+        }
     }
 }

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<String, Object> filteredMap = XContentHelper.convertToMap(filtered.content(), false, XContentType.JSON).v2();
+            assertThat(filteredMap, notNullValue());
+            assertThat(filteredMap, hasKey("source"));
+            assertThat(filteredMap, hasKey("dest"));
+        }
+    }
 }

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<String> 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<XContentType, Map<String, Object>> result = XContentHelper.convertToMap(
+                restRequest.requiredContent(),
+                true,
+                restRequest.getXContentType()
+            );
+            final Map<String, Object> 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<String, Object> transformBody(Map<String, Object> map) {
+        return XContentMapValues.filter(map, null, excludeFields);
+    }
+}

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<String> 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<XContentType, Map<String, Object>> result = XContentHelper.convertToMap(
-                            restRequest.requiredContent(),
-                            true,
-                            restRequest.getXContentType()
-                        );
-                        Map<String, Object> 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<String> getFilteredFields();
+
 }