jlstrater commented on a change in pull request #1282:
URL: https://github.com/apache/groovy/pull/1282#discussion_r441959751
##########
File path:
src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
##########
@@ -94,24 +94,28 @@
import java.util.Map;
/**
- * This customizer allows securing source code by controlling what code
constructs are allowed. For example, if you only
- * want to allow arithmetic operations in a groovy shell, you can configure
this customizer to restrict package imports,
- * method calls and so on.
+ * This customizer allows securing source code by controlling what code
constructs are permitted.
+ * This is typically done when using Groovy for its scripting or domain
specific language (DSL) features.
+ * For example, if you only want to allow arithmetic operations in a groovy
shell,
+ * you can configure this customizer to restrict package imports, method calls
and so on.
* <p>
- * Most of the security customization options found in this class work with
either blacklist or whitelist. This means that, for a
- * single option, you can set a whitelist OR a blacklist, but not both. You
can mix whitelist/blacklist strategies for
- * different options. For example, you can have import whitelist and tokens
blacklist.
+ * Most of the security customization options found in this class work with
either <i>allowed</i> or <i>disallowed</i> lists.
+ * This means that, for a single option, you can set an allowed list OR a
disallowed list, but not both.
+ * You can mix allowed/disallowed strategies for different options.
+ * For example, you can have an allowed import list and a disallowed tokens
list.
* <p>
- * The recommended way of securing shells is to use whitelists because it is
guaranteed that future features of the
- * Groovy language won't be allowed by defaut. Using blacklists, you can limit
the features of the languages by opting
- * out, but new language features would require you to update your
configuration.
+ * The recommended way of securing shells is to use allowed lists because it
is guaranteed that future features of the
+ * Groovy language won't be accidentally allowed unless explicitly added to
the allowed list.
+ * Using disallowed lists, you can limit the features of the language
constructs supported by your shell by opting
+ * out, but new language features are then implicitly also available and this
may not be desirable.
+ * The implication is that you might need to update your configuration with
each new release.
* <p>
- * If you set neither a whitelist nor a blacklist, then everything is
authorized.
+ * If you set neither an allowed list nor a disallowed list, then everything
is permitted.
Review comment:
```suggestion
* If neither an allowed list nor a disallowed list is set, then everything
is permitted.
```
##########
File path:
src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
##########
@@ -94,24 +94,28 @@
import java.util.Map;
/**
- * This customizer allows securing source code by controlling what code
constructs are allowed. For example, if you only
- * want to allow arithmetic operations in a groovy shell, you can configure
this customizer to restrict package imports,
- * method calls and so on.
+ * This customizer allows securing source code by controlling what code
constructs are permitted.
+ * This is typically done when using Groovy for its scripting or domain
specific language (DSL) features.
+ * For example, if you only want to allow arithmetic operations in a groovy
shell,
+ * you can configure this customizer to restrict package imports, method calls
and so on.
* <p>
- * Most of the security customization options found in this class work with
either blacklist or whitelist. This means that, for a
- * single option, you can set a whitelist OR a blacklist, but not both. You
can mix whitelist/blacklist strategies for
- * different options. For example, you can have import whitelist and tokens
blacklist.
+ * Most of the security customization options found in this class work with
either <i>allowed</i> or <i>disallowed</i> lists.
+ * This means that, for a single option, you can set an allowed list OR a
disallowed list, but not both.
+ * You can mix allowed/disallowed strategies for different options.
+ * For example, you can have an allowed import list and a disallowed tokens
list.
* <p>
- * The recommended way of securing shells is to use whitelists because it is
guaranteed that future features of the
- * Groovy language won't be allowed by defaut. Using blacklists, you can limit
the features of the languages by opting
- * out, but new language features would require you to update your
configuration.
+ * The recommended way of securing shells is to use allowed lists because it
is guaranteed that future features of the
+ * Groovy language won't be accidentally allowed unless explicitly added to
the allowed list.
+ * Using disallowed lists, you can limit the features of the language
constructs supported by your shell by opting
+ * out, but new language features are then implicitly also available and this
may not be desirable.
+ * The implication is that you might need to update your configuration with
each new release.
* <p>
- * If you set neither a whitelist nor a blacklist, then everything is
authorized.
+ * If you set neither an allowed list nor a disallowed list, then everything
is permitted.
Review comment:
while you're in there...
I tripped over this when reading it the first time. I think this might be
clearer?
##########
File path:
src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
##########
@@ -462,120 +662,200 @@ public void addExpressionCheckers(ExpressionChecker...
checkers) {
expressionCheckers.addAll(Arrays.asList(checkers));
}
+ public List<String> getDisallowedConstantTypes() {
+ return disallowedConstantTypes;
+ }
+
+ /**
+ * Legacy alias for {@link #getDisallowedConstantTypes()}
+ */
public List<String> getConstantTypesBlackList() {
- return constantTypesBlackList;
+ return getDisallowedConstantTypes();
}
public void setConstantTypesBlackList(final List<String>
constantTypesBlackList) {
- if (constantTypesWhiteList != null) {
- throw new IllegalArgumentException("You are not allowed to set
both whitelist and blacklist");
+ if (allowedConstantTypes != null) {
+ throw new IllegalArgumentException("You are not allowed to set
both an allowed list and a disallowed list");
}
- this.constantTypesBlackList = constantTypesBlackList;
+ this.disallowedConstantTypes = constantTypesBlackList;
+ }
+
+ public List<String> getAllowedConstantTypes() {
+ return allowedConstantTypes;
}
+ /**
+ * Legacy alias for {@link #getAllowedStatements()}
+ */
public List<String> getConstantTypesWhiteList() {
- return constantTypesWhiteList;
+ return getAllowedConstantTypes();
}
- public void setConstantTypesWhiteList(final List<String>
constantTypesWhiteList) {
- if (constantTypesBlackList != null) {
- throw new IllegalArgumentException("You are not allowed to set
both whitelist and blacklist");
+ public void setAllowedConstantTypes(final List<String>
allowedConstantTypes) {
+ if (disallowedConstantTypes != null) {
+ throw new IllegalArgumentException("You are not allowed to set
both an allowed list and a disallowed list");
}
- this.constantTypesWhiteList = constantTypesWhiteList;
+ this.allowedConstantTypes = allowedConstantTypes;
+ }
+
+ /**
+ * Legacy alias for {@link #setAllowedConstantTypes(List)}
+ */
+ public void setConstantTypesWhiteList(final List<String>
allowedConstantTypes) {
+ setAllowedConstantTypes(allowedConstantTypes);
}
/**
* An alternative way of setting constant types.
*
- * @param constantTypesWhiteList a list of classes.
+ * @param allowedConstantTypes a list of classes.
*/
- public void setConstantTypesClassesWhiteList(final List<Class>
constantTypesWhiteList) {
+ public void setAllowedConstantTypesClasses(final List<Class>
allowedConstantTypes) {
List<String> values = new LinkedList<>();
- for (Class aClass : constantTypesWhiteList) {
+ for (Class aClass : allowedConstantTypes) {
values.add(aClass.getName());
}
setConstantTypesWhiteList(values);
}
+ /**
+ * Legacy alias for {@link #setAllowedConstantTypesClasses(List)}
+ */
+ public void setConstantTypesClassesWhiteList(final List<Class>
allowedConstantTypes) {
+ setAllowedConstantTypesClasses(allowedConstantTypes);
+ }
+
/**
* An alternative way of setting constant types.
*
- * @param constantTypesBlackList a list of classes.
+ * @param disallowedConstantTypes a list of classes.
*/
- public void setConstantTypesClassesBlackList(final List<Class>
constantTypesBlackList) {
+ public void setDisallowedConstantTypesClasses(final List<Class>
disallowedConstantTypes) {
List<String> values = new LinkedList<>();
- for (Class aClass : constantTypesBlackList) {
+ for (Class aClass : disallowedConstantTypes) {
values.add(aClass.getName());
}
setConstantTypesBlackList(values);
}
+ /**
+ * Legacy alias for {@link #setDisallowedConstantTypesClasses(List)}
+ */
+ public void setConstantTypesClassesBlackList(final List<Class>
disallowedConstantTypes) {
+ setDisallowedConstantTypesClasses(disallowedConstantTypes);
+ }
+
+ public List<String> getDisallowedReceivers() {
+ return disallowedReceivers;
+ }
+
+ /**
+ * Legacy alias for {@link #getDisallowedReceivers()}
+ */
public List<String> getReceiversBlackList() {
- return receiversBlackList;
+ return getDisallowedReceivers();
}
/**
* Sets the list of classes which deny method calls.
*
* Please note that since Groovy is a dynamic language, and
- * this class performs a static type check, it will be reletively
- * simple to bypass any blacklist unless the receivers blacklist contains,
at
+ * this class performs a static type check, it will be relatively
+ * simple to bypass any disallowed list unless the receivers disallowed
list contains, at
Review comment:
both here and below, I think this was supposed to be 'disallowed
receivers list' instead of 'receiver[']s disallowed list'?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
