Permalink
Browse files

Make groovy sandbox method blacklist dynamically additive

Using the `script.groovy.sandbox.method_blacklist_patch` setting, the
blacklist can be dynamically *added* to by specifying a comma-separated
list of methods (for example, "toString,size" would add .toString and
.size to the blacklist).

When the `script.groovy.sandbox.method_blacklist_patch` setting is
changed, the script cache is cleared to force new scripts to be
recompiled. Additionally the on-disk cache is cleared so that scripts in
the `config/scripts` directory are re-compiled as well.

This also fixes an issue where script engines were injected more than
once, which can cause multiple instances of the script engine per node.

Conflicts:
	src/main/java/org/elasticsearch/script/ScriptService.java
	src/main/java/org/elasticsearch/watcher/FileWatcher.java
	src/test/java/org/elasticsearch/script/ScriptServiceTests.java
  • Loading branch information...
dakrone committed Jan 28, 2015
1 parent a8f555d commit 68c4a6201e6c889b272c1b64550237fe6d172b47
@@ -34,6 +34,7 @@
import org.elasticsearch.indices.recovery.RecoverySettings;
import org.elasticsearch.indices.store.IndicesStore;
import org.elasticsearch.indices.ttl.IndicesTTLService;
import org.elasticsearch.script.groovy.GroovyScriptEngineService;
import org.elasticsearch.threadpool.ThreadPool;
/**
@@ -97,6 +98,7 @@ public ClusterDynamicSettingsModule() {
clusterDynamicSettings.addDynamicSetting(HierarchyCircuitBreakerService.FIELDDATA_CIRCUIT_BREAKER_OVERHEAD_SETTING, Validator.NON_NEGATIVE_DOUBLE);
clusterDynamicSettings.addDynamicSetting(HierarchyCircuitBreakerService.REQUEST_CIRCUIT_BREAKER_LIMIT_SETTING, Validator.MEMORY_SIZE);
clusterDynamicSettings.addDynamicSetting(HierarchyCircuitBreakerService.REQUEST_CIRCUIT_BREAKER_OVERHEAD_SETTING, Validator.NON_NEGATIVE_DOUBLE);
clusterDynamicSettings.addDynamicSetting(GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH);
}
public void addDynamicSettings(String... settings) {
@@ -63,7 +63,7 @@ protected void configure() {
MapBinder<String, NativeScriptFactory> scriptsBinder
= MapBinder.newMapBinder(binder(), String.class, NativeScriptFactory.class);
for (Map.Entry<String, Class<? extends NativeScriptFactory>> entry : scripts.entrySet()) {
scriptsBinder.addBinding(entry.getKey()).to(entry.getValue());
scriptsBinder.addBinding(entry.getKey()).to(entry.getValue()).asEagerSingleton();
}
// now, check for config based ones
@@ -74,35 +74,35 @@ protected void configure() {
if (type == NativeScriptFactory.class) {
throw new ElasticsearchIllegalArgumentException("type is missing for native script [" + name + "]");
}
scriptsBinder.addBinding(name).to(type);
scriptsBinder.addBinding(name).to(type).asEagerSingleton();
}
Multibinder<ScriptEngineService> multibinder = Multibinder.newSetBinder(binder(), ScriptEngineService.class);
multibinder.addBinding().to(NativeScriptEngineService.class);
try {
settings.getClassLoader().loadClass("groovy.lang.GroovyClassLoader");
multibinder.addBinding().to(GroovyScriptEngineService.class);
multibinder.addBinding().to(GroovyScriptEngineService.class).asEagerSingleton();
} catch (Throwable t) {
Loggers.getLogger(ScriptService.class, settings).debug("failed to load groovy", t);
}
try {
settings.getClassLoader().loadClass("com.github.mustachejava.Mustache");
multibinder.addBinding().to(MustacheScriptEngineService.class);
multibinder.addBinding().to(MustacheScriptEngineService.class).asEagerSingleton();
} catch (Throwable t) {
Loggers.getLogger(ScriptService.class, settings).debug("failed to load mustache", t);
}
try {
settings.getClassLoader().loadClass("org.apache.lucene.expressions.Expression");
multibinder.addBinding().to(ExpressionScriptEngineService.class);
multibinder.addBinding().to(ExpressionScriptEngineService.class).asEagerSingleton();
} catch (Throwable t) {
Loggers.getLogger(ScriptService.class, settings).debug("failed to load lucene expressions", t);
}
for (Class<? extends ScriptEngineService> scriptEngine : scriptEngines) {
multibinder.addBinding().to(scriptEngine);
multibinder.addBinding().to(scriptEngine).asEagerSingleton();
}
bind(ScriptService.class).asEagerSingleton();
@@ -57,6 +57,8 @@
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.env.Environment;
import org.elasticsearch.index.query.TemplateQueryParser;
import org.elasticsearch.node.settings.NodeSettingsService;
import org.elasticsearch.script.groovy.GroovyScriptEngineService;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.watcher.FileChangesListener;
import org.elasticsearch.watcher.FileWatcher;
@@ -66,6 +68,7 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
@@ -93,6 +96,7 @@
private final Cache<CacheKey, CompiledScript> cache;
private final File scriptsDirectory;
private final FileWatcher fileWatcher;
private final DynamicScriptDisabling dynamicScriptingDisabled;
@@ -209,9 +213,26 @@ public static void writeTo(ScriptType scriptType, StreamOutput out) throws IOExc
}
}
class ApplySettings implements NodeSettingsService.Listener {
@Override
public void onRefreshSettings(Settings settings) {
GroovyScriptEngineService engine = (GroovyScriptEngineService) ScriptService.this.scriptEngines.get("groovy");
String[] patches = settings.getAsArray(GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH, Strings.EMPTY_ARRAY);
if (Arrays.equals(patches, engine.blacklistAdditions()) == false) {
logger.info("updating [{}] from {} to {}", GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH,
engine.blacklistAdditions(), patches);
engine.blacklistAdditions(patches);
engine.reloadConfig();
// Because the GroovyScriptEngineService knows nothing about the
// cache, we need to clear it here if the setting changes
ScriptService.this.clearCache();
}
}
}
@Inject
public ScriptService(Settings settings, Environment env, Set<ScriptEngineService> scriptEngines,
ResourceWatcherService resourceWatcherService) {
ResourceWatcherService resourceWatcherService, NodeSettingsService nodeSettingsService) throws IOException {
super(settings);
int cacheMaxSize = settings.getAsInt(SCRIPT_CACHE_SIZE_SETTING, 100);
@@ -244,7 +265,7 @@ public ScriptService(Settings settings, Environment env, Set<ScriptEngineService
if (logger.isTraceEnabled()) {
logger.trace("Using scripts directory [{}] ", scriptsDirectory);
}
FileWatcher fileWatcher = new FileWatcher(scriptsDirectory);
this.fileWatcher = new FileWatcher(scriptsDirectory);
fileWatcher.addListener(new ScriptChangesListener());
if (componentSettings.getAsBoolean("auto_reload_enabled", true)) {
@@ -254,6 +275,7 @@ public ScriptService(Settings settings, Environment env, Set<ScriptEngineService
// automatic reload is disable just load scripts once
fileWatcher.init();
}
nodeSettingsService.addListener(new ApplySettings());
}
//This isn't set in the ctor because doing so creates a guice circular
@@ -276,6 +298,21 @@ public CompiledScript compile(String lang, String script) {
return compile(lang, script, ScriptType.INLINE);
}
/**
* Clear both the in memory and on disk compiled script caches. Files on
* disk will be treated as if they are new and recompiled.
* */
public void clearCache() {
logger.debug("clearing script cache");
// Clear the in-memory script caches
this.cache.invalidateAll();
this.cache.cleanUp();
// Clear the cache of on-disk scripts
this.staticCache.clear();
// Clear the file watcher's state so it re-compiles on-disk scripts
this.fileWatcher.clearState();
}
public CompiledScript compile(String lang, String script, ScriptType scriptType) {
if (logger.isTraceEnabled()) {
logger.trace("Compiling lang: [{}] type: [{}] script: {}", lang, scriptType, script);
@@ -49,18 +49,22 @@
public static String GROOVY_SCRIPT_SANDBOX_RECEIVER_WHITELIST = "script.groovy.sandbox.receiver_whitelist";
private final Set<String> methodBlacklist;
private final Set<String> additionalMethodBlacklist;
private final Set<String> packageWhitelist;
private final Set<String> classWhitelist;
public GroovySandboxExpressionChecker(Settings settings) {
public GroovySandboxExpressionChecker(Settings settings, String[] blacklistAdditions) {
this.methodBlacklist = ImmutableSet.copyOf(settings.getAsArray(GROOVY_SANDBOX_METHOD_BLACKLIST, defaultMethodBlacklist, true));
this.additionalMethodBlacklist = ImmutableSet.copyOf(blacklistAdditions);
this.packageWhitelist = ImmutableSet.copyOf(settings.getAsArray(GROOVY_SANDBOX_PACKAGE_WHITELIST, defaultPackageWhitelist, true));
this.classWhitelist = ImmutableSet.copyOf(settings.getAsArray(GROOVY_SANDBOX_CLASS_WHITELIST, defaultClassConstructionWhitelist, true));
}
// Never allow calling these methods, regardless of the object type
public static String[] defaultMethodBlacklist = new String[]{
"getClass",
"class",
"forName",
"wait",
"notify",
"notifyAll",
@@ -121,6 +125,8 @@ public boolean isAuthorized(Expression expression) {
String methodName = mce.getMethodAsString();
if (methodBlacklist.contains(methodName)) {
return false;
} else if (additionalMethodBlacklist.contains(methodName)) {
return false;
} else if (methodName == null && mce.getMethod() instanceof GStringExpression) {
// We do not allow GStrings for method invocation, they are a security risk
return false;
@@ -142,7 +148,7 @@ public boolean isAuthorized(Expression expression) {
* Returns a customized ASTCustomizer that includes the whitelists and
* expression checker.
*/
public static SecureASTCustomizer getSecureASTCustomizer(Settings settings) {
public static SecureASTCustomizer getSecureASTCustomizer(Settings settings, String[] blacklistAdditions) {
SecureASTCustomizer scz = new SecureASTCustomizer();
// Closures are allowed
scz.setClosuresAllowed(true);
@@ -158,7 +164,7 @@ public static SecureASTCustomizer getSecureASTCustomizer(Settings settings) {
String[] receiverWhitelist = settings.getAsArray(GROOVY_SCRIPT_SANDBOX_RECEIVER_WHITELIST, defaultReceiverWhitelist, true);
scz.setReceiversWhiteList(newArrayList(receiverWhitelist));
// Add the customized expression checker for finer-grained checking
scz.addExpressionCheckers(new GroovySandboxExpressionChecker(settings));
scz.addExpressionCheckers(new GroovySandboxExpressionChecker(settings, blacklistAdditions));
return scz;
}
}
@@ -37,6 +37,7 @@
import org.codehaus.groovy.control.customizers.ImportCustomizer;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.ESLogger;
@@ -56,22 +57,36 @@
public class GroovyScriptEngineService extends AbstractComponent implements ScriptEngineService {
public static String GROOVY_SCRIPT_SANDBOX_ENABLED = "script.groovy.sandbox.enabled";
public static String GROOVY_SCRIPT_BLACKLIST_PATCH = "script.groovy.sandbox.method_blacklist_patch";
private final AtomicLong counter = new AtomicLong();
private final GroovyClassLoader loader;
private final boolean sandboxed;
private volatile GroovyClassLoader loader;
private volatile String[] blacklistAdditions = Strings.EMPTY_ARRAY;
@Inject
public GroovyScriptEngineService(Settings settings) {
super(settings);
this.sandboxed = settings.getAsBoolean(GROOVY_SCRIPT_SANDBOX_ENABLED, true);
reloadConfig();
}
public String[] blacklistAdditions() {
return this.blacklistAdditions;
}
public void blacklistAdditions(String[] additions) {
this.blacklistAdditions = additions;
}
public void reloadConfig() {
ImportCustomizer imports = new ImportCustomizer();
imports.addStarImports("org.joda.time");
imports.addStaticStars("java.lang.Math");
CompilerConfiguration config = new CompilerConfiguration();
config.addCompilationCustomizers(imports);
this.sandboxed = settings.getAsBoolean(GROOVY_SCRIPT_SANDBOX_ENABLED, true);
if (this.sandboxed) {
config.addCompilationCustomizers(GroovySandboxExpressionChecker.getSecureASTCustomizer(settings));
config.addCompilationCustomizers(GroovySandboxExpressionChecker.getSecureASTCustomizer(settings, this.blacklistAdditions));
}
// Add BigDecimal -> Double transformer
config.addCompilationCustomizers(new GroovyBigDecimalTransformer(CompilePhase.CONVERSION));
@@ -32,16 +32,26 @@
public class FileWatcher extends AbstractResourceWatcher<FileChangesListener> {
private FileObserver rootFileObserver;
private File file;
private static final ESLogger logger = Loggers.getLogger(FileWatcher.class);
/**
* Creates new file watcher on the given directory
*/
public FileWatcher(File file) {
this.file = file;
rootFileObserver = new FileObserver(file);
}
/**
* Clears any state with the FileWatcher, making all files show up as new
*/
public void clearState() {
rootFileObserver = new FileObserver(file);
rootFileObserver.init(false);
}
@Override
protected void doInit() {
rootFileObserver.init(true);
@@ -22,6 +22,9 @@
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.groovy.GroovyScriptEngineService;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.junit.Test;
@@ -31,6 +34,7 @@
/**
* Tests for the Groovy scripting sandbox
*/
@ElasticsearchIntegrationTest.ClusterScope(scope = ElasticsearchIntegrationTest.Scope.TEST)
public class GroovySandboxScriptTests extends ElasticsearchIntegrationTest {
@Test
@@ -62,7 +66,7 @@ public void testSandboxedGroovyScript() {
"Expression [MethodCallExpression] is not allowed: d.$(get + Class)().$(getDeclared + Method)(now).$(set + Accessible)(false)");
testFailure("Class.forName(\\\"DateTime\\\").getDeclaredMethod(\\\"plus\\\").setAccessible(true)",
"Method calls not allowed on [java.lang.Class]");
"Expression [MethodCallExpression] is not allowed: java.lang.Class.forName(DateTime)");
testFailure("Eval.me('2 + 2')", "Method calls not allowed on [groovy.util.Eval]");
@@ -90,6 +94,37 @@ public void testSandboxedGroovyScript() {
"Expression [MethodCallExpression] is not allowed: java.lang.Runtime.$(get + Runtime)().$methodNameec(touch /tmp/gotcha2)");
}
@Test
public void testDynamicBlacklist() throws Exception {
client().prepareIndex("test", "doc", "1").setSource("foo", 5).setRefresh(true).get();
testSuccess("[doc['foo'].value, 3, 4].isEmpty()");
testSuccess("[doc['foo'].value, 3, 4].size()");
// Now we blacklist two methods, .isEmpty() and .size()
Settings blacklistSettings = ImmutableSettings.builder()
.put(GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH, "isEmpty,size")
.build();
client().admin().cluster().prepareUpdateSettings().setTransientSettings(blacklistSettings).get();
testFailure("[doc['foo'].value, 3, 4].isEmpty()",
"Expression [MethodCallExpression] is not allowed: [doc[foo].value, 3, 4].isEmpty()");
testFailure("[doc['foo'].value, 3, 4].size()",
"Expression [MethodCallExpression] is not allowed: [doc[foo].value, 3, 4].size()");
// Undo the blacklist addition and make sure the scripts still work
Settings emptyBlacklistSettings = ImmutableSettings.builder()
.put(GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH, "")
.build();
client().admin().cluster().prepareUpdateSettings().setTransientSettings(emptyBlacklistSettings).get();
testSuccess("[doc['foo'].value, 3, 4].isEmpty()");
testSuccess("[doc['foo'].value, 3, 4].size()");
}
public void testSuccess(String script) {
logger.info("--> script: " + script);
SearchResponse resp = client().prepareSearch("test")
@@ -24,6 +24,7 @@
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.node.settings.NodeSettingsService;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.watcher.ResourceWatcherService;
@@ -56,7 +57,8 @@ public void testScriptsWithoutExtensions() throws IOException {
ResourceWatcherService resourceWatcherService = new ResourceWatcherService(settings, null);
logger.info("--> setup script service");
ScriptService scriptService = new ScriptService(settings, environment, ImmutableSet.of(new TestEngineService()), resourceWatcherService);
ScriptService scriptService = new ScriptService(settings, environment,
ImmutableSet.of(new TestEngineService()), resourceWatcherService, new NodeSettingsService(settings));
File scriptsFile = new File(genericConfigFolder, "scripts");
assertThat(scriptsFile.mkdir(), equalTo(true));
resourceWatcherService.notifyNow();

0 comments on commit 68c4a62

Please sign in to comment.