Skip to content

Commit

Permalink
branch-3.0: [Fix](http)Enhanced Security Checks for Audit Log File Na…
Browse files Browse the repository at this point in the history
…mes #44612 (#44832)

Cherry-picked from #44612

Co-authored-by: Calvin Kirs <[email protected]>
  • Loading branch information
github-actions[bot] and CalvinKirs authored Dec 7, 2024
1 parent a364a71 commit 16d5f02
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

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;
Expand All @@ -51,6 +53,23 @@
*/
@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})
Expand Down Expand Up @@ -79,7 +98,13 @@ public Object execute(HttpServletRequest request, HttpServletResponse response)
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());
Expand All @@ -97,6 +122,17 @@ public Object execute(HttpServletRequest request, HttpServletResponse response)
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")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 /"));


}
}

0 comments on commit 16d5f02

Please sign in to comment.