diff --git a/CHANGES.txt b/CHANGES.txt index eda42deec..a9ce9bcdd 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,6 @@ 0.5.0 ----- + * Tighten path containment in Live Migration file route (CASSSIDECAR-479) * Implement durable operational job tracker (CASSSIDECAR-374) * Remove filesystem path from Http response (CASSSIDECAR-477) * Add basic configuration retrieval logic to ConfigurationManager (CASSSIDECAR-427) 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 8def795f9..256b185c9 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 LiveMigrationTaskNotFoundException(String message) } } + /** + * 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 df4453240..89b2cce2f 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.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 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 @@ protected Void extractParamsOrThrow(RoutingContext context) 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 @@ protected Void extractParamsOrThrow(RoutingContext context) } 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,99 @@ protected void handleInternal(RoutingContext rc, 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; + ResolvedPath resolved; try { - localFile = LiveMigrationInstanceMetadataUtil.localPath(normalizedPath, instanceMeta).toString(); + resolved = LiveMigrationInstanceMetadataUtil.resolveLexically(normalizedPath, instanceMeta); + } + catch (UnknownMigrationPrefixException e) + { + // The URL is well-formed but matches no configured live-migration directory on this + // instance, so it addresses no resource here - report it as not found. + rc.fail(wrapHttpException(HttpResponseStatus.NOT_FOUND, e.getMessage(), e)); + return; } catch (IllegalArgumentException e) { - LOGGER.warn("Invalid path", e); - rc.response().setStatusCode(HttpResponseStatus.NOT_FOUND.code()).end(); + // URL must have been malformed, report it as bad request + rc.fail(wrapHttpException(HttpResponseStatus.BAD_REQUEST, e.getMessage(), e)); return; } - Path path = Paths.get(localFile); - + // 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(() -> isInvalidPath(rc, path, reqPath, instanceMeta)) - .onSuccess(invalid -> { - if (!invalid) - { - rc.put(FileStreamHandler.FILE_PATH_CONTEXT_KEY, localFile); - rc.next(); - } - }); + .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)); } - 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 382fffdfc..dfcca8170 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; @@ -28,12 +31,15 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import org.slf4j.Logger; 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; @@ -58,6 +64,14 @@ public class LiveMigrationInstanceMetadataUtil { private static final Logger LOGGER = LoggerFactory.getLogger(LiveMigrationInstanceMetadataUtil.class); + /** + * Caches the canonical (symlinks resolved) form of each base directory keyed by its lexical + * path. Base directories come from {@link InstanceMetadata}, which is fixed at startup, so the + * canonical form is stable for the process lifetime. The set of distinct base directories is + * bounded by configuration (~5–10 per instance), so no eviction is required. + */ + private static final ConcurrentMap BASE_DIR_CANONICAL = new ConcurrentHashMap<>(); + /** * Encapsulates all metadata for a specific directory instance in live migration. * Each descriptor represents one physical directory with its associated index, URL route, and placeholder. @@ -263,14 +277,52 @@ public static Map> placeholderDirsMap(InstanceMetadata insta } /** - * 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(); + } + + private static Path canonicalBaseDir(Path baseDir) throws IOException + { + Path cached = BASE_DIR_CANONICAL.get(baseDir); + if (cached != null) + { + return cached; + } + Path canonical = baseDir.toRealPath(); + Path existing = BASE_DIR_CANONICAL.putIfAbsent(baseDir, canonical); + return existing != null ? existing : canonical; + } + + /** + * 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 +351,66 @@ public static Path localPath(@NotNull String fileUrl, 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 canonical (symlinks resolved) path stays inside the canonical base directory. + * 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. + * + *

Performs blocking filesystem I/O - must be called from a worker thread, not the event + * loop. Throws {@link NoSuchFileException} before any canonical-path resolution runs, so + * callers can distinguish "file missing" from "path escapes via symlink". + * + * @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 canonical path escapes the base directory + */ + public void verifyContainment() throws IOException + { + if (!Files.exists(resolvedPath)) + { + throw new NoSuchFileException(resolvedPath.toString()); + } + Path canonical = resolvedPath.toRealPath(); + if (!canonical.startsWith(canonicalBaseDir(baseDir))) + { + LOGGER.error("Resolved path escapes base directory for {}", resolvedPath); + throw new IllegalArgumentException("Resolved path escapes base directory"); + } + } } private static Map 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 48ea12cde..5181a9f67 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 @@ VertxRoute getAllDataCopyTasksRoute(RouteBuilder.Factory factory, 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 5d083891c..8bf505fef 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 @@ 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 void testRouteFailsForNonExistentFile(VertxTestContext context) 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 -> + // //d1/data, so "../../secret.txt" would resolve to //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 e2e401936..6a99c39e3 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 @@ public void testRequestUsingEncodedDots(VertxTestContext context) throws IOExcep @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 //d1/data would walk up to /, 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 -> //d1/data, so "../../secret.txt" would resolve to //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 @@ public void testRouteHavingDirIndexThatDoesNotExist(VertxTestContext context) th 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 1c4b3854d..4c65c0bfe 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.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.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.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 @@ public void testRelativeLocalPaths() 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() {