From 9f73ac6df21f85e7f104df566a399015ac699aba Mon Sep 17 00:00:00 2001 From: zhuol Date: Mon, 24 Aug 2015 13:11:42 -0500 Subject: [PATCH 1/6] Spark-4223 Support * in acls. --- .../org/apache/spark/SecurityManager.scala | 4 ++-- .../apache/spark/SecurityManagerSuite.scala | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/SecurityManager.scala b/core/src/main/scala/org/apache/spark/SecurityManager.scala index 673ef49e7c1c..fae69539b3fd 100644 --- a/core/src/main/scala/org/apache/spark/SecurityManager.scala +++ b/core/src/main/scala/org/apache/spark/SecurityManager.scala @@ -394,7 +394,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf) def checkUIViewPermissions(user: String): Boolean = { logDebug("user=" + user + " aclsEnabled=" + aclsEnabled() + " viewAcls=" + viewAcls.mkString(",")) - !aclsEnabled || user == null || viewAcls.contains(user) + !aclsEnabled || user == null || viewAcls.contains(user) || viewAcls.contains("*") } /** @@ -409,7 +409,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf) def checkModifyPermissions(user: String): Boolean = { logDebug("user=" + user + " aclsEnabled=" + aclsEnabled() + " modifyAcls=" + modifyAcls.mkString(",")) - !aclsEnabled || user == null || modifyAcls.contains(user) + !aclsEnabled || user == null || modifyAcls.contains(user) || modifyAcls.contains("*") } diff --git a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala index f34aefca4eb1..6c0514e61480 100644 --- a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala +++ b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala @@ -125,6 +125,27 @@ class SecurityManagerSuite extends SparkFunSuite { } + test("set security with * in acls") { + val conf = new SparkConf + conf.set("spark.ui.acls.enable", "true") + conf.set("spark.admin.acls", "user1,user2") + conf.set("spark.ui.view.acls", "*") + conf.set("spark.modify.acls", "user4") + + val securityManager = new SecurityManager(conf); + assert(securityManager.aclsEnabled() === true) + + assert(securityManager.checkUIViewPermissions("user1") === true) + assert(securityManager.checkUIViewPermissions("user5") === true) + assert(securityManager.checkUIViewPermissions("user6") === true) + assert(securityManager.checkModifyPermissions("user4") === true) + assert(securityManager.checkModifyPermissions("user7") === false) + assert(securityManager.checkModifyPermissions("user8") === false) + securityManager.setModifyAcls(Set("user4"), "*") + assert(securityManager.checkModifyPermissions("user7") === true) + assert(securityManager.checkModifyPermissions("user8") === true) + } + test("ssl on setup") { val conf = SSLSampleConfigs.sparkSSLConfig() val expectedAlgorithms = Set( From 666996973d0e3af9a41539b5d50f1be5a642dea3 Mon Sep 17 00:00:00 2001 From: zhuol Date: Mon, 24 Aug 2015 15:48:38 -0500 Subject: [PATCH 2/6] Make sure that * works for Yarn application kill since it did not support 'defaultuser, *' pattern --- .../scala/org/apache/spark/SecurityManager.scala | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/SecurityManager.scala b/core/src/main/scala/org/apache/spark/SecurityManager.scala index fae69539b3fd..6885076207c3 100644 --- a/core/src/main/scala/org/apache/spark/SecurityManager.scala +++ b/core/src/main/scala/org/apache/spark/SecurityManager.scala @@ -310,7 +310,13 @@ private[spark] class SecurityManager(sparkConf: SparkConf) setViewAcls(Set[String](defaultUser), allowedUsers) } - def getViewAcls: String = viewAcls.mkString(",") + def getViewAcls: String = { + if (viewAcls.contains("*")) { + "*" + } else { + viewAcls.mkString(",") + } + } /** * Admin acls should be set before the view or modify acls. If you modify the admin @@ -321,7 +327,13 @@ private[spark] class SecurityManager(sparkConf: SparkConf) logInfo("Changing modify acls to: " + modifyAcls.mkString(",")) } - def getModifyAcls: String = modifyAcls.mkString(",") + def getModifyAcls: String = { + if (modifyAcls.contains("*")) { + "*" + } else { + modifyAcls.mkString(",") + } + } /** * Admin acls should be set before the view or modify acls. If you modify the admin From 0568bb1db8f8b332614a9ddf80fcd74e70597025 Mon Sep 17 00:00:00 2001 From: zhuol Date: Tue, 25 Aug 2015 10:46:58 -0500 Subject: [PATCH 3/6] Add comments, one more test for adminAcl with * --- .../org/apache/spark/SecurityManager.scala | 6 ++++++ .../org/apache/spark/SecurityManagerSuite.scala | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/core/src/main/scala/org/apache/spark/SecurityManager.scala b/core/src/main/scala/org/apache/spark/SecurityManager.scala index 6885076207c3..0d7acef3833a 100644 --- a/core/src/main/scala/org/apache/spark/SecurityManager.scala +++ b/core/src/main/scala/org/apache/spark/SecurityManager.scala @@ -310,6 +310,9 @@ private[spark] class SecurityManager(sparkConf: SparkConf) setViewAcls(Set[String](defaultUser), allowedUsers) } + /** + * Checking the existence of "*" is necessary as Yarn can't recognize the "*" in "defaultuser,*" + */ def getViewAcls: String = { if (viewAcls.contains("*")) { "*" @@ -327,6 +330,9 @@ private[spark] class SecurityManager(sparkConf: SparkConf) logInfo("Changing modify acls to: " + modifyAcls.mkString(",")) } + /** + * Checking the existence of "*" is necessary as Yarn can't recognize the "*" in "defaultuser,*" + */ def getModifyAcls: String = { if (modifyAcls.contains("*")) { "*" diff --git a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala index 6c0514e61480..127204f44642 100644 --- a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala +++ b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala @@ -135,6 +135,7 @@ class SecurityManagerSuite extends SparkFunSuite { val securityManager = new SecurityManager(conf); assert(securityManager.aclsEnabled() === true) + // check for viewAcls and modifyAcls with * assert(securityManager.checkUIViewPermissions("user1") === true) assert(securityManager.checkUIViewPermissions("user5") === true) assert(securityManager.checkUIViewPermissions("user6") === true) @@ -144,6 +145,22 @@ class SecurityManagerSuite extends SparkFunSuite { securityManager.setModifyAcls(Set("user4"), "*") assert(securityManager.checkModifyPermissions("user7") === true) assert(securityManager.checkModifyPermissions("user8") === true) + + // check for adminAcls with * + securityManager.setAdminAcls("user1,user2") + securityManager.setModifyAcls(Set("user1"), "user2") + securityManager.setViewAcls(Set("user1"), "user2") + assert(securityManager.checkUIViewPermissions("user5") === false) + assert(securityManager.checkUIViewPermissions("user6") === false) + assert(securityManager.checkModifyPermissions("user7") === false) + assert(securityManager.checkModifyPermissions("user8") === false) + securityManager.setAdminAcls("user1,*") + securityManager.setModifyAcls(Set("user1"), "user2") + securityManager.setViewAcls(Set("user1"), "user2") + assert(securityManager.checkUIViewPermissions("user5") === true) + assert(securityManager.checkUIViewPermissions("user6") === true) + assert(securityManager.checkModifyPermissions("user7") === true) + assert(securityManager.checkModifyPermissions("user8") === true) } test("ssl on setup") { From b857d8f8effd6114da5b0a3173ab690dd90e6123 Mon Sep 17 00:00:00 2001 From: zhuol Date: Wed, 26 Aug 2015 17:19:12 -0500 Subject: [PATCH 4/6] Address comments --- .../main/scala/org/apache/spark/SecurityManager.scala | 4 ++-- .../scala/org/apache/spark/SecurityManagerSuite.scala | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/SecurityManager.scala b/core/src/main/scala/org/apache/spark/SecurityManager.scala index 0d7acef3833a..ee0e32494f6e 100644 --- a/core/src/main/scala/org/apache/spark/SecurityManager.scala +++ b/core/src/main/scala/org/apache/spark/SecurityManager.scala @@ -311,7 +311,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf) } /** - * Checking the existence of "*" is necessary as Yarn can't recognize the "*" in "defaultuser,*" + * Checking the existence of "*" is necessary as YARN can't recognize the "*" in "defaultuser,*" */ def getViewAcls: String = { if (viewAcls.contains("*")) { @@ -331,7 +331,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf) } /** - * Checking the existence of "*" is necessary as Yarn can't recognize the "*" in "defaultuser,*" + * Checking the existence of "*" is necessary as YARN can't recognize the "*" in "defaultuser,*" */ def getModifyAcls: String = { if (modifyAcls.contains("*")) { diff --git a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala index 127204f44642..f29160d83408 100644 --- a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala +++ b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala @@ -132,21 +132,22 @@ class SecurityManagerSuite extends SparkFunSuite { conf.set("spark.ui.view.acls", "*") conf.set("spark.modify.acls", "user4") - val securityManager = new SecurityManager(conf); + val securityManager = new SecurityManager(conf) assert(securityManager.aclsEnabled() === true) - // check for viewAcls and modifyAcls with * + // check for viewAcls with * assert(securityManager.checkUIViewPermissions("user1") === true) assert(securityManager.checkUIViewPermissions("user5") === true) assert(securityManager.checkUIViewPermissions("user6") === true) assert(securityManager.checkModifyPermissions("user4") === true) assert(securityManager.checkModifyPermissions("user7") === false) assert(securityManager.checkModifyPermissions("user8") === false) + + // check for modifyAcls with * securityManager.setModifyAcls(Set("user4"), "*") assert(securityManager.checkModifyPermissions("user7") === true) assert(securityManager.checkModifyPermissions("user8") === true) - // check for adminAcls with * securityManager.setAdminAcls("user1,user2") securityManager.setModifyAcls(Set("user1"), "user2") securityManager.setViewAcls(Set("user1"), "user2") @@ -154,6 +155,8 @@ class SecurityManagerSuite extends SparkFunSuite { assert(securityManager.checkUIViewPermissions("user6") === false) assert(securityManager.checkModifyPermissions("user7") === false) assert(securityManager.checkModifyPermissions("user8") === false) + + // check for adminAcls with * securityManager.setAdminAcls("user1,*") securityManager.setModifyAcls(Set("user1"), "user2") securityManager.setViewAcls(Set("user1"), "user2") From b1d49b32a7b1e85c265a8cee8930d0138fd3bd8d Mon Sep 17 00:00:00 2001 From: zhuol Date: Thu, 27 Aug 2015 09:28:38 -0500 Subject: [PATCH 5/6] Modify the docs --- docs/configuration.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 4a6e4dd05b66..b12638bfaf76 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1296,7 +1296,8 @@ Apart from these, the following properties are also available, and may be useful Comma separated list of users/administrators that have view and modify access to all Spark jobs. This can be used if you run on a shared cluster and have a set of administrators or devs who - help debug when things work. + help debug when things work. Putting a "*" in the list means any user can have the priviledge + of admin. @@ -1337,7 +1338,8 @@ Apart from these, the following properties are also available, and may be useful Empty Comma separated list of users that have modify access to the Spark job. By default only the - user that started the Spark job has access to modify it (kill it for example). + user that started the Spark job has access to modify it (kill it for example). Putting a "*" in + the list means any user can have access to modify it. @@ -1359,7 +1361,8 @@ Apart from these, the following properties are also available, and may be useful Empty Comma separated list of users that have view access to the Spark web ui. By default only the - user that started the Spark job has view access. + user that started the Spark job has view access. Putting a "*" in the list means any user can + have view access to this Spark job. From 77869c927a65082c78f97442648e84d02d4e5b51 Mon Sep 17 00:00:00 2001 From: zhuol Date: Fri, 28 Aug 2015 09:13:14 -0500 Subject: [PATCH 6/6] Remove a space --- core/src/main/scala/org/apache/spark/SecurityManager.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/SecurityManager.scala b/core/src/main/scala/org/apache/spark/SecurityManager.scala index ee0e32494f6e..746d2081d439 100644 --- a/core/src/main/scala/org/apache/spark/SecurityManager.scala +++ b/core/src/main/scala/org/apache/spark/SecurityManager.scala @@ -332,7 +332,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf) /** * Checking the existence of "*" is necessary as YARN can't recognize the "*" in "defaultuser,*" - */ + */ def getModifyAcls: String = { if (modifyAcls.contains("*")) { "*"