Merge pull request #15262 from rmuir/filter_classes_in_scripts

Filter classes loaded by scripts

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.
+ * <p>
+ * Examples:
+ * <p>
+ * Allow permission to {@code java.util.List}
+ * <pre>permission org.elasticsearch.script.ClassPermission "java.util.List";</pre>
+ * Allow permission to classes underneath {@code java.util} (and its subpackages such as {@code java.util.zip})
+ * <pre>permission org.elasticsearch.script.ClassPermission "java.util.*";</pre>
+ * Allow permission to standard predefined list of basic classes (see list below)
+ * <pre>permission org.elasticsearch.script.ClassPermission "&lt;&lt;STANDARD&gt;&gt;";</pre>
+ * Allow permission to all classes
+ * <pre>permission org.elasticsearch.script.ClassPermission "*";</pre>
+ * <p>
+ * Set of classes (allowed by special value <code>&lt;&lt;STANDARD&gt;&gt;</code>):
+ * <ul>
+ *   <li>{@link java.lang.Boolean}</li>
+ *   <li>{@link java.lang.Byte}</li>
+ *   <li>{@link java.lang.Character}</li>
+ *   <li>{@link java.lang.Double}</li>
+ *   <li>{@link java.lang.Integer}</li>
+ *   <li>{@link java.lang.Long}</li>
+ *   <li>{@link java.lang.Math}</li>
+ *   <li>{@link java.lang.Object}</li>
+ *   <li>{@link java.lang.Short}</li>
+ *   <li>{@link java.lang.String}</li>
+ *   <li>{@link java.math.BigDecimal}</li>
+ *   <li>{@link java.util.ArrayList}</li>
+ *   <li>{@link java.util.Arrays}</li>
+ *   <li>{@link java.util.Date}</li>
+ *   <li>{@link java.util.HashMap}</li>
+ *   <li>{@link java.util.HashSet}</li>
+ *   <li>{@link java.util.Iterator}</li>
+ *   <li>{@link java.util.List}</li>
+ *   <li>{@link java.util.Map}</li>
+ *   <li>{@link java.util.Set}</li>
+ *   <li>{@link java.util.UUID}</li>
+ *   <li>{@link org.joda.time.DateTime}</li>
+ *   <li>{@link org.joda.time.DateTimeUtils}</li>
+ *   <li>{@link org.joda.time.DateTimeZone}</li>
+ *   <li>{@link org.joda.time.Instant}</li>
+ * </ul>
+ */
+public final class ClassPermission extends BasicPermission {
+    private static final long serialVersionUID = 3530711429252193884L;
+
+    public static final String STANDARD = "<<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<String> 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 <<STANDARD>> 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<Permission> elements() {
+                return impl.elements();
+            }
+        };
+    }
+}

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";
 };

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("<<STANDARD>>").implies(new ClassPermission("java.lang.Math")));
+        assertFalse(new ClassPermission("<<STANDARD>>").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("<<STANDARD>>");
+        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")));
+    }
+}

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;
@@ -44,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;
@@ -95,7 +97,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 +105,24 @@ 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 {
+                                    engineContext.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);
                 }

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";
 };

modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java

@@ -98,6 +98,16 @@ public class MoreExpressionTests extends ESIntegTestCase {
         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");
         ensureGreen("test");

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 <code>invokedynamic</code> instruction support in Java 7+.
-     * <p>
-     * Note: If this is disabled because <code>invokedynamic</code> is causing issues, then the Groovy
-     * <code>indy</code> jar needs to be replaced by the non-<code>indy</code> variant of it on the classpath (e.g.,
-     * <code>groovy-all-2.4.4-indy.jar</code> should be replaced by <code>groovy-all-2.4.4.jar</code>).
-     * <p>
-     * 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 <code>invokedynamic</code> 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
+        // always enable invokeDynamic, not the crazy softreference-based stuff
-        if (settings.getAsBoolean(GROOVY_INDY_ENABLED, true)) {
-            // maintain any default optimizations
         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<GroovyClassLoader>() {
             @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);
             }
         });
     }

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 "<<STANDARD>>";
+  // 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";
 };

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" "<<ALL FILES>>" "execute")]
+        assertFailure("pr = Runtime.getRuntime().exec(\"touch /tmp/gotcha\"); pr.waitFor()", MissingPropertyException.class);
-        assertFailure("pr = Runtime.getRuntime().exec(\"touch /tmp/gotcha\"); pr.waitFor()");
 
-        // AccessControlException[access denied ("java.lang.RuntimePermission" "accessClassInPackage.sun.reflect")]
+        // infamous:
-        assertFailure("d = new DateTime(); d.getClass().getDeclaredMethod(\"year\").setAccessible(true)");
+        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)");
+                        "\"${'getDeclared' + 'Method'}\"(\"year\").\"${'set' + 'Accessible'}\"(false)", SecurityException.class);
-        assertFailure("Class.forName(\"org.joda.time.DateTime\").getDeclaredMethod(\"year\").setAccessible(true)");
+        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')", MissingPropertyException.class);
-        assertFailure("Eval.me('2 + 2')");
+        assertFailure("Eval.x(5, 'x + 2')", MissingPropertyException.class);
-        assertFailure("Eval.x(5, 'x + 2')");
 
-        // 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" "<<ALL FILES>>" "execute")]
+        assertFailure("def methodName = 'ex'; Runtime.\"${'get' + 'Runtime'}\"().\"${methodName}ec\"(\"touch /tmp/gotcha2\")", MissingPropertyException.class);
-        assertFailure("def methodName = 'ex'; Runtime.\"${'get' + 'Runtime'}\"().\"${methodName}ec\"(\"touch /tmp/gotcha2\")");
 
-        // AccessControlException[access denied ("java.lang.RuntimePermission" "modifyThreadGroup")]
+        assertFailure("t = new Thread({ println 3 });", MultipleCompilationErrorsException.class);
-        assertFailure("t = new Thread({ println 3 });");
 
         // 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()", MultipleCompilationErrorsException.class);
-            assertFailure("new File(\"" + dir + "\").exists()");
         }
     }
 
@@ -115,23 +129,35 @@ public class GroovySecurityTests extends ESTestCase {
         Map<String, Object> vars = new HashMap<String, Object>();
         // 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<Object,Integer>(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<Object> getList() { return Arrays.asList(new MyObject()); }
+    }
+
     /** asserts that a script runs without exception */
     private void assertSuccess(String script) {
         doTest(script);
     }
 
     /** asserts that a script triggers securityexception */
-    private void assertFailure(String script) {
+    private void assertFailure(String script, Class<? extends Throwable> 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);
+            }
         }
     }
 }

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<ClassLoader>() {
+                @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<String, Object> 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<String, Object> 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();

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 "<<STANDARD>>";
+  // 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";
 };

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<? extends Throwable> 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);
     }
 }

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;
@@ -61,14 +66,30 @@ public class PythonScriptEngineService extends AbstractComponent implements Scri
         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<PythonInterpreter> () {
             @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;
             }
         });
     }

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 "<<STANDARD>>";
 };

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:
+            // we do a gross hack for now
-            // assertNotNull("null cause for exception: " + expected, cause);
+            DecimalFormatSymbols symbols = DecimalFormatSymbols.getInstance(Locale.getDefault());
-            assertNotNull("null cause for exception", cause);
+            if (symbols.getZeroDigit() == '0') {
-            assertTrue("unexpected exception: " + cause, cause instanceof SecurityException);
+                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();
+    }
 }