This is an automated email from the ASF dual-hosted git repository.
jkevan pushed a commit to branch unomi-1.4.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/unomi-1.4.x by this push:
new c55af15 UNOMI-399: load allow/forbid script from files instead of
configuration property to avoid script code conflicting with property value
parsing (#213)
c55af15 is described below
commit c55af15c10893782071379d8c0095d16cffd05e2
Author: kevan Jahanshahi <[email protected]>
AuthorDate: Mon Nov 16 13:17:34 2020 +0100
UNOMI-399: load allow/forbid script from files instead of configuration
property to avoid script code conflicting with property value parsing (#213)
---
.../main/resources/etc/custom.system.properties | 28 ++++++++++--
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, 90 insertions(+), 34 deletions(-)
diff --git a/package/src/main/resources/etc/custom.system.properties
b/package/src/main/resources/etc/custom.system.properties
index d0490bf..85c9488 100644
--- a/package/src/main/resources/etc/custom.system.properties
+++ b/package/src/main/resources/etc/custom.system.properties
@@ -31,11 +31,33 @@
org.apache.unomi.hazelcast.network.port=${env:UNOMI_HAZELCAST_NETWORK_PORT:-5701
## Security settings
##
#######################################################################################################################
org.apache.unomi.security.root.password=${env:UNOMI_ROOT_PASSWORD:-karaf}
+
+# These parameters control the list of classes that are allowed or forbidden
when executing expressions.
org.apache.unomi.scripting.allow=${env:UNOMI_ALLOW_SCRIPTING_CLASSES:-org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,ognl.*,java.lang.Object,java.util.Map,java.util.HashMap,java.lang.Integer,org.mvel2.*}
org.apache.unomi.scripting.forbid=${env:UNOMI_FORBID_SCRIPTING_CLASSES:-}
-org.apache.unomi.scripting.filter.allow=${env:UNOMI_SCRIPTING_FILTER_ALLOW:-all}
-org.apache.unomi.scripting.filter.forbid=${env:UNOMI_SCRIPTING_FILTER_FORBID:-.*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*}
+
+# This parameter controls the whole expression filtering system. It is not
recommended to turn it off. The main reason
+# to turn it off would be to check if it is interfering with something, but it
should always be active in production.
+org.apache.unomi.scripting.filter.activated=${env:UNOMI_SCRIPTING_FILTER_ACTIVATED:-true}
+
+# The following parameters control the filtering using regular expressions for
each scripting sub-system.
+# The "collections" parameter tells the expression filtering system which
configurations to expect. By default only
+# MVEL and/or OGNL are accepted values, but in the future these might be
replaced by new scripting sub-systems.
+# For each scripting sub-system, there is an allow and a forbid value. It is
NOT recommended to change the built-in
+# "forbid" value unless you are having issues with its value.
+# 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:-${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.
org.apache.unomi.security.properties.useOGNLScripting=${env:UNOMI_SCRIPTING_USE_OGNL:-false}
+# This parameter controls the condition sanitizing done on the ContextServlet
(/context.json). If will remove any
+# expressions that start with "script::". It is not recommended to change this
value, unless you run into compatibility issues.
org.apache.unomi.security.personalization.sanitizeConditions=${env:UNOMI_SECURITY_SANITIZEPERSONALIZATIONCONDITIONS:-true}
#######################################################################################################################
@@ -164,7 +186,7 @@
org.apache.unomi.shell.sshIdleTimeOut=${env:UNOMI_SHELL_SSHIDLETIMEOUT:-1800000}
org.apache.unomi.shell.hostKey=${env:UNOMI_SHELL_HOSTKEY:-${karaf.etc}/host.key}
#
# The format used for hostKey.
-#�Possible values are simple (Karaf internal), or PEM (OpenSSH format)
+#�Possible values are simple (Karaf internal), or PEM (OpenSSH format)
#
org.apache.unomi.shell.hostKeyFormat=${env:UNOMI_SHELL_HOSTKEYFORMAT:-simple}
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() {