This is an automated email from the ASF dual-hosted git repository. jkevan pushed a commit to branch loadAllowForbidScriptsFromFiles in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 59101ec8c4dc3598c613963e6bc300b91abb1e04 Author: Kevan <[email protected]> AuthorDate: Mon Nov 16 13:08:54 2020 +0100 UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing --- .../main/resources/etc/custom.system.properties | 8 ++-- package/src/main/resources/etc/mvel-allow.json | 1 + package/src/main/resources/etc/mvel-forbid.json | 19 ++++++++ package/src/main/resources/etc/ognl-allow.json | 1 + package/src/main/resources/etc/ognl-forbid.json | 19 ++++++++ scripting/pom.xml | 5 +++ .../internal/ExpressionFilterFactoryImpl.java | 51 +++++++++------------- 7 files changed, 69 insertions(+), 35 deletions(-) diff --git a/package/src/main/resources/etc/custom.system.properties b/package/src/main/resources/etc/custom.system.properties index 9660b68..2c9b395 100644 --- a/package/src/main/resources/etc/custom.system.properties +++ b/package/src/main/resources/etc/custom.system.properties @@ -48,10 +48,10 @@ org.apache.unomi.scripting.filter.activated=${env:UNOMI_SCRIPTING_FILTER_ACTIVAT # It is however fully expected to add new expressions to the "allow" value, although it is better to add them inside # any plugins you may be adding. This configuration is only designed to compensate for the cases where something was not properly designed or to deal with compatibility issues. Just be VERY careful to make your patterns AS SPECIFIC AS POSSIBLE in order to avoid introducing a way to abuse the expression filtering. org.apache.unomi.scripting.filter.collections=${env:UNOMI_SCRIPTING_FILTER_COLLECTIONS:-mvel,ognl} -org.apache.unomi.scripting.filter.mvel.allow=${env:UNOMI_SCRIPTING_FILTER_MVEL_ALLOW:-} -org.apache.unomi.scripting.filter.mvel.forbid=${env:UNOMI_SCRIPTING_FILTER_MVEL_FORBID:-.*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*,eval} -org.apache.unomi.scripting.filter.ognl.allow=${env:UNOMI_SCRIPTING_FILTER_OGNL_ALLOW:-} -org.apache.unomi.scripting.filter.ognl.forbid=${env:UNOMI_SCRIPTING_FILTER_OGNL_FORBID:-.*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*,eval} +org.apache.unomi.scripting.filter.mvel.allow=${env:UNOMI_SCRIPTING_FILTER_MVEL_ALLOW:-${karaf.etc}/mvel-allow.json} +org.apache.unomi.scripting.filter.mvel.forbid=${env:UNOMI_SCRIPTING_FILTER_MVEL_FORBID:-${karaf.etc}/mvel-forbid.json} +org.apache.unomi.scripting.filter.ognl.allow=${env:UNOMI_SCRIPTING_FILTER_OGNL_ALLOW:-${karaf.etc}/ognl-allow.json} +org.apache.unomi.scripting.filter.ognl.forbid=${env:UNOMI_SCRIPTING_FILTER_OGNL_FORBID:-${karaf.etc}/ognl-forbid.json} # This parameter controls whether OGNL scripting is allowed in expressions. Because of security reasons it is # deactivated by default. If you run into compatibility issues you could reactivate it but it is at your own risk. diff --git a/package/src/main/resources/etc/mvel-allow.json b/package/src/main/resources/etc/mvel-allow.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/package/src/main/resources/etc/mvel-allow.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/package/src/main/resources/etc/mvel-forbid.json b/package/src/main/resources/etc/mvel-forbid.json new file mode 100644 index 0000000..07e8ee6 --- /dev/null +++ b/package/src/main/resources/etc/mvel-forbid.json @@ -0,0 +1,19 @@ +[ + ".*Runtime.*", + ".*ProcessBuilder.*", + ".*exec.*", + ".*invoke.*", + ".*getClass.*", + ".*Class.*", + ".*ClassLoader.*", + ".*System.*", + ".*Method.*", + ".*method.*", + ".*Compiler.*", + ".*Thread.*", + ".*FileWriter.*", + ".*forName.*", + ".*Socket.*", + ".*DriverManager.*", + "eval" +] \ No newline at end of file diff --git a/package/src/main/resources/etc/ognl-allow.json b/package/src/main/resources/etc/ognl-allow.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/package/src/main/resources/etc/ognl-allow.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/package/src/main/resources/etc/ognl-forbid.json b/package/src/main/resources/etc/ognl-forbid.json new file mode 100644 index 0000000..07e8ee6 --- /dev/null +++ b/package/src/main/resources/etc/ognl-forbid.json @@ -0,0 +1,19 @@ +[ + ".*Runtime.*", + ".*ProcessBuilder.*", + ".*exec.*", + ".*invoke.*", + ".*getClass.*", + ".*Class.*", + ".*ClassLoader.*", + ".*System.*", + ".*Method.*", + ".*method.*", + ".*Compiler.*", + ".*Thread.*", + ".*FileWriter.*", + ".*forName.*", + ".*Socket.*", + ".*DriverManager.*", + "eval" +] \ No newline at end of file diff --git a/scripting/pom.xml b/scripting/pom.xml index 61c10e6..e468457 100644 --- a/scripting/pom.xml +++ b/scripting/pom.xml @@ -43,6 +43,11 @@ </dependency> <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-lang3</artifactId> + </dependency> + + <dependency> <groupId>org.apache.unomi</groupId> <artifactId>unomi-api</artifactId> <version>2.0.0-SNAPSHOT</version> diff --git a/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java b/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java index 6278fe3..5831842 100644 --- a/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java +++ b/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java @@ -18,6 +18,7 @@ package org.apache.unomi.scripting.internal; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang3.StringUtils; import org.apache.unomi.scripting.ExpressionFilter; import org.apache.unomi.scripting.ExpressionFilterFactory; import org.osgi.framework.Bundle; @@ -27,6 +28,7 @@ import org.osgi.framework.BundleListener; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; import java.io.IOException; import java.net.URL; import java.util.*; @@ -58,37 +60,8 @@ public class ExpressionFilterFactoryImpl implements ExpressionFilterFactory,Bund String[] initialFilterCollectionParts = initialFilterCollections.split(","); if (initialFilterCollectionParts != null) { for (String initialFilterCollection : initialFilterCollectionParts) { - String systemAllowedPatterns = System.getProperty("org.apache.unomi.scripting.filter."+initialFilterCollection+".allow", null); - if (systemAllowedPatterns != null) { - Set<Pattern> collectionAllowedExpressionPatterns = new HashSet<>(); - if ("all".equals(systemAllowedPatterns.trim())) { - collectionAllowedExpressionPatterns = null; - } else { - if (systemAllowedPatterns.trim().length() > 0) { - String[] systemAllowedPatternParts = systemAllowedPatterns.split(","); - collectionAllowedExpressionPatterns = new HashSet<>(); - for (String systemAllowedPatternPart : systemAllowedPatternParts) { - collectionAllowedExpressionPatterns.add(Pattern.compile(systemAllowedPatternPart)); - } - } - } - allowedExpressionPatternsByCollection.put(initialFilterCollection, collectionAllowedExpressionPatterns); - } - - String systemForbiddenPatterns = System.getProperty("org.apache.unomi.scripting.filter."+initialFilterCollection+".forbid", ".*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*,eval"); - if (systemForbiddenPatterns != null) { - Set<Pattern> collectionForbiddenExpressionPatterns = new HashSet<>(); - if (systemForbiddenPatterns.trim().length() > 0) { - String[] systemForbiddenPatternParts = systemForbiddenPatterns.split(","); - collectionForbiddenExpressionPatterns = new HashSet<>(); - for (String systemForbiddenPatternPart : systemForbiddenPatternParts) { - collectionForbiddenExpressionPatterns.add(Pattern.compile(systemForbiddenPatternPart)); - } - } else { - collectionForbiddenExpressionPatterns = null; - } - forbiddenExpressionPatternsByCollection.put(initialFilterCollection, collectionForbiddenExpressionPatterns); - } + allowedExpressionPatternsByCollection.put(initialFilterCollection, loadPatternsFromConfig("org.apache.unomi.scripting.filter."+initialFilterCollection+".allow")); + forbiddenExpressionPatternsByCollection.put(initialFilterCollection, loadPatternsFromConfig("org.apache.unomi.scripting.filter."+initialFilterCollection+".forbid")); } } @@ -102,7 +75,23 @@ public class ExpressionFilterFactoryImpl implements ExpressionFilterFactory,Bund bundleContext.addBundleListener(this); } + } + private Set<Pattern> loadPatternsFromConfig(String propertyKey) { + String patternsFile = System.getProperty(propertyKey, null); + if (StringUtils.isNotEmpty(patternsFile)) { + Set<Pattern> patterns = new HashSet<>(); + try { + JsonNode jsonPatterns = objectMapper.readTree(new File(patternsFile)); + for (JsonNode jsonPattern : jsonPatterns) { + patterns.add(Pattern.compile(jsonPattern.asText())); + } + } catch (IOException e) { + logger.error("Error while loading expressions definition from " + propertyKey, e); + } + return patterns; + } + return null; } public void destroy() {
