From 8ff42834e9755ca64dacb71b13de75242128262a Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Sun, 4 Oct 2015 17:13:47 -0400 Subject: [PATCH] lock down javascript and python permissions --- .../bootstrap/BootstrapInfo.java | 9 +- .../org/elasticsearch/bootstrap/ESPolicy.java | 32 +++---- .../elasticsearch/bootstrap/security.policy | 4 +- .../{groovy.policy => untrusted.policy} | 4 +- .../bootstrap/ESPolicyTests.java | 9 +- .../bootstrap/MockPluginPolicy.java | 15 ++- .../groovy/GroovyScriptEngineService.java | 13 ++- .../JavaScriptScriptEngineService.java | 11 ++- .../javascript/JavaScriptSecurityTests.java | 89 ++++++++++++++++++ .../python/PythonScriptEngineService.java | 33 ++++++- .../script/python/PythonSecurityTests.java | 92 +++++++++++++++++++ 11 files changed, 279 insertions(+), 32 deletions(-) rename core/src/main/resources/org/elasticsearch/bootstrap/{groovy.policy => untrusted.policy} (90%) create mode 100644 plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java create mode 100644 plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java index 76485bb86e5..f1278af96f4 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java @@ -45,9 +45,16 @@ public final class BootstrapInfo { } /** - * Returns true if secure computing mode is enabled (linux/amd64 only) + * Returns true if secure computing mode is enabled (linux/amd64, OS X only) */ public static boolean isSeccompInstalled() { return Natives.isSeccompInstalled(); } + + /** + * codebase location for untrusted scripts (provide some additional safety) + *

+ * This is not a full URL, just a path. + */ + public static final String UNTRUSTED_CODEBASE = "/untrusted"; } diff --git a/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java b/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java index 9db66ca9c14..ae993f25814 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java @@ -26,29 +26,27 @@ import java.net.URL; import java.security.CodeSource; import java.security.Permission; import java.security.PermissionCollection; -import java.security.Permissions; import java.security.Policy; import java.security.ProtectionDomain; import java.security.URIParameter; -import java.util.PropertyPermission; /** custom policy for union of static and dynamic permissions */ final class ESPolicy extends Policy { /** template policy file, the one used in tests */ static final String POLICY_RESOURCE = "security.policy"; - /** limited policy for groovy scripts */ - static final String GROOVY_RESOURCE = "groovy.policy"; + /** limited policy for scripts */ + static final String UNTRUSTED_RESOURCE = "untrusted.policy"; final Policy template; - final Policy groovy; + final Policy untrusted; final PermissionCollection dynamic; public ESPolicy(PermissionCollection dynamic) throws Exception { URI policyUri = getClass().getResource(POLICY_RESOURCE).toURI(); - URI groovyUri = getClass().getResource(GROOVY_RESOURCE).toURI(); + URI untrustedUri = getClass().getResource(UNTRUSTED_RESOURCE).toURI(); this.template = Policy.getInstance("JavaPolicy", new URIParameter(policyUri)); - this.groovy = Policy.getInstance("JavaPolicy", new URIParameter(groovyUri)); + this.untrusted = Policy.getInstance("JavaPolicy", new URIParameter(untrustedUri)); this.dynamic = dynamic; } @@ -56,15 +54,17 @@ final class ESPolicy extends Policy { public boolean implies(ProtectionDomain domain, Permission permission) { CodeSource codeSource = domain.getCodeSource(); // codesource can be null when reducing privileges via doPrivileged() - if (codeSource != null) { - URL location = codeSource.getLocation(); - // location can be null... ??? nobody knows - // https://bugs.openjdk.java.net/browse/JDK-8129972 - if (location != null) { - // run groovy scripts with no permissions (except logging property) - if ("/groovy/script".equals(location.getFile())) { - return groovy.implies(domain, permission); - } + if (codeSource == null) { + return false; + } + + URL location = codeSource.getLocation(); + // location can be null... ??? nobody knows + // https://bugs.openjdk.java.net/browse/JDK-8129972 + if (location != null) { + // run scripts with limited permissions + if (BootstrapInfo.UNTRUSTED_CODEBASE.equals(location.getFile())) { + return untrusted.implies(domain, permission); } } diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy index ae2320feaf8..11268245670 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -69,8 +69,8 @@ grant codeBase "${es.security.plugin.lang-groovy}" { permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect"; // needed by GroovyScriptEngineService to close its classloader (why?) permission java.lang.RuntimePermission "closeClassLoader"; - // Allow executing groovy scripts with codesource of /groovy/script - permission groovy.security.GroovyCodeSourcePermission "/groovy/script"; + // Allow executing groovy scripts with codesource of /untrusted + permission groovy.security.GroovyCodeSourcePermission "/untrusted"; }; grant codeBase "${es.security.plugin.lang-javascript}" { diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/groovy.policy b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy similarity index 90% rename from core/src/main/resources/org/elasticsearch/bootstrap/groovy.policy rename to core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy index 4e1275827d9..2475c56e814 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/groovy.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy @@ -18,8 +18,8 @@ */ /* - * Limited security policy for groovy scripts. - * This is what is needed for its invokeDynamic functionality to work. + * Limited security policy for scripts. + * This is what is needed for invokeDynamic functionality to work. */ grant { diff --git a/core/src/test/java/org/elasticsearch/bootstrap/ESPolicyTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ESPolicyTests.java index 5423e68b555..b7ed195e0f9 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/ESPolicyTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/ESPolicyTests.java @@ -24,7 +24,9 @@ import org.elasticsearch.test.ESTestCase; import java.io.FilePermission; import java.security.AccessControlContext; import java.security.AccessController; +import java.security.AllPermission; import java.security.CodeSource; +import java.security.Permission; import java.security.PermissionCollection; import java.security.Permissions; import java.security.PrivilegedAction; @@ -48,8 +50,13 @@ public class ESPolicyTests extends ESTestCase { */ public void testNullCodeSource() throws Exception { assumeTrue("test cannot run with security manager", System.getSecurityManager() == null); + // create a policy with AllPermission + Permission all = new AllPermission(); + PermissionCollection allCollection = all.newPermissionCollection(); + allCollection.add(all); + ESPolicy policy = new ESPolicy(allCollection); + // restrict ourselves to NoPermission PermissionCollection noPermissions = new Permissions(); - ESPolicy policy = new ESPolicy(noPermissions); assertFalse(policy.implies(new ProtectionDomain(null, noPermissions), new FilePermission("foo", "read"))); } diff --git a/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java b/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java index bd366a28c13..c301ec78b33 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/MockPluginPolicy.java @@ -35,7 +35,6 @@ import java.security.ProtectionDomain; import java.security.cert.Certificate; import java.util.Collections; import java.util.HashSet; -import java.util.Objects; import java.util.Set; /** @@ -99,18 +98,24 @@ final class MockPluginPolicy extends Policy { excludedSources.add(RandomizedRunner.class.getProtectionDomain().getCodeSource()); // junit library excludedSources.add(Assert.class.getProtectionDomain().getCodeSource()); - // groovy scripts - excludedSources.add(new CodeSource(new URL("file:/groovy/script"), (Certificate[])null)); + // scripts + excludedSources.add(new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[])null)); Loggers.getLogger(getClass()).debug("Apply permissions [{}] excluding codebases [{}]", extraPermissions, excludedSources); } @Override public boolean implies(ProtectionDomain domain, Permission permission) { + CodeSource codeSource = domain.getCodeSource(); + // codesource can be null when reducing privileges via doPrivileged() + if (codeSource == null) { + return false; + } + if (standardPolicy.implies(domain, permission)) { return true; - } else if (excludedSources.contains(domain.getCodeSource()) == false && - Objects.toString(domain.getCodeSource()).contains("test-classes") == false) { + } else if (excludedSources.contains(codeSource) == false && + codeSource.toString().contains("test-classes") == false) { return extraPermissions.implies(permission); } else { return false; diff --git a/plugins/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java b/plugins/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java index cd478bef604..1644effb16d 100644 --- a/plugins/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java +++ b/plugins/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java @@ -21,6 +21,7 @@ package org.elasticsearch.script.groovy; import groovy.lang.Binding; import groovy.lang.GroovyClassLoader; +import groovy.lang.GroovyCodeSource; import groovy.lang.Script; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.Scorer; @@ -36,6 +37,8 @@ import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.CompilationCustomizer; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.elasticsearch.SpecialPermission; +import org.elasticsearch.bootstrap.BootstrapInfo; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.hash.MessageDigests; @@ -168,7 +171,15 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri if (sm != null) { sm.checkPermission(new SpecialPermission()); } - return loader.parseClass(script, MessageDigests.toHexString(MessageDigests.sha1().digest(script.getBytes(StandardCharsets.UTF_8)))); + String fake = MessageDigests.toHexString(MessageDigests.sha1().digest(script.getBytes(StandardCharsets.UTF_8))); + // same logic as GroovyClassLoader.parseClass() but with a different codesource string: + GroovyCodeSource gcs = AccessController.doPrivileged(new PrivilegedAction() { + public GroovyCodeSource run() { + return new GroovyCodeSource(script, fake, BootstrapInfo.UNTRUSTED_CODEBASE); + } + }); + gcs.setCachable(false); + return loader.parseClass(gcs); } catch (Throwable e) { if (logger.isTraceEnabled()) { logger.trace("exception compiling Groovy script:", e); 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 f2078971ab6..0adf9ca5f60 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 @@ -22,6 +22,7 @@ package org.elasticsearch.script.javascript; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.Scorer; import org.elasticsearch.SpecialPermission; +import org.elasticsearch.bootstrap.BootstrapInfo; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; @@ -36,6 +37,10 @@ import org.mozilla.javascript.*; import org.mozilla.javascript.Script; import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.security.CodeSource; +import java.security.cert.Certificate; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicLong; @@ -105,7 +110,11 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements try { ctx.setWrapFactory(wrapFactory); ctx.setOptimizationLevel(optimizationLevel); - return ctx.compileString(script, generateScriptName(), 1, null); + ctx.setSecurityController(new PolicySecurityController()); + return ctx.compileString(script, generateScriptName(), 1, + new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[]) null)); + } catch (MalformedURLException e) { + throw new RuntimeException(e); } finally { Context.exit(); } diff --git a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java new file mode 100644 index 00000000000..36636eb0cc4 --- /dev/null +++ b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java @@ -0,0 +1,89 @@ +/* + * 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.javascript; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.test.ESTestCase; +import org.junit.After; +import org.junit.Before; +import org.mozilla.javascript.WrappedException; + +import java.util.HashMap; +import java.util.Map; + +/** + * Tests for the Javascript security permissions + */ +public class JavaScriptSecurityTests extends ESTestCase { + + private JavaScriptScriptEngineService se; + + @Before + public void setup() { + se = new JavaScriptScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + } + + @After + public void close() { + se.close(); + } + + /** runs a script */ + private void doTest(String script) { + Map vars = new HashMap(); + se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "test", "js", se.compile(script)), vars); + } + + /** asserts that a script runs without exception */ + private void assertSuccess(String script) { + doTest(script); + } + + /** assert that a security exception is hit */ + private void assertFailure(String script) { + 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); + } + } + + /** Test some javascripts that are ok */ + public void testOK() { + assertSuccess("1 + 2"); + assertSuccess("Math.cos(Math.PI)"); + } + + /** Test some javascripts that should hit security exception */ + public void testNotOK() { + // sanity check :) + assertFailure("java.lang.Runtime.getRuntime().halt(0)"); + // check a few things more restrictive than the ordinary policy + // no network + assertFailure("new java.net.Socket(\"localhost\", 1024)"); + // no files + assertFailure("java.io.File.createTempFile(\"test\", \"tmp\")"); + } +} 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 f4d83cf502d..87bfbb5af15 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 @@ -20,8 +20,11 @@ package org.elasticsearch.script.python; import java.io.IOException; +import java.security.AccessControlContext; import java.security.AccessController; +import java.security.Permissions; import java.security.PrivilegedAction; +import java.security.ProtectionDomain; import java.util.Map; import org.apache.lucene.index.LeafReaderContext; @@ -125,7 +128,8 @@ public class PythonScriptEngineService extends AbstractComponent implements Scri public Object execute(CompiledScript compiledScript, Map vars) { PyObject pyVars = Py.java2py(vars); interp.setLocals(pyVars); - PyObject ret = interp.eval((PyCode) compiledScript.compiled()); + // eval the script with reduced privileges + PyObject ret = evalRestricted((PyCode) compiledScript.compiled()); if (ret == null) { return null; } @@ -171,7 +175,8 @@ public class PythonScriptEngineService extends AbstractComponent implements Scri @Override public Object run() { interp.setLocals(pyVars); - PyObject ret = interp.eval(code); + // eval the script with reduced privileges + PyObject ret = evalRestricted(code); if (ret == null) { return null; } @@ -229,7 +234,8 @@ public class PythonScriptEngineService extends AbstractComponent implements Scri @Override public Object run() { interp.setLocals(pyVars); - PyObject ret = interp.eval(code); + // eval the script with reduced privileges + PyObject ret = evalRestricted(code); if (ret == null) { return null; } @@ -257,6 +263,27 @@ public class PythonScriptEngineService extends AbstractComponent implements Scri } } + // we don't have a way to specify codesource for generated jython classes, + // so we just run them with a special context to reduce privileges + private static final AccessControlContext PY_CONTEXT; + static { + Permissions none = new Permissions(); + none.setReadOnly(); + PY_CONTEXT = new AccessControlContext(new ProtectionDomain[] { + new ProtectionDomain(null, none) + }); + } + + /** Evaluates with reduced privileges */ + private final PyObject evalRestricted(final PyCode code) { + // eval the script with reduced privileges + return AccessController.doPrivileged(new PrivilegedAction() { + @Override + public PyObject run() { + return interp.eval(code); + } + }, PY_CONTEXT); + } public static Object unwrapValue(Object value) { if (value == null) { diff --git a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java new file mode 100644 index 00000000000..745a109d5f1 --- /dev/null +++ b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java @@ -0,0 +1,92 @@ +/* + * 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.python; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.test.ESTestCase; +import org.junit.After; +import org.junit.Before; +import org.python.core.PyException; + +import java.util.HashMap; +import java.util.Map; + +/** + * Tests for Python security permissions + */ +public class PythonSecurityTests extends ESTestCase { + + private PythonScriptEngineService se; + + @Before + public void setup() { + se = new PythonScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + } + + @After + public void close() { + // We need to clear some system properties + System.clearProperty("python.cachedir.skip"); + System.clearProperty("python.console.encoding"); + se.close(); + } + + /** runs a script */ + private void doTest(String script) { + Map vars = new HashMap(); + se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "test", "python", se.compile(script)), vars); + } + + /** asserts that a script runs without exception */ + private void assertSuccess(String script) { + doTest(script); + } + + /** assert that a security exception is hit */ + private void assertFailure(String script) { + try { + doTest(script); + fail("did not get expected exception"); + } catch (PyException expected) { + Throwable cause = expected.getCause(); + assertNotNull("null cause for exception: " + expected, cause); + assertTrue("unexpected exception: " + cause, cause instanceof SecurityException); + } + } + + /** Test some py scripts that are ok */ + public void testOK() { + assertSuccess("1 + 2"); + assertSuccess("from java.lang import Math\nMath.cos(0)"); + } + + /** Test some py scripts that should hit security exception */ + public void testNotOK() { + // sanity check :) + assertFailure("from java.lang import Runtime\nRuntime.getRuntime().halt(0)"); + // check a few things more restrictive than the ordinary policy + // no network + assertFailure("from java.net import Socket\nSocket(\"localhost\", 1024)"); + // no files + assertFailure("from java.io import File\nFile.createTempFile(\"test\", \"tmp\")"); + } +}