You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by fe...@apache.org on 2022/07/05 09:51:01 UTC
[incubator-kyuubi] branch master updated: [KYUUBI #2801][FOLLOWUP] Also check whether the batch main resource path is in local dir allow list
This is an automated email from the ASF dual-hosted git repository.
feiwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new c922ae28a [KYUUBI #2801][FOLLOWUP] Also check whether the batch main resource path is in local dir allow list
c922ae28a is described below
commit c922ae28ab39898b3deedd6cfaded70140855724
Author: Fei Wang <fw...@ebay.com>
AuthorDate: Tue Jul 5 17:50:54 2022 +0800
[KYUUBI #2801][FOLLOWUP] Also check whether the batch main resource path is in local dir allow list
### _Why are the changes needed?_
Also check whether the main resource path is in the local dir allow list.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
Closes #3002 from turboFei/check_resource.
Closes #2801
0e2f303d [Fei Wang] fix ut
cfa61e58 [Fei Wang] check main resource
Authored-by: Fei Wang <fw...@ebay.com>
Signed-off-by: Fei Wang <fw...@ebay.com>
---
.../kyuubi/engine/KyuubiApplicationManager.scala | 43 ++++++++++++----------
.../kyuubi/session/KyuubiBatchSessionImpl.scala | 3 ++
.../engine/KyuubiApplicationManagerSuite.scala | 28 +++++++-------
3 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KyuubiApplicationManager.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KyuubiApplicationManager.scala
index fc8d63ea4..0f236a8b4 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KyuubiApplicationManager.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KyuubiApplicationManager.scala
@@ -109,36 +109,39 @@ object KyuubiApplicationManager {
conf.set(FlinkProcessBuilder.TAG_KEY, newTag)
}
- private[kyuubi] def checkApplicationPathURI(uri: URI, conf: KyuubiConf): Unit = {
+ private[kyuubi] def checkApplicationAccessPath(path: String, conf: KyuubiConf): Unit = {
val localDirAllowList = conf.get(KyuubiConf.SESSION_LOCAL_DIR_ALLOW_LIST)
- if (localDirAllowList.nonEmpty && (uri.getScheme == null || uri.getScheme == "file")) {
- if (!uri.getPath.startsWith(File.separator)) {
- throw new KyuubiException(
- s"Relative path ${uri.getPath} is not allowed, please use absolute path.")
- }
-
- if (!localDirAllowList.exists(uri.getPath.startsWith(_))) {
- throw new KyuubiException(
- s"The file ${uri.getPath} to access is not in the local dir allow list" +
- s" [${localDirAllowList.mkString(",")}].")
+ if (localDirAllowList.nonEmpty) {
+ val uri =
+ try {
+ new URI(path)
+ } catch {
+ case e: URISyntaxException => throw new IllegalArgumentException(e)
+ }
+
+ if (uri.getScheme == null || uri.getScheme == "file") {
+ if (!uri.getPath.startsWith(File.separator)) {
+ throw new KyuubiException(
+ s"Relative path ${uri.getPath} is not allowed, please use absolute path.")
+ }
+
+ if (!localDirAllowList.exists(uri.getPath.startsWith(_))) {
+ throw new KyuubiException(
+ s"The file ${uri.getPath} to access is not in the local dir allow list" +
+ s" [${localDirAllowList.mkString(",")}].")
+ }
}
}
}
- private def checkSparkPathURIs(
+ private def checkSparkAccessPaths(
appConf: Map[String, String],
kyuubiConf: KyuubiConf): Unit = {
if (kyuubiConf.get(KyuubiConf.SESSION_LOCAL_DIR_ALLOW_LIST).nonEmpty) {
SparkProcessBuilder.PATH_CONFIGS.flatMap { key =>
appConf.get(key).map(_.split(",")).getOrElse(Array.empty)
}.filter(_.nonEmpty).foreach { path =>
- val uri =
- try {
- new URI(path)
- } catch {
- case e: URISyntaxException => throw new IllegalArgumentException(e)
- }
- checkApplicationPathURI(uri, kyuubiConf)
+ checkApplicationAccessPath(path, kyuubiConf)
}
}
}
@@ -175,7 +178,7 @@ object KyuubiApplicationManager {
appConf: Map[String, String],
kyuubiConf: KyuubiConf): Unit = {
applicationType.toUpperCase(Locale.ROOT) match {
- case appType if appType.startsWith("SPARK") => checkSparkPathURIs(appConf, kyuubiConf)
+ case appType if appType.startsWith("SPARK") => checkSparkAccessPaths(appConf, kyuubiConf)
case appType if appType.startsWith("FLINK") => // TODO: check flink app access local paths
case _ =>
}
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala
index d236fc97d..7193ab4a7 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiBatchSessionImpl.scala
@@ -100,6 +100,9 @@ class KyuubiBatchSessionImpl(
batchRequest.getBatchType,
normalizedConf,
sessionManager.getConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(
+ batchRequest.getResource,
+ sessionManager.getConf)
}
override def open(): Unit = {
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/KyuubiApplicationManagerSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/KyuubiApplicationManagerSuite.scala
index c4bc06c0d..93ef02cdc 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/KyuubiApplicationManagerSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/KyuubiApplicationManagerSuite.scala
@@ -17,38 +17,36 @@
package org.apache.kyuubi.engine
-import java.net.URI
-
import org.apache.kyuubi.{KyuubiException, KyuubiFunSuite}
import org.apache.kyuubi.config.KyuubiConf
class KyuubiApplicationManagerSuite extends KyuubiFunSuite {
- test("application access path uri") {
+ test("application access path") {
val localDirLimitConf = KyuubiConf()
.set(KyuubiConf.SESSION_LOCAL_DIR_ALLOW_LIST, Seq("/apache/kyuubi"))
val noLocalDirLimitConf = KyuubiConf()
- var uri = new URI("/apache/kyuubi/a.jar")
- KyuubiApplicationManager.checkApplicationPathURI(uri, localDirLimitConf)
- KyuubiApplicationManager.checkApplicationPathURI(uri, noLocalDirLimitConf)
+ var path = "/apache/kyuubi/a.jar"
+ KyuubiApplicationManager.checkApplicationAccessPath(path, localDirLimitConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(path, noLocalDirLimitConf)
- uri = new URI("/apache/kyuubijar")
+ path = "/apache/kyuubijar"
var e = intercept[KyuubiException] {
- KyuubiApplicationManager.checkApplicationPathURI(uri, localDirLimitConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(path, localDirLimitConf)
}
assert(e.getMessage.contains("is not in the local dir allow list"))
- KyuubiApplicationManager.checkApplicationPathURI(uri, noLocalDirLimitConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(path, noLocalDirLimitConf)
- uri = new URI("hdfs:/apache/kyuubijar")
- KyuubiApplicationManager.checkApplicationPathURI(uri, localDirLimitConf)
- KyuubiApplicationManager.checkApplicationPathURI(uri, noLocalDirLimitConf)
+ path = "hdfs:/apache/kyuubijar"
+ KyuubiApplicationManager.checkApplicationAccessPath(path, localDirLimitConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(path, noLocalDirLimitConf)
- uri = new URI("path/to/kyuubijar")
+ path = "path/to/kyuubijar"
e = intercept[KyuubiException] {
- KyuubiApplicationManager.checkApplicationPathURI(uri, localDirLimitConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(path, localDirLimitConf)
}
assert(e.getMessage.contains("please use absolute path"))
- KyuubiApplicationManager.checkApplicationPathURI(uri, noLocalDirLimitConf)
+ KyuubiApplicationManager.checkApplicationAccessPath(path, noLocalDirLimitConf)
var appConf = Map("spark.files" -> "/apache/kyuubi/jars/a.jar")
KyuubiApplicationManager.checkApplicationAccessPaths(