This is an automated email from the ASF dual-hosted git repository. liyang-kylin pushed a commit to branch kylin5 in repository https://gitbox.apache.org/repos/asf/kylin.git
commit dbf365d5a3da1ed589b30eb93d7dade43b4bc2a9 Author: Li Yang <[email protected]> AuthorDate: Mon Oct 5 13:55:16 2026 +0800 KYLIN-6117 require service token and confine backup_dir on metadata backup RPC Gate the internal metadata-backup broadcast RPC endpoints behind a global admin or a valid broadcast service token, attach the token on the internal senders, and reject a caller-supplied backup_dir that is blank, contains a path separator, or is '.'/'..' before any HDFS delete or write. --- .../kylin/rest/controller/NSystemController.java | 8 ++++++-- .../rest/controller/NSystemControllerTest.java | 22 ++++++++++++++++++---- .../org/apache/kylin/rest/service/FileService.java | 15 +++++++++++++++ .../apache/kylin/rest/service/FileServiceTest.java | 7 +++++++ .../apache/kylin/rest/service/ScheduleService.java | 5 +++++ 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java b/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java index 0f47deb3df..6616c611af 100644 --- a/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java +++ b/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java @@ -44,6 +44,7 @@ import org.apache.kylin.metadata.project.EnhancedUnitOfWork; import org.apache.kylin.metadata.project.ProjectInstance; import org.apache.kylin.rest.request.MetadataBackupRequest; import org.apache.kylin.rest.response.EnvelopeResponse; +import org.apache.kylin.rest.security.InternalRpcSecurity; import org.apache.kylin.rest.service.FileService; import org.apache.kylin.rest.service.ProjectService; import org.apache.kylin.rest.service.ScheduleService; @@ -187,9 +188,11 @@ public class NSystemController extends NBasicController { */ @PostMapping(value = "broadcast_metadata_backup") @ResponseBody - public EnvelopeResponse<String> broadcastMetadataBackup(@RequestBody MetadataBackupRequest request) { + public EnvelopeResponse<String> broadcastMetadataBackup(@RequestBody MetadataBackupRequest request, + HttpServletRequest servletRequest) { log.info("ResourceGroup[{}] broadcastMetadataBackup tmpFilePath : {}", request.getResourceGroupId(), request.getTmpFilePath()); + InternalRpcSecurity.requireGlobalAdminOrServiceToken(servletRequest); checkServer(request.getFromHost()); fileService.saveBroadcastMetadataBackup(request.getBackupDir(), request.getTmpFilePath(), request.getTmpFileSize(), request.getResourceGroupId(), request.getFromHost()); @@ -205,9 +208,10 @@ public class NSystemController extends NBasicController { @PostMapping(value = "metadata_backup_tmp_file") @ResponseBody public EnvelopeResponse<String> downloadMetadataBackTmpFile(@RequestBody MetadataBackupRequest request, - HttpServletResponse response) throws IOException { + HttpServletRequest servletRequest, HttpServletResponse response) throws IOException { log.info("ResourceGroup[{}] downloadMetadataBackTmpFile tmpFilePath : {}", request.getResourceGroupId(), request.getTmpFilePath()); + InternalRpcSecurity.requireGlobalAdminOrServiceToken(servletRequest); InputStream backupInputStream = fileService.getMetadataBackupFromTmpPath(request.getTmpFilePath(), request.getTmpFileSize()); setDownloadResponse(backupInputStream, METADATA_FILE, MediaType.APPLICATION_OCTET_STREAM_VALUE, response); diff --git a/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java b/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java index 11b63c2d2b..9aec46f801 100644 --- a/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java +++ b/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java @@ -20,6 +20,7 @@ package org.apache.kylin.rest.controller; import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_JSON; import org.apache.kylin.common.KylinConfigBase; +import org.apache.kylin.common.exception.KylinException; import org.apache.kylin.common.util.JsonUtil; import org.apache.kylin.common.util.NLocalFileMetadataTestCase; import org.apache.kylin.junit.rule.TransactionExceptedException; @@ -27,6 +28,7 @@ import org.apache.kylin.rest.constant.Constant; import org.apache.kylin.rest.request.MetadataBackupRequest; import org.apache.kylin.rest.service.SystemService; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -35,6 +37,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.springframework.http.MediaType; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.test.web.servlet.MockMvc; @@ -107,21 +110,32 @@ public class NSystemControllerTest extends NLocalFileMetadataTestCase { @Test public void broadcastMetadataBackup() throws Exception { MetadataBackupRequest request = new MetadataBackupRequest(); - Mockito.doAnswer(x -> null).when(nSystemController).broadcastMetadataBackup(Mockito.any()); + Mockito.doAnswer(x -> null).when(nSystemController).broadcastMetadataBackup(Mockito.any(), Mockito.any()); mockMvc.perform(MockMvcRequestBuilders.post("/api/system/broadcast_metadata_backup") .contentType(MediaType.APPLICATION_JSON).accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON)) .content(JsonUtil.writeValueAsString(request))).andExpect(MockMvcResultMatchers.status().isOk()); - Mockito.verify(nSystemController).broadcastMetadataBackup(Mockito.any(MetadataBackupRequest.class)); + Mockito.verify(nSystemController).broadcastMetadataBackup(Mockito.any(MetadataBackupRequest.class), + Mockito.any()); + } + + @Test + public void broadcastMetadataBackupRequiresAdminOrServiceToken() { + SecurityContextHolder.getContext() + .setAuthentication(new TestingAuthenticationToken("analyst", "analyst", Constant.ROLE_ANALYST)); + MetadataBackupRequest request = new MetadataBackupRequest(); + Assert.assertThrows(KylinException.class, + () -> nSystemController.broadcastMetadataBackup(request, new MockHttpServletRequest())); } @Test public void downloadMetadataBackTmpFile() throws Exception { MetadataBackupRequest request = new MetadataBackupRequest(); - Mockito.doAnswer(x -> null).when(nSystemController).downloadMetadataBackTmpFile(Mockito.any(), Mockito.any()); + Mockito.doAnswer(x -> null).when(nSystemController).downloadMetadataBackTmpFile(Mockito.any(), Mockito.any(), + Mockito.any()); mockMvc.perform(MockMvcRequestBuilders.post("/api/system/metadata_backup_tmp_file") .contentType(MediaType.APPLICATION_JSON).accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON)) .content(JsonUtil.writeValueAsString(request))).andExpect(MockMvcResultMatchers.status().isOk()); Mockito.verify(nSystemController).downloadMetadataBackTmpFile(Mockito.any(MetadataBackupRequest.class), - Mockito.any()); + Mockito.any(), Mockito.any()); } } diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/service/FileService.java b/src/common-service/src/main/java/org/apache/kylin/rest/service/FileService.java index 894b87cb47..4cd09dc4a5 100644 --- a/src/common-service/src/main/java/org/apache/kylin/rest/service/FileService.java +++ b/src/common-service/src/main/java/org/apache/kylin/rest/service/FileService.java @@ -22,6 +22,7 @@ import static org.apache.kylin.common.constant.Constants.BACKSLASH; import static org.apache.kylin.common.constant.Constants.METADATA_FILE; import static org.apache.kylin.common.constant.Constants.SYSTEM_TMP_DIR; import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON; +import static org.apache.kylin.common.exception.ServerErrorCode.PERMISSION_DENIED; import java.io.File; import java.io.FileNotFoundException; @@ -38,7 +39,9 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.fs.Path; import org.apache.kylin.common.KylinConfig; +import org.apache.kylin.common.exception.KylinException; import org.apache.kylin.common.exception.KylinRuntimeException; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; import org.apache.kylin.common.util.HadoopUtil; import org.apache.kylin.common.util.JsonUtil; import org.apache.kylin.common.util.Pair; @@ -171,6 +174,10 @@ public class FileService extends BasicService { val httpHeaders = new HttpHeaders(); httpHeaders.add(HttpHeaders.CONTENT_TYPE, HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON); httpHeaders.setAccept(Arrays.asList(MediaType.APPLICATION_OCTET_STREAM, MediaType.ALL)); + String broadcastToken = KylinConfig.getInstanceFromEnv().getBroadcastToken(); + if (StringUtils.isNotBlank(broadcastToken)) { + httpHeaders.add(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, broadcastToken); + } val httpEntity = new HttpEntity<>(JsonUtil.writeValueAsBytes(req), httpHeaders); RequestCallback requestCallback = restTemplate.httpEntityCallback(httpEntity); return restTemplate.execute(url, HttpMethod.POST, requestCallback, clientResponse -> { @@ -182,6 +189,7 @@ public class FileService extends BasicService { public void saveBroadcastMetadataBackup(String backupDir, String filePath, Long fileSize, String resourceGroupId, String fromHost) { + checkBackupDir(backupDir); var tmpFilePath = ""; try { tmpFilePath = downloadMetadataBackTmpFile(filePath, fileSize, resourceGroupId, fromHost); @@ -198,4 +206,11 @@ public class FileService extends BasicService { } } } + + private static void checkBackupDir(String backupDir) { + if (StringUtils.isBlank(backupDir) || backupDir.contains(BACKSLASH) || backupDir.contains("\\") + || ".".equals(backupDir) || "..".equals(backupDir)) { + throw new KylinException(PERMISSION_DENIED, "Illegal metadata backup directory: " + backupDir); + } + } } diff --git a/src/common-service/src/test/java/org/apache/kylin/rest/service/FileServiceTest.java b/src/common-service/src/test/java/org/apache/kylin/rest/service/FileServiceTest.java index e2332ff5c0..e8be154227 100644 --- a/src/common-service/src/test/java/org/apache/kylin/rest/service/FileServiceTest.java +++ b/src/common-service/src/test/java/org/apache/kylin/rest/service/FileServiceTest.java @@ -45,6 +45,7 @@ import java.util.Objects; import org.apache.hadoop.fs.Path; import org.apache.kylin.common.KylinConfig; +import org.apache.kylin.common.exception.KylinException; import org.apache.kylin.common.util.AddressUtil; import org.apache.kylin.common.util.HadoopUtil; import org.apache.kylin.common.util.Pair; @@ -459,4 +460,10 @@ class FileServiceTest { assertTrue(exception.getMessage().contains("Path not kylin metadata tmp directory")); } } + + @Test + void saveBroadcastMetadataBackupRejectsTraversal() { + assertThrows(KylinException.class, () -> fileService.saveBroadcastMetadataBackup("../evil", "tmp-file", 3L, + RandomUtil.randomUUIDStr(), AddressUtil.getLocalInstance())); + } } \ No newline at end of file diff --git a/src/job-service/src/main/java/org/apache/kylin/rest/service/ScheduleService.java b/src/job-service/src/main/java/org/apache/kylin/rest/service/ScheduleService.java index 31ee5b0fa6..079639a6bc 100644 --- a/src/job-service/src/main/java/org/apache/kylin/rest/service/ScheduleService.java +++ b/src/job-service/src/main/java/org/apache/kylin/rest/service/ScheduleService.java @@ -53,6 +53,7 @@ import org.apache.kylin.common.exception.KylinRuntimeException; import org.apache.kylin.common.metrics.MetricsCategory; import org.apache.kylin.common.metrics.MetricsGroup; import org.apache.kylin.common.metrics.MetricsName; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; import org.apache.kylin.common.response.RestResponse; import org.apache.kylin.common.util.AddressUtil; import org.apache.kylin.common.util.JsonUtil; @@ -334,6 +335,10 @@ public class ScheduleService extends BasicService { req.put("from_host", AddressUtil.getLocalInstance()); val httpHeaders = new HttpHeaders(); httpHeaders.add(HttpHeaders.CONTENT_TYPE, HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON); + String broadcastToken = KylinConfig.getInstanceFromEnv().getBroadcastToken(); + if (StringUtils.isNotBlank(broadcastToken)) { + httpHeaders.add(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, broadcastToken); + } val exchange = restTemplate.exchange(url, HttpMethod.POST, new HttpEntity<>(JsonUtil.writeValueAsBytes(req), httpHeaders), String.class); receive(exchange, "noticeToTenantNode");
