Merge branch 'main' into feature/hostname-doc-values-sparse-index-setting

salvatore-campagna · web-flow · commit 0a40c32a8ac9 · 2025-02-05T21:06:55.000+01:00
diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java
@@ -17,8 +17,11 @@
 import java.util.List;
 import java.util.Objects;
 
+import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
+
 public final class FileAccessTree {
     public static final FileAccessTree EMPTY = new FileAccessTree(List.of());
+    private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator();
 
     private final String[] readPaths;
     private final String[] writePaths;
@@ -27,11 +30,11 @@ private FileAccessTree(List<FileEntitlement> fileEntitlements) {
         List<String> readPaths = new ArrayList<>();
         List<String> writePaths = new ArrayList<>();
         for (FileEntitlement fileEntitlement : fileEntitlements) {
-            var mode = fileEntitlement.mode();
-            if (mode == FileEntitlement.Mode.READ_WRITE) {
-                writePaths.add(fileEntitlement.path());
+            String path = normalizePath(Path.of(fileEntitlement.path()));
+            if (fileEntitlement.mode() == FileEntitlement.Mode.READ_WRITE) {
+                writePaths.add(path);
             }
-            readPaths.add(fileEntitlement.path());
+            readPaths.add(path);
         }
 
         readPaths.sort(String::compareTo);
@@ -46,14 +49,20 @@ public static FileAccessTree of(List<FileEntitlement> fileEntitlements) {
     }
 
     boolean canRead(Path path) {
-        return checkPath(normalize(path), readPaths);
+        return checkPath(normalizePath(path), readPaths);
     }
 
     boolean canWrite(Path path) {
-        return checkPath(normalize(path), writePaths);
+        return checkPath(normalizePath(path), writePaths);
     }
 
-    private static String normalize(Path path) {
+    /**
+     * @return the "canonical" form of the given {@code path}, to be used for entitlement checks.
+     */
+    static String normalizePath(Path path) {
+        // Note that toAbsolutePath produces paths separated by the default file separator,
+        // so on Windows, if the given path uses forward slashes, this consistently
+        // converts it to backslashes.
         return path.toAbsolutePath().normalize().toString();
     }
 
@@ -64,7 +73,7 @@ private static boolean checkPath(String path, String[] paths) {
         int ndx = Arrays.binarySearch(paths, path);
         if (ndx < -1) {
             String maybeParent = paths[-ndx - 2];
-            return path.startsWith(maybeParent);
+            return path.startsWith(maybeParent) && path.startsWith(FILE_SEPARATOR, maybeParent.length());
         }
         return ndx >= 0;
     }
diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FileEntitlement.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FileEntitlement.java
@@ -12,10 +12,12 @@
 import org.elasticsearch.entitlement.runtime.policy.ExternalEntitlement;
 import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
 
-import java.nio.file.Paths;
-
 /**
- * Describes a file entitlement with a path and mode.
+ * Describes entitlement to access files at a particular location.
+ *
+ * @param path the location of the files. For directories, implicitly includes access to
+ *            all contained files and (recursively) subdirectories.
+ * @param mode the type of operation
  */
 public record FileEntitlement(String path, Mode mode) implements Entitlement {
 
@@ -24,14 +26,6 @@ public enum Mode {
         READ_WRITE
     }
 
-    public FileEntitlement {
-        path = normalizePath(path);
-    }
-
-    private static String normalizePath(String path) {
-        return Paths.get(path).toAbsolutePath().normalize().toString();
-    }
-
     private static Mode parseMode(String mode) {
         if (mode.equals("read")) {
             return Mode.READ;
diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java
@@ -16,6 +16,7 @@
 import java.nio.file.Path;
 import java.util.List;
 
+import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
 import static org.hamcrest.Matchers.is;
 
 public class FileAccessTreeTests extends ESTestCase {
@@ -41,7 +42,9 @@ public void testRead() {
         var tree = FileAccessTree.of(List.of(entitlement("foo", "read")));
         assertThat(tree.canRead(path("foo")), is(true));
         assertThat(tree.canRead(path("foo/subdir")), is(true));
+        assertThat(tree.canRead(path("food")), is(false));
         assertThat(tree.canWrite(path("foo")), is(false));
+        assertThat(tree.canWrite(path("food")), is(false));
 
         assertThat(tree.canRead(path("before")), is(false));
         assertThat(tree.canRead(path("later")), is(false));
@@ -51,7 +54,9 @@ public void testWrite() {
         var tree = FileAccessTree.of(List.of(entitlement("foo", "read_write")));
         assertThat(tree.canWrite(path("foo")), is(true));
         assertThat(tree.canWrite(path("foo/subdir")), is(true));
+        assertThat(tree.canWrite(path("food")), is(false));
         assertThat(tree.canRead(path("foo")), is(true));
+        assertThat(tree.canRead(path("food")), is(false));
 
         assertThat(tree.canWrite(path("before")), is(false));
         assertThat(tree.canWrite(path("later")), is(false));
@@ -83,6 +88,22 @@ public void testNormalizePath() {
         assertThat(tree.canRead(path("")), is(false));
     }
 
+    public void testForwardSlashes() {
+        String sep = getDefaultFileSystem().getSeparator();
+        var tree = FileAccessTree.of(List.of(entitlement("a/b", "read"), entitlement("m" + sep + "n", "read")));
+
+        // Native separators work
+        assertThat(tree.canRead(path("a" + sep + "b")), is(true));
+        assertThat(tree.canRead(path("m" + sep + "n")), is(true));
+
+        // Forward slashes also work
+        assertThat(tree.canRead(path("a/b")), is(true));
+        assertThat(tree.canRead(path("m/n")), is(true));
+
+        // In case the native separator is a backslash, don't treat that as an escape
+        assertThat(tree.canRead(path("m\n")), is(false));
+    }
+
     FileEntitlement entitlement(String path, String mode) {
         Path p = path(path);
         return FileEntitlement.create(p.toString(), mode);
diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java
@@ -118,6 +118,14 @@ public static List<TransportVersion> getAllVersions() {
         return VersionsHolder.ALL_VERSIONS;
     }
 
+    /**
+     * @return whether this is a known {@link TransportVersion}, i.e. one declared in {@link TransportVersions}. Other versions may exist
+     *         in the wild (they're sent over the wire by numeric ID) but we don't know how to communicate using such versions.
+     */
+    public boolean isKnown() {
+        return VersionsHolder.ALL_VERSIONS_MAP.containsKey(id);
+    }
+
     public static TransportVersion fromString(String str) {
         return TransportVersion.fromId(Integer.parseInt(str));
     }
diff --git a/server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java b/server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java
@@ -11,16 +11,18 @@
 
 import org.elasticsearch.Build;
 import org.elasticsearch.TransportVersion;
-import org.elasticsearch.TransportVersions;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.metrics.CounterMetric;
+import org.elasticsearch.core.Strings;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.core.UpdateForV9;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.EOFException;
@@ -126,6 +128,8 @@ final class TransportHandshaker {
      * [3] Parent task ID should be empty; see org.elasticsearch.tasks.TaskId.writeTo for its structure.
      */
 
+    private static final Logger logger = LogManager.getLogger(TransportHandshaker.class);
+
     static final TransportVersion V8_HANDSHAKE_VERSION = TransportVersion.fromId(7_17_00_99);
     static final TransportVersion V9_HANDSHAKE_VERSION = TransportVersion.fromId(8_800_00_0);
     static final Set<TransportVersion> ALLOWED_HANDSHAKE_VERSIONS = Set.of(V8_HANDSHAKE_VERSION, V9_HANDSHAKE_VERSION);
@@ -159,7 +163,7 @@ void sendHandshake(
         ActionListener<TransportVersion> listener
     ) {
         numHandshakes.inc();
-        final HandshakeResponseHandler handler = new HandshakeResponseHandler(requestId, listener);
+        final HandshakeResponseHandler handler = new HandshakeResponseHandler(requestId, channel, listener);
         pendingHandshakes.put(requestId, handler);
         channel.addCloseListener(
             ActionListener.running(() -> handler.handleLocalException(new TransportException("handshake failed because connection reset")))
@@ -185,9 +189,9 @@ void sendHandshake(
     }
 
     void handleHandshake(TransportChannel channel, long requestId, StreamInput stream) throws IOException {
+        final HandshakeRequest handshakeRequest;
         try {
-            // Must read the handshake request to exhaust the stream
-            new HandshakeRequest(stream);
+            handshakeRequest = new HandshakeRequest(stream);
         } catch (Exception e) {
             assert ignoreDeserializationErrors : e;
             throw e;
@@ -206,9 +210,44 @@ void handleHandshake(TransportChannel channel, long requestId, StreamInput strea
             assert ignoreDeserializationErrors : exception;
             throw exception;
         }
+        ensureCompatibleVersion(version, handshakeRequest.transportVersion, handshakeRequest.releaseVersion, channel);
         channel.sendResponse(new HandshakeResponse(this.version, Build.current().version()));
     }
 
+    static void ensureCompatibleVersion(
+        TransportVersion localTransportVersion,
+        TransportVersion remoteTransportVersion,
+        String releaseVersion,
+        Object channel
+    ) {
+        if (TransportVersion.isCompatible(remoteTransportVersion)) {
+            if (remoteTransportVersion.onOrAfter(localTransportVersion)) {
+                // Remote is newer than us, so we will be using our transport protocol and it's up to the other end to decide whether it
+                // knows how to do that.
+                return;
+            }
+            if (remoteTransportVersion.isKnown()) {
+                // Remote is older than us, so we will be using its transport protocol, which we can only do if and only if its protocol
+                // version is known to us.
+                return;
+            }
+        }
+
+        final var message = Strings.format(
+            """
+                Rejecting unreadable transport handshake from remote node with version [%s/%s] received on [%s] since this node has \
+                version [%s/%s] which has an incompatible wire format.""",
+            releaseVersion,
+            remoteTransportVersion,
+            channel,
+            Build.current().version(),
+            localTransportVersion
+        );
+        logger.warn(message);
+        throw new IllegalStateException(message);
+
+    }
+
     TransportResponseHandler<HandshakeResponse> removeHandlerForHandshake(long requestId) {
         return pendingHandshakes.remove(requestId);
     }
@@ -224,11 +263,13 @@ long getNumHandshakes() {
     private class HandshakeResponseHandler implements TransportResponseHandler<HandshakeResponse> {
 
         private final long requestId;
+        private final TcpChannel channel;
         private final ActionListener<TransportVersion> listener;
         private final AtomicBoolean isDone = new AtomicBoolean(false);
 
-        private HandshakeResponseHandler(long requestId, ActionListener<TransportVersion> listener) {
+        private HandshakeResponseHandler(long requestId, TcpChannel channel, ActionListener<TransportVersion> listener) {
             this.requestId = requestId;
+            this.channel = channel;
             this.listener = listener;
         }
 
@@ -245,20 +286,13 @@ public Executor executor() {
         @Override
         public void handleResponse(HandshakeResponse response) {
             if (isDone.compareAndSet(false, true)) {
-                TransportVersion responseVersion = response.transportVersion;
-                if (TransportVersion.isCompatible(responseVersion) == false) {
-                    listener.onFailure(
-                        new IllegalStateException(
-                            "Received message from unsupported version: ["
-                                + responseVersion
-                                + "] minimal compatible version is: ["
-                                + TransportVersions.MINIMUM_COMPATIBLE
-                                + "]"
-                        )
-                    );
-                } else {
-                    listener.onResponse(TransportVersion.min(TransportHandshaker.this.version, response.getTransportVersion()));
-                }
+                ActionListener.completeWith(listener, () -> {
+                    ensureCompatibleVersion(version, response.getTransportVersion(), response.getReleaseVersion(), channel);
+                    final var resultVersion = TransportVersion.min(TransportHandshaker.this.version, response.getTransportVersion());
+                    assert TransportVersion.current().before(version) // simulating a newer-version transport service for test purposes
+                        || resultVersion.isKnown() : "negotiated unknown version " + resultVersion;
+                    return resultVersion;
+                });
             }
         }
 
diff --git a/server/src/test/java/org/elasticsearch/transport/InboundHandlerTests.java b/server/src/test/java/org/elasticsearch/transport/InboundHandlerTests.java
@@ -290,7 +290,9 @@ public void testLogsSlowInboundProcessing() throws Exception {
             );
             BytesStreamOutput byteData = new BytesStreamOutput();
             TaskId.EMPTY_TASK_ID.writeTo(byteData);
+            // simulate bytes of a transport handshake: vInt transport version then release version string
             TransportVersion.writeVersion(remoteVersion, byteData);
+            byteData.writeString(randomIdentifier());
             final InboundMessage requestMessage = new InboundMessage(
                 requestHeader,
                 ReleasableBytesReference.wrap(byteData.bytes()),
diff --git a/server/src/test/java/org/elasticsearch/transport/TransportHandshakerTests.java b/server/src/test/java/org/elasticsearch/transport/TransportHandshakerTests.java

Original file line number	Diff line number	Diff line change
`@@ -118,6 +118,14 @@ public static List<TransportVersion> getAllVersions() {`
`118`	`118`	`return VersionsHolder.ALL_VERSIONS;`
`119`	`119`	`}`
`120`	`120`
	`121`	`+ /**`
	`122`	`+ * @return whether this is a known {@link TransportVersion}, i.e. one declared in {@link TransportVersions}. Other versions may exist`
	`123`	`+ * in the wild (they're sent over the wire by numeric ID) but we don't know how to communicate using such versions.`
	`124`	`+ */`
	`125`	`+ public boolean isKnown() {`
	`126`	`+ return VersionsHolder.ALL_VERSIONS_MAP.containsKey(id);`
	`127`	`+ }`
	`128`	`+`
`121`	`129`	`public static TransportVersion fromString(String str) {`
`122`	`130`	`return TransportVersion.fromId(Integer.parseInt(str));`
`123`	`131`	`}`