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 e05034536062c77e44de8139c33478a75e54241e Author: Li Yang <[email protected]> AuthorDate: Fri Oct 2 13:25:51 2026 +0800 KYLIN-6107 improve broadcast access control --- .../src/main/resources/config/init.properties | 2 + .../src/main/resources/config/init_min.properties | 2 + .../rest/config/initialize/BroadcastListener.java | 3 +- .../kylin/rest/controller/BroadcastController.java | 34 +++++- .../rest/controller/BroadcastEventValidator.java | 77 +++++++++++++ .../rest/controller/BroadcastControllerTest.java | 123 +++++++++++++++++++++ .../rest/security/BroadcastSecurityContext.java | 76 +++++++++++++ .../apache/kylin/rest/service/AccessService.java | 8 +- .../apache/kylin/rest/service/AclTCRService.java | 8 +- .../kylin/rest/service/AccessServiceTest.java | 15 +++ .../org/apache/kylin/common/KylinConfigBase.java | 6 +- .../transaction/BroadcastEventReadyNotifier.java | 2 + .../apache/kylin/tool/restclient/RestClient.java | 5 + .../src/main/resources/kylin-defaults0.properties | 2 + .../kylin/tool/restclient/RestClientTest.java | 68 ++++++++++++ .../src/main/resources/config/init.properties | 2 + .../src/main/resources/config/init_min.properties | 2 + .../src/main/resources/config/init.properties | 2 + .../src/main/resources/config/init_min.properties | 2 + .../src/main/resources/config/init.properties | 2 + .../src/main/resources/config/init_min.properties | 2 + .../src/main/resources/config/init.properties | 2 + .../src/main/resources/config/init_min.properties | 2 + 23 files changed, 438 insertions(+), 9 deletions(-) diff --git a/src/common-booter/src/main/resources/config/init.properties b/src/common-booter/src/main/resources/config/init.properties index 834a92bab6..dc40446cd1 100644 --- a/src/common-booter/src/main/resources/config/init.properties +++ b/src/common-booter/src/main/resources/config/init.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/common-booter/src/main/resources/config/init_min.properties b/src/common-booter/src/main/resources/config/init_min.properties index 1f10abe807..c12e8cabc1 100644 --- a/src/common-booter/src/main/resources/config/init_min.properties +++ b/src/common-booter/src/main/resources/config/init_min.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/config/initialize/BroadcastListener.java b/src/common-server/src/main/java/org/apache/kylin/rest/config/initialize/BroadcastListener.java index 01140ef806..55ad017f77 100644 --- a/src/common-server/src/main/java/org/apache/kylin/rest/config/initialize/BroadcastListener.java +++ b/src/common-server/src/main/java/org/apache/kylin/rest/config/initialize/BroadcastListener.java @@ -44,6 +44,7 @@ import org.apache.kylin.rest.service.AuditLogService; import org.apache.kylin.rest.service.JobService; import org.apache.kylin.rest.service.QueryService; import org.apache.kylin.rest.service.UserAclService; +import org.apache.kylin.rest.security.BroadcastSecurityContext; import org.apache.spark.sql.LogicalViewLoader; import org.apache.spark.sql.SparderEnv; import org.springframework.beans.factory.annotation.Autowired; @@ -131,6 +132,6 @@ public class BroadcastListener implements BroadcastEventHandler { @Override public void handleLocally(BroadcastEventReadyNotifier notifier) throws IOException { - handle(notifier); + BroadcastSecurityContext.runAsTrusted(() -> handle(notifier)); } } diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastController.java b/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastController.java index 91c038f9db..43dc8a9a14 100644 --- a/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastController.java +++ b/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastController.java @@ -20,13 +20,21 @@ package org.apache.kylin.rest.controller; import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_JSON; 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.IOException; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import javax.servlet.http.HttpServletRequest; + +import org.apache.commons.lang3.StringUtils; +import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.exception.KylinException; import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; import org.apache.kylin.rest.config.initialize.BroadcastListener; import org.apache.kylin.rest.response.EnvelopeResponse; +import org.apache.kylin.rest.security.BroadcastSecurityContext; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.PostMapping; @@ -35,6 +43,8 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; +import com.fasterxml.jackson.databind.JsonNode; + @Controller @RequestMapping(value = "/api/broadcast", produces = { HTTP_VND_APACHE_KYLIN_JSON, HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON }) @@ -45,12 +55,32 @@ public class BroadcastController extends NBasicController { @PostMapping(value = "") @ResponseBody - public EnvelopeResponse<String> broadcastReceive(@RequestBody BroadcastEventReadyNotifier notifier) + public EnvelopeResponse<String> broadcastReceive(@RequestBody JsonNode body, HttpServletRequest request) throws IOException { - localHandler.handle(notifier); + BroadcastEventReadyNotifier notifier = BroadcastEventValidator.validateAndDeserialize(body); + authorizeBroadcast(request); + BroadcastSecurityContext.runAsTrusted(() -> localHandler.handle(notifier)); return new EnvelopeResponse<>(KylinException.CODE_SUCCESS, "", ""); } + private void authorizeBroadcast(HttpServletRequest request) { + if (isAdmin()) { + return; + } + + String configuredToken = KylinConfig.getInstanceFromEnv().getBroadcastToken(); + String providedToken = request == null ? null + : request.getHeader(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER); + if (StringUtils.isNotBlank(configuredToken) && providedToken != null + && MessageDigest.isEqual(configuredToken.getBytes(StandardCharsets.UTF_8), + providedToken.getBytes(StandardCharsets.UTF_8))) { + return; + } + + throw new KylinException(PERMISSION_DENIED, + "Broadcast endpoint requires an authenticated global admin or a valid broadcast token."); + } + @PutMapping(value = "/capacity/refresh_all") @ResponseBody public EnvelopeResponse<String> innerRefreshAll() { diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastEventValidator.java b/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastEventValidator.java new file mode 100644 index 0000000000..028df7ed77 --- /dev/null +++ b/src/common-server/src/main/java/org/apache/kylin/rest/controller/BroadcastEventValidator.java @@ -0,0 +1,77 @@ +/* + * 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.kylin.rest.controller; + +import static org.apache.kylin.common.exception.ServerErrorCode.INVALID_PARAMETER; + +import java.io.IOException; +import java.util.Set; + +import org.apache.kylin.common.exception.KylinException; +import org.apache.kylin.common.persistence.transaction.AccessBatchGrantEventNotifier; +import org.apache.kylin.common.persistence.transaction.AccessGrantEventNotifier; +import org.apache.kylin.common.persistence.transaction.AccessRevokeEventNotifier; +import org.apache.kylin.common.persistence.transaction.AclGrantEventNotifier; +import org.apache.kylin.common.persistence.transaction.AclRevokeEventNotifier; +import org.apache.kylin.common.persistence.transaction.AclTCRRevokeEventNotifier; +import org.apache.kylin.common.persistence.transaction.AddCredentialToSparkBroadcastEventNotifier; +import org.apache.kylin.common.persistence.transaction.AuditLogBroadcastEventNotifier; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; +import org.apache.kylin.common.persistence.transaction.EpochCheckBroadcastNotifier; +import org.apache.kylin.common.persistence.transaction.LogicalViewBroadcastNotifier; +import org.apache.kylin.common.persistence.transaction.RefreshVolumeBroadcastEventNotifier; +import org.apache.kylin.common.persistence.transaction.StopQueryBroadcastEventNotifier; +import org.apache.kylin.common.util.JsonUtil; +import org.apache.kylin.guava30.shaded.common.collect.ImmutableSet; +import org.apache.kylin.rest.security.AdminUserSyncEventNotifier; + +import com.fasterxml.jackson.databind.JsonNode; + +final class BroadcastEventValidator { + + private static final Set<String> ALLOWED_EVENT_TYPES = ImmutableSet.of( + BroadcastEventReadyNotifier.class.getName(), + AccessBatchGrantEventNotifier.class.getName(), + AccessGrantEventNotifier.class.getName(), + AccessRevokeEventNotifier.class.getName(), + AclGrantEventNotifier.class.getName(), + AclRevokeEventNotifier.class.getName(), + AclTCRRevokeEventNotifier.class.getName(), + AddCredentialToSparkBroadcastEventNotifier.class.getName(), + AdminUserSyncEventNotifier.class.getName(), + AuditLogBroadcastEventNotifier.class.getName(), + EpochCheckBroadcastNotifier.class.getName(), + LogicalViewBroadcastNotifier.class.getName(), + RefreshVolumeBroadcastEventNotifier.class.getName(), + StopQueryBroadcastEventNotifier.class.getName()); + + private BroadcastEventValidator() { + } + + static BroadcastEventReadyNotifier validateAndDeserialize(JsonNode body) throws IOException { + if (body == null) { + throw new KylinException(INVALID_PARAMETER, "Broadcast event body is required."); + } + + JsonNode typeNode = body.get("@class"); + if (typeNode == null || !typeNode.isTextual() || !ALLOWED_EVENT_TYPES.contains(typeNode.asText())) { + throw new KylinException(INVALID_PARAMETER, "Unsupported broadcast event type."); + } + return JsonUtil.readValue(body.toString(), BroadcastEventReadyNotifier.class); + } +} diff --git a/src/common-server/src/test/java/org/apache/kylin/rest/controller/BroadcastControllerTest.java b/src/common-server/src/test/java/org/apache/kylin/rest/controller/BroadcastControllerTest.java new file mode 100644 index 0000000000..f46413a8c4 --- /dev/null +++ b/src/common-server/src/test/java/org/apache/kylin/rest/controller/BroadcastControllerTest.java @@ -0,0 +1,123 @@ +/* + * 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.kylin.rest.controller; + +import java.util.Collections; + +import org.apache.kylin.common.exception.KylinException; +import org.apache.kylin.common.persistence.transaction.AclGrantEventNotifier; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; +import org.apache.kylin.common.util.JsonUtil; +import org.apache.kylin.common.util.NLocalFileMetadataTestCase; +import org.apache.kylin.rest.config.initialize.BroadcastListener; +import org.apache.kylin.rest.constant.Constant; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +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.util.ReflectionTestUtils; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; +import org.springframework.test.web.servlet.result.MockMvcResultMatchers; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; + +import com.fasterxml.jackson.databind.JsonNode; + +public class BroadcastControllerTest extends NLocalFileMetadataTestCase { + + private BroadcastController controller; + private BroadcastListener listener; + + @Before + public void setUp() { + createTestMetadata(); + listener = Mockito.mock(BroadcastListener.class); + controller = new BroadcastController(); + ReflectionTestUtils.setField(controller, "localHandler", listener); + SecurityContextHolder.clearContext(); + getTestConfig().setProperty("kylin.server.broadcast-token", ""); + } + + @After + public void tearDown() { + SecurityContextHolder.clearContext(); + cleanupTestMetadata(); + } + + @Test + public void testUnauthenticatedAclBroadcastRejected() throws Exception { + JsonNode event = aclGrantEvent(); + + Assert.assertThrows(KylinException.class, + () -> controller.broadcastReceive(event, new MockHttpServletRequest())); + Mockito.verifyNoInteractions(listener); + } + + @Test + public void testGlobalAdminAclBroadcastAccepted() throws Exception { + SecurityContextHolder.getContext() + .setAuthentication(new TestingAuthenticationToken("ADMIN", "ADMIN", Constant.ROLE_ADMIN)); + + controller.broadcastReceive(aclGrantEvent(), new MockHttpServletRequest()); + + Mockito.verify(listener).handle(Mockito.any(AclGrantEventNotifier.class)); + } + + @Test + public void testBroadcastTokenAcceptedWithoutUserAuthentication() throws Exception { + getTestConfig().setProperty("kylin.server.broadcast-token", "secret"); + MockMvc mockMvc = MockMvcBuilders.standaloneSetup(controller).build(); + + mockMvc.perform(MockMvcRequestBuilders.post("/api/broadcast").contentType(MediaType.APPLICATION_JSON) + .header(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, "secret") + .content(aclGrantEvent().toString())).andExpect(MockMvcResultMatchers.status().isOk()); + + Mockito.verify(listener).handle(Mockito.any(AclGrantEventNotifier.class)); + } + + @Test + public void testInvalidBroadcastTokenRejected() throws Exception { + getTestConfig().setProperty("kylin.server.broadcast-token", "secret"); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, "wrong"); + + Assert.assertThrows(KylinException.class, () -> controller.broadcastReceive(aclGrantEvent(), request)); + Mockito.verifyNoInteractions(listener); + } + + @Test + public void testUnsupportedBroadcastTypeRejected() throws Exception { + JsonNode event = JsonUtil.readValueAsTree( + "{\"@class\":\"org.apache.kylin.common.persistence.event.ResourceDeleteEvent\"}"); + + Assert.assertThrows(KylinException.class, + () -> controller.broadcastReceive(event, new MockHttpServletRequest())); + Mockito.verifyNoInteractions(listener); + } + + private JsonNode aclGrantEvent() throws Exception { + AclGrantEventNotifier notifier = new AclGrantEventNotifier("project-uuid", + JsonUtil.writeValueAsString(Collections.emptyList())); + return JsonUtil.readValueAsTree(JsonUtil.writeValueAsString(notifier)); + } +} diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/security/BroadcastSecurityContext.java b/src/common-service/src/main/java/org/apache/kylin/rest/security/BroadcastSecurityContext.java new file mode 100644 index 0000000000..57f3563ab3 --- /dev/null +++ b/src/common-service/src/main/java/org/apache/kylin/rest/security/BroadcastSecurityContext.java @@ -0,0 +1,76 @@ +/* + * 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.kylin.rest.security; + +import static org.apache.kylin.common.exception.ServerErrorCode.PERMISSION_DENIED; + +import java.io.IOException; + +import org.apache.kylin.common.exception.KylinException; +import org.apache.kylin.rest.constant.Constant; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public final class BroadcastSecurityContext { + + private static final ThreadLocal<Boolean> TRUSTED_BROADCAST = new ThreadLocal<>(); + + private BroadcastSecurityContext() { + } + + public static boolean isTrustedBroadcast() { + return Boolean.TRUE.equals(TRUSTED_BROADCAST.get()); + } + + public static void runAsTrusted(CheckedRunnable action) throws IOException { + boolean previous = isTrustedBroadcast(); + TRUSTED_BROADCAST.set(Boolean.TRUE); + try { + action.run(); + } finally { + if (previous) { + TRUSTED_BROADCAST.set(Boolean.TRUE); + } else { + TRUSTED_BROADCAST.remove(); + } + } + } + + public static void requireTrustedBroadcastOrGlobalAdmin() { + if (isTrustedBroadcast()) { + return; + } + + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + boolean globalAdmin = authentication != null && authentication.isAuthenticated() + && authentication.getAuthorities().stream() + .anyMatch(authority -> Constant.ROLE_ADMIN.equals(authority.getAuthority())); + if (!globalAdmin) { + log.warn("Rejected an untrusted broadcast-originated ACL mutation"); + throw new KylinException(PERMISSION_DENIED, "Broadcast ACL mutation requires a trusted internal caller."); + } + } + + @FunctionalInterface + public interface CheckedRunnable { + void run() throws IOException; + } +} diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/service/AccessService.java b/src/common-service/src/main/java/org/apache/kylin/rest/service/AccessService.java index 8df4923063..72dd889d3e 100644 --- a/src/common-service/src/main/java/org/apache/kylin/rest/service/AccessService.java +++ b/src/common-service/src/main/java/org/apache/kylin/rest/service/AccessService.java @@ -78,6 +78,7 @@ import org.apache.kylin.rest.security.AclEntityType; import org.apache.kylin.rest.security.AclPermission; import org.apache.kylin.rest.security.AclPermissionFactory; import org.apache.kylin.rest.security.AclRecord; +import org.apache.kylin.rest.security.BroadcastSecurityContext; import org.apache.kylin.rest.security.CompositeAclPermission; import org.apache.kylin.rest.security.ExternalAclProvider; import org.apache.kylin.rest.security.MutableAclRecord; @@ -927,7 +928,7 @@ public class AccessService extends BasicService { throws IOException { AccessGrantEventNotifier notifier = new AccessGrantEventNotifier(UnitOfWork.GLOBAL_UNIT, ae.getId(), identifier, isPrincipal, permission); - updateAccess(notifier, null, null); + BroadcastSecurityContext.runAsTrusted(() -> updateAccess(notifier, null, null)); return true; } @@ -936,7 +937,7 @@ public class AccessService extends BasicService { public boolean batchGrantAccess(List<AccessRequest> requests, AclEntity ae) throws IOException { AccessBatchGrantEventNotifier notifier = new AccessBatchGrantEventNotifier(UnitOfWork.GLOBAL_UNIT, ae.getId(), JsonUtil.writeValueAsString(requests)); - updateAccess(null, notifier, null); + BroadcastSecurityContext.runAsTrusted(() -> updateAccess(null, notifier, null)); return true; } @@ -945,13 +946,14 @@ public class AccessService extends BasicService { public boolean revokeAccess(AclEntity ae, String name, boolean principal) throws IOException { AccessRevokeEventNotifier notifier = new AccessRevokeEventNotifier(UnitOfWork.GLOBAL_UNIT, ae.getId(), name, principal); - updateAccess(null, null, notifier); + BroadcastSecurityContext.runAsTrusted(() -> updateAccess(null, null, notifier)); return true; } @Transaction public void updateAccess(AccessGrantEventNotifier grantNotifier, AccessBatchGrantEventNotifier batchGrantNotifier, AccessRevokeEventNotifier revokeNotifier) throws IOException { + BroadcastSecurityContext.requireTrustedBroadcastOrGlobalAdmin(); if (grantNotifier != null) { AclEntity ae = getAclEntity(AclEntityType.PROJECT_INSTANCE, grantNotifier.getEntityId()); grant(ae, grantNotifier.getIdentifier(), grantNotifier.getIsPrincipal(), grantNotifier.getPermission()); diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/service/AclTCRService.java b/src/common-service/src/main/java/org/apache/kylin/rest/service/AclTCRService.java index d100a7e9cc..18dc62b484 100644 --- a/src/common-service/src/main/java/org/apache/kylin/rest/service/AclTCRService.java +++ b/src/common-service/src/main/java/org/apache/kylin/rest/service/AclTCRService.java @@ -69,6 +69,7 @@ import org.apache.kylin.rest.aspect.Transaction; import org.apache.kylin.rest.request.AccessRequest; import org.apache.kylin.rest.request.AclTCRRequest; import org.apache.kylin.rest.response.AclTCRResponse; +import org.apache.kylin.rest.security.BroadcastSecurityContext; import org.apache.kylin.rest.security.MutableAclRecord; import org.apache.kylin.rest.util.AclEvaluate; import org.apache.kylin.rest.util.AclPermissionUtil; @@ -102,6 +103,7 @@ public class AclTCRService extends BasicService implements AclTCRServiceSupporte private UserService userService; public void revokeAclTCR(String uuid, String sid, boolean principal) { + BroadcastSecurityContext.requireTrustedBroadcastOrGlobalAdmin(); // permission already has been checked in AccessService#revokeAcl getManager(NProjectManager.class).listAllProjects().stream().filter(p -> p.getUuid().equals(uuid)).findFirst() .ifPresent(prj -> EnhancedUnitOfWork.doInTransactionWithCheckAndRetry(() -> { @@ -111,6 +113,7 @@ public class AclTCRService extends BasicService implements AclTCRServiceSupporte } public void revokeAclTCR(String sid, boolean principal) { + BroadcastSecurityContext.requireTrustedBroadcastOrGlobalAdmin(); // only global admin has permission // permission already has been checked in UserController, UserGroupController projectService.getOwnedProjects().parallelStream() @@ -1200,18 +1203,19 @@ public class AclTCRService extends BasicService implements AclTCRServiceSupporte public boolean remoteGrantACL(String projectId, List<AccessRequest> accessRequests) throws IOException { AclGrantEventNotifier notifier = new AclGrantEventNotifier(projectId, JsonUtil.writeValueAsString(accessRequests)); - updateAclFromRemote(notifier, null); + BroadcastSecurityContext.runAsTrusted(() -> updateAclFromRemote(notifier, null)); return true; } public boolean remoteRevokeACL(String projectId, String sid, boolean principal) throws IOException { AclRevokeEventNotifier notifier = new AclRevokeEventNotifier(projectId, sid, principal); - updateAclFromRemote(null, notifier); + BroadcastSecurityContext.runAsTrusted(() -> updateAclFromRemote(null, notifier)); return true; } public void updateAclFromRemote(AclGrantEventNotifier grantEventNotifier, AclRevokeEventNotifier revokeEventNotifier) throws IOException { + BroadcastSecurityContext.requireTrustedBroadcastOrGlobalAdmin(); if (grantEventNotifier != null) { List<AccessRequest> accessRequest = JsonUtil.readValue(grantEventNotifier.getRawAclTCRRequests(), new TypeReference<List<AccessRequest>>() { diff --git a/src/common-service/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java b/src/common-service/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java index 69bafd16f2..f59a45e39d 100644 --- a/src/common-service/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java +++ b/src/common-service/src/test/java/org/apache/kylin/rest/service/AccessServiceTest.java @@ -60,6 +60,7 @@ import org.apache.kylin.rest.security.AclEntityType; import org.apache.kylin.rest.security.AclPermission; import org.apache.kylin.rest.security.AclPermissionFactory; import org.apache.kylin.rest.security.AclRecord; +import org.apache.kylin.rest.security.BroadcastSecurityContext; import org.apache.kylin.rest.security.CompositeAclPermission; import org.apache.kylin.rest.security.MutableAclRecord; import org.apache.kylin.rest.security.UserAclManager; @@ -945,4 +946,18 @@ public class AccessServiceTest extends NLocalFileMetadataTestCase { SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("ADMIN", "ADMIN", Constant.ROLE_ADMIN)); } + + @Test + public void testUpdateAccessRejectsUntrustedCaller() { + SecurityContextHolder.clearContext(); + + Assert.assertThrows(KylinException.class, () -> accessService.updateAccess(null, null, null)); + } + + @Test + public void testUpdateAccessAllowsTrustedInternalCaller() throws IOException { + SecurityContextHolder.clearContext(); + + BroadcastSecurityContext.runAsTrusted(() -> accessService.updateAccess(null, null, null)); + } } diff --git a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java index 5058970dcf..060aa42c5f 100644 --- a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java +++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java @@ -774,6 +774,10 @@ public abstract class KylinConfigBase implements Serializable { return getOptional("kylin.server.https.keystore-password", "changeit"); } + public String getBroadcastToken() { + return EncryptUtil.getDecryptedValue(getOptional("kylin.server.broadcast-token", "")); + } + public String getServerHttpsKeyAlias() { return getOptional("kylin.server.https.key-alias", null); } @@ -4558,7 +4562,7 @@ public abstract class KylinConfigBase implements Serializable { } return Sets.newHashSet(config.split(",")); } - + public boolean isForcedToPushDown() { return Boolean.parseBoolean(getOptional("kylin.query.pushdown-force", FALSE)); } diff --git a/src/core-common/src/main/java/org/apache/kylin/common/persistence/transaction/BroadcastEventReadyNotifier.java b/src/core-common/src/main/java/org/apache/kylin/common/persistence/transaction/BroadcastEventReadyNotifier.java index d7112348f3..7f84a29c23 100644 --- a/src/core-common/src/main/java/org/apache/kylin/common/persistence/transaction/BroadcastEventReadyNotifier.java +++ b/src/core-common/src/main/java/org/apache/kylin/common/persistence/transaction/BroadcastEventReadyNotifier.java @@ -40,6 +40,8 @@ import lombok.Setter; @Setter public class BroadcastEventReadyNotifier extends SchedulerEventNotifier { + public static final String BROADCAST_TOKEN_HEADER = "X-Kylin-Broadcast-Token"; + public BroadcastScopeEnum getBroadcastScope() { return BroadcastScopeEnum.WHOLE_NODES; } diff --git a/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java b/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java index 2292749031..c17308b172 100644 --- a/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java +++ b/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.lang3.StringUtils; import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; import org.apache.http.NameValuePair; @@ -181,6 +182,10 @@ public class RestClient { String url = baseUrl + "/broadcast"; HttpPost post = newPost(url); post.addHeader(ROUTED, "true"); + String broadcastToken = KylinConfig.getInstanceFromEnv().getBroadcastToken(); + if (StringUtils.isNotBlank(broadcastToken)) { + post.addHeader(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, broadcastToken); + } HttpResponse response = null; try { post.setEntity(new ByteArrayEntity(JsonUtil.writeValueAsBytes(notifier), ContentType.APPLICATION_JSON)); diff --git a/src/core-common/src/main/resources/kylin-defaults0.properties b/src/core-common/src/main/resources/kylin-defaults0.properties index dc1b57b9ed..51d6ca5516 100644 --- a/src/core-common/src/main/resources/kylin-defaults0.properties +++ b/src/core-common/src/main/resources/kylin-defaults0.properties @@ -56,6 +56,8 @@ kylin.server.https.port=7444 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/core-common/src/test/java/org/apache/kylin/tool/restclient/RestClientTest.java b/src/core-common/src/test/java/org/apache/kylin/tool/restclient/RestClientTest.java new file mode 100644 index 0000000000..28f508b357 --- /dev/null +++ b/src/core-common/src/test/java/org/apache/kylin/tool/restclient/RestClientTest.java @@ -0,0 +1,68 @@ +/* + * 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.kylin.tool.restclient; + +import org.apache.http.HttpStatus; +import org.apache.http.HttpVersion; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.StringEntity; +import org.apache.http.impl.client.DefaultHttpClient; +import org.apache.http.message.BasicStatusLine; +import org.apache.kylin.common.persistence.transaction.AuditLogBroadcastEventNotifier; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; +import org.apache.kylin.common.util.NLocalFileMetadataTestCase; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +public class RestClientTest extends NLocalFileMetadataTestCase { + + @Before + public void setUp() { + createTestMetadata(); + } + + @After + public void tearDown() { + cleanupTestMetadata(); + } + + @Test + public void testNotifyAddsBroadcastToken() throws Exception { + getTestConfig().setProperty("kylin.server.broadcast-token", "secret"); + RestClient restClient = new RestClient("localhost", 7070, null, null); + DefaultHttpClient httpClient = Mockito.mock(DefaultHttpClient.class); + restClient.client = httpClient; + CloseableHttpResponse response = Mockito.mock(CloseableHttpResponse.class); + Mockito.when(response.getStatusLine()) + .thenReturn(new BasicStatusLine(HttpVersion.HTTP_1_1, HttpStatus.SC_OK, "OK")); + Mockito.when(response.getEntity()).thenReturn(new StringEntity("")); + Mockito.when(httpClient.execute(Mockito.any(HttpPost.class))).thenReturn(response); + + restClient.notify(new AuditLogBroadcastEventNotifier()); + + ArgumentCaptor<HttpPost> requestCaptor = ArgumentCaptor.forClass(HttpPost.class); + Mockito.verify(httpClient).execute(requestCaptor.capture()); + Assert.assertEquals("secret", + requestCaptor.getValue().getFirstHeader(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER).getValue()); + } +} diff --git a/src/data-loading-booter/src/main/resources/config/init.properties b/src/data-loading-booter/src/main/resources/config/init.properties index 5ded54ce8f..08e7f48f02 100644 --- a/src/data-loading-booter/src/main/resources/config/init.properties +++ b/src/data-loading-booter/src/main/resources/config/init.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/data-loading-booter/src/main/resources/config/init_min.properties b/src/data-loading-booter/src/main/resources/config/init_min.properties index 8afffc1fdd..fa4ce65af4 100644 --- a/src/data-loading-booter/src/main/resources/config/init_min.properties +++ b/src/data-loading-booter/src/main/resources/config/init_min.properties @@ -50,6 +50,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/ops-booter/src/main/resources/config/init.properties b/src/ops-booter/src/main/resources/config/init.properties index 68d3e5eba3..b535fff500 100644 --- a/src/ops-booter/src/main/resources/config/init.properties +++ b/src/ops-booter/src/main/resources/config/init.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/ops-booter/src/main/resources/config/init_min.properties b/src/ops-booter/src/main/resources/config/init_min.properties index 87b9b75fc4..6df6f3b23c 100644 --- a/src/ops-booter/src/main/resources/config/init_min.properties +++ b/src/ops-booter/src/main/resources/config/init_min.properties @@ -50,6 +50,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/query-booter/src/main/resources/config/init.properties b/src/query-booter/src/main/resources/config/init.properties index 04cbd3bbe2..f31c0ff319 100644 --- a/src/query-booter/src/main/resources/config/init.properties +++ b/src/query-booter/src/main/resources/config/init.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/query-booter/src/main/resources/config/init_min.properties b/src/query-booter/src/main/resources/config/init_min.properties index b85b2e1d9f..94527c0c5a 100644 --- a/src/query-booter/src/main/resources/config/init_min.properties +++ b/src/query-booter/src/main/resources/config/init_min.properties @@ -50,6 +50,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/rec-booter/src/main/resources/config/init.properties b/src/rec-booter/src/main/resources/config/init.properties index a2ccdd4909..f6760ac8e5 100644 --- a/src/rec-booter/src/main/resources/config/init.properties +++ b/src/rec-booter/src/main/resources/config/init.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false diff --git a/src/rec-booter/src/main/resources/config/init_min.properties b/src/rec-booter/src/main/resources/config/init_min.properties index 85e97153e7..46ced5dfe6 100644 --- a/src/rec-booter/src/main/resources/config/init_min.properties +++ b/src/rec-booter/src/main/resources/config/init_min.properties @@ -51,6 +51,8 @@ kylin.server.https.port=7443 kylin.server.https.keystore-type=JKS kylin.server.https.keystore-file=${KYLIN_HOME}/server/.keystore kylin.server.https.keystore-password=changeit +# Shared secret for authenticating inter-node broadcast requests. Required for multi-node deployments. +#kylin.server.broadcast-token= # Set true to enable CORS for any origins kylin.server.cors.allow-all=false
