From b6a23185638197cfa5f2cab612c558b9e73734fe Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 3 Nov 2015 09:52:25 -0500 Subject: [PATCH] upgrade rhino for plugins/lang-javascript the current jar is over 3 years old, we should upgrade it for bugfixes. the current integration could be more secure: set a global policy and enforce additional (compile-time) checks closes #14466 --- .../elasticsearch/bootstrap/untrusted.policy | 5 ++- plugins/lang-javascript/build.gradle | 2 +- .../licenses/rhino-1.7.7.jar.sha1 | 1 + .../licenses/rhino-1.7R4.jar.sha1 | 1 - .../plugin/javascript/JavaScriptPlugin.java | 5 +++ .../JavaScriptScriptEngineService.java | 40 ++++++++++++++----- .../javascript/JavaScriptSecurityTests.java | 23 ++++++++--- .../script/python/PythonSecurityTests.java | 14 ++++--- 8 files changed, 65 insertions(+), 26 deletions(-) create mode 100644 plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 delete mode 100644 plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy index 2475c56e814..dbbc4f14d7e 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy @@ -19,13 +19,16 @@ /* * Limited security policy for scripts. - * This is what is needed for invokeDynamic functionality to work. + * This is what is needed for basic functionality to work. */ grant { // groovy IndyInterface bootstrap requires this property for indy logging permission java.util.PropertyPermission "groovy.indy.logging", "read"; + // needed by Rhino engine exception handling + permission java.util.PropertyPermission "rhino.stack.style", "read"; + // needed IndyInterface selectMethod (setCallSiteTarget) permission java.lang.RuntimePermission "getClassLoader"; }; diff --git a/plugins/lang-javascript/build.gradle b/plugins/lang-javascript/build.gradle index 938d7cc5e73..ead459f29d1 100644 --- a/plugins/lang-javascript/build.gradle +++ b/plugins/lang-javascript/build.gradle @@ -23,7 +23,7 @@ esplugin { } dependencies { - compile 'org.mozilla:rhino:1.7R4' + compile 'org.mozilla:rhino:1.7.7' } compileJava.options.compilerArgs << "-Xlint:-rawtypes,-unchecked" diff --git a/plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 b/plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 new file mode 100644 index 00000000000..8c997d41c2b --- /dev/null +++ b/plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 @@ -0,0 +1 @@ +3a9ea863b86126b0ed8f2fe2230412747cd3c254 \ No newline at end of file diff --git a/plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 b/plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 deleted file mode 100644 index 3432f18a5de..00000000000 --- a/plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -e982f2136574b9a423186fbaeaaa98dc3e5a5288 diff --git a/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java b/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java index a6832fe7afe..9ca36bd9f86 100644 --- a/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java +++ b/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java @@ -28,6 +28,11 @@ import org.elasticsearch.script.javascript.JavaScriptScriptEngineService; */ public class JavaScriptPlugin extends Plugin { + static { + // install rhino policy on plugin init + JavaScriptScriptEngineService.init(); + } + @Override public String name() { return "lang-javascript"; diff --git a/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java b/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java index 70fc5f46d63..621d338128f 100644 --- a/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java +++ b/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java @@ -58,6 +58,34 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements private Scriptable globalScope; + // one time initialization of rhino security manager integration + private static final CodeSource DOMAIN; + static { + try { + DOMAIN = new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[]) null); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + SecurityController.initGlobal(new PolicySecurityController() { + @Override + public GeneratedClassLoader createClassLoader(ClassLoader parent, Object securityDomain) { + // don't let scripts compile other scripts + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + sm.checkPermission(new SpecialPermission()); + } + // check the domain, this is all we allow + if (securityDomain != DOMAIN) { + throw new SecurityException("illegal securityDomain: " + securityDomain); + } + return super.createClassLoader(parent, securityDomain); + } + }); + } + + /** ensures this engine is initialized */ + public static void init() {} + @Inject public JavaScriptScriptEngineService(Settings settings) { super(settings); @@ -100,21 +128,11 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements @Override public Object compile(String script) { - // we don't know why kind of safeguards rhino has, - // but just be safe - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new SpecialPermission()); - } Context ctx = Context.enter(); try { ctx.setWrapFactory(wrapFactory); ctx.setOptimizationLevel(optimizationLevel); - ctx.setSecurityController(new PolicySecurityController()); - return ctx.compileString(script, generateScriptName(), 1, - new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[]) null)); - } catch (MalformedURLException e) { - throw new RuntimeException(e); + return ctx.compileString(script, generateScriptName(), 1, DOMAIN); } finally { Context.exit(); } diff --git a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java index 887317fb744..410099de0a7 100644 --- a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java +++ b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java @@ -23,8 +23,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; -import org.junit.After; -import org.junit.Before; import org.mozilla.javascript.WrappedException; import java.util.HashMap; @@ -37,14 +35,18 @@ public class JavaScriptSecurityTests extends ESTestCase { private JavaScriptScriptEngineService se; - @Before - public void setup() { + @Override + public void setUp() throws Exception { + super.setUp(); se = new JavaScriptScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + // otherwise will exit your VM and other bad stuff + assumeTrue("test requires security manager to be enabled", System.getSecurityManager() != null); } - @After - public void close() { + @Override + public void tearDown() throws Exception { se.close(); + super.tearDown(); } /** runs a script */ @@ -86,4 +88,13 @@ public class JavaScriptSecurityTests extends ESTestCase { // no files assertFailure("java.io.File.createTempFile(\"test\", \"tmp\")"); } + + public void testDefinitelyNotOK() { + // no mucking with security controller + assertFailure("var ctx = org.mozilla.javascript.Context.getCurrentContext(); " + + "ctx.setSecurityController(new org.mozilla.javascript.PolicySecurityController());"); + // no compiling scripts from scripts + assertFailure("var ctx = org.mozilla.javascript.Context.getCurrentContext(); " + + "ctx.compileString(\"1 + 1\", \"foobar\", 1, null); "); + } } diff --git a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java index fd60607e2e6..c4a80a41916 100644 --- a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java +++ b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java @@ -23,8 +23,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; -import org.junit.After; -import org.junit.Before; import org.python.core.PyException; import java.util.HashMap; @@ -37,17 +35,21 @@ public class PythonSecurityTests extends ESTestCase { private PythonScriptEngineService se; - @Before - public void setup() { + @Override + public void setUp() throws Exception { + super.setUp(); se = new PythonScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + // otherwise will exit your VM and other bad stuff + assumeTrue("test requires security manager to be enabled", System.getSecurityManager() != null); } - @After - public void close() { + @Override + public void tearDown() throws Exception { // We need to clear some system properties System.clearProperty("python.cachedir.skip"); System.clearProperty("python.console.encoding"); se.close(); + super.tearDown(); } /** runs a script */