From 1e0544e4a104587db24f474c6b5c79ebbd78d9f3 Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Tue, 10 Sep 2024 12:30:06 +0800 Subject: [PATCH] [fix](metadata)Add FE metadata-related file checks --- .../java/org/apache/doris/common/Config.java | 4 ++ .../org/apache/doris/master/MetaHelper.java | 44 ++++++++++++----- .../apache/doris/master/MetaHelperTest.java | 49 +++++++++++++++++++ 3 files changed, 86 insertions(+), 11 deletions(-) diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java index 360ec7ae6035c7..f12fff59c0355f 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java +++ b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java @@ -3052,4 +3052,8 @@ public static int metaServiceRpcRetryTimes() { @ConfField(mutable = true, description = {"表示最大锁持有时间，超过该时间会打印告警日志，单位秒", "Maximum lock hold time; logs a warning if exceeded"}) public static long max_lock_hold_threshold_seconds = 10; + + @ConfField(mutable = true, description = {"元数据同步是否开启安全模式", + "Is metadata synchronization enabled in safe mode"}) + public static boolean meta_helper_security_mode = false; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java b/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java index 81c2bb25856434..cf63a82cd870d8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java +++ b/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java @@ -18,6 +18,7 @@ package org.apache.doris.master; import org.apache.doris.catalog.Env; +import org.apache.doris.common.Config; import org.apache.doris.common.io.IOUtils; import org.apache.doris.common.util.HttpURLUtil; import org.apache.doris.httpv2.entity.ResponseBody; @@ -46,7 +47,7 @@ public class MetaHelper { public static final String X_IMAGE_MD5 = "X-Image-Md5"; private static final int BUFFER_BYTES = 8 * 1024; private static final int CHECKPOINT_LIMIT_BYTES = 30 * 1024 * 1024; - private static final String VALID_FILENAME_REGEX = "^image\\.\\d+(\\.part)?$"; + private static final String VALID_FILENAME_REGEX = "^image\\.\\d+(\\.part)?$"; public static File getMasterImageDir() { @@ -58,14 +59,10 @@ public static int getLimit() { return CHECKPOINT_LIMIT_BYTES; } - // rename the .PART_SUFFIX file to filename - public static File complete(String filename, File dir) throws IOException { - // Validate that the filename does not contain illegal path elements - checkIsValidFileName(filename); - - File file = new File(dir, filename + MetaHelper.PART_SUFFIX); // Original file with a specific suffix - File newFile = new File(dir, filename); // Target file without the suffix - + private static void completeCheck(File dir, File file, File newFile) throws IOException { + if (!Config.meta_helper_security_mode) { + return; + } String dirPath = dir.getCanonicalPath(); // Get the canonical path of the directory String filePath = file.getCanonicalPath(); // Get the canonical path of the original file String newFilePath = newFile.getCanonicalPath(); // Get the canonical path of the new file @@ -80,6 +77,17 @@ public static File complete(String filename, File dir) throws IOException { throw new IOException("Source file does not exist or is not a valid file."); } + } + + // rename the .PART_SUFFIX file to filename + public static File complete(String filename, File dir) throws IOException { + // Validate that the filename does not contain illegal path elements + checkIsValidFileName(filename); + + File file = new File(dir, filename + MetaHelper.PART_SUFFIX); // Original file with a specific suffix + File newFile = new File(dir, filename); // Target file without the suffix + + completeCheck(dir, file, newFile); // Attempt to rename the file. If it fails, throw an exception if (!file.renameTo(newFile)) { throw new IOException("Complete file " + filename + " failed"); @@ -91,22 +99,36 @@ public static File complete(String filename, File dir) throws IOException { public static File getFile(String filename, File dir) throws IOException { checkIsValidFileName(filename); File file = new File(dir, filename + MetaHelper.PART_SUFFIX); + checkFile(dir, file); + return file; + } + + private static void checkFile(File dir, File file) throws IOException { + if (!Config.meta_helper_security_mode) { + return; + } String dirPath = dir.getCanonicalPath(); String filePath = file.getCanonicalPath(); if (!filePath.startsWith(dirPath)) { throw new SecurityException("File path traversal attempt detected."); } - return file; } + private static void checkIsValidFileName(String filename) { + if (!Config.meta_helper_security_mode) { + return; + } if (!filename.matches(VALID_FILENAME_REGEX)) { throw new IllegalArgumentException("Invalid filename"); } } private static void checkFile(File file) throws IOException { + if (!Config.meta_helper_security_mode) { + return; + } if (!file.getAbsolutePath().startsWith(file.getCanonicalFile().getParent())) { throw new IllegalArgumentException("Invalid file path"); } @@ -175,7 +197,7 @@ public static void getRemoteFile(String urlStr, int timeout, File file) if (out != null) { out.close(); } - if (!md5Matched && file.exists()) { + if (!md5Matched && file.exists() & Config.meta_helper_security_mode) { file.delete(); } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java b/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java index 070979494bfd6c..ae2e1a9d58aaf7 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java @@ -17,6 +17,7 @@ package org.apache.doris.master; +import org.apache.doris.common.Config; import org.apache.doris.httpv2.entity.ResponseBody; import org.apache.doris.httpv2.rest.RestApiStatusCode; import org.apache.doris.persist.StorageInfo; @@ -25,6 +26,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Assert; import org.junit.Test; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; + +import java.io.File; +import java.io.IOException; public class MetaHelperTest { @@ -49,4 +55,47 @@ private ResponseBody buildResponseBody() { bodyBefore.setMsg("msg"); return bodyBefore; } + + File tempDir = new File(System.getProperty("java.io.tmpdir"), "tempDir"); + + @BeforeEach + void setUp() { + + if (tempDir.exists()) { + tempDir.delete(); + } + tempDir.mkdir(); + } + + @Test + public void testFile() throws IOException { + + String errorFilename = "testfile."; + File errorFileWithSuffix = new File(tempDir, errorFilename); + String rightFilename = "image.1"; + File rightFileWithSuffix = new File(tempDir, rightFilename); + + Config.meta_helper_security_mode = true; + + if (errorFileWithSuffix.exists()) { + errorFileWithSuffix.delete(); + } + Assert.assertTrue(errorFileWithSuffix.createNewFile()); + Assert.assertThrows(IllegalArgumentException.class, () -> MetaHelper.complete(errorFilename, tempDir)); + Assert.assertThrows(IllegalArgumentException.class, () -> MetaHelper.getFile(errorFilename, tempDir)); + if (rightFileWithSuffix.exists()) { + rightFileWithSuffix.delete(); + } + Assert.assertTrue(rightFileWithSuffix.createNewFile()); + Assert.assertEquals(rightFileWithSuffix.getName() + ".part", MetaHelper.getFile(rightFilename, tempDir).getName()); + + } + + @AfterEach + public void tearDown() { + if (tempDir.exists()) { + tempDir.delete(); + } + } + }