Skip to content
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

Change default for script.disable_dynamic #5853

Closed
rtoma opened this issue Apr 17, 2014 · 11 comments · Fixed by #5943

Comments

@rtoma
Copy link

commented Apr 17, 2014

Please make setting script.disable_dynamic=true the default.
The current default: 'false' allows for nasty business via a _search api call. The docs already mention this, but does not mention the high level of nastiness. More details available. PM is probably better suited.

@kimchy

This comment has been minimized.

Copy link
Member

commented Apr 17, 2014

I am +1 on this, better be safe and we can do it for the 1.2 release with proper documentation.

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2014

++

@nik9000

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2014

Dynamic scripts are so useful though! It lets me upgrade my application without messing with Elasticsearch at all. I can try out new features with no trouble. I imagine you can do a lot with them, but can it be mitigated with better sandboxing?

@kimchy

This comment has been minimized.

Copy link
Member

commented Apr 17, 2014

@nik9000 agreed!, the path forward is to first fix the problem we have with mvel to pick a better scripting engine (by default). Once we do, then see how we can sandbox said scripting engine in an optimized manner. Once we do, then we can re-enable them by default, but thats a longer task.

@byronvoorbach

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2014

Note: If you disable the dynamic scripting, it will still be possible to use stored scripts.
That way, disabling it by default just requires some extra explanation for current users that use this feature.

@rtoma

This comment has been minimized.

Copy link
Author

commented Apr 17, 2014

Credits to @byronvoorbach for showing me very nasty proof of concepts.

@dakrone

This comment has been minimized.

Copy link
Member

commented Apr 25, 2014

@kimchy @s1monw how do we feel about making the script.disable_dynamic setting dynamic so it can be changed? I am in favor of making it dynamic as it makes dealing with this breaking change a little bit easier.

@kimchy

This comment has been minimized.

Copy link
Member

commented Apr 25, 2014

I think its safer to not allow for it to be dynamic I think, so I vote to not make it dynamic.

@dakrone

This comment has been minimized.

Copy link
Member

commented Apr 25, 2014

Hmm.. looking at this further, how do we feel about allowing dynamic mustache scripting by default? It looks like mustache is only templating instead of execution. Without allowing it, you can't allow template queries unless you enable dynamic scripting.

@kimchy

This comment has been minimized.

Copy link
Member

commented Apr 25, 2014

I think mustache should be safe, maybe we can have a flag that states for a scripting engine if its a template one or scripting?

dakrone added a commit to dakrone/elasticsearch that referenced this issue Apr 25, 2014
dakrone added a commit that referenced this issue Apr 25, 2014
@kimchy

This comment has been minimized.

Copy link
Member

commented May 1, 2014

Btw, it is documented that this should be disabled is someone is concerned about the security implications: http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/modules-scripting.html#_disabling_dynamic_scripts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.