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 8cb1bc2b787060f19da4e2fd5b7a0a03191fecff Author: Li Yang <[email protected]> AuthorDate: Fri Oct 2 23:44:31 2026 +0800 KYLIN-6115 restrict config export and harden encryption key --- .../kylin/rest/controller/NConfigController.java | 6 ++ .../rest/controller/NConfigControllerTest.java | 25 +++++ .../org/apache/kylin/common/util/EncryptUtil.java | 106 +++++++++++++++++---- .../apache/kylin/common/util/EncryptUtilsTest.java | 45 +++++++++ 4 files changed, 164 insertions(+), 18 deletions(-) diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/controller/NConfigController.java b/src/common-server/src/main/java/org/apache/kylin/rest/controller/NConfigController.java index e8c46ef056..b3a7c2c5e0 100644 --- a/src/common-server/src/main/java/org/apache/kylin/rest/controller/NConfigController.java +++ b/src/common-server/src/main/java/org/apache/kylin/rest/controller/NConfigController.java @@ -26,6 +26,8 @@ import org.apache.kylin.common.KapConfig; import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.exception.KylinException; import org.apache.kylin.rest.response.EnvelopeResponse; +import org.apache.kylin.rest.util.AclEvaluate; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; @@ -37,6 +39,9 @@ import io.swagger.annotations.ApiOperation; @RequestMapping(value = "/api/config", produces = { HTTP_VND_APACHE_KYLIN_JSON, HTTP_VND_APACHE_KYLIN_V4_PUBLIC_JSON }) public class NConfigController extends NBasicController { + @Autowired + private AclEvaluate aclEvaluate; + @ApiOperation(value = "is cloud", tags = { "MID" }) @GetMapping(value = "/is_cloud") @ResponseBody @@ -50,6 +55,7 @@ public class NConfigController extends NBasicController { @GetMapping(value = "/all") @ResponseBody public EnvelopeResponse<Properties> fetchAll() { + aclEvaluate.checkIsGlobalAdmin(); return new EnvelopeResponse<>(KylinException.CODE_SUCCESS, KylinConfig.getInstanceFromEnv().exportToProperties(), ""); } diff --git a/src/common-server/src/test/java/org/apache/kylin/rest/controller/NConfigControllerTest.java b/src/common-server/src/test/java/org/apache/kylin/rest/controller/NConfigControllerTest.java index d817fdea6a..da3072ec36 100644 --- a/src/common-server/src/test/java/org/apache/kylin/rest/controller/NConfigControllerTest.java +++ b/src/common-server/src/test/java/org/apache/kylin/rest/controller/NConfigControllerTest.java @@ -20,14 +20,19 @@ package org.apache.kylin.rest.controller; import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLIN_JSON; import org.apache.kylin.common.util.NLocalFileMetadataTestCase; +import org.apache.kylin.rest.util.AclEvaluate; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; import org.springframework.http.MediaType; +import org.springframework.security.access.AccessDeniedException; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.result.MockMvcResultMatchers; @@ -41,11 +46,15 @@ public class NConfigControllerTest extends NLocalFileMetadataTestCase { @Rule public ExpectedException thrown = ExpectedException.none(); + @Mock + private AclEvaluate aclEvaluate; + @InjectMocks private NConfigController nConfigController = Mockito.spy(new NConfigController()); @Before public void setUp() { + MockitoAnnotations.initMocks(this); mockMvc = MockMvcBuilders.standaloneSetup(nConfigController) // .defaultRequest(MockMvcRequestBuilders.get("/")).build(); @@ -70,4 +79,20 @@ public class NConfigControllerTest extends NLocalFileMetadataTestCase { Mockito.verify(nConfigController).isCloud(); } + @Test + public void testFetchAllAllowedForGlobalAdmin() throws Exception { + mockMvc.perform(MockMvcRequestBuilders.get("/api/config/all") // + .contentType(MediaType.APPLICATION_JSON) // + .accept(MediaType.parseMediaType(APPLICATION_JSON))) + .andExpect(MockMvcResultMatchers.status().isOk()); + Mockito.verify(aclEvaluate).checkIsGlobalAdmin(); + } + + @Test + public void testFetchAllRejectedForNonAdmin() { + Mockito.doThrow(new AccessDeniedException("denied")).when(aclEvaluate).checkIsGlobalAdmin(); + + Assert.assertThrows(AccessDeniedException.class, () -> nConfigController.fetchAll()); + Mockito.verify(aclEvaluate).checkIsGlobalAdmin(); + } } diff --git a/src/core-common/src/main/java/org/apache/kylin/common/util/EncryptUtil.java b/src/core-common/src/main/java/org/apache/kylin/common/util/EncryptUtil.java index 08b3d4ed62..724b6a171b 100644 --- a/src/core-common/src/main/java/org/apache/kylin/common/util/EncryptUtil.java +++ b/src/core-common/src/main/java/org/apache/kylin/common/util/EncryptUtil.java @@ -19,6 +19,8 @@ package org.apache.kylin.common.util; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import javax.crypto.Cipher; import javax.crypto.spec.SecretKeySpec; @@ -30,30 +32,37 @@ import lombok.extern.slf4j.Slf4j; @Slf4j public class EncryptUtil { + /** - * thisIsAsecretKey + * Built-in key that older releases used for every deployment. It is kept only so that + * configuration values encrypted by those releases remain readable. New values should be + * encrypted with a deployment-specific key configured through {@link #ENCRYPTION_KEY_PROPERTY} + * or {@link #ENCRYPTION_KEY_ENV}. */ - private static final byte[] key = { 0x74, 0x68, 0x69, 0x73, 0x49, 0x73, 0x41, 0x53, 0x65, 0x63, 0x72, 0x65, 0x74, - 0x4b, 0x65, 0x79 }; + private static final byte[] LEGACY_KEY = { 0x74, 0x68, 0x69, 0x73, 0x49, 0x73, 0x41, 0x53, 0x65, 0x63, 0x72, 0x65, + 0x74, 0x4b, 0x65, 0x79 }; + + public static final String ENCRYPTION_KEY_PROPERTY = "kylin.security.encryption-key"; + + public static final String ENCRYPTION_KEY_ENV = "KYLIN_SECURITY_ENCRYPTION_KEY"; public static final String ENC_PREFIX = "ENC('"; public static final String ENC_SUBFIX = "')"; public static final String DEC_FLAG = "DEC"; + private static final String AES_TRANSFORMATION = "AES/ECB/PKCS5Padding"; + private static final String SHA_256 = "SHA-256"; + private static final int AES_KEY_LENGTH = 16; + + private static volatile boolean legacyKeyWarningLogged = false; + public static boolean isEncrypted(String value) { return StringUtils.isNotEmpty(value) && value.startsWith(ENC_PREFIX) && value.endsWith(ENC_SUBFIX); } public static String encrypt(String strToEncrypt) { - try { - Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding"); - final SecretKeySpec secretKey = new SecretKeySpec(key, "AES"); - cipher.init(Cipher.ENCRYPT_MODE, secretKey); - return Base64.encodeBase64String(cipher.doFinal(strToEncrypt.getBytes(Charset.defaultCharset()))); - } catch (Exception e) { - throw new RuntimeException(e.getMessage(), e); - } + return encrypt(strToEncrypt, encryptionKey()); } public static String encryptWithPrefix(String value) { @@ -61,14 +70,16 @@ public class EncryptUtil { } public static String decrypt(String strToDecrypt) { - try { - Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5PADDING"); - final SecretKeySpec secretKey = new SecretKeySpec(key, "AES"); - cipher.init(Cipher.DECRYPT_MODE, secretKey); - return new String(cipher.doFinal(Base64.decodeBase64(strToDecrypt)), Charset.defaultCharset()); - } catch (Exception e) { - throw new RuntimeException(e.getMessage(), e); + byte[] configuredKey = configuredKey(); + if (configuredKey != null) { + try { + return decrypt(strToDecrypt, configuredKey); + } catch (Exception e) { + // Values encrypted before the configured key was introduced still use the legacy key. + log.debug("Failed to decrypt with the configured encryption key, retrying with the legacy key"); + } } + return decrypt(strToDecrypt, LEGACY_KEY); } public static String decryptPassInKylin(String value) { @@ -101,4 +112,63 @@ public class EncryptUtil { // for encrypt password like LDAP password System.out.println(EncryptUtil.encrypt(passwordTxt)); } + + private static byte[] encryptionKey() { + byte[] configuredKey = configuredKey(); + if (configuredKey != null) { + return configuredKey; + } + if (!legacyKeyWarningLogged) { + log.warn("{} is not configured. Falling back to the built-in legacy encryption key, which is shared " + + "by every Apache Kylin deployment. Configure a deployment-specific key to protect stored " + + "credentials.", ENCRYPTION_KEY_PROPERTY); + legacyKeyWarningLogged = true; + } + return LEGACY_KEY; + } + + private static byte[] configuredKey() { + String key = System.getProperty(ENCRYPTION_KEY_PROPERTY); + if (StringUtils.isBlank(key)) { + key = System.getenv(ENCRYPTION_KEY_ENV); + } + if (StringUtils.isBlank(key)) { + return null; + } + return deriveKey(key); + } + + private static byte[] deriveKey(String secret) { + try { + MessageDigest digest = MessageDigest.getInstance(SHA_256); + byte[] hash = digest.digest(secret.getBytes(StandardCharsets.UTF_8)); + byte[] key = new byte[AES_KEY_LENGTH]; + System.arraycopy(hash, 0, key, 0, AES_KEY_LENGTH); + return key; + } catch (Exception e) { + throw new RuntimeException(e.getMessage(), e); + } + } + + private static String encrypt(String strToEncrypt, byte[] key) { + try { + Cipher cipher = Cipher.getInstance(AES_TRANSFORMATION); + final SecretKeySpec secretKey = new SecretKeySpec(key, "AES"); + cipher.init(Cipher.ENCRYPT_MODE, secretKey); + return Base64.encodeBase64String(cipher.doFinal(strToEncrypt.getBytes(Charset.defaultCharset()))); + } catch (Exception e) { + throw new RuntimeException(e.getMessage(), e); + } + } + + private static String decrypt(String strToDecrypt, byte[] key) { + try { + Cipher cipher = Cipher.getInstance(AES_TRANSFORMATION); + final SecretKeySpec secretKey = new SecretKeySpec(key, "AES"); + cipher.init(Cipher.DECRYPT_MODE, secretKey); + return new String(cipher.doFinal(Base64.decodeBase64(strToDecrypt)), Charset.defaultCharset()); + } catch (Exception e) { + throw new RuntimeException(e.getMessage(), e); + } + } } diff --git a/src/core-common/src/test/java/org/apache/kylin/common/util/EncryptUtilsTest.java b/src/core-common/src/test/java/org/apache/kylin/common/util/EncryptUtilsTest.java index d1e0ca2a12..756a27a1cd 100644 --- a/src/core-common/src/test/java/org/apache/kylin/common/util/EncryptUtilsTest.java +++ b/src/core-common/src/test/java/org/apache/kylin/common/util/EncryptUtilsTest.java @@ -18,6 +18,7 @@ package org.apache.kylin.common.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import org.junit.jupiter.api.Test; @@ -47,4 +48,48 @@ class EncryptUtilsTest { String text = "ENC('1YeqVr9MakSFbgxEec9sBwg==')"; assertNull(EncryptUtil.getDecryptedValue(text)); } + + @Test + void testEncryptWithConfiguredKey() { + System.setProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY, "deployment-specific-secret"); + try { + String encrypted = EncryptUtil.encrypt("kylin"); + assertNotEquals("YeqVr9MakSFbgxEec9sBwg==", encrypted); + assertEquals("kylin", EncryptUtil.decrypt(encrypted)); + assertEquals("kylin", + EncryptUtil.getDecryptedValue(EncryptUtil.ENC_PREFIX + encrypted + EncryptUtil.ENC_SUBFIX)); + } finally { + System.clearProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY); + } + } + + @Test + void testLegacyCiphertextStillDecryptsWithConfiguredKey() { + System.setProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY, "deployment-specific-secret"); + try { + assertEquals("kylin", EncryptUtil.getDecryptedValue("ENC('YeqVr9MakSFbgxEec9sBwg==')")); + } finally { + System.clearProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY); + } + } + + @Test + void testConfiguredKeysAreDeploymentScoped() { + System.setProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY, "deployment-a"); + String encryptedByDeploymentA; + try { + encryptedByDeploymentA = EncryptUtil.encrypt("kylin"); + } finally { + System.clearProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY); + } + + System.setProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY, "deployment-b"); + try { + assertNotEquals(encryptedByDeploymentA, EncryptUtil.encrypt("kylin")); + assertNull(EncryptUtil.getDecryptedValue( + EncryptUtil.ENC_PREFIX + encryptedByDeploymentA + EncryptUtil.ENC_SUBFIX)); + } finally { + System.clearProperty(EncryptUtil.ENCRYPTION_KEY_PROPERTY); + } + } }
