This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push: new d4e0976 JEXL-357: added and updated Javadoc; - refined code to reduce clutter; - refined tests to improve coverage; d4e0976 is described below commit d4e09765d3f044ded3b9d8502c9a0eefbeab0ff2 Author: henrib <hen...@apache.org> AuthorDate: Mon Feb 7 17:43:51 2022 +0100 JEXL-357: added and updated Javadoc; - refined code to reduce clutter; - refined tests to improve coverage; --- RELEASE-NOTES.txt | 12 +++ src/changes/changes.xml | 9 ++ .../java/org/apache/commons/jexl3/JexlBuilder.java | 57 ++++++++-- .../java/org/apache/commons/jexl3/JexlOptions.java | 7 +- .../apache/commons/jexl3/annotations/NoJexl.java | 4 +- .../org/apache/commons/jexl3/internal/Engine.java | 34 +++++- .../jexl3/internal/introspection/Introspector.java | 2 +- .../jexl3/internal/introspection/Permissions.java | 51 +++------ .../internal/introspection/PermissionsParser.java | 4 +- .../jexl3/internal/introspection/Uberspect.java | 5 +- .../jexl3/introspection/JexlPermissions.java | 115 +++++++++++++++++++-- .../org/apache/commons/jexl3/JexlTestCase.java | 30 +++++- .../internal/introspection/PermissionsTest.java | 52 ++++++++-- 13 files changed, 308 insertions(+), 74 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 74b2e63..be26679 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -28,6 +28,18 @@ Compatibility with previous releases ==================================== Version 3.3 is source and binary compatible with 3.2. +What's new in 3.3: +================== +JEXL 3.3 brings the ability to configure permissions on libraries in the manner pioneered +with the @NoJexl annotation on source code. This is achieved through a crude but light mechanism akin to +a security manager that controls what JEXL can introspect and thus expose to scripts. +Used in conjunction with options (JexlOptions) and features (JexlFeatures), the permissions (JexlPermissions) +allow fine-tuning the scripting integration into any project. + +New Features in 3.3: +==================== +* JEXL-357: Configure accessible packages/classes/methods/fields + Bugs Fixed in 3.3: ================== * JEXL-354: #pragma does not handle negative integer or real literals diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6e81e57..74640cc 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -27,6 +27,15 @@ <body> <release version="3.3" date="YYYY-MM-DD"> <!-- UPDATE --> + <action dev="henrib" type="add" issue="JEXL-357" > + Configure accessible packages/classes/methods/fields + </action> + <action dev="henrib" type="fix" issue="JEXL-354" due-to="William Price"> + #pragma does not handle negative integer or real literals + </action> + <action dev="henrib" type="fix" issue="JEXL-353" due-to="Mr.Z"> + Documentation error for not-in/not-match operator + </action> <action dev="ggregory" type="update" due-to="Dependabot"> Bump checkstyle from 9.2 to 9.2.1 #72. </action> diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java index df19317..c9e7f93 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java +++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java @@ -18,6 +18,7 @@ package org.apache.commons.jexl3; import org.apache.commons.jexl3.internal.Engine; +import org.apache.commons.jexl3.introspection.JexlPermissions; import org.apache.commons.jexl3.introspection.JexlSandbox; import org.apache.commons.jexl3.introspection.JexlUberspect; import org.apache.commons.logging.Log; @@ -26,13 +27,27 @@ import java.util.Map; import java.nio.charset.Charset; /** - * Configure and builds a JexlEngine. + * Configures and builds a JexlEngine. * - * <p>The <code>setSilent</code> and <code>setStrict</code> methods allow to fine-tune an engine instance behavior - * according to various error control needs. The strict flag tells the engine when and if null as operand is - * considered an error, the silent flag tells the engine what to do with the error - * (log as warning or throw exception).</p> + * <p>The builder allow fine-tuning an engine instance behavior according to various control needs.</p> * + * <p>Broad configurations elements are controlled through the features ({@link JexlFeatures}) that can restrict JEXL + * syntax - for instance, only expressions with no-side effects - and permissions ({@link JexlPermissions}) that control + * the visible set of objects - for instance, avoiding access to any object in java.rmi.* -. </p> + * + * <p>Fine error control and runtime-overridable behaviors are implemented through options ({@link JexlOptions}). Most + * common flags accessible from the builder are reflected in its options ({@link #options()}). + * <p>The <code>silent</code> flag tells the engine what to do with the error; when true, errors are logged as + * warning, when false, they throw {@link JexlException} exceptions.</p> + * <p>The <code>strict</code> flag tells the engine when and if null as operand is considered an error. The <code>safe</code> + * flog determines if safe-navigation is used. Safe-navigation allows an evaluation shortcut and return null in expressions + * that attempts dereferencing null, typically a method call or accessing a property.</p> + * <p>The <code>lexical</code> and <code>lexicalShade</code> flags can be used to enforce a lexical scope for + * variables and parameters. The <code>lexicalShade</code> can be used to further ensure no global variable can be + * used with the same name as a local one even after it goes out of scope. The corresponding feature flags should be + * preferred since they will detect violations at parsing time. (see {@link JexlFeatures})</p> + * + * <p>The following rules apply on silent and strict flags:</p> * <ul> * <li>When "silent" & "not-strict": * <p> 0 & null should be indicators of "default" values so that even in an case of error, @@ -55,6 +70,7 @@ import java.nio.charset.Charset; * </p> * </li> * </ul> + * */ public class JexlBuilder { @@ -67,6 +83,9 @@ public class JexlBuilder { /** The strategy strategy. */ private JexlUberspect.ResolverStrategy strategy = null; + /** The set of permissions. */ + private JexlPermissions permissions = null; + /** The sandbox. */ private JexlSandbox sandbox = null; @@ -123,6 +142,22 @@ public class JexlBuilder { } /** + * Sets the JexlPermissions instance the engine will use. + * + * @param p the permissions + * @return this builder + */ + public JexlBuilder permissions(final JexlPermissions p) { + this.permissions = p; + return this; + } + + /** @return the permissions */ + public JexlPermissions permissions() { + return this.permissions; + } + + /** * Sets the JexlUberspect strategy strategy the engine will use. * <p>This is ignored if the uberspect has been set. * @@ -319,7 +354,9 @@ public class JexlBuilder { /** * Sets whether the engine will throw JexlException during evaluation when an error is triggered. - * + * <p>When <em>not</em> silent, the engine throws an exception when the evaluation triggers an exception or an + * error.</p> + * <p>It is recommended to use <em>silent(true)</em> as an explicit default.</p> * @param flag true means no JexlException will occur, false allows them * @return this builder */ @@ -336,6 +373,9 @@ public class JexlBuilder { /** * Sets whether the engine considers unknown variables, methods, functions and constructors as errors or * evaluates them as null. + * <p>When <em>not</em> strict, operators or functions using null operands return null on evaluation. When + * strict, those raise exceptions.</p> + * <p>It is recommended to use <em>strict(true)</em> as an explicit default.</p> * * @param flag true means strict error reporting, false allows them to be evaluated as null * @return this builder @@ -352,9 +392,10 @@ public class JexlBuilder { /** * Sets whether the engine considers dereferencing null in navigation expressions - * as errors or evaluates them as null. + * as null or triggers an error. * <p><code>x.y()</code> if x is null throws an exception when not safe, - * return null and warns if it is.<p> + * return null and warns if it is.</p> + * <p>It is recommended to use <em>safe(false)</em> as an explicit default.</p> * * @param flag true means safe navigation, false throws exception when dereferencing null * @return this builder diff --git a/src/main/java/org/apache/commons/jexl3/JexlOptions.java b/src/main/java/org/apache/commons/jexl3/JexlOptions.java index 1c12b1e..ab9e573 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlOptions.java +++ b/src/main/java/org/apache/commons/jexl3/JexlOptions.java @@ -27,7 +27,7 @@ import org.apache.commons.jexl3.internal.Engine; * The flags, briefly explained, are the following: * <ul> * <li>silent: whether errors throw exception</li> - * <li>safe: whether navigation through null is an error</li> + * <li>safe: whether navigation through null is <em>not</em>an error</li> * <li>cancellable: whether thread interruption is an error</li> * <li>lexical: whether redefining local variables is an error</li> * <li>lexicalShade: whether local variables shade global ones even outside their scope</li> @@ -312,8 +312,11 @@ public final class JexlOptions { } /** - * Sets whether the engine considers null in navigation expression as errors + * Sets whether the engine considers null in navigation expression as null or as errors * during evaluation. + * <p>If safe, encountering null during a navigation expression - dereferencing a method or a field through a null + * object or property - will <em>not</em> be considered an error but evaluated as <em>null</em>. It is recommended + * to use <em>setSafe(false)</em> as an explicit default.</p> * @param flag true if safe, false otherwise */ public void setSafe(final boolean flag) { diff --git a/src/main/java/org/apache/commons/jexl3/annotations/NoJexl.java b/src/main/java/org/apache/commons/jexl3/annotations/NoJexl.java index 14ce643..7c2c21d 100644 --- a/src/main/java/org/apache/commons/jexl3/annotations/NoJexl.java +++ b/src/main/java/org/apache/commons/jexl3/annotations/NoJexl.java @@ -29,7 +29,9 @@ import java.lang.annotation.Target; * This allows to completely hide a package, class, interface, constructor, method or field from * JEXL; a NoJexl annotated element will not be usable through any kind of JEXL expression or script. * </p> - * See {@link org.apache.commons.jexl3.introspection.JexlSandbox} for another way to restrict JEXL access. + * See {@link org.apache.commons.jexl3.introspection.JexlPermissions} for the general mechanism to restrict + * what JEXL allows in scripts. + * See {@link org.apache.commons.jexl3.introspection.JexlSandbox} for another way to further constrain JEXL access. */ @Documented @Retention(RetentionPolicy.RUNTIME) diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java b/src/main/java/org/apache/commons/jexl3/internal/Engine.java index 61de330..11d6338 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java @@ -25,9 +25,11 @@ import org.apache.commons.jexl3.JexlFeatures; import org.apache.commons.jexl3.JexlInfo; import org.apache.commons.jexl3.JexlOptions; import org.apache.commons.jexl3.JexlScript; +import org.apache.commons.jexl3.internal.introspection.Permissions; import org.apache.commons.jexl3.internal.introspection.SandboxUberspect; import org.apache.commons.jexl3.internal.introspection.Uberspect; import org.apache.commons.jexl3.introspection.JexlMethod; +import org.apache.commons.jexl3.introspection.JexlPermissions; import org.apache.commons.jexl3.introspection.JexlSandbox; import org.apache.commons.jexl3.introspection.JexlUberspect; import org.apache.commons.jexl3.parser.ASTArrayAccess; @@ -73,7 +75,9 @@ public class Engine extends JexlEngine { private static final class UberspectHolder { /** The default uberspector that handles all introspection patterns. */ private static final Uberspect UBERSPECT = - new Uberspect(LogFactory.getLog(JexlEngine.class), JexlUberspect.JEXL_STRATEGY); + new Uberspect(LogFactory.getLog(JexlEngine.class), + JexlUberspect.JEXL_STRATEGY, + JexlPermissions.parse()); /** Non-instantiable. */ private UberspectHolder() {} @@ -194,7 +198,9 @@ public class Engine extends JexlEngine { this.collectMode = conf.collectMode(); this.stackOverflow = conf.stackOverflow() > 0? conf.stackOverflow() : Integer.MAX_VALUE; // core properties: - final JexlUberspect uber = conf.uberspect() == null ? getUberspect(conf.logger(), conf.strategy()) : conf.uberspect(); + final JexlUberspect uber = conf.uberspect() == null + ? getUberspect(conf.logger(), conf.strategy(), conf.permissions()) + : conf.uberspect(); final ClassLoader loader = conf.loader(); if (loader != null) { uber.setClassLoader(loader); @@ -238,14 +244,32 @@ public class Engine extends JexlEngine { * instead of the default one.</p> * @param logger the logger to use for the underlying Uberspect * @param strategy the property resolver strategy + * @param permissions the introspection permissions * @return Uberspect the default uberspector instance. + * @since 3.3 */ - public static Uberspect getUberspect(final Log logger, final JexlUberspect.ResolverStrategy strategy) { + public static Uberspect getUberspect( + final Log logger, + final JexlUberspect.ResolverStrategy strategy, + final JexlPermissions permissions) { if ((logger == null || logger.equals(LogFactory.getLog(JexlEngine.class))) - && (strategy == null || strategy == JexlUberspect.JEXL_STRATEGY)) { + && (strategy == null || strategy == JexlUberspect.JEXL_STRATEGY) + && permissions == null || permissions == Permissions.DEFAULT) { return UberspectHolder.UBERSPECT; } - return new Uberspect(logger, strategy); + return new Uberspect(logger, strategy, permissions); + } + + /** + * Use {@link Engine#getUberspect(Log, JexlUberspect.ResolverStrategy, JexlPermissions)}. + * @deprecated 3.3 + * @param logger the logger + * @param strategy the strategy + * @return an Uberspect instance + */ + @Deprecated + public static Uberspect getUberspect(final Log logger, final JexlUberspect.ResolverStrategy strategy) { + return getUberspect(logger, strategy, null); } @Override diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java index ee534da..08b5290 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java @@ -106,7 +106,7 @@ public final class Introspector { public Introspector(final Log log, final ClassLoader cloader, final JexlPermissions perms) { this.logger = log; this.loader = cloader; - this.permissions = perms != null? perms : Permissions.SECURE; + this.permissions = perms; } /** diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java index 7c6ad9f..911f395 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java @@ -33,19 +33,21 @@ import org.apache.commons.jexl3.introspection.JexlPermissions; /** * Checks whether an element (ctor, field or method) is visible by JEXL introspection. - * Default implementation does this by checking if element has been annotated with NoJexl. + * <p>Default implementation does this by checking if element has been annotated with NoJexl.</p> * - * The NoJexl annotation allows a fine grain permissions on executable objects (methods, fields, constructors). - * NoJexl of a package implies all classes (including derived classes) and all interfaces - * of that package are invisible to Jexl; - * NoJexl on a class implies this class and all its derived classes are invisible to Jexl; - * NoJexl on a (public) field makes it not visible as a property to Jexl; - * NoJexl on a constructor prevents that constructor to be used to instantiate through 'new'; - * NoJexl on a method prevents that method and any of its overrides to be visible to Jexl; - * NoJexl on an interface prevents all methods of that interface and their overrides to be visible to Jexl. - * - * It is possible to further refine permissions on classes used through libraries where source code form can - * not be altered. The @link(PermissionsParser). + * <p>The NoJexl annotation allows a fine grain permissions on executable objects (methods, fields, constructors). + * </p> + * <ul> + * <li>NoJexl of a package implies all classes (including derived classes) and all interfaces + * of that package are invisible to JEXL.</li> + * <li>NoJexl on a class implies this class and all its derived classes are invisible to JEXL.</li> + * <li>NoJexl on a (public) field makes it not visible as a property to JEXL.</li> + * <li>NoJexl on a constructor prevents that constructor to be used to instantiate through 'new'.</li> + * <li>NoJexl on a method prevents that method and any of its overrides to be visible to JEXL.</li> + * <li>NoJexl on an interface prevents all methods of that interface and their overrides to be visible to JEXL.</li> + * </ul> + * <p> It is possible to further refine permissions on classes used through libraries where source code form can + * not be altered using an instance of permissions using {@link JexlPermissions#parse(String...)}.</p> */ public class Permissions implements JexlPermissions { @@ -235,31 +237,6 @@ public class Permissions implements JexlPermissions { public static final Permissions DEFAULT = new Permissions(); /** - * A very secure singleton. - */ - public static final Permissions SECURE = new PermissionsParser().parse( - "# Secure Uberspect Permissions", - "java.nio.*", - "java.io.*", - "java.lang.*", - "java.math.*", - "java.text.*", - "java.util.*", - "org.w3c.dom.*", - "org.apache.commons.jexl3.*", - "java.lang { Runtime {} System {} ProcessBuilder {} Class {} }", - "java.lang.annotation {}", - "java.lang.instrument {}", - "java.lang.invoke {}", - "java.lang.management {}", - "java.lang.ref {}", - "java.lang.reflect {}", - "java.net {}", - "java.io { File { } }", - "java.nio { Path { } Paths { } Files { } }" - ); - - /** * @return the packages */ Map<String, NoJexlPackage> getPackages() { diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java index 8f05737..165aef3 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java @@ -151,6 +151,7 @@ public class PermissionsParser { */ private int readIdentifier(StringBuilder id, int offset, boolean dot, boolean star) { int begin = -1; + boolean starf = star; int i = offset; char c = 0; while (i < size) { @@ -167,8 +168,9 @@ public class PermissionsParser { } id.append('.'); begin = -1; - } else if (star && c == '*') { + } else if (starf && c == '*') { id.append('*'); + starf = false; // only one star } else { break; } diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java index 64bf0d6..589a84d 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java @@ -20,6 +20,7 @@ import org.apache.commons.jexl3.JexlArithmetic; import org.apache.commons.jexl3.JexlEngine; import org.apache.commons.jexl3.JexlOperator; import org.apache.commons.jexl3.introspection.JexlMethod; +import org.apache.commons.jexl3.introspection.JexlPermissions; import org.apache.commons.jexl3.introspection.JexlPropertyGet; import org.apache.commons.jexl3.introspection.JexlPropertySet; import org.apache.commons.jexl3.introspection.JexlUberspect; @@ -58,7 +59,7 @@ public class Uberspect implements JexlUberspect { /** The resolver strategy. */ private final JexlUberspect.ResolverStrategy strategy; /** The permissions. */ - private final Permissions permissions; + private final JexlPermissions permissions; /** The introspector version. */ private final AtomicInteger version; /** The soft reference to the introspector currently in use. */ @@ -88,7 +89,7 @@ public class Uberspect implements JexlUberspect { * @param sty the resolver strategy * @param perms the introspector permissions */ - public Uberspect(final Log runtimeLogger, final JexlUberspect.ResolverStrategy sty, final Permissions perms) { + public Uberspect(final Log runtimeLogger, final JexlUberspect.ResolverStrategy sty, final JexlPermissions perms) { logger = runtimeLogger == null? LogFactory.getLog(JexlEngine.class) : runtimeLogger; strategy = sty == null? JexlUberspect.JEXL_STRATEGY : sty; permissions = perms == null? Permissions.DEFAULT : perms; diff --git a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java index 910bf53..f0cad14 100644 --- a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java +++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java @@ -16,6 +16,7 @@ */ package org.apache.commons.jexl3.introspection; +import org.apache.commons.jexl3.internal.introspection.Permissions; import org.apache.commons.jexl3.internal.introspection.PermissionsParser; import java.lang.reflect.Constructor; @@ -23,54 +24,148 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; /** - * A JEXL dedicated 'security manager' that constraints which packages/classes/constructors/fields/methods - * are made visible to JEXL scripts. + * This interface describes permissions used by JEXL introspection that constrain which + * packages/classes/constructors/fields/methods are made visible to JEXL scripts. + * <p>By specifying or implementing permissions, it is possible to constrain precisely which objects can be manipulated + * by JEXL, allowing users to enter their own expressions or scripts whilst maintaining tight control + * over what can be executed. JEXL introspection mechanism will check whether it is permitted to + * access a constructor, method or field before exposition to the {@link JexlUberspect}. The restrictions + * are applied in all cases, for any {@link org.apache.commons.jexl3.introspection.JexlUberspect.ResolverStrategy}. + * </p> + * <p>This complements using a dedicated {@link ClassLoader} and/or {@link SecurityManager} - being deprecated - + * and possibly {@link JexlSandbox} with a simpler mechanism. The {@link org.apache.commons.jexl3.annotations.NoJexl} + * annotation processing is actually performed using the result of calling {@link #parse(String...)} with no arguments; + * implementations shall delegate calls to its methods for {@link org.apache.commons.jexl3.annotations.NoJexl} to be + * processed.</p> + * <p>A simple textual configuration can be used to create user defined permissions using + * {@link JexlPermissions#parse(String...)}.</p> + *<p>To instantiate a JEXL engine using permissions, one should use a {@link org.apache.commons.jexl3.JexlBuilder} + * and call {@link org.apache.commons.jexl3.JexlBuilder#permissions(JexlPermissions)}. Another approach would + * be to instantiate a {@link JexlUberspect} with those permissions and call + * {@link org.apache.commons.jexl3.JexlBuilder#uberspect(JexlUberspect)}.</p> + * @since 3.3 */ public interface JexlPermissions { /** - * Checks whether a package explicitly disallows JEXL introspection. + * Checks whether a package allows JEXL introspection. + * <p>If the package disallows JEXL introspection, none of its classes or interfaces are visible + * to JEXL and can not be used in scripts or expression.</p> * @param pack the package * @return true if JEXL is allowed to introspect, false otherwise + * @since 3.3 */ boolean allow(final Package pack); /** - * Checks whether a class or one of its super-classes or implemented interfaces - * explicitly disallows JEXL introspection. + * Checks whether a class allows JEXL introspection. + * <p>If the class disallows JEXL introspection, none of its constructors, methods or fields + * as well as derived classes are visible to JEXL and can not be used in scripts or expressions. + * If one of its super-classes is not allowed, tbe class is not allowed either.</p> + * <p>For interfaces, only methods and fields are disallowed in derived interfaces or implementing classes.</p> * @param clazz the class to check * @return true if JEXL is allowed to introspect, false otherwise + * @since 3.3 */ boolean allow(final Class<?> clazz); /** - * Checks whether a constructor explicitly disallows JEXL introspection. + * Checks whether a constructor allows JEXL introspection. + * <p>If a constructor is not allowed, the new operator can not be used to instantiate its declared class + * in scripts or expressions.</p> * @param ctor the constructor to check * @return true if JEXL is allowed to introspect, false otherwise + * @since 3.3 */ boolean allow(final Constructor<?> ctor); /** - * Checks whether a method explicitly disallows JEXL introspection. - * <p>Since methods can be overridden, this also checks that no superclass or interface + * Checks whether a method allows JEXL introspection. + * <p>If a method is not allowed, it can not resolved and called in scripts or expressions.</p> + * <p>Since methods can be overridden and overloaded, this also checks that no superclass or interface * explicitly disallows this methods.</p> * @param method the method to check * @return true if JEXL is allowed to introspect, false otherwise + * @since 3.3 */ boolean allow(final Method method); /** * Checks whether a field explicitly disallows JEXL introspection. + * <p>If a field is not allowed, it can not resolved and accessed in scripts or expressions.</p> * @param field the field to check * @return true if JEXL is allowed to introspect, false otherwise + * @since 3.3 */ boolean allow(final Field field); /** * Parses a set of permissions. - * @param src the permissions source + * <p> + * In JEXL 3.3, the syntax recognizes 2 types of permissions: + * </p> + * <ul> + * <li>Allowing access to a wildcard restricted set of packages. </li> + * <li>Denying access to packages, classes (and inner classes), methods and fields</li> + * </ul> + * <p>Wildcards specifications determine the set of allowed packages. When empty, all packages can be + * used. When using JEXL to expose functional elements, their packages should be exposed through wildcards. + * These allow composing the volume of what is allowed by addition.</p> + * <p>Restrictions behave exactly like the {@link org.apache.commons.jexl3.annotations.NoJexl} annotation; + * they can restrict access to package, class, inner-class, methods and fields. + * These allow refining the volume of what is allowed by extrusion.</p> + * An example of a tight environment that would not allow scripts to wander could be: + * <pre> + * # allow a very restricted set of base classes + * java.math.* + * java.text.* + * java.util.* + * # deny classes that could pose a security risk + * java.lang { Runtime {} System {} ProcessBuilder {} Class {} } + * org.apache.commons.jexl3 { JexlBuilder {} } + * </pre> + * <p> + * Syntax for wildcards is the name of the package suffixed by <code>.*</code>. Syntax for restrictions is + * a list of package restrictions. A package restriction is a package name followed by a block + * (as in curly-bracket block {}) that contains a list of class restrictions. A class restriction is a class name + * followed by a block of member restrictions. A member restriction can be a class restriction - to restrict + * nested classes -, a field which is the Java field name suffixed with <code>;</code>, a method composed of + * its Java name suffixed with <code>();</code>. Constructor restrictions are specified like methods using the + * class name as method name. + * </p> + * <p> + * All overrides and overloads of a constructors or method are allowed or restricted at the same time, + * the restriction being based on their names, not their whole signature. This differs from the @NoJexl annotation. + * </p> + * <pre> + * # some wildcards + * java.lang.*; # java.lang is pretty much a must have + * my.allowed.package0.* + * another.allowed.package1.* + * # nojexl like restrictions + * my.package.internal {} # the whole package is hidden + * my.package { + * class0 { + * class1 {} # the whole class1 is hidden + * class2 { + * class2(); # class2 constructors can not be invoked + * class3 { + * aMethod(); # aMethod can not be called + * aField; # aField can not be accessed + * } + * } # end of class2 + * class0(); # class0 constructors can not be invoked + * method(); # method can not be called + * field; # field can not be accessed + * } # end class0 + * } # end package my.package + * </pre> + * + * @param src the permissions source, the default (NoJexl aware) permissions if null * @return the permissions instance + * @since 3.3 */ static JexlPermissions parse(String... src) { - return new PermissionsParser().parse(src); + return src == null || src.length == 0? Permissions.DEFAULT : new PermissionsParser().parse(src); } + } diff --git a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java index d9f515a..56d031c 100644 --- a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java +++ b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java @@ -24,6 +24,7 @@ import java.lang.reflect.Method; import org.apache.commons.jexl3.internal.Util; import org.apache.commons.jexl3.internal.introspection.Permissions; import org.apache.commons.jexl3.internal.introspection.Uberspect; +import org.apache.commons.jexl3.introspection.JexlPermissions; import org.junit.After; import org.junit.Assert; @@ -68,9 +69,36 @@ public class JexlTestCase { return new JexlBuilder().create(); } + /** + * A very secure singleton. + */ + public static final JexlPermissions SECURE = JexlPermissions.parse( + "# Secure Uberspect Permissions", + "java.nio.*", + "java.io.*", + "java.lang.*", + "java.math.*", + "java.text.*", + "java.util.*", + "org.w3c.dom.*", + "org.apache.commons.jexl3.*", + "org.apache.commons.jexl3 { JexlBuilder {} }", + "org.apache.commons.jexl3.internal { Engine {} }", + "java.lang { Runtime {} System {} ProcessBuilder {} Class {} }", + "java.lang.annotation {}", + "java.lang.instrument {}", + "java.lang.invoke {}", + "java.lang.management {}", + "java.lang.ref {}", + "java.lang.reflect {}", + "java.net {}", + "java.io { File { } }", + "java.nio { Path { } Paths { } Files { } }" + ); + public static JexlEngine createEngine(final boolean lenient) { return new JexlBuilder() - .uberspect(new Uberspect(null, null, Permissions.SECURE)) + .uberspect(new Uberspect(null, null, SECURE)) .arithmetic(new JexlArithmetic(!lenient)).cache(128).create(); } diff --git a/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java b/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java index 0f8161c..f9fdf4d 100644 --- a/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java +++ b/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java @@ -16,6 +16,11 @@ */ package org.apache.commons.jexl3.internal.introspection; +import org.apache.commons.jexl3.JexlBuilder; +import org.apache.commons.jexl3.JexlEngine; +import org.apache.commons.jexl3.JexlException; +import org.apache.commons.jexl3.JexlScript; +import org.apache.commons.jexl3.JexlTestCase; import org.apache.commons.jexl3.internal.introspection.nojexlpackage.Invisible; import org.apache.commons.jexl3.introspection.JexlPermissions; import org.junit.Assert; @@ -24,15 +29,12 @@ import org.junit.Test; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; -import java.math.BigDecimal; import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; -import static org.apache.commons.jexl3.internal.introspection.Permissions.SECURE; /** * Checks the CacheMap.MethodKey implementation @@ -40,6 +42,7 @@ import static org.apache.commons.jexl3.internal.introspection.Permissions.SECURE public class PermissionsTest { + public static class A { public int i; public A() {} @@ -205,6 +208,8 @@ public class PermissionsTest { Set<String> wildcards = p.getWildcards(); Assert.assertEquals(4, wildcards.size()); + JexlEngine jexl = new JexlBuilder().permissions(p).safe(false).lexical(true).create(); + Method exit = getMethod(java.lang.Runtime.class,"exit"); Assert.assertNotNull(exit); Assert.assertFalse(p.allow(exit)); @@ -214,10 +219,24 @@ public class PermissionsTest { Method callMeNot = getMethod(Outer.Inner.class, "callMeNot"); Assert.assertNotNull(callMeNot); Assert.assertFalse(p.allow(callMeNot)); + JexlScript script = jexl.createScript("o.callMeNot()", "o"); + try { + Object result = script.execute(null, new Outer.Inner()); + Assert.fail("callMeNot should be uncallable"); + } catch(JexlException.Method xany) { + Assert.assertEquals("callMeNot", xany.getMethod()); + } Method uncallable = getMethod(Invisible.class, "uncallable"); Assert.assertFalse(p.allow(uncallable)); Package ip = Invisible.class.getPackage(); Assert.assertFalse(p.allow(ip)); + script = jexl.createScript("o.uncallable()", "o"); + try { + Object result = script.execute(null, new Invisible()); + Assert.fail("uncallable should be uncallable"); + } catch(JexlException.Method xany) { + Assert.assertEquals("uncallable", xany.getMethod()); + } } @Test @@ -233,7 +252,7 @@ public class PermissionsTest { @Test public void testSecurePermissions() { - Assert.assertNotNull(SECURE); + Assert.assertNotNull(JexlTestCase.SECURE); List<Class<?>> acs = Arrays.asList( java.lang.Runtime.class, java.math.BigDecimal.class, @@ -242,7 +261,7 @@ public class PermissionsTest { for(Class<?> ac: acs) { Package p = ac.getPackage(); Assert.assertNotNull(ac.getName(), p); - Assert.assertTrue(ac.getName(), SECURE.allow(p)); + Assert.assertTrue(ac.getName(), JexlTestCase.SECURE.allow(p)); } List<Class<?>> nacs = Arrays.asList( java.lang.annotation.ElementType.class, @@ -254,8 +273,29 @@ public class PermissionsTest { for(Class<?> nac : nacs) { Package p = nac.getPackage(); Assert.assertNotNull(nac.getName(), p); - Assert.assertFalse(nac.getName(), SECURE.allow(p)); + Assert.assertFalse(nac.getName(), JexlTestCase.SECURE.allow(p)); } } + + @Test + public void testParsePermissionsFailures() { + String[] srcs = new String[]{ + "java.lang.*.*", + "java.math.*.", + "java.text.*;", + "java.lang {{ Runtime {} }", + "java.rmi {}}", + "java.io { Text File {} }", + "java.io { File { m.x } }" + }; + for(String src : srcs) { + try { + Permissions p = (Permissions) JexlPermissions.parse(src); + Assert.fail(src); + } catch(IllegalStateException xill) { + Assert.assertNotNull(xill); + } + } + } }