This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new 16d5f027695 branch-3.0: [Fix](http)Enhanced Security Checks for Audit
Log File Names #44612 (#44832)
16d5f027695 is described below
commit 16d5f027695d474ca73b8acc45704f702585a434
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Dec 6 22:12:25 2024 -0800
branch-3.0: [Fix](http)Enhanced Security Checks for Audit Log File Names
#44612 (#44832)
Cherry-picked from #44612
Co-authored-by: Calvin Kirs <[email protected]>
---
.../apache/doris/httpv2/rest/GetLogFileAction.java | 38 +++++++++++++-
.../apache/doris/httpv2/GetLogFileActionTest.java | 60 ++++++++++++++++++++++
2 files changed, 97 insertions(+), 1 deletion(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java
b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java
index 475ee5ace1e..87c4c4cfa90 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/GetLogFileAction.java
@@ -32,6 +32,8 @@ import org.springframework.web.bind.annotation.RestController;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.Map;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
@@ -51,6 +53,23 @@ import javax.servlet.http.HttpServletResponse;
*/
@RestController
public class GetLogFileAction extends RestBaseController {
+ /**
+ * This method fetches internal logs via HTTP, which is no longer
recommended and will
+ * be deprecated in future versions.
+ * <p>
+ * Using HTTP to fetch logs introduces serious security and performance
issues:
+ * - **Security Risks**: Log content may expose sensitive information,
allowing attackers to exploit the exposed
+ * HTTP endpoints.
+ * - **Performance Problems**: Frequent HTTP requests can cause
significant system load, affecting the
+ * responsiveness and stability of the application.
+ * <p>
+ * It is strongly advised not to use this approach for accessing logs. Any
new requirements should be
+ * handled using more secure, reliable, and efficient methods such as log
aggregation tools (e.g., ELK, Splunk)
+ * or dedicated internal APIs.
+ * <p>
+ * **Note**: No new HTTP endpoints or types for log access will be
accepted.
+ * Any further attempts to extend this HTTP-based log retrieval method
will not be supported.
+ */
private final Set<String> logFileTypes = Sets.newHashSet("fe.audit.log");
@RequestMapping(path = "/api/get_log_file", method = {RequestMethod.GET,
RequestMethod.HEAD})
@@ -79,7 +98,13 @@ public class GetLogFileAction extends RestBaseController {
String fileInfos = getFileInfos(logType);
response.setHeader("file_infos", fileInfos);
return ResponseEntityBuilder.ok();
- } else if (method.equals(RequestMethod.GET.name())) {
+ }
+ if (method.equals(RequestMethod.GET.name())) {
+ try {
+ checkAuditLogFileName(logFile);
+ } catch (SecurityException e) {
+ return ResponseEntityBuilder.internalError(e.getMessage());
+ }
File log = getLogFile(logType, logFile);
if (!log.exists() || !log.isFile()) {
return ResponseEntityBuilder.okWithCommonError("Log file not
exist: " + log.getName());
@@ -97,6 +122,17 @@ public class GetLogFileAction extends RestBaseController {
return ResponseEntityBuilder.ok();
}
+ private void checkAuditLogFileName(String logFile) {
+ if (!logFile.matches("^[a-zA-Z0-9._-]+$")) {
+ throw new SecurityException("Invalid file name");
+ }
+ Path normalizedPath =
Paths.get(Config.audit_log_dir).resolve(logFile).normalize();
+ // check path is valid or not
+ if (!normalizedPath.startsWith(Config.audit_log_dir)) {
+ throw new SecurityException("Invalid file path: Access outside of
permitted directory");
+ }
+ }
+
private String getFileInfos(String logType) {
Map<String, Long> fileInfos = Maps.newTreeMap();
if (logType.equals("fe.audit.log")) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/httpv2/GetLogFileActionTest.java
b/fe/fe-core/src/test/java/org/apache/doris/httpv2/GetLogFileActionTest.java
new file mode 100644
index 00000000000..8d4cac9b6ad
--- /dev/null
+++ b/fe/fe-core/src/test/java/org/apache/doris/httpv2/GetLogFileActionTest.java
@@ -0,0 +1,60 @@
+// 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.doris.httpv2;
+
+import org.apache.doris.common.Config;
+import org.apache.doris.httpv2.rest.GetLogFileAction;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+public class GetLogFileActionTest {
+
+ @TempDir
+ public File tempDir;
+
+ @BeforeAll
+ public static void before() {
+ File tempDir = new File("test/audit.log");
+ tempDir.mkdir();
+ Config.audit_log_dir = tempDir.getAbsolutePath();
+ }
+
+ @Test
+ public void testCheckAuditLogFileName() throws NoSuchMethodException,
InvocationTargetException, IllegalAccessException {
+ //private method checkAuditLogFileName
+ GetLogFileAction action = new GetLogFileAction();
+ Method method =
GetLogFileAction.class.getDeclaredMethod("checkAuditLogFileName", String.class);
+ method.setAccessible(true);
+ method.invoke(action, "audit.log");
+ method.invoke(action, "fe.audit.log.20241104-1");
+ Assertions.assertThrows(InvocationTargetException.class, () ->
method.invoke(action, "../etc/passwd"));
+ Assertions.assertThrows(InvocationTargetException.class, () ->
method.invoke(action,
+ "fe.audit.log.20241104-1/../../etc/passwd"));
+ Assertions.assertThrows(InvocationTargetException.class,
+ () -> method.invoke(action, "fe.audit.log.20241104-1; rm -rf
/"));
+
+
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
