-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SPARK-1890 and SPARK-1891- add admin and modify acls #1196
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,10 +41,19 @@ import org.apache.spark.deploy.SparkHadoopUtil | |
* secure the UI if it has data that other users should not be allowed to see. The javax | ||
* servlet filter specified by the user can authenticate the user and then once the user | ||
* is logged in, Spark can compare that user versus the view acls to make sure they are | ||
* authorized to view the UI. The configs 'spark.ui.acls.enable' and 'spark.ui.view.acls' | ||
* authorized to view the UI. The configs 'spark.acls.enable' and 'spark.ui.view.acls' | ||
* control the behavior of the acls. Note that the person who started the application | ||
* always has view access to the UI. | ||
* | ||
* Spark has a set of modify acls (`spark.modify.acls`) that controls which users have permission | ||
* to modify a single application. This would include things like killing the application. By | ||
* default the person who started the application has modify access. For modify access through | ||
* the UI, you must have a filter that does authentication in place for the modify acls to work | ||
* properly. | ||
* | ||
* Spark also has a set of admin acls (`spark.admin.acls`) which is a set of users/administrators | ||
* who always have permission to view or modify the Spark application. | ||
* | ||
* Spark does not currently support encryption after authentication. | ||
* | ||
* At this point spark has multiple communication protocols that need to be secured and | ||
|
@@ -137,18 +146,32 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging { | |
private val sparkSecretLookupKey = "sparkCookie" | ||
|
||
private val authOn = sparkConf.getBoolean("spark.authenticate", false) | ||
private var uiAclsOn = sparkConf.getBoolean("spark.ui.acls.enable", false) | ||
// keep spark.ui.acls.enable for backwards compatibility with 1.0 | ||
private var aclsOn = sparkConf.getOption("spark.acls.enable").getOrElse( | ||
sparkConf.get("spark.ui.acls.enable", "false")).toBoolean | ||
|
||
// admin acls should be set before view or modify acls | ||
private var adminAcls: Set[String] = | ||
stringToSet(sparkConf.get("spark.admin.acls", "")) | ||
|
||
private var viewAcls: Set[String] = _ | ||
|
||
// list of users who have permission to modify the application. This should | ||
// apply to both UI and CLI for things like killing the application. | ||
private var modifyAcls: Set[String] = _ | ||
|
||
// always add the current user and SPARK_USER to the viewAcls | ||
private val defaultAclUsers = Seq[String](System.getProperty("user.name", ""), | ||
private val defaultAclUsers = Set[String](System.getProperty("user.name", ""), | ||
Option(System.getenv("SPARK_USER")).getOrElse("")) | ||
|
||
setViewAcls(defaultAclUsers, sparkConf.get("spark.ui.view.acls", "")) | ||
setModifyAcls(defaultAclUsers, sparkConf.get("spark.modify.acls", "")) | ||
|
||
private val secretKey = generateSecretKey() | ||
logInfo("SecurityManager: authentication " + (if (authOn) "enabled" else "disabled") + | ||
"; ui acls " + (if (uiAclsOn) "enabled" else "disabled") + | ||
"; users with view permissions: " + viewAcls.toString()) | ||
"; ui acls " + (if (aclsOn) "enabled" else "disabled") + | ||
"; users with view permissions: " + viewAcls.toString() + | ||
"; users with modify permissions: " + modifyAcls.toString()) | ||
|
||
// Set our own authenticator to properly negotiate user/password for HTTP connections. | ||
// This is needed by the HTTP client fetching from the HttpServer. Put here so its | ||
|
@@ -169,18 +192,43 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging { | |
) | ||
} | ||
|
||
private[spark] def setViewAcls(defaultUsers: Seq[String], allowedUsers: String) { | ||
viewAcls = (defaultUsers ++ allowedUsers.split(',')).map(_.trim()).filter(!_.isEmpty).toSet | ||
/** | ||
* Split a comma separated String, filter out any empty items, and return a Set of strings | ||
*/ | ||
private def stringToSet(list: String): Set[String] = { | ||
(list.split(',')).map(_.trim()).filter(!_.isEmpty).toSet | ||
} | ||
|
||
private[spark] def setViewAcls(defaultUsers: Set[String], allowedUsers: String) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More out of curiosity: since the class is already "private[spark]", is the modifier also needed here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no I don't believe it is needed, I'll remove them. |
||
viewAcls = (adminAcls ++ defaultUsers ++ stringToSet(allowedUsers)) | ||
logInfo("Changing view acls to: " + viewAcls.mkString(",")) | ||
} | ||
|
||
private[spark] def setViewAcls(defaultUser: String, allowedUsers: String) { | ||
setViewAcls(Seq[String](defaultUser), allowedUsers) | ||
setViewAcls(Set[String](defaultUser), allowedUsers) | ||
} | ||
|
||
private[spark] def getViewAcls: String = viewAcls.mkString(",") | ||
|
||
private[spark] def setModifyAcls(defaultUsers: Set[String], allowedUsers: String) { | ||
modifyAcls = (adminAcls ++ defaultUsers ++ stringToSet(allowedUsers)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this require that I think it would be better to either calculate the view ACLs as part of the getter, or to call this as part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes it requires it set before. I went back and forth on this a bit and choose to keep it this way since its private and only really called in once place at this point (history ui). And actually only the view one is called the modify one isn't called anywhere outside of this class. We could add the additional logic but I kind of see it as just overhead right now. Normally everything is initialized just when you create the securityManager and so these routines aren't called outside of here. I could be swayed to change it. I should atleast add a comment here also. I have it in some other places, but should add here too. |
||
logInfo("Changing modify acls to: " + modifyAcls.mkString(",")) | ||
} | ||
|
||
private[spark] def setUIAcls(aclSetting: Boolean) { | ||
uiAclsOn = aclSetting | ||
logInfo("Changing acls enabled to: " + uiAclsOn) | ||
private[spark] def getModifyAcls: String = modifyAcls.mkString(",") | ||
|
||
/** | ||
* Admin acls should be set before the view or modify acls. If you modify the admin | ||
* acls you should also set the view and modify acls again to pick up the changes. | ||
*/ | ||
private[spark] def setAdminAcls(adminUsers: String) { | ||
adminAcls = stringToSet(adminUsers) | ||
logInfo("Changing admin acls to: " + adminAcls.mkString(",")) | ||
} | ||
|
||
private[spark] def setAcls(aclSetting: Boolean) { | ||
aclsOn = aclSetting | ||
logInfo("Changing acls enabled to: " + aclsOn) | ||
} | ||
|
||
/** | ||
|
@@ -224,22 +272,39 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging { | |
* Check to see if Acls for the UI are enabled | ||
* @return true if UI authentication is enabled, otherwise false | ||
*/ | ||
def uiAclsEnabled(): Boolean = uiAclsOn | ||
def aclsEnabled(): Boolean = aclsOn | ||
|
||
/** | ||
* Checks the given user against the view acl list to see if they have | ||
* authorization to view the UI. If the UI acls must are disabled | ||
* via spark.ui.acls.enable, all users have view access. | ||
* authorization to view the UI. If the UI acls are disabled | ||
* via spark.acls.enable, all users have view access. If the user is null | ||
* it is assumed authentication isn't turned on and all users have access. | ||
* | ||
* @param user to see if is authorized | ||
* @return true is the user has permission, otherwise false | ||
*/ | ||
def checkUIViewPermissions(user: String): Boolean = { | ||
logDebug("user=" + user + " uiAclsEnabled=" + uiAclsEnabled() + " viewAcls=" + | ||
logDebug("user=" + user + " aclsEnabled=" + aclsEnabled() + " viewAcls=" + | ||
viewAcls.mkString(",")) | ||
if (uiAclsEnabled() && (user != null) && (!viewAcls.contains(user))) false else true | ||
if (aclsEnabled() && (user != null) && (!viewAcls.contains(user))) false else true | ||
} | ||
|
||
/** | ||
* Checks the given user against the modify acl list to see if they have | ||
* authorization to modify the application. If the UI acls are disabled | ||
* via spark.acls.enable, all users have modify access. If the user is null | ||
* it is assumed authentication isn't turned on and all users have access. | ||
* | ||
* @param user to see if is authorized | ||
* @return true is the user has permission, otherwise false | ||
*/ | ||
def checkModifyPermissions(user: String): Boolean = { | ||
logDebug("user=" + user + " aclsEnabled=" + aclsEnabled() + " modifyAcls=" + | ||
modifyAcls.mkString(",")) | ||
if (aclsEnabled() && (user != null) && (!modifyAcls.contains(user))) false else true | ||
} | ||
|
||
|
||
/** | ||
* Check to see if authentication for the Spark communication protocols is enabled | ||
* @return true if authentication is enabled, otherwise false | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: too many parentheses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed a couple.