This is an automated email from the ASF dual-hosted git repository.
frankgh pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push:
new 74d400ce CASSSIDECAR-479: Tighten path containment in Live Migration
file route (#367)
74d400ce is described below
commit 74d400cefce464702dbc2c59ae9f22a5bf2d3541
Author: N V Harikrishna <[email protected]>
AuthorDate: Sat Aug 29 04:42:45 2026 +0530
CASSSIDECAR-479: Tighten path containment in Live Migration file route
(#367)
Patch by N V Harikrishna; reviewed by Yifan Cai and Francisco Guerrero for
CASSSIDECAR-479
---
CHANGES.txt | 1 +
.../exceptions/LiveMigrationExceptions.java | 16 +++
.../LiveMigrationFileResolveHandler.java | 110 ++++++++++++-------
.../LiveMigrationInstanceMetadataUtil.java | 103 +++++++++++++++++-
.../sidecar/modules/LiveMigrationModule.java | 10 +-
.../LiveMigrationFileDigestHandlerTest.java | 36 +++++++
.../livemigration/LiveMigrationFileStreamTest.java | 62 ++++++++++-
.../LiveMigrationInstanceMetadataUtilTest.java | 120 +++++++++++++++++++++
8 files changed, 410 insertions(+), 48 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 588af8c4..3bfb2983 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,6 @@
0.5.0
-----
+ * Tighten path containment in Live Migration file route (CASSSIDECAR-479)
* Add fast_forward_enabled flag to enable eager pipelining of staging and
importing in coordinated writes (CASSSIDECAR-463)
* CachingSchemaStore should be reloaded when Schemastore kafka configs are
changed (CASSSIDECAR-493)
* Support retrying on HTTP 429 responses and fix duplicate immediate
execution in RequestExecutor retry scheduling (CASSSIDECAR-465)
diff --git
a/server/src/main/java/org/apache/cassandra/sidecar/exceptions/LiveMigrationExceptions.java
b/server/src/main/java/org/apache/cassandra/sidecar/exceptions/LiveMigrationExceptions.java
index 8def795f..256b185c 100644
---
a/server/src/main/java/org/apache/cassandra/sidecar/exceptions/LiveMigrationExceptions.java
+++
b/server/src/main/java/org/apache/cassandra/sidecar/exceptions/LiveMigrationExceptions.java
@@ -72,6 +72,22 @@ public class LiveMigrationExceptions
}
}
+ /**
+ * Thrown when a live-migration file URL is well-formed but matches no
configured directory prefix
+ * on this instance, i.e. it addresses no resource here. Extends {@link
IllegalArgumentException} so
+ * that destination-side callers - which map file URLs obtained from the
source's file-list API onto
+ * local paths - keep treating an unmatched prefix (a source/destination
directory-configuration
+ * mismatch) as a bad argument, while the source-side request handler can
catch this specific type to
+ * distinguish "no such resource" (HTTP 404) from a malformed URL (HTTP
400).
+ */
+ public static class UnknownMigrationPrefixException extends
IllegalArgumentException
+ {
+ public UnknownMigrationPrefixException(String message)
+ {
+ super(message);
+ }
+ }
+
/**
* Exception thrown when file verification fails during live migration.
*/
diff --git
a/server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileResolveHandler.java
b/server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileResolveHandler.java
index df445324..25d66018 100644
---
a/server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileResolveHandler.java
+++
b/server/src/main/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileResolveHandler.java
@@ -18,13 +18,13 @@
package org.apache.cassandra.sidecar.handlers.livemigration;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
+import java.io.IOException;
+import java.net.URI;
import java.nio.file.FileSystems;
import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
-import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -42,16 +42,17 @@ import io.vertx.core.http.HttpServerRequest;
import io.vertx.core.net.SocketAddress;
import io.vertx.ext.auth.authorization.Authorization;
import io.vertx.ext.web.RoutingContext;
-import io.vertx.ext.web.handler.HttpException;
import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions;
import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
import org.apache.cassandra.sidecar.config.LiveMigrationConfiguration;
import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+import
org.apache.cassandra.sidecar.exceptions.LiveMigrationExceptions.UnknownMigrationPrefixException;
import org.apache.cassandra.sidecar.handlers.AbstractHandler;
import org.apache.cassandra.sidecar.handlers.AccessProtected;
import org.apache.cassandra.sidecar.handlers.FileStreamHandler;
import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil;
+import
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil.ResolvedPath;
import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
import org.jetbrains.annotations.NotNull;
@@ -60,6 +61,7 @@ import org.jetbrains.annotations.Nullable;
import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_INDEX_PARAM;
import static
org.apache.cassandra.sidecar.common.ApiEndpointsV1.DIR_TYPE_PARAM;
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.replacePlaceholder;
+import static
org.apache.cassandra.sidecar.utils.HttpExceptions.wrapHttpException;
/**
* Handler that resolves and validates file paths for live migration
operations.
@@ -94,7 +96,7 @@ public class LiveMigrationFileResolveHandler extends
AbstractHandler<Void> imple
String dirType = context.pathParam(DIR_TYPE_PARAM);
if (null == dirType || dirType.isEmpty() || null ==
LiveMigrationDirType.find(dirType))
{
- throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directory type: " + dirType);
+ throw wrapHttpException(HttpResponseStatus.BAD_REQUEST, "Invalid
directory type: " + dirType);
}
String dirIndex = context.pathParam(DIR_INDEX_PARAM);
@@ -105,11 +107,11 @@ public class LiveMigrationFileResolveHandler extends
AbstractHandler<Void> imple
}
catch (NumberFormatException formatException)
{
- throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ throw wrapHttpException(HttpResponseStatus.BAD_REQUEST, "Invalid
directoryIndex: " + dirIndex);
}
if (index < 0)
{
- throw new HttpException(HttpResponseStatus.BAD_REQUEST.code(),
"Invalid directoryIndex: " + dirIndex);
+ throw wrapHttpException(HttpResponseStatus.BAD_REQUEST, "Invalid
directoryIndex: " + dirIndex);
}
// Path params are not used further, hence returning null.
@@ -123,65 +125,95 @@ public class LiveMigrationFileResolveHandler extends
AbstractHandler<Void> imple
SocketAddress remoteAddress,
@Nullable Void request)
{
- String reqPath = URLDecoder.decode(rc.request().path(),
StandardCharsets.UTF_8);
+ String reqPath;
+ try
+ {
+ reqPath = URI.create(rc.request().path()).getPath();
+ }
+ catch (IllegalArgumentException e)
+ {
+ rc.fail(wrapHttpException(HttpResponseStatus.BAD_REQUEST,
"Malformed request path", e));
+ return;
+ }
if (reqPath.contains("/../") || reqPath.endsWith("/.."))
{
LOGGER.warn("Tried to access file using relative path({}).
Rejecting the request.", reqPath);
-
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ rc.fail(wrapHttpException(HttpResponseStatus.BAD_REQUEST,
+ "Tried to access file using relative
path: " + reqPath));
return;
}
InstanceMetadata instanceMeta = metadataFetcher.instance(host);
String normalizedPath = rc.normalizedPath();
- String localFile;
- try
+ ResolvedPath resolved =
LiveMigrationInstanceMetadataUtil.resolveLexically(normalizedPath,
instanceMeta);
+
+ // Only the filesystem-touching checks (verifyContainment,
isDirectory, isExcluded) run on
+ // the worker thread; the lexical resolve above is pure string work
and stays on the event
+ // loop. validate() throws HttpException on any failure, which flows
through processFailure
+ // -> context.fail() so the framework's failure handler renders a JSON
error response.
+ String localFile = resolved.resolvedPath().toString();
+ executorPools.service()
+ .executeBlocking(() -> {
+ validate(resolved, instanceMeta);
+ return localFile;
+ })
+ .onSuccess(file -> {
+ rc.put(FileStreamHandler.FILE_PATH_CONTEXT_KEY, file);
+ rc.next();
+ })
+ .onFailure(cause -> processFailure(cause, rc, host,
remoteAddress, request));
+ }
+
+ @Override
+ protected void processFailure(Throwable cause, RoutingContext context,
String host, SocketAddress remoteAddress, Void request)
+ {
+ if (cause instanceof UnknownMigrationPrefixException)
{
- localFile =
LiveMigrationInstanceMetadataUtil.localPath(normalizedPath,
instanceMeta).toString();
+ context.fail(wrapHttpException(HttpResponseStatus.NOT_FOUND,
cause.getMessage(), cause));
}
- catch (IllegalArgumentException e)
+ else
{
- LOGGER.warn("Invalid path", e);
-
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
- return;
+ super.processFailure(cause, context, host, remoteAddress, request);
}
-
- Path path = Paths.get(localFile);
-
- executorPools.service()
- .executeBlocking(() -> isInvalidPath(rc, path, reqPath,
instanceMeta))
- .onSuccess(invalid -> {
- if (!invalid)
- {
- rc.put(FileStreamHandler.FILE_PATH_CONTEXT_KEY,
localFile);
- rc.next();
- }
- });
}
- private boolean isInvalidPath(RoutingContext rc, Path path, String
reqPath, InstanceMetadata instanceMeta)
+ private void validate(ResolvedPath resolved, InstanceMetadata instanceMeta)
{
- if (!Files.exists(path))
+ Path path = resolved.resolvedPath();
+ try
+ {
+ resolved.verifyContainment();
+ }
+ catch (NoSuchFileException e)
{
LOGGER.info("Requested file is not found. file={}", path);
-
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
- return true;
+ throw wrapHttpException(HttpResponseStatus.NOT_FOUND, "File not
found", e);
}
+ catch (IllegalArgumentException e)
+ {
+ throw wrapHttpException(HttpResponseStatus.FORBIDDEN,
e.getMessage(), e);
+ }
+ catch (IOException e)
+ {
+ LOGGER.error("Filesystem error while resolving {}", path, e);
+ throw wrapHttpException(HttpResponseStatus.INTERNAL_SERVER_ERROR,
+ "Filesystem error while resolving
requested path", e);
+ }
+
if (Files.isDirectory(path))
{
- LOGGER.info("Cannot transfer directory. path={}.", reqPath);
-
rc.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
- return true;
+ LOGGER.info("Cannot transfer directory. path={}", path);
+ throw wrapHttpException(HttpResponseStatus.BAD_REQUEST, "Cannot
transfer directory");
}
if (isExcluded(path, instanceMeta))
{
LOGGER.debug("Requested path or one of its parent directories is
excluded from Live Migration. " +
- "path={}", reqPath);
-
rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end();
- return true;
+ "path={}", path);
+ throw wrapHttpException(HttpResponseStatus.NOT_FOUND,
+ "Requested path is excluded from live
migration");
}
- return false;
}
private boolean isExcluded(Path localFile, InstanceMetadata
instanceMetadata)
diff --git
a/server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtil.java
b/server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtil.java
index 382fffdf..1fbff507 100644
---
a/server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtil.java
+++
b/server/src/main/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtil.java
@@ -18,6 +18,9 @@
package org.apache.cassandra.sidecar.livemigration;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -34,6 +37,7 @@ import org.slf4j.LoggerFactory;
import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
import org.apache.cassandra.sidecar.common.ApiEndpointsV1;
+import
org.apache.cassandra.sidecar.exceptions.LiveMigrationExceptions.UnknownMigrationPrefixException;
import
org.apache.cassandra.sidecar.handlers.livemigration.LiveMigrationDirType;
import org.jetbrains.annotations.NotNull;
@@ -263,14 +267,40 @@ public class LiveMigrationInstanceMetadataUtil
}
/**
- * Converts given live migration file download URL to local path.
+ * Resolves a live migration file download URL to a local path. Performs
only a lexical
+ * containment check; symlinks are not resolved and the file is not
required to exist.
+ * Suitable for destination-side callers that place files into
operator-controlled directories.
+ * Source-side callers that serve existing files in response to remote
requests must additionally
+ * call {@link ResolvedPath#verifyContainment()} on the returned object
before using the path.
*
* @param fileUrl Live migration file download URL
* @param metadata Cassandra instance metadata
- * @return local path for given live migration file download URL
+ * @return lexically-resolved local path for given live migration file
download URL
+ * @throws IllegalArgumentException if the URL is malformed or the lexical
path escapes
+ * the base directory
*/
public static Path localPath(@NotNull String fileUrl,
@NotNull InstanceMetadata metadata)
+ {
+ return resolveLexically(fileUrl, metadata).resolvedPath();
+ }
+
+ /**
+ * Lexically resolves a live migration file download URL to a {@link
ResolvedPath}. Performs only
+ * string-level validation; the filesystem is not touched and the file is
not required to exist.
+ *
+ * @param fileUrl Live migration file download URL
+ * @param metadata Cassandra instance metadata
+ * @return the lexically-resolved {@link ResolvedPath}
+ * @throws IllegalArgumentException if the URL is malformed - it
contains a relative traversal
+ * segment ({@code /../}) or
lexically escapes the configured base directory
+ * @throws UnknownMigrationPrefixException if the URL does not match any
configured live-migration directory
+ * prefix; the URL is well-formed
but addresses no resource on this
+ * instance. This is a subtype of
{@link IllegalArgumentException}, so
+ * callers that only care about
"bad URL" need not distinguish it
+ */
+ public static ResolvedPath resolveLexically(@NotNull String fileUrl,
+ @NotNull InstanceMetadata
metadata)
{
Objects.requireNonNull(fileUrl, "fileUrl cannot be null");
Objects.requireNonNull(metadata, "metadata cannot be null");
@@ -299,11 +329,76 @@ public class LiveMigrationInstanceMetadataUtil
throw new IllegalArgumentException(errorMessage);
}
- return resolvedPath;
+ return new ResolvedPath(baseDir, resolvedPath);
}
}
- throw new IllegalArgumentException("File url " + fileUrl + " is
unknown.");
+ LOGGER.warn("File url {} does not match any configured live-migration
directory prefix.", fileUrl);
+ throw new UnknownMigrationPrefixException("File url " + fileUrl + " is
unknown.");
+ }
+
+ /**
+ * Lexical resolution of a live-migration URL: the configured base
directory paired with the
+ * local path it maps to.
+ */
+ public static final class ResolvedPath
+ {
+ private final Path baseDir;
+ private final Path resolvedPath;
+
+ ResolvedPath(Path baseDir, Path resolvedPath)
+ {
+ this.baseDir = baseDir;
+ this.resolvedPath = resolvedPath;
+ }
+
+ public Path resolvedPath()
+ {
+ return resolvedPath;
+ }
+
+ /**
+ * Verifies that the source-side file represented by this {@code
ResolvedPath} exists and
+ * that its real (symlinks resolved) path stays inside the real base
directory path.
+ * Use this on the source side after
+ * {@link LiveMigrationInstanceMetadataUtil#resolveLexically(String,
InstanceMetadata)} to
+ * enforce that the file the operator is about to serve cannot escape
the configured
+ * directory through a symlink. Callers continue to use {@link
#resolvedPath()} (the
+ * lexical form) for exclusion matching, logging, and serving, since
exclusion patterns
+ * and operator-facing logs are configured against the lexical form.
+ *
+ * <p>Performs blocking filesystem I/O - must be called from a worker
thread, not the event
+ * loop. Throws {@link NoSuchFileException} before any real-path
resolution runs, so
+ * callers can distinguish "file missing" from "path escapes via
symlink". The base directory
+ * is only resolved when the file's real path does not already start
with it, so a base
+ * directory that is not itself a symlink costs no extra filesystem
calls.
+ *
+ * @throws NoSuchFileException if the resolved file does not exist
+ * @throws IOException if {@link Path#toRealPath} fails
for an I/O reason
+ * other than missing file
+ * @throws IllegalArgumentException if the real path escapes the base
directory
+ */
+ public void verifyContainment() throws IOException
+ {
+ if (!Files.exists(resolvedPath))
+ {
+ throw new NoSuchFileException(resolvedPath.toString());
+ }
+ Path realPath = resolvedPath.toRealPath();
+ // When the base directory is not itself behind a symlink, the
file's real path still
+ // starts with the lexical base directory, which already proves
containment.
+ if (realPath.startsWith(baseDir))
+ {
+ return;
+ }
+ // The base directory itself may be a symlink (for example a data
dir pointing at a
+ // mounted volume), so compare against its real path before
rejecting.
+ if (!realPath.startsWith(baseDir.toRealPath()))
+ {
+ LOGGER.error("Resolved path escapes base directory for {}",
resolvedPath);
+ throw new IllegalArgumentException("Resolved path escapes base
directory");
+ }
+ }
}
private static Map<String, String>
migrationUrlLocalDirMap(InstanceMetadata instanceMetadata)
diff --git
a/server/src/main/java/org/apache/cassandra/sidecar/modules/LiveMigrationModule.java
b/server/src/main/java/org/apache/cassandra/sidecar/modules/LiveMigrationModule.java
index 48ea12cd..5181a9f6 100644
---
a/server/src/main/java/org/apache/cassandra/sidecar/modules/LiveMigrationModule.java
+++
b/server/src/main/java/org/apache/cassandra/sidecar/modules/LiveMigrationModule.java
@@ -196,8 +196,16 @@ public class LiveMigrationModule extends AbstractModule
responseCode = "200",
content = @Content(mediaType = "application/json",
schema = @Schema(implementation = DigestResponse.class)))
+ @APIResponse(responseCode = "400",
+ description = "Invalid path parameter (e.g., non-numeric directory index)
or a directory was requested",
+ content = @Content(mediaType = "application/json",
+ schema = @Schema(type = SchemaType.OBJECT)))
+ @APIResponse(responseCode = "403",
+ description = "Resolved path is outside the configured live-migration
directories",
+ content = @Content(mediaType = "application/json",
+ schema = @Schema(type = SchemaType.OBJECT)))
@APIResponse(responseCode = "404",
- description = "Live migration not enabled or node not configured as
source",
+ description = "Live migration not enabled, node not configured as source,
or requested file not found",
content = @Content(mediaType = "application/json",
schema = @Schema(type = SchemaType.OBJECT)))
@APIResponse(responseCode = "503",
diff --git
a/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileDigestHandlerTest.java
b/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileDigestHandlerTest.java
index 5d083891..8bf505fe 100644
---
a/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileDigestHandlerTest.java
+++
b/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileDigestHandlerTest.java
@@ -20,7 +20,9 @@ package org.apache.cassandra.sidecar.handlers.livemigration;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
@@ -278,6 +280,40 @@ public class LiveMigrationFileDigestHandlerTest
shouldFail(context, testRoute, FIRST_INSTANCE_IP, SECOND_INSTANCE_IP,
FIRST_INSTANCE_IP, 404);
}
+ @Test
+ public void testRequestUsingDoubleEncodedDots(VertxTestContext context)
throws IOException
+ {
+ // Double-encoded traversal: "%252e%252e%2f" decodes only once in
Vert.x's normalizer to "%2e%2e/"
+ // ("%25" is a reserved char that is never decoded), so "%252e%252e"
stays a literal path segment and
+ // is never collapsed into "..". Plant a secret two levels above the
data dir (data/0 ->
+ // <tempDir>/<id>/d1/data, so "../../secret.txt" would resolve to
<tempDir>/<id>/secret.txt) and
+ // confirm the double-encoded request cannot traverse out to digest
it. The resolve handler runs
+ // before the digest handler, so this guard protects the digest
endpoint too.
+ createFile("super-secret",
tempDir.resolve(String.valueOf(FIRST_ID)).resolve("secret.txt").toString());
+
+ String testRoute = LIVE_MIGRATION_FILES_ROUTE +
"/data/0/%252e%252e/%252e%252e/secret.txt?digestAlgorithm=md5";
+ shouldFail(context, testRoute, FIRST_INSTANCE_IP, SECOND_INSTANCE_IP,
FIRST_INSTANCE_IP, 404);
+ }
+
+ @Test
+ public void testRouteFailsForSymlinkEscapingDataDir(VertxTestContext
context) throws IOException
+ {
+ // A symlink inside the data dir pointing outside the migration tree
cannot be caught by the lexical
+ // ".." containment check. realPath() resolves symlinks via
toRealPath() and must reject the escape
+ // with 403, so no digest is computed for the file behind the symlink.
The resolve handler runs
+ // before the digest handler, so this guard protects the digest
endpoint too.
+ Path outside = tempDir.resolve("outside-tree");
+ Files.createDirectories(outside);
+ createFile("top-secret", outside.resolve("secret.txt").toString());
+
+ Path dataDir = Paths.get(firstInstanceDataDirs.get(0));
+ Files.createDirectories(dataDir);
+ Files.createSymbolicLink(dataDir.resolve("escape"), outside);
+
+ String testRoute = LIVE_MIGRATION_FILES_ROUTE +
"/data/0/escape/secret.txt?digestAlgorithm=md5";
+ shouldFail(context, testRoute, FIRST_INSTANCE_IP, SECOND_INSTANCE_IP,
FIRST_INSTANCE_IP, 403);
+ }
+
@Test
public void testRouteSucceedsForEmptyFile(VertxTestContext context) throws
IOException
{
diff --git
a/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamTest.java
b/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamTest.java
index e2e40193..6a99c39e 100644
---
a/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamTest.java
+++
b/server/src/test/java/org/apache/cassandra/sidecar/handlers/livemigration/LiveMigrationFileStreamTest.java
@@ -19,7 +19,9 @@
package org.apache.cassandra.sidecar.handlers.livemigration;
import java.io.IOException;
+import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -205,14 +207,64 @@ class LiveMigrationFileStreamTest
@Test
public void testRequestInvalidPathUsingEncodedDots(VertxTestContext
context) throws IOException
{
- String filePath = "/ks/tb-1234/ks-tb-1234-Data.db";
- createFile(DUMMY_CONTENT, firstInstanceDataDirs.get(0) + filePath);
+ // If %2E%2E were wrongly decoded and the path were collapsed
unchecked, three "../"
+ // segments from <tempDir>/<id>/d1/data would walk up to <tempDir>/,
where this decoy
+ // lives. Plant it so the test actually proves the request can't reach
it. Vert.x rejects
+ // 3+ consecutive encoded ".." patterns, so the request never reaches
the resolver.
+ createFile("decoy", tempDir.resolve("secrets").toString());
- // Vertx rejects 3+ consecutive %2E%2E (encoded ..) patterns
shouldThrowError(context, LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/%2E%2E/%2E%2E/%2E%2E/secrets",
FIRST_INSTANCE_IP, THIRD_INSTANCE_IP,
FIRST_INSTANCE_IP, 404);
}
+ @Test
+ public void testRequestUsingDoubleEncodedDots(VertxTestContext context)
throws IOException
+ {
+ // Double-encoded traversal: "%252e%252e%2f" decodes only once in
Vert.x's normalizer to "%2e%2e/"
+ // ("%25" is a reserved char that is never decoded), so "%252e%252e"
stays a literal path segment
+ // and is never collapsed into "..". Plant a secret two levels above
the data dir
+ // (data/0 -> <tempDir>/<id>/d1/data, so "../../secret.txt" would
resolve to <tempDir>/<id>/secret.txt)
+ // and confirm the double-encoded request cannot traverse out to reach
it.
+ createFile("super-secret",
tempDir.resolve(String.valueOf(FIRST_ID)).resolve("secret.txt").toString());
+ createFile(DUMMY_CONTENT, firstInstanceDataDirs.get(0) +
"/ks/tb-1234/ks-tb-1234-Data.db");
+
+ String testRoute = LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/%252e%252e/%252e%252e/secret.txt";
+ shouldThrowError(context, testRoute, FIRST_INSTANCE_IP,
SECOND_INSTANCE_IP, FIRST_INSTANCE_IP, 404);
+ }
+
+ @Test
+ public void testRouteFailsForSymlinkEscapingDataDir(VertxTestContext
context) throws IOException
+ {
+ // A symlink planted inside the data dir that points outside the
migration tree cannot be caught by
+ // the lexical ".." containment check. realPath() resolves symlinks
via toRealPath() and rejects
+ // the escape with 403, so the file behind the symlink is not served.
+ Path outside = tempDir.resolve("outside-tree");
+ Files.createDirectories(outside);
+ createFile("top-secret", outside.resolve("secret.txt").toString());
+
+ Path dataDir = Paths.get(firstInstanceDataDirs.get(0));
+ Files.createDirectories(dataDir);
+ Files.createSymbolicLink(dataDir.resolve("escape"), outside);
+
+ String testRoute = LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/escape/secret.txt";
+ shouldThrowError(context, testRoute, FIRST_INSTANCE_IP,
SECOND_INSTANCE_IP, FIRST_INSTANCE_IP, 403);
+ }
+
+ @Test
+ public void testRouteSucceedsForSymlinkWithinDataDir(VertxTestContext
context) throws IOException
+ {
+ // A symlink that resolves to a target still inside the data dir is
legitimate and must be served -
+ // the real-path containment check must not over-block symlinks within
the migration tree.
+ String filePath = "/ks/tb-1234/ks-tb-1234-Data.db";
+ createFile(DUMMY_CONTENT, firstInstanceDataDirs.get(0) + filePath);
+
+ Path dataDir = Paths.get(firstInstanceDataDirs.get(0));
+ Files.createSymbolicLink(dataDir.resolve("link-ks"),
dataDir.resolve("ks"));
+
+ String testRoute = LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/link-ks/tb-1234/ks-tb-1234-Data.db";
+ shouldSucceed(context, testRoute, FIRST_INSTANCE_IP,
SECOND_INSTANCE_IP, FIRST_INSTANCE_IP);
+ }
+
@Test
public void testRequestDirectory(VertxTestContext context) throws
IOException
@@ -263,7 +315,9 @@ class LiveMigrationFileStreamTest
String filePath = "/ks/tb-1234/ks-tb-1234-Data.db";
createFile(DUMMY_CONTENT, firstInstanceDataDirs.get(0) + filePath);
- // This route has 100 as dataHomeDir index which is greater than
instance data home dir count. It should fail.
+ // dirIndex 100 is greater than the configured data dir count, so the
URL doesn't match
+ // any configured prefix - reports it as a missing resource (404),
since
+ // the URL is well-formed but addresses no directory configured on
this instance.
shouldThrowError(context, LIVE_MIGRATION_DATA_FILE_DIR_PATH + "/100" +
filePath,
FIRST_INSTANCE_IP, THIRD_INSTANCE_IP,
FIRST_INSTANCE_IP, 404);
}
diff --git
a/server/src/test/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtilTest.java
b/server/src/test/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtilTest.java
index 1c4b3854..4c65c0bf 100644
---
a/server/src/test/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtilTest.java
+++
b/server/src/test/java/org/apache/cassandra/sidecar/livemigration/LiveMigrationInstanceMetadataUtilTest.java
@@ -18,7 +18,12 @@
package org.apache.cassandra.sidecar.livemigration;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
@@ -31,6 +36,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import
org.apache.cassandra.sidecar.exceptions.LiveMigrationExceptions.UnknownMigrationPrefixException;
import org.mockito.Mockito;
import static
org.apache.cassandra.sidecar.handlers.livemigration.InstanceMetadataTestUtil.LIVE_MIGRATION_CDC_RAW_DIR_PATH;
@@ -40,6 +46,7 @@ import static
org.apache.cassandra.sidecar.handlers.livemigration.InstanceMetada
import static
org.apache.cassandra.sidecar.handlers.livemigration.InstanceMetadataTestUtil.LIVE_MIGRATION_LOCAL_SYSTEM_DATA_FILE_DIR_PATH;
import static
org.apache.cassandra.sidecar.handlers.livemigration.InstanceMetadataTestUtil.LIVE_MIGRATION_SAVED_CACHES_DIR_PATH;
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil.localPath;
+import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationInstanceMetadataUtil.resolveLexically;
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.CDC_RAW_DIR_PLACEHOLDER;
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.COMMITLOG_DIR_PLACEHOLDER;
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.DATA_FILE_DIR_PLACEHOLDER;
@@ -47,6 +54,7 @@ import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholde
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.LOCAL_SYSTEM_DATA_FILE_DIR_PLACEHOLDER;
import static
org.apache.cassandra.sidecar.livemigration.LiveMigrationPlaceholderUtil.SAVED_CACHES_DIR_PLACEHOLDER;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static
org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.entry;
import static org.mockito.Mockito.when;
@@ -308,6 +316,118 @@ class LiveMigrationInstanceMetadataUtilTest
instanceMetadata);
}
+ @Test
+ public void testLocalPathDoesNotEscapeForEncodedTraversal()
+ {
+ // Vert.x's normalizedPath() does a single unreserved-char decode
followed by dot-segment removal.
+ // A double-encoded "%252e%252e" survives as the literal "%252e%252e"
(because "%25" is reserved and
+ // is never decoded), and an encoded slash "%2f" stays encoded so
"..%2f..%2f" is a single literal
+ // path element rather than parent-directory segments. In both cases
the sequence reaches localPath()
+ // as ordinary file-name text, so the resolved path must stay *inside*
the data directory and never
+ // escape it. This locks in that the reported double-encoding bypass
cannot traverse out of the root.
+ String cassandraHomeDir =
tempDir.resolve("encodedTraversal").toString();
+ InstanceMetadata instanceMetadata =
getInstanceMetadata(cassandraHomeDir);
+ String dataDir = instanceMetadata.dataDirs().get(0);
+
+ // Double-encoded ".." segments (what survives Vert.x normalization
for %252e%252e input)
+ // resolve as literal directory names inside the data dir, not as
parent-directory hops.
+ assertThat(localPath(LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/%252e%252e/%252e%252e/etc/passwd",
+ instanceMetadata).toString())
+ .isEqualTo(dataDir + "/%252e%252e/%252e%252e/etc/passwd");
+
+ // Encoded-slash sequence (what survives normalization for %2e%2e%2f
input) stays a single literal
+ // path element, so it too resolves inside the data dir.
+ assertThat(localPath(LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/ks/..%2f..%2fetc/passwd",
+ instanceMetadata).toString())
+ .isEqualTo(dataDir + "/ks/..%2f..%2fetc/passwd");
+ }
+
+ @Test
+ public void testRealPathRejectsSymlinkEscape() throws IOException
+ {
+ // A symlink inside the data dir pointing outside the migration tree
must be rejected by the
+ // realpath-based containment check; the lexical localPath() check
alone cannot detect this.
+ String cassandraHomeDir = tempDir.resolve("symlinkEscape").toString();
+ InstanceMetadata instanceMetadata =
getInstanceMetadata(cassandraHomeDir);
+
+ Path outside = tempDir.resolve("outside-tree");
+ Files.createDirectories(outside);
+ Files.write(outside.resolve("secret.txt"),
"secret".getBytes(StandardCharsets.UTF_8));
+
+ Path dataDir = Paths.get(instanceMetadata.dataDirs().get(0));
+ Files.createDirectories(dataDir);
+ Files.createSymbolicLink(dataDir.resolve("escape"), outside);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> resolveLexically(
+ LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/escape/secret.txt", instanceMetadata)
+ .verifyContainment());
+ }
+
+ @Test
+ public void testRealPathReturnsLexicalForSymlinkWithinDir() throws
IOException
+ {
+ // A symlink whose target is still inside the data dir is legitimate;
verifyContainment
+ // accepts it without throwing, and resolveLexically returns the
lexical (URL-derived)
+ // path so downstream consumers operate against the configured
directory namespace.
+ String cassandraHomeDir = tempDir.resolve("symlinkWithin").toString();
+ InstanceMetadata instanceMetadata =
getInstanceMetadata(cassandraHomeDir);
+
+ Path dataDir = Paths.get(instanceMetadata.dataDirs().get(0));
+ Files.createDirectories(dataDir.resolve("ks"));
+ Files.write(dataDir.resolve("ks/file.db"),
"data".getBytes(StandardCharsets.UTF_8));
+ Files.createSymbolicLink(dataDir.resolve("link-ks"),
dataDir.resolve("ks"));
+
+ LiveMigrationInstanceMetadataUtil.ResolvedPath resolved =
+ resolveLexically(LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/link-ks/file.db", instanceMetadata);
+ resolved.verifyContainment();
+
assertThat(resolved.resolvedPath()).isEqualTo(dataDir.resolve("link-ks/file.db"));
+ }
+
+ @Test
+ public void testRealPathThrowsForNonExistentPath()
+ {
+ // verifyContainment does an explicit Files.exists check before
toRealPath and throws
+ // NoSuchFileException when the resolved file is missing, so callers
can distinguish
+ // "file not found" cleanly from "path escapes base directory"
(IllegalArgumentException).
+ String cassandraHomeDir = tempDir.resolve("nonexistent").toString();
+ InstanceMetadata instanceMetadata =
getInstanceMetadata(cassandraHomeDir);
+
+ assertThatExceptionOfType(NoSuchFileException.class)
+ .isThrownBy(() -> resolveLexically(
+ LIVE_MIGRATION_DATA_FILE_DIR_PATH + "/0/ks/foo.db",
instanceMetadata)
+ .verifyContainment());
+ }
+
+ @Test
+ public void testRealPathRejectsLexicalTraversal()
+ {
+ // resolveLexically rejects /../ patterns up-front, before any
filesystem I/O runs — same
+ // guard that localPath uses.
+ String cassandraHomeDir = tempDir.resolve("lexicalEscape").toString();
+ InstanceMetadata instanceMetadata =
getInstanceMetadata(cassandraHomeDir);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> resolveLexically(
+ LIVE_MIGRATION_DATA_FILE_DIR_PATH +
"/0/../../etc/passwd", instanceMetadata));
+ }
+
+ @Test
+ public void testResolveLexicallyThrowsForUnknownPrefix()
+ {
+ // A well-formed URL whose prefix matches no configured directory is
reported via
+ // UnknownMigrationPrefixException. It is a subtype of
IllegalArgumentException (so destination-side
+ // localPath callers keep treating it as a bad argument), while the
source-side handler catches this
+ // specific type to answer 404 - "no such resource" - instead of 400
for a malformed URL.
+ String cassandraHomeDir = tempDir.resolve("unknownPrefix").toString();
+ InstanceMetadata instanceMetadata =
getInstanceMetadata(cassandraHomeDir);
+ when(instanceMetadata.cdcDir()).thenReturn(null);
+
+ assertThatExceptionOfType(UnknownMigrationPrefixException.class)
+ .isThrownBy(() -> resolveLexically(LIVE_MIGRATION_CDC_RAW_DIR_PATH +
"/0/" + FILE_NAME, instanceMetadata))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
@Test
public void testLocalPathNonExistingDirs()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
