From 2169a123a5b3296d47eb87b4f89ecba8a08c20bc Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Sat, 5 Dec 2015 21:46:52 -0500 Subject: [PATCH 1/2] Filter classes loaded by scripts Since 2.2 we run all scripts with minimal privileges, similar to applets in your browser. The problem is, they have unrestricted access to other things they can muck with (ES, JDK, whatever). So they can still easily do tons of bad things This PR restricts what classes scripts can load via the classloader mechanism, to make life more difficult. The "standard" list was populated from the old list used for the groovy sandbox: though a few more were needed for tests to pass (java.lang.String, java.util.Iterator, nothing scary there). Additionally, each scripting engine typically needs permissions to some runtime stuff. That is the downside of this "good old classloader" approach, but I like the transparency and simplicity, and I don't want to waste my time with any feature provided by the engine itself for this, I don't trust them. This is not perfect and the engines are not perfect but you gotta start somewhere. For expert users that need to tweak the permissions, we already support that via the standard java security configuration files, the specification is simple, supports wildcards, etc (though we do not use them ourselves). --- .../elasticsearch/script/ClassPermission.java | 171 ++++++++++++++++++ .../elasticsearch/bootstrap/untrusted.policy | 1 + .../script/ClassPermissionTests.java | 79 ++++++++ .../ExpressionScriptEngineService.java | 19 +- .../plugin-metadata/plugin-security.policy | 9 + .../expression/MoreExpressionTests.java | 10 + .../groovy/GroovyScriptEngineService.java | 36 ++-- .../plugin-metadata/plugin-security.policy | 20 ++ .../script/groovy/GroovySecurityTests.java | 68 ++++--- .../JavaScriptScriptEngineService.java | 48 +++-- .../plugin-metadata/plugin-security.policy | 11 ++ .../javascript/JavaScriptSecurityTests.java | 23 ++- .../python/PythonScriptEngineService.java | 27 ++- .../plugin-metadata/plugin-security.policy | 2 + .../script/python/PythonSecurityTests.java | 24 ++- 15 files changed, 479 insertions(+), 69 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/script/ClassPermission.java create mode 100644 core/src/test/java/org/elasticsearch/script/ClassPermissionTests.java diff --git a/core/src/main/java/org/elasticsearch/script/ClassPermission.java b/core/src/main/java/org/elasticsearch/script/ClassPermission.java new file mode 100644 index 00000000000..eb580bac3ea --- /dev/null +++ b/core/src/main/java/org/elasticsearch/script/ClassPermission.java @@ -0,0 +1,171 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.script; + +import java.security.BasicPermission; +import java.security.Permission; +import java.security.PermissionCollection; +import java.util.Arrays; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; + +/** + * Checked by scripting engines to allow loading a java class. + *

+ * Examples: + *

+ * Allow permission to {@code java.util.List} + * 

permission org.elasticsearch.script.ClassPermission "java.util.List";
+ * Allow permission to classes underneath {@code java.util} (and its subpackages such as {@code java.util.zip}) + * 
permission org.elasticsearch.script.ClassPermission "java.util.*";
+ * Allow permission to standard predefined list of basic classes (see list below) + * 
permission org.elasticsearch.script.ClassPermission "<<STANDARD>>";
+ * Allow permission to all classes + * 
permission org.elasticsearch.script.ClassPermission "*";
+ *

+ * Set of classes (allowed by special value <<STANDARD>>): + *

+ */ +public final class ClassPermission extends BasicPermission { + private static final long serialVersionUID = 3530711429252193884L; + + public static final String STANDARD = "<>"; + /** Typical set of classes for scripting: basic data types, math, dates, and simple collections */ + // this is the list from the old grovy sandbox impl (+ some things like String, Iterator, etc that were missing) + public static final Set STANDARD_CLASSES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + // jdk classes + java.lang.Boolean.class.getName(), + java.lang.Byte.class.getName(), + java.lang.Character.class.getName(), + java.lang.Double.class.getName(), + java.lang.Integer.class.getName(), + java.lang.Long.class.getName(), + java.lang.Math.class.getName(), + java.lang.Object.class.getName(), + java.lang.Short.class.getName(), + java.lang.String.class.getName(), + java.math.BigDecimal.class.getName(), + java.util.ArrayList.class.getName(), + java.util.Arrays.class.getName(), + java.util.Date.class.getName(), + java.util.HashMap.class.getName(), + java.util.HashSet.class.getName(), + java.util.Iterator.class.getName(), + java.util.List.class.getName(), + java.util.Map.class.getName(), + java.util.Set.class.getName(), + java.util.UUID.class.getName(), + // joda-time + org.joda.time.DateTime.class.getName(), + org.joda.time.DateTimeUtils.class.getName(), + org.joda.time.DateTimeZone.class.getName(), + org.joda.time.Instant.class.getName() + ))); + + /** + * Creates a new ClassPermission object. + * + * @param name class to grant permission to + */ + public ClassPermission(String name) { + super(name); + } + + /** + * Creates a new ClassPermission object. + * This constructor exists for use by the {@code Policy} object to instantiate new Permission objects. + * + * @param name class to grant permission to + * @param actions ignored + */ + public ClassPermission(String name, String actions) { + this(name); + } + + @Override + public boolean implies(Permission p) { + // check for a special value of STANDARD to imply the basic set + if (p != null && p.getClass() == getClass()) { + ClassPermission other = (ClassPermission) p; + if (STANDARD.equals(getName()) && STANDARD_CLASSES.contains(other.getName())) { + return true; + } + } + return super.implies(p); + } + + @Override + public PermissionCollection newPermissionCollection() { + // BasicPermissionCollection only handles wildcards, we expand <> here + PermissionCollection impl = super.newPermissionCollection(); + return new PermissionCollection() { + private static final long serialVersionUID = 6792220143549780002L; + + @Override + public void add(Permission permission) { + if (permission instanceof ClassPermission && STANDARD.equals(permission.getName())) { + for (String clazz : STANDARD_CLASSES) { + impl.add(new ClassPermission(clazz)); + } + } else { + impl.add(permission); + } + } + + @Override + public boolean implies(Permission permission) { + return impl.implies(permission); + } + + @Override + public Enumeration elements() { + return impl.elements(); + } + }; + } +} diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy index d32ea6a2435..8e7ca8d8b6e 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy @@ -34,5 +34,6 @@ grant { permission java.util.PropertyPermission "rhino.stack.style", "read"; // needed IndyInterface selectMethod (setCallSiteTarget) + // TODO: clean this up / only give it to engines that really must have it permission java.lang.RuntimePermission "getClassLoader"; }; diff --git a/core/src/test/java/org/elasticsearch/script/ClassPermissionTests.java b/core/src/test/java/org/elasticsearch/script/ClassPermissionTests.java new file mode 100644 index 00000000000..05a65363ff5 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/script/ClassPermissionTests.java @@ -0,0 +1,79 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.script; + +import org.elasticsearch.test.ESTestCase; + +import java.security.AllPermission; +import java.security.PermissionCollection; + +/** Very simple sanity checks for {@link ClassPermission} */ +public class ClassPermissionTests extends ESTestCase { + + public void testEquals() { + assertEquals(new ClassPermission("pkg.MyClass"), new ClassPermission("pkg.MyClass")); + assertFalse(new ClassPermission("pkg.MyClass").equals(new AllPermission())); + } + + public void testImplies() { + assertTrue(new ClassPermission("pkg.MyClass").implies(new ClassPermission("pkg.MyClass"))); + assertFalse(new ClassPermission("pkg.MyClass").implies(new ClassPermission("pkg.MyOtherClass"))); + assertFalse(new ClassPermission("pkg.MyClass").implies(null)); + assertFalse(new ClassPermission("pkg.MyClass").implies(new AllPermission())); + } + + public void testStandard() { + assertTrue(new ClassPermission("<>").implies(new ClassPermission("java.lang.Math"))); + assertFalse(new ClassPermission("<>").implies(new ClassPermission("pkg.MyClass"))); + } + + public void testPermissionCollection() { + ClassPermission math = new ClassPermission("java.lang.Math"); + PermissionCollection collection = math.newPermissionCollection(); + collection.add(math); + assertTrue(collection.implies(new ClassPermission("java.lang.Math"))); + assertFalse(collection.implies(new ClassPermission("pkg.MyClass"))); + } + + public void testPermissionCollectionStandard() { + ClassPermission standard = new ClassPermission("<>"); + PermissionCollection collection = standard.newPermissionCollection(); + collection.add(standard); + assertTrue(collection.implies(new ClassPermission("java.lang.Math"))); + assertFalse(collection.implies(new ClassPermission("pkg.MyClass"))); + } + + /** not recommended but we test anyway */ + public void testWildcards() { + assertTrue(new ClassPermission("*").implies(new ClassPermission("pkg.MyClass"))); + assertTrue(new ClassPermission("pkg.*").implies(new ClassPermission("pkg.MyClass"))); + assertTrue(new ClassPermission("pkg.*").implies(new ClassPermission("pkg.sub.MyClass"))); + assertFalse(new ClassPermission("pkg.My*").implies(new ClassPermission("pkg.MyClass"))); + assertFalse(new ClassPermission("pkg*").implies(new ClassPermission("pkg.MyClass"))); + } + + public void testPermissionCollectionWildcards() { + ClassPermission lang = new ClassPermission("java.lang.*"); + PermissionCollection collection = lang.newPermissionCollection(); + collection.add(lang); + assertTrue(collection.implies(new ClassPermission("java.lang.Math"))); + assertFalse(collection.implies(new ClassPermission("pkg.MyClass"))); + } +} diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java index 72a1dd7d5c6..d773d5a8866 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java @@ -36,6 +36,7 @@ import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.core.DateFieldMapper; import org.elasticsearch.index.mapper.core.NumberFieldMapper; +import org.elasticsearch.script.ClassPermission; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.ScriptEngineService; @@ -95,7 +96,7 @@ public class ExpressionScriptEngineService extends AbstractComponent implements @Override public Object compile(String script) { // classloader created here - SecurityManager sm = System.getSecurityManager(); + final SecurityManager sm = System.getSecurityManager(); if (sm != null) { sm.checkPermission(new SpecialPermission()); } @@ -103,8 +104,22 @@ public class ExpressionScriptEngineService extends AbstractComponent implements @Override public Expression run() { try { + ClassLoader loader = getClass().getClassLoader(); + if (sm != null) { + loader = new ClassLoader(loader) { + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + try { + sm.checkPermission(new ClassPermission(name)); + } catch (SecurityException e) { + throw new ClassNotFoundException(name, e); + } + return super.loadClass(name, resolve); + } + }; + } // NOTE: validation is delayed to allow runtime vars, and we don't have access to per index stuff here - return JavascriptCompiler.compile(script); + return JavascriptCompiler.compile(script, JavascriptCompiler.DEFAULT_FUNCTIONS, loader); } catch (ParseException e) { throw new ScriptException("Failed to parse expression: " + script, e); } diff --git a/modules/lang-expression/src/main/plugin-metadata/plugin-security.policy b/modules/lang-expression/src/main/plugin-metadata/plugin-security.policy index 5bada15755e..9f50be3dd05 100644 --- a/modules/lang-expression/src/main/plugin-metadata/plugin-security.policy +++ b/modules/lang-expression/src/main/plugin-metadata/plugin-security.policy @@ -22,4 +22,13 @@ grant { permission java.lang.RuntimePermission "createClassLoader"; // needed because of security problems in JavascriptCompiler permission java.lang.RuntimePermission "getClassLoader"; + + // expression runtime + permission org.elasticsearch.script.ClassPermission "java.lang.String"; + permission org.elasticsearch.script.ClassPermission "org.apache.lucene.expressions.Expression"; + permission org.elasticsearch.script.ClassPermission "org.apache.lucene.queries.function.FunctionValues"; + // available functions + permission org.elasticsearch.script.ClassPermission "java.lang.Math"; + permission org.elasticsearch.script.ClassPermission "org.apache.lucene.util.MathUtil"; + permission org.elasticsearch.script.ClassPermission "org.apache.lucene.util.SloppyMath"; }; diff --git a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java index 4ac4b402c17..89a5be7ff1c 100644 --- a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java +++ b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java @@ -97,6 +97,16 @@ public class MoreExpressionTests extends ESIntegTestCase { assertEquals(1, rsp.getHits().getTotalHits()); assertEquals(5.0, rsp.getHits().getAt(0).field("foo").getValue(), 0.0D); } + + public void testFunction() throws Exception { + createIndex("test"); + ensureGreen("test"); + client().prepareIndex("test", "doc", "1").setSource("foo", 4).setRefresh(true).get(); + SearchResponse rsp = buildRequest("doc['foo'] + abs(1)").get(); + assertSearchResponse(rsp); + assertEquals(1, rsp.getHits().getTotalHits()); + assertEquals(5.0, rsp.getHits().getAt(0).field("foo").getValue(), 0.0D); + } public void testBasicUsingDotValue() throws Exception { createIndex("test"); diff --git a/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java b/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java index 42ee05a7e03..85f57694ce6 100644 --- a/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java +++ b/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java @@ -51,6 +51,7 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; +import java.security.AccessControlContext; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.HashMap; @@ -65,16 +66,6 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri * The name of the scripting engine/language. */ public static final String NAME = "groovy"; - /** - * The setting to enable or disable invokedynamic instruction support in Java 7+. - *

- * Note: If this is disabled because invokedynamic is causing issues, then the Groovy - * indy jar needs to be replaced by the non-indy variant of it on the classpath (e.g., - * groovy-all-2.4.4-indy.jar should be replaced by groovy-all-2.4.4.jar). - *

- * Defaults to {@code true}. - */ - public static final String GROOVY_INDY_ENABLED = "script.groovy.indy"; /** * The name of the Groovy compiler setting to use associated with activating invokedynamic support. */ @@ -96,22 +87,33 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri // Add BigDecimal -> Double transformer config.addCompilationCustomizers(new GroovyBigDecimalTransformer(CompilePhase.CONVERSION)); - // Implicitly requires Java 7u60 or later to get valid support - if (settings.getAsBoolean(GROOVY_INDY_ENABLED, true)) { - // maintain any default optimizations - config.getOptimizationOptions().put(GROOVY_INDY_SETTING_NAME, true); - } + // always enable invokeDynamic, not the crazy softreference-based stuff + config.getOptimizationOptions().put(GROOVY_INDY_SETTING_NAME, true); // Groovy class loader to isolate Groovy-land code // classloader created here - SecurityManager sm = System.getSecurityManager(); + final SecurityManager sm = System.getSecurityManager(); if (sm != null) { sm.checkPermission(new SpecialPermission()); } this.loader = AccessController.doPrivileged(new PrivilegedAction() { @Override public GroovyClassLoader run() { - return new GroovyClassLoader(getClass().getClassLoader(), config); + // snapshot our context (which has permissions for classes), since the script has none + final AccessControlContext engineContext = AccessController.getContext(); + return new GroovyClassLoader(new ClassLoader(getClass().getClassLoader()) { + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (sm != null) { + try { + engineContext.checkPermission(new ClassPermission(name)); + } catch (SecurityException e) { + throw new ClassNotFoundException(name, e); + } + } + return super.loadClass(name, resolve); + } + }, config); } }); } diff --git a/modules/lang-groovy/src/main/plugin-metadata/plugin-security.policy b/modules/lang-groovy/src/main/plugin-metadata/plugin-security.policy index 4481203994e..7de3e1a62aa 100644 --- a/modules/lang-groovy/src/main/plugin-metadata/plugin-security.policy +++ b/modules/lang-groovy/src/main/plugin-metadata/plugin-security.policy @@ -28,4 +28,24 @@ grant { permission java.lang.RuntimePermission "closeClassLoader"; // Allow executing groovy scripts with codesource of /untrusted permission groovy.security.GroovyCodeSourcePermission "/untrusted"; + + // Standard set of classes + permission org.elasticsearch.script.ClassPermission "<>"; + // groovy runtime (TODO: clean these up if possible) + permission org.elasticsearch.script.ClassPermission "groovy.grape.GrabAnnotationTransformation"; + permission org.elasticsearch.script.ClassPermission "groovy.json.JsonOutput"; + permission org.elasticsearch.script.ClassPermission "groovy.lang.Binding"; + permission org.elasticsearch.script.ClassPermission "groovy.lang.GroovyObject"; + permission org.elasticsearch.script.ClassPermission "groovy.lang.GString"; + permission org.elasticsearch.script.ClassPermission "groovy.lang.Script"; + permission org.elasticsearch.script.ClassPermission "groovy.util.GroovyCollections"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.ast.builder.AstBuilderTransformation"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.reflection.ClassInfo"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.runtime.GStringImpl"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.runtime.powerassert.ValueRecorder"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.runtime.powerassert.AssertionRenderer"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.runtime.ScriptBytecodeAdapter"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation"; + permission org.elasticsearch.script.ClassPermission "org.codehaus.groovy.vmplugin.v7.IndyInterface"; + permission org.elasticsearch.script.ClassPermission "sun.reflect.ConstructorAccessorImpl"; }; diff --git a/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java b/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java index 258d957d77e..5f91631c021 100644 --- a/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java +++ b/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java @@ -20,16 +20,22 @@ package org.elasticsearch.script.groovy; import org.apache.lucene.util.Constants; +import org.codehaus.groovy.control.MultipleCompilationErrorsException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; +import groovy.lang.MissingPropertyException; + import java.nio.file.Path; +import java.security.PrivilegedActionException; import java.util.AbstractMap; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; /** @@ -48,7 +54,7 @@ public class GroovySecurityTests extends ESTestCase { @Override public void setUp() throws Exception { super.setUp(); - se = new GroovyScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + se = new GroovyScriptEngineService(Settings.EMPTY); // otherwise will exit your VM and other bad stuff assumeTrue("test requires security manager to be enabled", System.getSecurityManager() != null); } @@ -62,8 +68,16 @@ public class GroovySecurityTests extends ESTestCase { public void testEvilGroovyScripts() throws Exception { // Plain test assertSuccess(""); - // field access + // field access (via map) assertSuccess("def foo = doc['foo'].value; if (foo == null) { return 5; }"); + // field access (via list) + assertSuccess("def foo = mylist[0]; if (foo == null) { return 5; }"); + // field access (via array) + assertSuccess("def foo = myarray[0]; if (foo == null) { return 5; }"); + // field access (via object) + assertSuccess("def foo = myobject.primitive.toString(); if (foo == null) { return 5; }"); + assertSuccess("def foo = myobject.object.toString(); if (foo == null) { return 5; }"); + assertSuccess("def foo = myobject.list[0].primitive.toString(); if (foo == null) { return 5; }"); // List assertSuccess("def list = [doc['foo'].value, 3, 4]; def v = list.get(1); list.add(10)"); // Ranges @@ -78,35 +92,35 @@ public class GroovySecurityTests extends ESTestCase { assertSuccess("def n = [1,2,3]; GroovyCollections.max(n)"); // Fail cases: - // AccessControlException[access denied ("java.io.FilePermission" "<>" "execute")] - assertFailure("pr = Runtime.getRuntime().exec(\"touch /tmp/gotcha\"); pr.waitFor()"); + assertFailure("pr = Runtime.getRuntime().exec(\"touch /tmp/gotcha\"); pr.waitFor()", MissingPropertyException.class); - // AccessControlException[access denied ("java.lang.RuntimePermission" "accessClassInPackage.sun.reflect")] - assertFailure("d = new DateTime(); d.getClass().getDeclaredMethod(\"year\").setAccessible(true)"); + // infamous: + assertFailure("java.lang.Math.class.forName(\"java.lang.Runtime\")", PrivilegedActionException.class); + // filtered directly by our classloader + assertFailure("getClass().getClassLoader().loadClass(\"java.lang.Runtime\").availableProcessors()", PrivilegedActionException.class); + // unfortunately, we have access to other classloaders (due to indy mechanism needing getClassLoader permission) + // but we can't do much with them directly at least. + assertFailure("myobject.getClass().getClassLoader().loadClass(\"java.lang.Runtime\").availableProcessors()", SecurityException.class); + assertFailure("d = new DateTime(); d.getClass().getDeclaredMethod(\"year\").setAccessible(true)", SecurityException.class); assertFailure("d = new DateTime(); d.\"${'get' + 'Class'}\"()." + - "\"${'getDeclared' + 'Method'}\"(\"year\").\"${'set' + 'Accessible'}\"(false)"); - assertFailure("Class.forName(\"org.joda.time.DateTime\").getDeclaredMethod(\"year\").setAccessible(true)"); + "\"${'getDeclared' + 'Method'}\"(\"year\").\"${'set' + 'Accessible'}\"(false)", SecurityException.class); + assertFailure("Class.forName(\"org.joda.time.DateTime\").getDeclaredMethod(\"year\").setAccessible(true)", MissingPropertyException.class); - // AccessControlException[access denied ("groovy.security.GroovyCodeSourcePermission" "/groovy/shell")] - assertFailure("Eval.me('2 + 2')"); - assertFailure("Eval.x(5, 'x + 2')"); + assertFailure("Eval.me('2 + 2')", MissingPropertyException.class); + assertFailure("Eval.x(5, 'x + 2')", MissingPropertyException.class); - // AccessControlException[access denied ("java.lang.RuntimePermission" "accessDeclaredMembers")] assertFailure("d = new Date(); java.lang.reflect.Field f = Date.class.getDeclaredField(\"fastTime\");" + - " f.setAccessible(true); f.get(\"fastTime\")"); + " f.setAccessible(true); f.get(\"fastTime\")", MultipleCompilationErrorsException.class); - // AccessControlException[access denied ("java.io.FilePermission" "<>" "execute")] - assertFailure("def methodName = 'ex'; Runtime.\"${'get' + 'Runtime'}\"().\"${methodName}ec\"(\"touch /tmp/gotcha2\")"); + assertFailure("def methodName = 'ex'; Runtime.\"${'get' + 'Runtime'}\"().\"${methodName}ec\"(\"touch /tmp/gotcha2\")", MissingPropertyException.class); - // AccessControlException[access denied ("java.lang.RuntimePermission" "modifyThreadGroup")] - assertFailure("t = new Thread({ println 3 });"); + assertFailure("t = new Thread({ println 3 });", MultipleCompilationErrorsException.class); // test a directory we normally have access to, but the groovy script does not. Path dir = createTempDir(); // TODO: figure out the necessary escaping for windows paths here :) if (!Constants.WINDOWS) { - // access denied ("java.io.FilePermission" ".../tempDir-00N" "read") - assertFailure("new File(\"" + dir + "\").exists()"); + assertFailure("new File(\"" + dir + "\").exists()", MultipleCompilationErrorsException.class); } } @@ -115,8 +129,18 @@ public class GroovySecurityTests extends ESTestCase { Map vars = new HashMap(); // we add a "mock document" containing a single field "foo" that returns 4 (abusing a jdk class with a getValue() method) vars.put("doc", Collections.singletonMap("foo", new AbstractMap.SimpleEntry(null, 4))); + vars.put("mylist", Arrays.asList("foo")); + vars.put("myarray", Arrays.asList("foo")); + vars.put("myobject", new MyObject()); + se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "test", "js", se.compile(script)), vars).run(); } + + public static class MyObject { + public int getPrimitive() { return 0; } + public Object getObject() { return "value"; } + public List getList() { return Arrays.asList(new MyObject()); } + } /** asserts that a script runs without exception */ private void assertSuccess(String script) { @@ -124,14 +148,16 @@ public class GroovySecurityTests extends ESTestCase { } /** asserts that a script triggers securityexception */ - private void assertFailure(String script) { + private void assertFailure(String script, Class exceptionClass) { try { doTest(script); fail("did not get expected exception"); } catch (ScriptException expected) { Throwable cause = expected.getCause(); assertNotNull(cause); - assertTrue("unexpected exception: " + cause, cause instanceof SecurityException); + if (exceptionClass.isAssignableFrom(cause.getClass()) == false) { + throw new AssertionError("unexpected exception: " + cause, expected); + } } } } 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 621d338128f..33a4e55801b 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 @@ -39,7 +39,10 @@ import org.mozilla.javascript.Script; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; +import java.security.AccessControlContext; +import java.security.AccessController; import java.security.CodeSource; +import java.security.PrivilegedAction; import java.security.cert.Certificate; import java.util.List; import java.util.Map; @@ -54,18 +57,47 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements private static WrapFactory wrapFactory = new CustomWrapFactory(); - private final int optimizationLevel; - private Scriptable globalScope; // one time initialization of rhino security manager integration private static final CodeSource DOMAIN; + private static final int OPTIMIZATION_LEVEL = 1; + static { try { DOMAIN = new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[]) null); } catch (MalformedURLException e) { throw new RuntimeException(e); } + ContextFactory factory = new ContextFactory() { + @Override + protected void onContextCreated(Context cx) { + cx.setWrapFactory(wrapFactory); + cx.setOptimizationLevel(OPTIMIZATION_LEVEL); + } + }; + if (System.getSecurityManager() != null) { + factory.initApplicationClassLoader(AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ClassLoader run() { + // snapshot our context (which has permissions for classes), since the script has none + final AccessControlContext engineContext = AccessController.getContext(); + return new ClassLoader(JavaScriptScriptEngineService.class.getClassLoader()) { + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + try { + engineContext.checkPermission(new ClassPermission(name)); + } catch (SecurityException e) { + throw new ClassNotFoundException(name, e); + } + return super.loadClass(name, resolve); + } + }; + } + })); + } + factory.seal(); + ContextFactory.initGlobal(factory); SecurityController.initGlobal(new PolicySecurityController() { @Override public GeneratedClassLoader createClassLoader(ClassLoader parent, Object securityDomain) { @@ -78,6 +110,7 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements if (securityDomain != DOMAIN) { throw new SecurityException("illegal securityDomain: " + securityDomain); } + return super.createClassLoader(parent, securityDomain); } }); @@ -90,11 +123,8 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements public JavaScriptScriptEngineService(Settings settings) { super(settings); - this.optimizationLevel = settings.getAsInt("script.javascript.optimization_level", 1); - Context ctx = Context.enter(); try { - ctx.setWrapFactory(wrapFactory); globalScope = ctx.initStandardObjects(null, true); } finally { Context.exit(); @@ -130,8 +160,6 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements public Object compile(String script) { Context ctx = Context.enter(); try { - ctx.setWrapFactory(wrapFactory); - ctx.setOptimizationLevel(optimizationLevel); return ctx.compileString(script, generateScriptName(), 1, DOMAIN); } finally { Context.exit(); @@ -142,8 +170,6 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements public ExecutableScript executable(CompiledScript compiledScript, Map vars) { Context ctx = Context.enter(); try { - ctx.setWrapFactory(wrapFactory); - Scriptable scope = ctx.newObject(globalScope); scope.setPrototype(globalScope); scope.setParentScope(null); @@ -161,8 +187,6 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements public SearchScript search(final CompiledScript compiledScript, final SearchLookup lookup, @Nullable final Map vars) { Context ctx = Context.enter(); try { - ctx.setWrapFactory(wrapFactory); - final Scriptable scope = ctx.newObject(globalScope); scope.setPrototype(globalScope); scope.setParentScope(null); @@ -215,7 +239,6 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements public Object run() { Context ctx = Context.enter(); try { - ctx.setWrapFactory(wrapFactory); return ScriptValueConverter.unwrapValue(script.exec(ctx, scope)); } finally { Context.exit(); @@ -276,7 +299,6 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements public Object run() { Context ctx = Context.enter(); try { - ctx.setWrapFactory(wrapFactory); return ScriptValueConverter.unwrapValue(script.exec(ctx, scope)); } finally { Context.exit(); diff --git a/plugins/lang-javascript/src/main/plugin-metadata/plugin-security.policy b/plugins/lang-javascript/src/main/plugin-metadata/plugin-security.policy index e45c1b86ceb..739a2531d2f 100644 --- a/plugins/lang-javascript/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/lang-javascript/src/main/plugin-metadata/plugin-security.policy @@ -20,4 +20,15 @@ grant { // needed to generate runtime classes permission java.lang.RuntimePermission "createClassLoader"; + + // Standard set of classes + permission org.elasticsearch.script.ClassPermission "<>"; + // rhino runtime (TODO: clean these up if possible) + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.ContextFactory"; + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.Callable"; + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.NativeFunction"; + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.Script"; + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.ScriptRuntime"; + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.Undefined"; + permission org.elasticsearch.script.ClassPermission "org.mozilla.javascript.optimizer.OptRuntime"; }; 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 410099de0a7..c6f9805f818 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,6 +23,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; +import org.mozilla.javascript.EcmaError; import org.mozilla.javascript.WrappedException; import java.util.HashMap; @@ -61,14 +62,20 @@ public class JavaScriptSecurityTests extends ESTestCase { } /** assert that a security exception is hit */ - private void assertFailure(String script) { + private void assertFailure(String script, Class exceptionClass) { try { doTest(script); fail("did not get expected exception"); } catch (WrappedException expected) { Throwable cause = expected.getCause(); assertNotNull(cause); - assertTrue("unexpected exception: " + cause, cause instanceof SecurityException); + if (exceptionClass.isAssignableFrom(cause.getClass()) == false) { + throw new AssertionError("unexpected exception: " + expected, expected); + } + } catch (EcmaError expected) { + if (exceptionClass.isAssignableFrom(expected.getClass()) == false) { + throw new AssertionError("unexpected exception: " + expected, expected); + } } } @@ -79,22 +86,22 @@ public class JavaScriptSecurityTests extends ESTestCase { } /** Test some javascripts that should hit security exception */ - public void testNotOK() { + public void testNotOK() throws Exception { // sanity check :) - assertFailure("java.lang.Runtime.getRuntime().halt(0)"); + assertFailure("java.lang.Runtime.getRuntime().halt(0)", EcmaError.class); // check a few things more restrictive than the ordinary policy // no network - assertFailure("new java.net.Socket(\"localhost\", 1024)"); + assertFailure("new java.net.Socket(\"localhost\", 1024)", EcmaError.class); // no files - assertFailure("java.io.File.createTempFile(\"test\", \"tmp\")"); + assertFailure("java.io.File.createTempFile(\"test\", \"tmp\")", EcmaError.class); } public void testDefinitelyNotOK() { // no mucking with security controller assertFailure("var ctx = org.mozilla.javascript.Context.getCurrentContext(); " + - "ctx.setSecurityController(new org.mozilla.javascript.PolicySecurityController());"); + "ctx.setSecurityController(new org.mozilla.javascript.PolicySecurityController());", EcmaError.class); // no compiling scripts from scripts assertFailure("var ctx = org.mozilla.javascript.Context.getCurrentContext(); " + - "ctx.compileString(\"1 + 1\", \"foobar\", 1, null); "); + "ctx.compileString(\"1 + 1\", \"foobar\", 1, null); ", EcmaError.class); } } diff --git a/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java b/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java index 3dfa4bcd0f9..1930f530671 100644 --- a/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java +++ b/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java @@ -25,7 +25,11 @@ import java.security.AccessController; import java.security.Permissions; import java.security.PrivilegedAction; import java.security.ProtectionDomain; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.Scorer; @@ -34,6 +38,7 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.ClassPermission; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.LeafSearchScript; @@ -55,20 +60,36 @@ import org.python.util.PythonInterpreter; public class PythonScriptEngineService extends AbstractComponent implements ScriptEngineService { private final PythonInterpreter interp; - + @Inject public PythonScriptEngineService(Settings settings) { super(settings); // classloader created here - SecurityManager sm = System.getSecurityManager(); + final SecurityManager sm = System.getSecurityManager(); if (sm != null) { sm.checkPermission(new SpecialPermission()); } this.interp = AccessController.doPrivileged(new PrivilegedAction () { @Override public PythonInterpreter run() { - return PythonInterpreter.threadLocalStateInterpreter(null); + // snapshot our context here for checks, as the script has no permissions + final AccessControlContext engineContext = AccessController.getContext(); + PythonInterpreter interp = PythonInterpreter.threadLocalStateInterpreter(null); + if (sm != null) { + interp.getSystemState().setClassLoader(new ClassLoader(getClass().getClassLoader()) { + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + try { + engineContext.checkPermission(new ClassPermission(name)); + } catch (SecurityException e) { + throw new ClassNotFoundException(name, e); + } + return super.loadClass(name, resolve); + } + }); + } + return interp; } }); } diff --git a/plugins/lang-python/src/main/plugin-metadata/plugin-security.policy b/plugins/lang-python/src/main/plugin-metadata/plugin-security.policy index 9ecbfdc7586..86f4df64db4 100644 --- a/plugins/lang-python/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/lang-python/src/main/plugin-metadata/plugin-security.policy @@ -22,4 +22,6 @@ grant { permission java.lang.RuntimePermission "createClassLoader"; // needed by PySystemState init (TODO: see if we can avoid this) permission java.lang.RuntimePermission "getClassLoader"; + // Standard set of classes + permission org.elasticsearch.script.ClassPermission "<>"; }; 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 dd25db815ac..e90ac503f13 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 @@ -25,7 +25,9 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; import org.python.core.PyException; +import java.text.DecimalFormatSymbols; import java.util.HashMap; +import java.util.Locale; import java.util.Map; /** @@ -66,12 +68,12 @@ public class PythonSecurityTests extends ESTestCase { doTest(script); fail("did not get expected exception"); } catch (PyException expected) { - Throwable cause = expected.getCause(); // TODO: fix jython localization bugs: https://github.com/elastic/elasticsearch/issues/13967 - // this is the correct assert: - // assertNotNull("null cause for exception: " + expected, cause); - assertNotNull("null cause for exception", cause); - assertTrue("unexpected exception: " + cause, cause instanceof SecurityException); + // we do a gross hack for now + DecimalFormatSymbols symbols = DecimalFormatSymbols.getInstance(Locale.getDefault()); + if (symbols.getZeroDigit() == '0') { + assertTrue(expected.toString().contains("cannot import")); + } } } @@ -91,4 +93,16 @@ public class PythonSecurityTests extends ESTestCase { // no files assertFailure("from java.io import File\nFile.createTempFile(\"test\", \"tmp\")"); } + + /** Test again from a new thread, python has complex threadlocal configuration */ + public void testNotOKFromSeparateThread() throws Exception { + Thread t = new Thread() { + @Override + public void run() { + assertFailure("from java.lang import Runtime\nRuntime.availableProcessors()"); + } + }; + t.start(); + t.join(); + } } From 3c419c2186fc690b4e95e87db8ade2787f971cf3 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Sat, 5 Dec 2015 22:08:40 -0500 Subject: [PATCH 2/2] do expressions consistently with other engines --- .../script/expression/ExpressionScriptEngineService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java index d773d5a8866..a7f93925119 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java @@ -45,6 +45,7 @@ import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.search.lookup.SearchLookup; +import java.security.AccessControlContext; import java.security.AccessController; import java.security.PrivilegedAction; import java.text.ParseException; @@ -104,13 +105,15 @@ public class ExpressionScriptEngineService extends AbstractComponent implements @Override public Expression run() { try { + // snapshot our context here, we check on behalf of the expression + AccessControlContext engineContext = AccessController.getContext(); ClassLoader loader = getClass().getClassLoader(); if (sm != null) { loader = new ClassLoader(loader) { @Override protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { try { - sm.checkPermission(new ClassPermission(name)); + engineContext.checkPermission(new ClassPermission(name)); } catch (SecurityException e) { throw new ClassNotFoundException(name, e); }