Clean up scripting permissions.

Now that groovy is factored out, we contain this dangerous stuff there.

TODO: look into those test hacks inspecting class protection domains, maybe we can
clean that one up too.

TODO: generalize the GroovyCodeSourcePermission to something all script engines check,
before entering accesscontrollerblocks. this way e.g. groovy script cannot coerce
python engine into creating something with more privs if it gets ahold of it... we
should probably protect the aws/gce hacks in the same way.

core/src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -166,6 +166,8 @@ final class Security {
         m.put("repository-s3",       "org.elasticsearch.plugin.repository.s3.S3RepositoryPlugin");
         m.put("discovery-ec2",       "org.elasticsearch.plugin.discovery.ec2.Ec2DiscoveryPlugin");
         m.put("cloud-gce",           "org.elasticsearch.plugin.cloud.gce.CloudGcePlugin");
+        m.put("lang-expression",     "org.elasticsearch.script.expression.ExpressionPlugin");
+        m.put("lang-groovy",         "org.elasticsearch.script.groovy.GroovyPlugin");
         m.put("lang-javascript",     "org.elasticsearch.plugin.javascript.JavaScriptPlugin");
         m.put("lang-python",         "org.elasticsearch.plugin.python.PythonPlugin");
         SPECIAL_PLUGINS = Collections.unmodifiableMap(m);

core/src/main/resources/org/elasticsearch/bootstrap/security.policy

@@ -57,6 +57,20 @@ grant codeBase "${es.security.plugin.cloud-gce}" {
   permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
 };
 
+grant codeBase "${es.security.plugin.lang-expression}" {
+  // needed to generate runtime classes
+  permission java.lang.RuntimePermission "createClassLoader";
+};
+
+grant codeBase "${es.security.plugin.lang-groovy}" {
+  // needed to generate runtime classes
+  permission java.lang.RuntimePermission "createClassLoader";
+  // needed by groovy engine
+  permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect";
+  // needed by GroovyScriptEngineService to close its classloader (why?)
+  permission java.lang.RuntimePermission "closeClassLoader";
+};
+
 grant codeBase "${es.security.plugin.lang-javascript}" {
   // needed to generate runtime classes
   permission java.lang.RuntimePermission "createClassLoader";
@@ -106,6 +120,8 @@ grant codeBase "${es.security.jar.randomizedtesting.junit4}" {
 grant {
 
   // Allow executing groovy scripts with codesource of /groovy/script
+  // TODO: make our own general ScriptServicePermission we check instead and 
+  // check-before-createClassLoader for all scripting engines.
   permission groovy.security.GroovyCodeSourcePermission "/groovy/script";
 
   // Allow connecting to the internet anywhere
@@ -114,15 +130,9 @@ grant {
   // Allow read/write to all system properties
   permission java.util.PropertyPermission "*", "read,write";
 
-  // needed by scripting engines, etc
-  permission java.lang.RuntimePermission "createClassLoader";
-
   // needed by lucene SPI currently 
   permission java.lang.RuntimePermission "getClassLoader";
 
-  // needed by GroovyScriptEngineService
-  permission java.lang.RuntimePermission "closeClassLoader";
-
   // needed by Settings
   permission java.lang.RuntimePermission "getenv.*";
 
@@ -130,12 +140,10 @@ grant {
   // otherwise can be provided only to test libraries
   permission java.lang.RuntimePermission "modifyThread";
 
-  // needed by groovy scripting
+  // needed by ExceptionSerializationTests and RestTestCase for
+  // some hackish things they do. otherwise only needed by groovy 
+  // (TODO: clean this up?)
   permission java.lang.RuntimePermission "getProtectionDomain";
-
-  // reflection hacks:
-  // needed by groovy engine
-  permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect";
   
   // likely not low hanging fruit...
   permission java.lang.RuntimePermission "accessDeclaredMembers";

plugins/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java

@@ -43,6 +43,8 @@ import org.elasticsearch.script.SearchScript;
 import org.elasticsearch.search.MultiValueMode;
 import org.elasticsearch.search.lookup.SearchLookup;
 
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.text.ParseException;
 import java.util.Calendar;
 import java.util.Map;
@@ -91,12 +93,18 @@ public class ExpressionScriptEngineService extends AbstractComponent implements
 
     @Override
     public Object compile(String script) {
-        try {
-            // NOTE: validation is delayed to allow runtime vars, and we don't have access to per index stuff here
-            return JavascriptCompiler.compile(script);
-        } catch (ParseException e) {
-            throw new ScriptException("Failed to parse expression: " + script, e);
-        }
+        // classloader created here
+        return AccessController.doPrivileged(new PrivilegedAction<Expression>() {
+            @Override
+            public Expression run() {
+                try {
+                    // NOTE: validation is delayed to allow runtime vars, and we don't have access to per index stuff here
+                    return JavascriptCompiler.compile(script);
+                } catch (ParseException e) {
+                    throw new ScriptException("Failed to parse expression: " + script, e);
+                }
+            }
+        });
     }
 
     @Override

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

@@ -423,6 +423,7 @@ public class MoreExpressionTests extends ESIntegTestCase {
 
     // series of unit test for using expressions as executable scripts
     public void testExecutableScripts() throws Exception {
+        assumeTrue("test creates classes directly, cannot run with security manager", System.getSecurityManager() == null);
         Map<String, Object> vars = new HashMap<>();
         vars.put("a", 2.5);
         vars.put("b", 3);

plugins/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java

@@ -20,10 +20,13 @@
 package org.elasticsearch.script.groovy;
 
 import java.nio.charset.StandardCharsets;
+
 import com.google.common.hash.Hashing;
+
 import groovy.lang.Binding;
 import groovy.lang.GroovyClassLoader;
 import groovy.lang.Script;
+
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Scorer;
 import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
@@ -49,6 +52,8 @@ import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
 import java.math.BigDecimal;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -99,17 +104,30 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri
         }
 
         // Groovy class loader to isolate Groovy-land code
-        this.loader = new GroovyClassLoader(getClass().getClassLoader(), config);
+        // classloader created here
+        this.loader = AccessController.doPrivileged(new PrivilegedAction<GroovyClassLoader>() {
+            @Override
+            public GroovyClassLoader run() {
+                return new GroovyClassLoader(getClass().getClassLoader(), config);
+            }
+        });
     }
 
     @Override
     public void close() {
         loader.clearCache();
-        try {
-            loader.close();
-        } catch (IOException e) {
-            logger.warn("Unable to close Groovy loader", e);
-        }
+        // close classloader here (why do we do this?)
+        AccessController.doPrivileged(new PrivilegedAction<Void>() {
+            @Override
+            public Void run() {
+                try {
+                    loader.close();
+                } catch (IOException e) {
+                    logger.warn("Unable to close Groovy loader", e);
+                }
+                return null;
+            }
+        });
     }
 
     @Override