From 3b30f3bb7e156597b85db0a96fe00d65a97d5b7c Mon Sep 17 00:00:00 2001 From: gaojun2048 Date: Mon, 14 Mar 2022 13:15:14 +0800 Subject: [PATCH] fix log read bug (#8852) * fix log * get DOLPHINSCHEDULER_HOME * add DOLPHINSCHEDULER_WORKER_HOME to env --- .../server/log/LoggerRequestProcessor.java | 38 +++++++++++-- .../log/LoggerRequestProcessorTest.java | 56 ++++++++++++++++++- dolphinscheduler-worker/src/main/bin/start.sh | 2 + 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/dolphinscheduler-log-server/src/main/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessor.java b/dolphinscheduler-log-server/src/main/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessor.java index 9c720c5471..5160c86043 100644 --- a/dolphinscheduler-log-server/src/main/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessor.java +++ b/dolphinscheduler-log-server/src/main/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessor.java @@ -76,21 +76,35 @@ public class LoggerRequestProcessor implements NettyRequestProcessor { case GET_LOG_BYTES_REQUEST: GetLogBytesRequestCommand getLogRequest = JSONUtils.parseObject( command.getBody(), GetLogBytesRequestCommand.class); - byte[] bytes = getFileContentBytes(getLogRequest.getPath()); + String path = getLogRequest.getPath(); + if (!checkPathSecurity(path)) { + throw new IllegalArgumentException("Illegal path"); + } + byte[] bytes = getFileContentBytes(path); GetLogBytesResponseCommand getLogResponse = new GetLogBytesResponseCommand(bytes); channel.writeAndFlush(getLogResponse.convert2Command(command.getOpaque())); break; case VIEW_WHOLE_LOG_REQUEST: ViewLogRequestCommand viewLogRequest = JSONUtils.parseObject( command.getBody(), ViewLogRequestCommand.class); - String msg = LoggerUtils.readWholeFileContent(viewLogRequest.getPath()); + String viewLogPath = viewLogRequest.getPath(); + if (!checkPathSecurity(viewLogPath)) { + throw new IllegalArgumentException("Illegal path"); + } + String msg = LoggerUtils.readWholeFileContent(viewLogPath); ViewLogResponseCommand viewLogResponse = new ViewLogResponseCommand(msg); channel.writeAndFlush(viewLogResponse.convert2Command(command.getOpaque())); break; case ROLL_VIEW_LOG_REQUEST: RollViewLogRequestCommand rollViewLogRequest = JSONUtils.parseObject( command.getBody(), RollViewLogRequestCommand.class); - List lines = readPartFileContent(rollViewLogRequest.getPath(), + + String rollViewLogPath = rollViewLogRequest.getPath(); + if (!checkPathSecurity(rollViewLogPath)) { + throw new IllegalArgumentException("Illegal path"); + } + + List lines = readPartFileContent(rollViewLogPath, rollViewLogRequest.getSkipLineNum(), rollViewLogRequest.getLimit()); StringBuilder builder = new StringBuilder(); for (String line : lines) { @@ -104,7 +118,9 @@ public class LoggerRequestProcessor implements NettyRequestProcessor { command.getBody(), RemoveTaskLogRequestCommand.class); String taskLogPath = removeTaskLogRequest.getPath(); - + if (!checkPathSecurity(taskLogPath)) { + throw new IllegalArgumentException("Illegal path"); + } File taskLogFile = new File(taskLogPath); boolean status = true; try { @@ -123,6 +139,20 @@ public class LoggerRequestProcessor implements NettyRequestProcessor { } } + /** + * LogServer only can read the logs dir. + * @param path + * @return + */ + private boolean checkPathSecurity(String path) { + String dsHome = System.getProperty("DOLPHINSCHEDULER_WORKER_HOME"); + if (path.startsWith(dsHome) && !path.contains("../") && path.endsWith(".log")) { + return true; + } + + return false; + } + public ExecutorService getExecutor() { return this.executor; } diff --git a/dolphinscheduler-log-server/src/test/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessorTest.java b/dolphinscheduler-log-server/src/test/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessorTest.java index 84baed34eb..fa095231f8 100644 --- a/dolphinscheduler-log-server/src/test/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessorTest.java +++ b/dolphinscheduler-log-server/src/test/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessorTest.java @@ -38,12 +38,66 @@ public class LoggerRequestProcessorTest { @Test public void testProcessViewWholeLogRequest() { + System.setProperty("DOLPHINSCHEDULER_WORKER_HOME", System.getProperty("user.dir")); Channel channel = PowerMockito.mock(Channel.class); PowerMockito.when(channel.writeAndFlush(Mockito.any(Command.class))).thenReturn(null); PowerMockito.mockStatic(LoggerUtils.class); PowerMockito.when(LoggerUtils.readWholeFileContent(Mockito.anyString())).thenReturn(""); + String userDir = System.getProperty("user.dir"); + ViewLogRequestCommand logRequestCommand = new ViewLogRequestCommand(userDir + "/log/path/a.log"); - ViewLogRequestCommand logRequestCommand = new ViewLogRequestCommand("/log/path"); + Command command = new Command(); + command.setType(CommandType.VIEW_WHOLE_LOG_REQUEST); + command.setBody(JSONUtils.toJsonByteArray(logRequestCommand)); + + LoggerRequestProcessor loggerRequestProcessor = new LoggerRequestProcessor(); + loggerRequestProcessor.process(channel, command); + } + + @Test(expected = IllegalArgumentException.class) + public void testProcessViewWholeLogRequestError() { + System.setProperty("DOLPHINSCHEDULER_WORKER_HOME", System.getProperty("user.dir")); + Channel channel = PowerMockito.mock(Channel.class); + PowerMockito.when(channel.writeAndFlush(Mockito.any(Command.class))).thenReturn(null); + PowerMockito.mockStatic(LoggerUtils.class); + PowerMockito.when(LoggerUtils.readWholeFileContent(Mockito.anyString())).thenReturn(""); + String userDir = System.getProperty("user.dir"); + ViewLogRequestCommand logRequestCommand = new ViewLogRequestCommand(userDir + "/log/path/a"); + + Command command = new Command(); + command.setType(CommandType.VIEW_WHOLE_LOG_REQUEST); + command.setBody(JSONUtils.toJsonByteArray(logRequestCommand)); + + LoggerRequestProcessor loggerRequestProcessor = new LoggerRequestProcessor(); + loggerRequestProcessor.process(channel, command); + } + + @Test(expected = IllegalArgumentException.class) + public void testProcessViewWholeLogRequestErrorRelativePath() { + System.setProperty("DOLPHINSCHEDULER_WORKER_HOME", System.getProperty("user.dir")); + Channel channel = PowerMockito.mock(Channel.class); + PowerMockito.when(channel.writeAndFlush(Mockito.any(Command.class))).thenReturn(null); + PowerMockito.mockStatic(LoggerUtils.class); + PowerMockito.when(LoggerUtils.readWholeFileContent(Mockito.anyString())).thenReturn(""); + String userDir = System.getProperty("user.dir"); + ViewLogRequestCommand logRequestCommand = new ViewLogRequestCommand(userDir + "/log/../../a.log"); + + Command command = new Command(); + command.setType(CommandType.VIEW_WHOLE_LOG_REQUEST); + command.setBody(JSONUtils.toJsonByteArray(logRequestCommand)); + + LoggerRequestProcessor loggerRequestProcessor = new LoggerRequestProcessor(); + loggerRequestProcessor.process(channel, command); + } + + @Test(expected = IllegalArgumentException.class) + public void testProcessViewWholeLogRequestErrorStartWith() { + System.setProperty("DOLPHINSCHEDULER_WORKER_HOME", System.getProperty("user.dir")); + Channel channel = PowerMockito.mock(Channel.class); + PowerMockito.when(channel.writeAndFlush(Mockito.any(Command.class))).thenReturn(null); + PowerMockito.mockStatic(LoggerUtils.class); + PowerMockito.when(LoggerUtils.readWholeFileContent(Mockito.anyString())).thenReturn(""); + ViewLogRequestCommand logRequestCommand = new ViewLogRequestCommand("/log/a.log"); Command command = new Command(); command.setType(CommandType.VIEW_WHOLE_LOG_REQUEST); diff --git a/dolphinscheduler-worker/src/main/bin/start.sh b/dolphinscheduler-worker/src/main/bin/start.sh index 0f21c64f80..4ec32d821c 100644 --- a/dolphinscheduler-worker/src/main/bin/start.sh +++ b/dolphinscheduler-worker/src/main/bin/start.sh @@ -19,6 +19,8 @@ BIN_DIR=$(dirname $0) DOLPHINSCHEDULER_HOME=${DOLPHINSCHEDULER_HOME:-$(cd $BIN_DIR/..; pwd)} +export DOLPHINSCHEDULER_WORK_HOME=${DOLPHINSCHEDULER_HOME} + JAVA_OPTS=${JAVA_OPTS:-"-server -Xms4g -Xmx4g -Xmn2g -XX:+PrintGCDetails -Xloggc:gc.log -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=dump.hprof"} if [[ "$DOCKER" == "true" ]]; then