You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by jk...@apache.org on 2020/11/16 12:09:12 UTC

[unomi] branch loadAllowForbidScriptsFromFiles created (now 59101ec)

This is an automated email from the ASF dual-hosted git repository.

jkevan pushed a change to branch loadAllowForbidScriptsFromFiles
in repository https://gitbox.apache.org/repos/asf/unomi.git.


      at 59101ec  UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing

This branch includes the following new commits:

     new 59101ec  UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[unomi] 01/01: UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing

Posted by jk...@apache.org.
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 <ke...@jahia.com>
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() {