Rewrite the scripting security docs #23930

nik9000 · 2017-04-05T21:21:16Z

They needed to be updated now that Painless is the default and
the non-sandboxed scripting languages are going away or gone.

I dropped the entire section about customizing the classloader
whitelists. In master this barely does anything (exposes more
things to expressions) and in 5.x it isn't advisable anyway.

They needed to be updated now that Painless is the default and the non-sandboxed scripting languages are going away or gone. I dropped the entire section about customizing the classloader whitelists. In master this barely does anything (exposes more things to expressions) and in 5.x it isn't advisable anyway.

jasontedor

I left some nits, they do not require a follow-up from me.

jasontedor · 2017-04-06T01:22:50Z

docs/reference/modules/scripting/security.asciidoc

+and `javascript` is super dangerous. These languages have unfettered access to
+Elasticsearch and can cause all kind of trouble. Worse, the only thing
+preventing them from causing real trouble is the Java security manager and
+SECCOMP/Seatbelt/ActiveProcessLimit. Assuming that those will prevent disaster


Nit: SECCOMP -> seccomp

jasontedor · 2017-04-06T01:25:55Z

docs/reference/modules/scripting/security.asciidoc

+Internet write scripts.
+
+Enabling dynamic scripting for non-sandboxed languages like `groovy`, `python`,
+and `javascript` is super dangerous. These languages have unfettered access to


Unless you want to refer to the modules/plugins by name here, I think it's better to not stylize these in monospace, but instead refer to them by their names: Groovy, Python, and JavaScript.

jasontedor · 2017-04-06T01:27:01Z

docs/reference/modules/scripting/security.asciidoc

-<2> Allow inline Groovy scripts for all operations
-<3> Allow stored Groovy scripts to be used for search and aggregations.
+<2> Allow inline Painless scripts for all operations
+<3> Allow stored Painless scripts to be used for search and aggregations.


You're inconsistent as far as whether or not these end with periods.

clintongormley

In general, good changes. I think the tone isn't quite right - it's conversational, which is suitable for a blog but not so much for documentation about security. You don't want opinions or conversations here, or anything vague that might be misunderstood, or presuming that users know what eg seccomp is. You want very clear, simple advice, with clear definitions etc.

clintongormley · 2017-04-06T07:03:39Z

docs/reference/modules/scripting/security.asciidoc

+First and foremost, never run Elasticsearch as the `root` user, as this would
+allow a script to access or do *anything* on your server, without limitations.
+Elasticsearch checks if it is running as `root` on startup but it won't
+refuse to start if it can't be sure so do your part to make sure it isn't


if it can't be sure? why wouldn't it be sure? I think simplify this (leaving out edge cases) to simply say: don't run as root.

clintongormley · 2017-04-06T07:04:56Z

docs/reference/modules/scripting/security.asciidoc

+running as `root`.
+
+You should not expose Elasticsearch directly to users, but instead have an
+application in between your users and Elasticsearch. If you *must* allow users


"an application to sanitise user input before running requests on ES" - or something similar

clintongormley · 2017-04-06T07:10:15Z

docs/reference/modules/scripting/security.asciidoc

-directly to your users, then you have to decide whether you trust them enough
-to run scripts on your box or not, and apply the appropriate safety measures.
-
-[[enable-dynamic-scripting]]


removing this anchor is probably going to break a lot of links in other docs - be aware that other books will need edits too

Do you think I should keep the anchor somewhere with a comment that it is historical? Something about how it is no longer needed in most cases?

No, I think it probably makes more sense to fix the places that link to it.

I got the docs build to verify this and it looks like nothing else links there.

clintongormley · 2017-04-06T07:17:37Z

docs/reference/modules/scripting/security.asciidoc

+https://en.wikipedia.org/wiki/Defense_in_depth_(computing)[layers], so you'll
+want to take some steps as well.
+
+First and foremost, never run Elasticsearch as the `root` user, as this would


Add

[float] === Don't run as root

clintongormley · 2017-04-06T07:18:18Z

docs/reference/modules/scripting/security.asciidoc

+Elasticsearch checks if it is running as `root` on startup but it won't
+refuse to start if it can't be sure so do your part to make sure it isn't
+running as `root`.
+


Add

[float] === Don't expose Elasticsearch to the Internet

clintongormley · 2017-04-06T07:20:10Z

docs/reference/modules/scripting/security.asciidoc

+determined malicious user to write searches that overwhelm the Elasticsearch
+cluster and bring it down.
+
+By default Elasticsearch enables dynamic scripting for sandboxed languages,


Add

[float] === Sandboxed scripting languages

clintongormley · 2017-04-06T07:44:32Z

docs/reference/modules/scripting/security.asciidoc

+By default Elasticsearch enables dynamic scripting for sandboxed languages,
+namely the scripting language `painless`, the template language `mustache`, and
+the expression language `expressions`. These *ought* to be safe to expose to
+trusted users and to your application servers. It is tempting fate to expose


Instead of "it is tempting fate", perhaps something like "Be aware that all software has bugs, and bugs can be exploited by determined malicious users"

clintongormley · 2017-04-06T07:49:50Z

docs/reference/modules/scripting/security.asciidoc

+and `javascript` is super dangerous. These languages have unfettered access to
+Elasticsearch and can cause all kind of trouble. Worse, the only thing
+preventing them from causing real trouble is the Java security manager and
+SECCOMP/Seatbelt/ActiveProcessLimit. Assuming that those will prevent disaster


I don't like this line starting "Assuming..." - it sounds like you're saying that the security manager etc is pretty much a waste of time.

I'd have a section explaining the other layers of security that we have such as JSM, seccomp, etc. Then say something like "Even with those in place, non-sandboxed languages provide the malicious user with too many opportunities to attack your cluster or to mount a denial of service attack"

clintongormley · 2017-04-06T07:51:52Z

docs/reference/modules/scripting/security.asciidoc

-script.inline: false
-script.stored: false
-script.file:   true
+script.inline: false  <1>


these are the defaults for non-sandboxed, not for all scripts

clintongormley · 2017-04-06T07:53:14Z

docs/reference/modules/scripting/security.asciidoc

-[[java-security-manager]]
-[float]
-=== Java Security Manager
-


While class whitelisting may not be useful for Painless, this section is probably still useful for plugin authors. Perhaps it should be moved to the plugin docs?

There is a section in the plugin/authors.asciidoc about the security manager. I'll see about what can be merged.

After another read it look like nothing can be merged. All the tips here are basically ways to expose more classes to groovy. The words in plugin/authors.asciidoc are more about making the security manager work for your plugin.

I wonder if it makes sense to only target this at 6.0? Or just cherry-pick some of it back to 5.x? The page is pretty out of date for 5.x but all this security manager stuff still kind of applies to groovy.

nik9000 · 2017-04-06T16:43:08Z

@clintongormley I believe this is ready for another round when you are ready for it.

clintongormley

Looks fantastic! (just one section that I think needs a little tweaking).

Regarding 5.x vs 6, I think it is worth backporting but in 5.x I'd leave the java security policy stuff, and in 6.0 I'd remove all mention of non-sandboxed languages as they won't exist anymore

clintongormley · 2017-04-07T07:41:24Z

docs/reference/modules/scripting/security.asciidoc

@@ -1,67 +1,108 @@
 [[modules-scripting-security]]
 === Scripting and security

-You should never run Elasticsearch as the `root` user, as this would allow a
-script to access or do *anything* on your server, without limitations.
+While Elasticsearch contributors make every effort to prevent scrips from


scrips -> scripts

clintongormley · 2017-04-07T07:44:00Z

docs/reference/modules/scripting/security.asciidoc

+is a poor choice because:
+1. This drops a layer of security, leaving only Elasticsearch's builtin
+<<modules-scripting-other-layers, security layers>>.
+2. Un-sandboxed scripts have unchecked access to Elasticsearch's internals and


Un-sandboxed -> Non-sandboxed

clintongormley · 2017-04-07T07:47:21Z

docs/reference/modules/scripting/security.asciidoc

+Which scripts Elasticsearch will execute where is controlled by settings
+starting with `scripts.`. The simplest settings allow scripts to be enabled
+or disabled based on where they are stored. The default values for
+non-sandboxed languages are the following:


This section is a bit confusing I think because you're showing the global settings, but using them to explain the default settings for non-sandboxed languages. I'd perhaps explain the default for sandboxed vs non-sandboxed first, then say "you can use these global settings to enable/disable ALL scripting languages"

jasontedor

I left a few more comments.

jasontedor · 2017-04-07T10:44:59Z

docs/reference/modules/scripting/security.asciidoc

+https://en.wikipedia.org/wiki/Seccomp[seccomp] in Linux,
+https://www.chromium.org/developers/design-documents/sandbox/osx-sandboxing-design[Seatbelt] in mac OS,
+and ActiveProcessLimit on Windows to prevent forking and other undesirable
+behaviors like writing to unexpected directories.


We do not use these mechanisms for preventing writes, only to prevent fork/exec. The Java security manager plays the role of guarding the filesystem.

I thought we used it for more than fork/exec. I know seccomp would support all kind of things and I thought we'd turned on some of them. Will reread for my own sanity.

Just reread. Only exec and fork as you say. Will fix.

jasontedor · 2017-04-07T10:45:09Z

docs/reference/modules/scripting/security.asciidoc

+
+Elasticsearch uses
+https://en.wikipedia.org/wiki/Seccomp[seccomp] in Linux,
+https://www.chromium.org/developers/design-documents/sandbox/osx-sandboxing-design[Seatbelt] in mac OS,


Nit: mac OS -> macOS

I keep remembering this one incorrectly.

jasontedor · 2017-04-07T10:46:57Z

docs/reference/modules/scripting/security.asciidoc

+Elasticsearch uses
+https://en.wikipedia.org/wiki/Seccomp[seccomp] in Linux,
+https://www.chromium.org/developers/design-documents/sandbox/osx-sandboxing-design[Seatbelt] in mac OS,
+and ActiveProcessLimit on Windows to prevent forking and other undesirable


A link here is https://msdn.microsoft.com/en-us/library/windows/desktop/ms684147.

I'd found a bunch of links but wasn't familiar enough with the windows API to know which one is canonical. Similarly, I used a link to the chromium docs for Seatbelt because it looked like it explained the mechanism better and actually used the word Seatbelt. I couldn't really follow the Apple links.

The Apple docs indeed leave room for improvement.

nik9000 · 2017-04-07T13:33:21Z

Regarding 5.x vs 6, I think it is worth backporting but in 5.x I'd leave the java security policy stuff, and in 6.0 I'd remove all mention of non-sandboxed languages as they won't exist anymore

I'd like to remove the non-sandboxed stuff in the master branch in a followup so I can backport this pretty cleanly to 5.x.

nik9000 · 2017-04-07T14:18:39Z

@clintongormley and @jasontedor, I pushed some more changes. Please have a look when you get a chance.

jasontedor

LGTM.

clintongormley

LGTM

They needed to be updated now that Painless is the default and the non-sandboxed scripting languages are going away or gone.

nik9000 · 2017-04-07T15:57:33Z

Thanks @jasontedor and @clintongormley! I've merged to master and backported to 5.x with 66fec71. I believe it keeps all the for the groovy workarounds.

Drops any mention of non-sandboxed scripting languages other than a brief "we don't support them and we shouldn't because A and B" statement. Relates to elastic#23930

Drops any mention of non-sandboxed scripting languages other than a brief "we don't support them and we shouldn't because A and B" statement. Relates to #23930

nik9000 added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >docs General docs changes review v5.4.0 v6.0.0-alpha1 labels Apr 5, 2017

nik9000 requested a review from clintongormley April 5, 2017 21:21

jasontedor approved these changes Apr 6, 2017

View reviewed changes

clintongormley suggested changes Apr 6, 2017

View reviewed changes

nik9000 added 2 commits April 6, 2017 11:21

More formal language

755c9e0

Fix default

5b73874

clintongormley approved these changes Apr 7, 2017

View reviewed changes

jasontedor reviewed Apr 7, 2017

View reviewed changes

nik9000 added 4 commits April 7, 2017 09:45

Fix section around forking

0517b0b

Spelling

e0eae40

Rewrite section about script locations

f16a36d

Rewrite section about defaults

57ca834

jasontedor approved these changes Apr 7, 2017

View reviewed changes

clintongormley approved these changes Apr 7, 2017

View reviewed changes

nik9000 merged commit 7fad7c6 into elastic:master Apr 7, 2017

nik9000 added a commit that referenced this pull request Apr 7, 2017

Rewrite the scripting security docs (#23930)

66fec71

They needed to be updated now that Painless is the default and the non-sandboxed scripting languages are going away or gone.

nik9000 mentioned this pull request Apr 7, 2017

Update scripts/security docs for sandboxed world #23977

Merged

nik9000 deleted the rewrite_scripting_security_docs branch June 7, 2017 14:52

Rewrite the scripting security docs #23930

Rewrite the scripting security docs #23930

Conversation

nik9000 commented Apr 5, 2017

jasontedor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintongormley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Apr 6, 2017

clintongormley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasontedor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Apr 7, 2017

nik9000 commented Apr 7, 2017 • edited Loading

jasontedor left a comment

Choose a reason for hiding this comment

clintongormley left a comment

Choose a reason for hiding this comment

nik9000 commented Apr 7, 2017

nik9000 commented Apr 7, 2017 •

edited

Loading