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() {

Reply via email to