diff --git a/integration-test/src/test/java/org/apache/iotdb/pipe/it/single/IoTDBPipeReceiverIT.java b/integration-test/src/test/java/org/apache/iotdb/pipe/it/single/IoTDBPipeReceiverIT.java index 5ca62ebb15f04..136943f00b017 100644 --- a/integration-test/src/test/java/org/apache/iotdb/pipe/it/single/IoTDBPipeReceiverIT.java +++ b/integration-test/src/test/java/org/apache/iotdb/pipe/it/single/IoTDBPipeReceiverIT.java @@ -24,12 +24,15 @@ import org.apache.iotdb.commons.conf.CommonDescriptor; import org.apache.iotdb.commons.conf.IoTDBConstant; import org.apache.iotdb.commons.path.MeasurementPath; +import org.apache.iotdb.commons.pipe.agent.plugin.meta.PipePluginMeta; import org.apache.iotdb.commons.pipe.sink.client.IoTDBSyncClient; import org.apache.iotdb.commons.pipe.sink.payload.thrift.common.PipeTransferHandshakeConstant; import org.apache.iotdb.commons.pipe.sink.payload.thrift.request.IoTDBSinkRequestVersion; import org.apache.iotdb.commons.pipe.sink.payload.thrift.request.PipeRequestType; +import org.apache.iotdb.confignode.consensus.request.write.pipe.plugin.CreatePipePluginPlan; import org.apache.iotdb.confignode.manager.pipe.sink.payload.PipeTransferConfigNodeHandshakeV1Req; import org.apache.iotdb.confignode.manager.pipe.sink.payload.PipeTransferConfigNodeHandshakeV2Req; +import org.apache.iotdb.confignode.manager.pipe.sink.payload.PipeTransferConfigPlanReq; import org.apache.iotdb.db.pipe.sink.payload.evolvable.request.PipeTransferDataNodeHandshakeV1Req; import org.apache.iotdb.db.pipe.sink.payload.evolvable.request.PipeTransferDataNodeHandshakeV2Req; import org.apache.iotdb.db.pipe.sink.payload.evolvable.request.PipeTransferTabletRawReq; @@ -56,6 +59,7 @@ import org.apache.tsfile.enums.TSDataType; import org.apache.tsfile.external.commons.io.FileUtils; import org.apache.tsfile.file.metadata.enums.TSEncoding; +import org.apache.tsfile.utils.Binary; import org.apache.tsfile.utils.ReadWriteIOUtils; import org.apache.tsfile.write.record.Tablet; import org.apache.tsfile.write.schema.MeasurementSchema; @@ -256,6 +260,21 @@ public void testConfigNodeReceiverSessionHandling() throws Exception { Assert.assertNotEquals( TSStatusCode.NOT_LOGIN.getStatusCode(), client.pipeTransfer(buildEmptyConfigPlanReq()).getStatus().getCode()); + Assert.assertEquals( + TSStatusCode.NO_PERMISSION.getStatusCode(), + client + .pipeTransfer( + PipeTransferConfigPlanReq.toTPipeTransferReq( + new CreatePipePluginPlan( + new PipePluginMeta( + "receiver-security-test-plugin", + "attacker.Plugin", + false, + "attacker.jar", + "deadbeef"), + new Binary(new byte[] {1})))) + .getStatus() + .getCode()); } } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/receiver/protocol/IoTDBConfigNodeReceiver.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/receiver/protocol/IoTDBConfigNodeReceiver.java index 281867d0ec58d..d76a1d2943c5c 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/receiver/protocol/IoTDBConfigNodeReceiver.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/receiver/protocol/IoTDBConfigNodeReceiver.java @@ -88,6 +88,7 @@ import org.apache.iotdb.confignode.consensus.request.write.table.view.SetViewPropertiesPlan; import org.apache.iotdb.confignode.consensus.request.write.template.CommitSetSchemaTemplatePlan; import org.apache.iotdb.confignode.consensus.request.write.template.CreateSchemaTemplatePlan; +import org.apache.iotdb.confignode.consensus.request.write.template.DropSchemaTemplatePlan; import org.apache.iotdb.confignode.consensus.request.write.template.ExtendSchemaTemplatePlan; import org.apache.iotdb.confignode.consensus.request.write.trigger.DeleteTriggerInTablePlan; import org.apache.iotdb.confignode.consensus.request.write.trigger.UpdateTriggerStateInTablePlan; @@ -413,6 +414,12 @@ private TSStatus checkPermission(final ConfigPhysicalPlan plan) throws IOExcepti case CreateSchemaTemplate: templateName = ((CreateSchemaTemplatePlan) plan).getTemplate().getName(); return checkGlobalStatus(userEntity, PrivilegeType.SYSTEM, templateName, true); + case DropSchemaTemplate: + return checkGlobalStatus( + userEntity, + PrivilegeType.SYSTEM, + ((DropSchemaTemplatePlan) plan).getTemplateName(), + true); case CommitSetSchemaTemplate: templateName = ((CommitSetSchemaTemplatePlan) plan).getName(); return checkGlobalStatus(userEntity, PrivilegeType.SYSTEM, templateName, true); @@ -552,6 +559,12 @@ private TSStatus checkPermission(final ConfigPhysicalPlan plan) throws IOExcepti PrivilegeType.DELETE, ((CommitDeleteTablePlan) plan).getDatabase(), ((CommitDeleteTablePlan) plan).getTableName()); + case PipeDeleteDevices: + return checkTableStatus( + userEntity, + PrivilegeType.DELETE, + ((PipeDeleteDevicesPlan) plan).getDatabase(), + ((PipeDeleteDevicesPlan) plan).getTableName()); case GrantRole: case GrantUser: case RevokeUser: @@ -747,7 +760,7 @@ private TSStatus checkPermission(final ConfigPhysicalPlan plan) throws IOExcepti return checkGlobalStatus( userEntity, PrivilegeType.MANAGE_ROLE, ((AuthorPlan) plan).getRoleName(), true); default: - return StatusUtils.OK; + return RpcUtils.getStatus(TSStatusCode.NO_PERMISSION); } } @@ -1204,10 +1217,14 @@ private TSStatus executePlan(final ConfigPhysicalPlan plan) throws ConsensusExce .getPermissionManager() .operatePermission((AuthorPlan) plan, shouldMarkAsPipeRequest.get()); case CreateSchemaTemplate: - default: + case DropSchemaTemplate: + // Only explicitly supported config-region pipe plans may be written to consensus. New plan + // types must be added to an explicit case after their authorization is implemented. return configManager .getConsensusManager() .write(shouldMarkAsPipeRequest.get() ? new PipeEnrichedPlan(plan) : plan); + default: + return RpcUtils.getStatus(TSStatusCode.NO_PERMISSION); } } diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/meta/PipePluginMeta.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/meta/PipePluginMeta.java index 19f6863be3ca3..f1d8a27998bdd 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/meta/PipePluginMeta.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/meta/PipePluginMeta.java @@ -19,6 +19,8 @@ package org.apache.iotdb.commons.pipe.agent.plugin.meta; +import org.apache.iotdb.commons.utils.FileUtils; + import org.apache.tsfile.utils.PublicBAOS; import org.apache.tsfile.utils.ReadWriteIOUtils; @@ -26,6 +28,8 @@ import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; +import java.nio.file.Paths; +import java.util.Locale; import java.util.Objects; public class PipePluginMeta { @@ -52,22 +56,24 @@ public PipePluginMeta( String jarName, String jarMD5, String pluginLoadingExceptionMessage) { - this.pluginName = Objects.requireNonNull(pluginName).toUpperCase(); + this.pluginName = + validatePathSegment(Objects.requireNonNull(pluginName)).toUpperCase(Locale.ROOT); this.className = Objects.requireNonNull(className); this.isBuiltin = isBuiltin; if (isBuiltin) { - this.jarName = jarName; + this.jarName = jarName == null ? null : validatePathSegment(jarName); this.jarMD5 = jarMD5; } else { - this.jarName = Objects.requireNonNull(jarName); + this.jarName = validatePathSegment(Objects.requireNonNull(jarName)); this.jarMD5 = Objects.requireNonNull(jarMD5); } this.pluginLoadingExceptionMessage = pluginLoadingExceptionMessage; } public PipePluginMeta(String pluginName, String className) { - this.pluginName = Objects.requireNonNull(pluginName).toUpperCase(); + this.pluginName = + validatePathSegment(Objects.requireNonNull(pluginName)).toUpperCase(Locale.ROOT); this.className = Objects.requireNonNull(className); this.isBuiltin = true; @@ -195,4 +201,13 @@ public String toString() { + '\'' + '}'; } + + private static String validatePathSegment(final String pathSegment) { + final String pathError = FileUtils.getIllegalError4Directory(pathSegment); + if (pathError != null) { + throw new IllegalArgumentException(pathError); + } + Paths.get(pathSegment); + return pathSegment; + } } diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManager.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManager.java index 42b559f30d5e8..48f931e5db04b 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManager.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManager.java @@ -36,6 +36,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Locale; public class PipePluginExecutableManager extends ExecutableManager { @@ -46,7 +47,7 @@ public PipePluginExecutableManager(String temporaryLibRoot, String libRoot) { } public boolean isLocalJarMatched(PipePluginMeta pipePluginMeta) throws PipeException { - final String pluginName = pipePluginMeta.getPluginName(); + final String pluginName = validatePathSegment(pipePluginMeta.getPluginName()); final String md5FilePath = pluginName + ".txt"; if (hasFileUnderTemporaryRoot(md5FilePath)) { @@ -94,40 +95,33 @@ public static PipePluginExecutableManager getInstance() { } public boolean hasPluginFileUnderInstallDir(String pluginName, String fileName) { - return Files.exists(Paths.get(getPluginInstallPathV2(pluginName, fileName))); + return Files.exists(getPluginInstallPathV2Path(pluginName, fileName)); } public String getPluginsDirPath(String pluginName) { - return this.libRoot + File.separator + INSTALL_DIR + File.separator + pluginName.toUpperCase(); + return getPluginDirectoryPath(pluginName).toString(); } public void removePluginFileUnderLibRoot(String pluginName, String fileName) throws IOException { - String pluginPath = getPluginInstallPathV2(pluginName, fileName); - Path path = Paths.get(pluginPath); + final Path path = getPluginInstallPathV2Path(pluginName, fileName); Files.deleteIfExists(path); Files.deleteIfExists(path.getParent()); } public String getPluginInstallPathV2(String pluginName, String fileName) { - return this.libRoot - + File.separator - + INSTALL_DIR - + File.separator - + pluginName.toUpperCase() - + File.separator - + fileName; + return getPluginInstallPathV2Path(pluginName, fileName).toString(); } public String getPluginInstallPathV1(String fileName) { - return this.libRoot + File.separator + INSTALL_DIR + File.separator + fileName; + return resolvePathUnderDirectory(getInstallDirectoryPath(), fileName).toString(); } public void linkExistedPlugin( final String oldPluginName, final String newPluginName, final String fileName) throws IOException { FileUtils.createHardLink( - new File(getPluginsDirPath(oldPluginName), fileName), - new File(getPluginsDirPath(newPluginName), fileName)); + getPluginInstallPathV2Path(oldPluginName, fileName).toFile(), + getPluginInstallPathV2Path(newPluginName, fileName).toFile()); } /** @@ -138,7 +132,48 @@ public void linkExistedPlugin( */ public void savePluginToInstallDir(ByteBuffer byteBuffer, String pluginName, String fileName) throws IOException { - String destination = getPluginInstallPathV2(pluginName, fileName); - saveToDir(byteBuffer, destination); + saveToDir(byteBuffer, getPluginInstallPathV2Path(pluginName, fileName).toString()); + } + + private Path getPluginInstallPathV2Path(final String pluginName, final String fileName) { + return resolvePathUnderDirectory(getPluginDirectoryPath(pluginName), fileName); + } + + private Path getPluginDirectoryPath(final String pluginName) { + final String validatedPluginName = validatePathSegment(pluginName); + return resolvePathUnderDirectory( + getInstallDirectoryPath(), validatedPluginName.toUpperCase(Locale.ROOT)); + } + + private Path getInstallDirectoryPath() { + return Paths.get(libRoot, INSTALL_DIR).toAbsolutePath().normalize(); + } + + /** + * Resolves a single untrusted path segment below {@code baseDirectory}. + * + *

The segment validation rejects separators and dot segments, while the normalized containment + * check remains as a defense in depth for absolute paths and future callers. + */ + private Path resolvePathUnderDirectory(final Path baseDirectory, final String pathSegment) { + validatePathSegment(pathSegment); + + final Path normalizedBaseDirectory = baseDirectory.toAbsolutePath().normalize(); + final Path normalizedTargetPath = + normalizedBaseDirectory.resolve(pathSegment).toAbsolutePath().normalize(); + if (!normalizedTargetPath.startsWith(normalizedBaseDirectory)) { + throw new IllegalArgumentException( + String.format(PipeMessages.ILLEGAL_FILENAME_PATH_TRAVERSAL, pathSegment)); + } + return normalizedTargetPath; + } + + private String validatePathSegment(final String pathSegment) { + final String pathError = FileUtils.getIllegalError4Directory(pathSegment); + if (pathError != null) { + throw new IllegalArgumentException(pathError); + } + Paths.get(pathSegment); + return pathSegment; } } diff --git a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManagerTest.java b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManagerTest.java new file mode 100644 index 0000000000000..129c850d99ecb --- /dev/null +++ b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/agent/plugin/service/PipePluginExecutableManagerTest.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.commons.pipe.agent.plugin.service; + +import org.junit.Assert; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; +import java.util.stream.Stream; + +public class PipePluginExecutableManagerTest { + + @Test + public void testPluginPathsAreConfinedToInstallDirectory() throws Exception { + final Path root = Files.createTempDirectory("pipe-plugin-executable-manager-test"); + final Path temporaryLibRoot = root.resolve("temporary"); + final Path libRoot = root.resolve("lib"); + final PipePluginExecutableManager manager = + new PipePluginExecutableManager(temporaryLibRoot.toString(), libRoot.toString()); + final String traversalFileName = ".." + File.separator + ".." + File.separator + "outside.jar"; + final Path outsideFile = libRoot.resolve("outside.jar"); + final byte[] pluginContent = "plugin".getBytes(StandardCharsets.UTF_8); + + try { + Assert.assertEquals( + libRoot + .toAbsolutePath() + .normalize() + .resolve("install") + .resolve("TEST-PLUGIN") + .resolve("test.jar"), + Path.of(manager.getPluginInstallPathV2("test-plugin", "test.jar"))); + manager.savePluginToInstallDir(ByteBuffer.wrap(pluginContent), "test-plugin", "test.jar"); + Assert.assertArrayEquals( + pluginContent, + Files.readAllBytes(Path.of(manager.getPluginInstallPathV2("test-plugin", "test.jar")))); + + Assert.assertThrows( + IllegalArgumentException.class, + () -> manager.getPluginsDirPath(".." + File.separator + "outside")); + Assert.assertThrows( + IllegalArgumentException.class, + () -> manager.getPluginsDirPath(".." + otherFileSeparator() + "outside")); + Assert.assertThrows( + IllegalArgumentException.class, () -> manager.getPluginInstallPathV1(traversalFileName)); + Assert.assertThrows( + IllegalArgumentException.class, + () -> + manager.savePluginToInstallDir( + ByteBuffer.wrap(new byte[] {1}), "plugin", traversalFileName)); + Assert.assertThrows( + IllegalArgumentException.class, + () -> + manager.savePluginToInstallDir( + ByteBuffer.wrap(new byte[] {1}), + ".." + otherFileSeparator() + "plugin", + "test.jar")); + Assert.assertFalse(Files.exists(outsideFile)); + + Files.createDirectories(outsideFile.getParent()); + Files.write(outsideFile, "preserve".getBytes(StandardCharsets.UTF_8)); + Assert.assertThrows( + IllegalArgumentException.class, + () -> manager.removePluginFileUnderLibRoot("plugin", traversalFileName)); + Assert.assertArrayEquals( + "preserve".getBytes(StandardCharsets.UTF_8), Files.readAllBytes(outsideFile)); + + Assert.assertThrows( + IllegalArgumentException.class, + () -> manager.linkExistedPlugin("source", "target", traversalFileName)); + } finally { + deleteRecursively(root); + } + } + + private static String otherFileSeparator() { + return File.separatorChar == '/' ? "\\" : "/"; + } + + private static void deleteRecursively(final Path path) throws IOException { + try (final Stream stream = Files.walk(path)) { + for (final Path subPath : + (Iterable) stream.sorted(Comparator.reverseOrder())::iterator) { + Files.deleteIfExists(subPath); + } + } + } +} diff --git a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/plugin/meta/PipePluginMetaTest.java b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/plugin/meta/PipePluginMetaTest.java index b5b854adc4de4..4aa8c5739a9b1 100644 --- a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/plugin/meta/PipePluginMetaTest.java +++ b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/pipe/plugin/meta/PipePluginMetaTest.java @@ -22,6 +22,7 @@ import org.apache.iotdb.commons.pipe.agent.plugin.builtin.BuiltinPipePlugin; import org.apache.iotdb.commons.pipe.agent.plugin.meta.ConfigNodePipePluginMetaKeeper; import org.apache.iotdb.commons.pipe.agent.plugin.meta.DataNodePipePluginMetaKeeper; +import org.apache.iotdb.commons.pipe.agent.plugin.meta.PipePluginMeta; import org.junit.Assert; import org.junit.Test; @@ -58,4 +59,20 @@ public void testDataNodePipePluginMetaKeeper() { BuiltinPipePlugin.IOTDB_EXTRACTOR.getPipePluginClass(), keeper.getBuiltinPluginClass(BuiltinPipePlugin.IOTDB_EXTRACTOR.getPipePluginName())); } + + @Test + public void testRejectPathTraversalInPluginMetadata() { + Assert.assertThrows( + IllegalArgumentException.class, + () -> new PipePluginMeta("../plugin", "test.Plugin", false, "test.jar", "md5")); + Assert.assertThrows( + IllegalArgumentException.class, + () -> new PipePluginMeta("plugin", "test.Plugin", false, "../test.jar", "md5")); + Assert.assertThrows( + IllegalArgumentException.class, + () -> new PipePluginMeta("plugin", "test.Plugin", false, "..\\test.jar", "md5")); + Assert.assertThrows( + IllegalArgumentException.class, + () -> new PipePluginMeta("plugin\0", "test.Plugin", false, "test.jar", "md5")); + } }