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 a4fb62a84acf632773bf86a936df10a147d7cf85 Author: Li Yang <[email protected]> AuthorDate: Fri Oct 2 09:56:23 2026 +0800 KYLIN-6104 improve spark job access control --- .../src/main/resources/kylinSecurity.xml | 2 + .../apache/kylin/rest/config/SecurityConfig.java | 2 +- .../org/apache/kylin/rest/util/SparkUIUtil.java | 99 ++++++++++++++++++++-- .../apache/kylin/rest/util/SparkUIUtilTest.java | 64 ++++++++++++++ .../src/main/resources/kylinSecurity.xml | 2 + .../kylin/rest/controller/JobController.java | 8 +- .../kylin/rest/controller/JobControllerTest.java | 30 ++++++- .../kylin/rest/util/SparkHistoryUIUtilTest.java | 5 +- .../src/main/resources/kylinSecurity.xml | 2 + src/server/src/main/resources/kylinSecurity.xml | 2 + 10 files changed, 206 insertions(+), 10 deletions(-) diff --git a/src/common-booter/src/main/resources/kylinSecurity.xml b/src/common-booter/src/main/resources/kylinSecurity.xml index a2978ad1d8..b1377eb6d7 100644 --- a/src/common-booter/src/main/resources/kylinSecurity.xml +++ b/src/common-booter/src/main/resources/kylinSecurity.xml @@ -310,6 +310,7 @@ <scr:intercept-url pattern="/api/error" access="permitAll"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:logout invalidate-session="true" delete-cookies="JSESSIONID" logout-url="/api/j_spring_security_logout" @@ -365,6 +366,7 @@ <scr:intercept-url pattern="/api/projects*/*" access="isAuthenticated()"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:form-login login-page="/login"/> diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/config/SecurityConfig.java b/src/common-server/src/main/java/org/apache/kylin/rest/config/SecurityConfig.java index 1b06aaedbb..57f2048085 100644 --- a/src/common-server/src/main/java/org/apache/kylin/rest/config/SecurityConfig.java +++ b/src/common-server/src/main/java/org/apache/kylin/rest/config/SecurityConfig.java @@ -94,7 +94,7 @@ public class SecurityConfig extends WebSecurityConfigurerAdapter { .and() .authorizeRequests() .antMatchers("/api/query*/**", "/api/metadata*/**", "/api/cubes*/**", "/api/models*/**", "/api/job*/**", - "/api/**") + "/driver_ui/**", "/api/**") .authenticated(); http.logout() diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/util/SparkUIUtil.java b/src/common-service/src/main/java/org/apache/kylin/rest/util/SparkUIUtil.java index e7b8c78472..36b562dbe1 100644 --- a/src/common-service/src/main/java/org/apache/kylin/rest/util/SparkUIUtil.java +++ b/src/common-service/src/main/java/org/apache/kylin/rest/util/SparkUIUtil.java @@ -18,7 +18,9 @@ package org.apache.kylin.rest.util; import java.io.IOException; +import java.net.InetAddress; import java.net.URI; +import java.net.UnknownHostException; import java.util.Collections; import java.util.Locale; import java.util.Objects; @@ -27,7 +29,11 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.kylin.common.KylinConfig; +import org.apache.kylin.common.exception.KylinRuntimeException; +import org.apache.kylin.common.util.AddressUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpHeaders; @@ -52,19 +58,21 @@ public class SparkUIUtil { public static void resendSparkUIRequest(HttpServletRequest servletRequest, HttpServletResponse servletResponse, String sparkUiUrl, String uriPath, String proxyLocationBase) throws IOException { - URI target = UriComponentsBuilder.fromHttpUrl(sparkUiUrl).path(uriPath).query(servletRequest.getQueryString()) + URI originTarget = validateProxyUrl(sparkUiUrl); + URI target = UriComponentsBuilder.fromUri(originTarget).path(uriPath).query(servletRequest.getQueryString()) .build(true).toUri(); final HttpMethod method = HttpMethod.resolve(servletRequest.getMethod()); try (ClientHttpResponse response = execute(target, method, proxyLocationBase)) { rewrite(response, servletResponse, method, servletRequest.getRequestURL().toString(), REDIRECT_THRESHOLD, - proxyLocationBase); + proxyLocationBase, originTarget); } } public static ClientHttpResponse execute(URI uri, HttpMethod method, String proxyLocationBase) throws IOException { - ClientHttpRequest clientHttpRequest = factory.createRequest(uri, method); + URI validatedUri = validateProxyUrl(uri.toString()); + ClientHttpRequest clientHttpRequest = factory.createRequest(validatedUri, method); clientHttpRequest.getHeaders().put(SPARK_UI_PROXY_HEADER, Collections.singletonList(proxyLocationBase)); return clientHttpRequest.execute(); } @@ -72,6 +80,12 @@ public class SparkUIUtil { private static void rewrite(final ClientHttpResponse response, final HttpServletResponse servletResponse, final HttpMethod originMethod, final String originUrlStr, final int depth, String proxyLocationBase) throws IOException { + rewrite(response, servletResponse, originMethod, originUrlStr, depth, proxyLocationBase, null); + } + + private static void rewrite(final ClientHttpResponse response, final HttpServletResponse servletResponse, + final HttpMethod originMethod, final String originUrlStr, final int depth, String proxyLocationBase, + final URI originTarget) throws IOException { if (depth <= 0) { final String msg = String.format(Locale.ROOT, "redirect exceed threshold: %d, origin request: [%s %s]", REDIRECT_THRESHOLD, originMethod, originUrlStr); @@ -81,8 +95,12 @@ public class SparkUIUtil { } HttpHeaders headers = response.getHeaders(); if (response.getStatusCode().is3xxRedirection()) { - try (ClientHttpResponse r = execute(headers.getLocation(), originMethod, proxyLocationBase)) { - rewrite(r, servletResponse, originMethod, originUrlStr, depth - 1, proxyLocationBase); + URI redirectTarget = validateProxyUrl(Objects.toString(headers.getLocation(), "")); + if (originTarget != null && !isSameAuthority(originTarget, redirectTarget)) { + throw new IOException("redirect target is not allowed: " + redirectTarget); + } + try (ClientHttpResponse r = execute(redirectTarget, originMethod, proxyLocationBase)) { + rewrite(r, servletResponse, originMethod, originUrlStr, depth - 1, proxyLocationBase, originTarget); } return; } @@ -97,4 +115,75 @@ public class SparkUIUtil { } + public static URI validateProxyUrl(String url) { + if (StringUtils.isBlank(url)) { + throw new KylinRuntimeException("Proxy target URL cannot be empty"); + } + + final URI target; + try { + target = URI.create(url); + } catch (IllegalArgumentException e) { + throw new KylinRuntimeException("Invalid proxy target URL: " + url, e); + } + + String scheme = target.getScheme(); + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + throw new KylinRuntimeException("Proxy target URL must use http or https"); + } + if (StringUtils.isBlank(target.getHost()) || target.getUserInfo() != null || target.getFragment() != null) { + throw new KylinRuntimeException("Invalid proxy target URL: " + url); + } + + String host = target.getPort() < 0 ? target.getHost() : target.getHost() + ":" + target.getPort(); + AddressUtil.validateHost(host); + return target; + } + + public static void validateJobTrackingUrl(String url, String requestHost) { + URI target = validateProxyUrl(url); + KylinConfig config = KylinConfig.getInstanceFromEnv(); + + String trackingUrlPattern = config.getJobTrackingURLPattern(); + if (StringUtils.isNotBlank(trackingUrlPattern) && url.matches(trackingUrlPattern)) { + return; + } + + String yarnStatusUrl = config.getYarnStatusCheckUrl(); + if (StringUtils.isNotBlank(yarnStatusUrl)) { + URI allowedYarnTarget = validateProxyUrl(yarnStatusUrl); + if (isSameAuthority(target, allowedYarnTarget)) { + return; + } + } + + if (StringUtils.isNotBlank(requestHost) && isSameHost(target.getHost(), requestHost)) { + return; + } + + throw new KylinRuntimeException("Tracking URL host is not allowed: " + target.getHost()); + } + + private static boolean isSameAuthority(URI first, URI second) { + return StringUtils.equalsIgnoreCase(first.getHost(), second.getHost()) + && getEffectivePort(first) == getEffectivePort(second); + } + + private static int getEffectivePort(URI uri) { + if (uri.getPort() >= 0) { + return uri.getPort(); + } + return "https".equalsIgnoreCase(uri.getScheme()) ? 443 : 80; + } + + private static boolean isSameHost(String firstHost, String secondHost) { + try { + InetAddress first = InetAddress.getByName(firstHost); + InetAddress second = InetAddress.getByName(secondHost); + return first.equals(second); + } catch (UnknownHostException e) { + return StringUtils.equalsIgnoreCase(firstHost, secondHost); + } + } + } diff --git a/src/common-service/src/test/java/org/apache/kylin/rest/util/SparkUIUtilTest.java b/src/common-service/src/test/java/org/apache/kylin/rest/util/SparkUIUtilTest.java new file mode 100644 index 0000000000..fefed0122e --- /dev/null +++ b/src/common-service/src/test/java/org/apache/kylin/rest/util/SparkUIUtilTest.java @@ -0,0 +1,64 @@ +/* + * 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.util; + +import static org.junit.Assert.assertEquals; + +import org.apache.kylin.common.exception.KylinRuntimeException; +import org.apache.kylin.common.util.NLocalFileMetadataTestCase; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class SparkUIUtilTest extends NLocalFileMetadataTestCase { + + @Before + public void setUp() { + createTestMetadata(); + overwriteSystemProp("kylin.job.yarn-app-rest-check-status-url", ""); + overwriteSystemProp("kylin.job.tracking-url-pattern", ""); + } + + @After + public void tearDown() { + cleanupTestMetadata(); + } + + @Test + public void testValidateProxyUrl() { + assertEquals("127.0.0.1", SparkUIUtil.validateProxyUrl("http://127.0.0.1:4040/jobs";).getHost()); + + Assert.assertThrows(KylinRuntimeException.class, () -> SparkUIUtil.validateProxyUrl("ftp://127.0.0.1";)); + Assert.assertThrows(KylinRuntimeException.class, + () -> SparkUIUtil.validateProxyUrl("http://user:[email protected]";)); + Assert.assertThrows(KylinRuntimeException.class, () -> SparkUIUtil.validateProxyUrl("http://127.0.0.1/#x";)); + } + + @Test + public void testValidateJobTrackingUrl() { + SparkUIUtil.validateJobTrackingUrl("http://127.0.0.1:4040";, "127.0.0.1"); + + Assert.assertThrows(KylinRuntimeException.class, + () -> SparkUIUtil.validateJobTrackingUrl("http://10.0.0.1:4040";, "127.0.0.1")); + Assert.assertThrows(KylinRuntimeException.class, + () -> SparkUIUtil.validateJobTrackingUrl("ftp://127.0.0.1";, "127.0.0.1")); + } +} + diff --git a/src/data-loading-booter/src/main/resources/kylinSecurity.xml b/src/data-loading-booter/src/main/resources/kylinSecurity.xml index c95cac54bc..73acdf4513 100644 --- a/src/data-loading-booter/src/main/resources/kylinSecurity.xml +++ b/src/data-loading-booter/src/main/resources/kylinSecurity.xml @@ -310,6 +310,7 @@ <scr:intercept-url pattern="/api/system/license/info" access="isAuthenticated()"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:logout invalidate-session="true" delete-cookies="JSESSIONID" logout-url="/api/j_spring_security_logout" @@ -414,6 +415,7 @@ <scr:intercept-url pattern="/api/projects*/*" access="isAuthenticated()"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:form-login login-page="/login"/> diff --git a/src/data-loading-server/src/main/java/org/apache/kylin/rest/controller/JobController.java b/src/data-loading-server/src/main/java/org/apache/kylin/rest/controller/JobController.java index 4d73a289df..dfba66a6d3 100644 --- a/src/data-loading-server/src/main/java/org/apache/kylin/rest/controller/JobController.java +++ b/src/data-loading-server/src/main/java/org/apache/kylin/rest/controller/JobController.java @@ -62,6 +62,7 @@ import org.apache.kylin.rest.response.JobStatisticsResponse; import org.apache.kylin.rest.service.JobInfoService; import org.apache.kylin.rest.service.JobService; import org.apache.kylin.rest.service.RouteService; +import org.apache.kylin.rest.util.SparkUIUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -309,8 +310,13 @@ public class JobController extends BaseController { @PutMapping(value = "/spark") @ApiOperation(value = "updateURL", tags = { "DW" }) @ResponseBody - public EnvelopeResponse<String> updateSparkJobInfo(@RequestBody SparkJobUpdateRequest sparkJobUpdateRequest) { + public EnvelopeResponse<String> updateSparkJobInfo(@RequestBody SparkJobUpdateRequest sparkJobUpdateRequest, + HttpServletRequest servletRequest) { checkProjectName(sparkJobUpdateRequest.getProject()); + if (StringUtils.isNotBlank(sparkJobUpdateRequest.getYarnAppUrl())) { + SparkUIUtil.validateJobTrackingUrl(sparkJobUpdateRequest.getYarnAppUrl(), + servletRequest.getRemoteAddr()); + } return updateJobInfoWithCheck(sparkJobUpdateRequest.getProject(), sparkJobUpdateRequest.getJobId(), sparkJobUpdateRequest.getJobLastRunningStartTime(), () -> jobInfoService.updateSparkJobInfo(sparkJobUpdateRequest)); diff --git a/src/data-loading-server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java b/src/data-loading-server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java index 231c5e8a15..9eb47f0214 100644 --- a/src/data-loading-server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java +++ b/src/data-loading-server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java @@ -331,7 +331,7 @@ public class JobControllerTest extends NLocalFileMetadataTestCase { request.setProject(job.getProject()); request.setJobId(job.getId()); request.setTaskId(job.getId() + "_00_00"); - request.setYarnAppUrl("url"); + request.setYarnAppUrl("http://127.0.0.1:4040";); request.setYarnAppId("app_id"); request.setCores("1"); request.setMemory("1024"); @@ -340,12 +340,38 @@ public class JobControllerTest extends NLocalFileMetadataTestCase { MvcResult result = mockMvc .perform(MockMvcRequestBuilders.put("/api/jobs/spark").contentType(MediaType.APPLICATION_JSON) .content(JsonUtil.writeValueAsString(request)) + .with(req -> { + req.setRemoteAddr("127.0.0.1"); + return req; + }) .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON))) .andExpect(MockMvcResultMatchers.status().isOk()).andReturn(); Map<String, String> response = JsonUtil.readValueAsMap(result.getResponse().getContentAsString()); Assert.assertEquals(response.get("code"), KylinException.CODE_SUCCESS); - Mockito.verify(jobController).updateSparkJobInfo(request); + Mockito.verify(jobController).updateSparkJobInfo(Mockito.eq(request), Mockito.any()); + } + + @Test + public void testUpdateSparkJobInfoRejectsUntrustedYarnAppUrl() throws Exception { + ExecutablePO job = mockJob(ExecutableState.RUNNING); + SparkJobUpdateRequest request = new SparkJobUpdateRequest(); + request.setJobLastRunningStartTime(String.valueOf(job.getOutput().getLastRunningStartTime())); + request.setProject(job.getProject()); + request.setJobId(job.getId()); + request.setTaskId(job.getId() + "_00_00"); + request.setYarnAppUrl("http://10.0.0.1:4040";); + + mockMvc.perform(MockMvcRequestBuilders.put("/api/jobs/spark").contentType(MediaType.APPLICATION_JSON) + .content(JsonUtil.writeValueAsString(request)) + .with(req -> { + req.setRemoteAddr("127.0.0.1"); + return req; + }) + .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON))) + .andExpect(MockMvcResultMatchers.status().isInternalServerError()); + + Mockito.verify(jobInfoService, Mockito.never()).updateSparkJobInfo(Mockito.any()); } @Test diff --git a/src/data-loading-service/src/test/java/org/apache/kylin/rest/util/SparkHistoryUIUtilTest.java b/src/data-loading-service/src/test/java/org/apache/kylin/rest/util/SparkHistoryUIUtilTest.java index ea09b2bda4..850de43c27 100644 --- a/src/data-loading-service/src/test/java/org/apache/kylin/rest/util/SparkHistoryUIUtilTest.java +++ b/src/data-loading-service/src/test/java/org/apache/kylin/rest/util/SparkHistoryUIUtilTest.java @@ -23,6 +23,7 @@ import static org.apache.spark.deploy.history.HistoryServerBuilder.createHistory import java.io.File; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.net.URI; import org.apache.commons.io.FileUtils; import org.apache.kylin.common.util.NLocalFileMetadataTestCase; @@ -77,7 +78,9 @@ public class SparkHistoryUIUtilTest extends NLocalFileMetadataTestCase { field.set(null, mockFactory); ClientHttpResponse mockClientHttpResponse = Mockito.mock(ClientHttpResponse.class); Mockito.when(mockClientHttpResponse.getStatusCode()).thenReturn(HttpStatus.FOUND); - Mockito.when(mockClientHttpResponse.getHeaders()).thenReturn(new HttpHeaders()); + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.setLocation(URI.create("http://localhost:18080/redirect";)); + Mockito.when(mockClientHttpResponse.getHeaders()).thenReturn(responseHeaders); ClientHttpRequest mockRequest = Mockito.mock(ClientHttpRequest.class); Mockito.when(mockRequest.getHeaders()).thenReturn(new HttpHeaders()); diff --git a/src/query-booter/src/main/resources/kylinSecurity.xml b/src/query-booter/src/main/resources/kylinSecurity.xml index 33c0f8ff98..f05e4a9871 100644 --- a/src/query-booter/src/main/resources/kylinSecurity.xml +++ b/src/query-booter/src/main/resources/kylinSecurity.xml @@ -309,6 +309,7 @@ <scr:intercept-url pattern="/api/error" access="permitAll"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:logout invalidate-session="true" delete-cookies="JSESSIONID" logout-url="/api/j_spring_security_logout" @@ -365,6 +366,7 @@ <scr:intercept-url pattern="/api/projects*/*" access="isAuthenticated()"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:form-login login-page="/login"/> diff --git a/src/server/src/main/resources/kylinSecurity.xml b/src/server/src/main/resources/kylinSecurity.xml index 2870174e7b..2b3cd7a33f 100644 --- a/src/server/src/main/resources/kylinSecurity.xml +++ b/src/server/src/main/resources/kylinSecurity.xml @@ -315,6 +315,7 @@ <scr:intercept-url pattern="/api/error" access="permitAll"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:logout invalidate-session="true" delete-cookies="JSESSIONID" logout-url="/api/j_spring_security_logout" @@ -373,6 +374,7 @@ <scr:intercept-url pattern="/api/projects*/*" access="isAuthenticated()"/> <scr:intercept-url pattern="/api/admin*/**" access="hasRole('ROLE_ADMIN')"/> <scr:intercept-url pattern="/api/**" access="isAuthenticated()"/> + <scr:intercept-url pattern="/driver_ui/**" access="isAuthenticated()"/> <scr:intercept-url pattern="/sparder/**" access="isAuthenticated()"/> <scr:form-login login-page="/login"/>
