-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Add privilege for script actions #48856
Comments
Pinging @elastic/es-security (:Security/Authorization) |
Rather than adding new permitted actions to the |
Do you mean that manage_scripts should be a global permission, and not restricted to watcher? Would you then also want to support more granular permission, so scripts can only be executed in single or multiple places (like watches)? |
@P1llus I originally meant to create a new cluster privilege to be able to use the GET/PUT/DELETE script APIs. What you are suggesting, being able to confine the usage of scripts to certain contexts, is a different thing. But I think this is an interesting proposal. I envisage being able to define a list of context names when PUTing the script. I wonder what folks from @elastic/es-core-features think about this? |
The reason the idea came up, is just like @spinscale mentioned, when the case is external scripts, just to minimize the possibility of some sort of vulnerability. While context level would be great, if that is a really big task, then we should at least be able to give access based on script type maybe? |
We discussed this in our weekly team meeting. We have decided on adding a What @P1llus is suggesting, to have fine-grained privilege on scripts, and enforce those privileges whenever a script is used, is difficult to achieve at the moment and would have a broad impact (it would be a breaking change). However, this is an interesting proposal that should be explored as part of the work to add the new |
@bytebilly offered to take the temperature of the requirement for fine-grained script privileges, as we currently don't have a clear idea about the solutions benefiting from such a privilege. |
@albertzaharovits Thanks for following this up! :) |
Sorry for the late update here: as of today, I got no evidence that fine-grained access is required. I'll keep an eye on that and will update here in case something will come out. |
@bytebilly thanks a ton for digging into this! Just to be sure I understand your answer correctly in the context of my initial question: Your answer only refers to the requirement for fine grained privileges, but not for the ask to be able to manage scripts as an watcher admin (which still is an open issue then to have a |
@spinscale yes that's the case. We don't have scheduled it yet, given the long list of current priorities. |
We plan to use the execute painless API when creating a Runtime Fields in the Index Pattern Editor. Most of the users that will interact with Runtime Fields at this stage are not eligible for the |
@jimczi do we need full control over scripts (add, remove, etc) or just execute? I'm not sure we want to grant full control over every stored script (as it would be all or nothing) to any user. Do we need to add it just to What do you think? |
+1, @javanna @jdconrad do you think it's ok to expect Kibana to always use the |
Runtime fields don't support stored scripts, so no need for add and remove privilege on our end. The plan of granting painless/execute role to kibana_system sounds good to me. |
+1 to what @javanna has already said - there shouldn't be anything dangerous about running these scripts through the execute API |
Can you clarify how we intend to use this? (*) Would that checking be just for compilation, or will it submit a sample document, or something else? My concern in the more general case, is that the execute API allow access to specific index metadata (existence of indices, mappings, etc) which Kibana system can access, but the user themselves may not have access to. We need to be sure that we're not opening a new path for the user to escalate their access. |
@tvernum The execute API takes in an example document created only for this request, but it also takes in an existing index name and pulls mappings from live indexes. If this is a security risk we need to re-think how we take in mappings. Painless Lab currently also uses this same strategy for score and filter contexts. I know in the future we would like to provide a doc id and pull that data so the user doesn't have to create an entire example doc -- but maybe this would be easier/better from the Kibana side as a separate request to fill in that data. Edit: I wonder if Kibana should pull in mappings as a separate request and then the execute api should process them in the same way as the strategy for grabbing an existing document. |
FYI, Kibana now has runtime field preview in 7.15.0 but I get an error with my Okta account;
Did we do this? Or still need to? |
btw, this is kind of blocking me on one of our internal clusters. It would be great if we could resolve it in 7.15.1. |
@LeeDr probably the simplest short-term fix to your problem is adding |
@bytebilly @LeeDr , we have a plan for this but it felt through the cracks :(. |
Thanks @jimczi. I'm not sure if the change will be something appropriate for a patch release, but 7.15.1 FF is about a week away. If we can't get it in there, or don't feel the change is appropriate for a patch release, I'll ask Infra to add Also, in case anybody wonders if there's a work-around for adding or editing a runtime field without this privilege, I did look into that. I figured I could use examples from https://www.elastic.co/guide/en/elasticsearch/reference/master/runtime-mapping-fields.html in the Dev Tools Console to add or replace the runtime field (without hitting the Preview path which I don't have privs for). But the Elasticsearch examples are adding runtime fields to an index mapping and that's not how Kibana does it (AFAIK). I think Kibana saves the runtime field in the Kibana index-pattern saved object and sends the query in searches each time. Or I could execute a command in the Dev Tools Console to modify the index-pattern in the .kibana index. |
This change refactors the painless execute action into two actions: * PainlessExecuteAction * PainlessExecuteIndexAction Script context that require an index to run should use the newly added PainlessExecuteIndexAction. This action checks if the user has the read permission on the index to run but unlike the existing PainlessExecuteAction, no cluster privilege is required. For scripts that run outside of an index context, the existing PainlessExecuteAction can still be used. Note that passing the index in the body continues to work for backward compatibility purpose but in such case it's the old action that requires the cluster privilege **and** the read privilege that will run. Relates elastic#48856
I'd like to revisit this as kibana is still unable to use the execute api. Granting privileges to the There will be another api created which with take a painless script and return the a field list - part of the UX for creating composite runtime fields. This new api will have similar needs. |
Could you please expand this a bit more? What do you mean by "can use"? I think that a user should be able to execute scripts when adding runtime fields, but not when just "using" them (e.g. getting results), as they are transparent. Is that what you meant? |
We have the need for admins to disable runtime field authoring in Kibana. Does it make sense to set that up as a privilege all users have by default which allows both execute for that user, and the authoring UX in Kibana. Admins can disable the privilege globally for Kibana, or by role. |
Sorry, perhaps 'add runtime fields' makes more sense |
Which is the main reason for this access control? It is about adding runtime fields to the mapping, or more broadly using them in search queries too? Since these features are available at the Elasticsearch API level, blocking at Kibana level may not be the best approach. We also don't provide privileges that are automatically granted to all users, or "negative" privileges. This would make it more complex to leverage the current privilege model to forbid the use of runtime fields. |
It might be best to take a step back and have a more thorough conversation about this.
|
I think that we should split this issue into separate — even if related — discussions:
We agreed to implement For the execute call ( For |
@bytebilly for #4, the customer concern centers around the performance impact of users who create Runtime Fields in Kibana without understanding the implications. I took that as concern for users creating them in the UX without knowing the impact. Runtime Fields fields living within the data view potentially increases the overhead for everyone. I agree with separating the issues, but that is why I was conflating them. I don't think we want a kill switch for any runtime field coming from Kibana, we want to give administrators the power to control who can author persisted runtime fields. |
If that the case then there's no reason to involve ES. |
I can't take responsibility for the implications of a design decision like that on the ES side. If you tell me that it can't inadvertently result in privileged escalation then I'll believe you. |
Painless execute allows users to validate their scripts. Some of the supported script contexts support providing a sample document as well as an index to pull the mappings from. The painless execute API requires cluster admin privileges today and while that's ok for the contexts that don't support providing an index, it is not ideal when an index is provided. In fact users can run scripts as part of the search API, which requires only the indices/read privilege on the indices that the users is reading from. This commit maps the painless execute action to an indices/read action when an index is specified, so that in that case the same privileges as a search action will be requested to run painless execute. Relates to elastic#48856
…#85512) Painless execute allows users to validate their scripts. Some of the supported script contexts support providing a sample document as well as an index to pull the mappings from. The painless execute API requires cluster admin privileges today and while that's ok for the contexts that don't support providing an index, it is not ideal when an index is provided. In fact users can run scripts as part of the search API, which requires only the indices/read privilege on the indices that the users is reading from. This commit maps the painless execute action to an indices/read action when an index is specified, so that in that case the same privileges as a search action will be requested to run painless execute. Relates to #48856 Closes #86428
Would need this specific privilege to allow user to add script without having 'manage,all' privilege. |
Yeah, we're interested in the same thing. We want to give engineers access to dev tools and the painless lab but only for the indices that they can already access. In our case they use painless scripts for building and testing watchers. |
Describe the feature: The
manage_watcher
privilege right now only contains the"cluster:admin/xpack/watcher/*", "cluster:monitor/xpack/watcher/*"
actions.I do think it makes sense to also add
cluster:admin/script/*
(get/put/delete scripts) in there as well, as a watcher admin may need to refer to external scripts.If you think that makes sense, I'm happy to open a PR for this.
The text was updated successfully, but these errors were encountered: