Make script.groovy.sandbox.method_blacklist_patch truly append-only

Additionally, this setting can be specified in elasticsearch.yml if
desired, to pre-populate the list of methods to be added to the default
blacklist.

When making a change to this setting dynamically, the entire blacklist
is logged as well.
This commit is contained in:
Lee Hinman 2015-01-28 16:36:08 -07:00
parent afcedb94ed
commit 86e52c30a1
4 changed files with 24 additions and 22 deletions

View File

@ -68,7 +68,6 @@ import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
@ -213,10 +212,10 @@ public class ScriptService extends AbstractComponent {
GroovyScriptEngineService engine = (GroovyScriptEngineService) ScriptService.this.scriptEngines.get("groovy");
if (engine != null) {
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);
boolean blacklistChanged = engine.addToBlacklist(patches);
if (blacklistChanged) {
logger.info("adding {} to [{}], new blacklisted methods: {}", patches,
GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH, engine.blacklistAdditions());
engine.reloadConfig();
// Because the GroovyScriptEngineService knows nothing about the
// cache, we need to clear it here if the setting changes

View File

@ -53,7 +53,7 @@ public class GroovySandboxExpressionChecker implements SecureASTCustomizer.Expre
private final Set<String> packageWhitelist;
private final Set<String> classWhitelist;
public GroovySandboxExpressionChecker(Settings settings, String[] blacklistAdditions) {
public GroovySandboxExpressionChecker(Settings settings, Set<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));
@ -148,7 +148,7 @@ public class GroovySandboxExpressionChecker implements SecureASTCustomizer.Expre
* Returns a customized ASTCustomizer that includes the whitelists and
* expression checker.
*/
public static SecureASTCustomizer getSecureASTCustomizer(Settings settings, String[] blacklistAdditions) {
public static SecureASTCustomizer getSecureASTCustomizer(Settings settings, Set<String> blacklistAdditions) {
SecureASTCustomizer scz = new SecureASTCustomizer();
// Closures are allowed
scz.setClosuresAllowed(true);

View File

@ -19,6 +19,7 @@
package org.elasticsearch.script.groovy;
import com.google.common.collect.ImmutableSet;
import groovy.lang.Binding;
import groovy.lang.GroovyClassLoader;
import groovy.lang.Script;
@ -48,7 +49,9 @@ import org.elasticsearch.search.lookup.SearchLookup;
import java.io.IOException;
import java.math.BigDecimal;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
/**
@ -62,21 +65,32 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri
private final AtomicLong counter = new AtomicLong();
private final boolean sandboxed;
private volatile GroovyClassLoader loader;
private volatile String[] blacklistAdditions = Strings.EMPTY_ARRAY;
private volatile Set<String> blacklistAdditions;
@Inject
public GroovyScriptEngineService(Settings settings) {
super(settings);
this.sandboxed = settings.getAsBoolean(GROOVY_SCRIPT_SANDBOX_ENABLED, true);
this.blacklistAdditions = ImmutableSet.copyOf(settings.getAsArray(GROOVY_SCRIPT_BLACKLIST_PATCH, Strings.EMPTY_ARRAY));
reloadConfig();
}
public String[] blacklistAdditions() {
public Set<String> blacklistAdditions() {
return this.blacklistAdditions;
}
public void blacklistAdditions(String[] additions) {
this.blacklistAdditions = additions;
/**
* Appends the additional blacklisted methods to the current blacklist,
* returns true if the black list has changed
*/
public boolean addToBlacklist(String... additions) {
Set<String> newBlackList = new HashSet<>(blacklistAdditions);
for (String addition : additions) {
newBlackList.add(addition);
}
boolean changed = this.blacklistAdditions.equals(newBlackList) == false;
this.blacklistAdditions = ImmutableSet.copyOf(newBlackList);
return changed;
}
public void reloadConfig() {

View File

@ -112,17 +112,6 @@ public class GroovySandboxScriptTests extends ElasticsearchIntegrationTest {
"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) {