WW-5268 Implement ability to specify exempt classes for package exclusions

This commit is contained in:
Kusal Kithul-Godage
2023-02-15 00:34:42 +11:00
parent 930c6de807
commit 5fcee890d5
5 changed files with 174 additions and 23 deletions
@@ -26,7 +26,13 @@ import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.TextParseUtil;
import com.opensymphony.xwork2.util.reflection.ReflectionException;
import ognl.*;
import ognl.ClassResolver;
import ognl.Ognl;
import ognl.OgnlContext;
import ognl.OgnlException;
import ognl.OgnlRuntime;
import ognl.SimpleNode;
import ognl.TypeConverter;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -37,7 +43,12 @@ import java.beans.IntrospectionException;
import java.beans.Introspector;
import java.beans.PropertyDescriptor;
import java.lang.reflect.Method;
import java.util.*;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
@@ -66,10 +77,12 @@ public class OgnlUtil {
private Set<Class<?>> excludedClasses;
private Set<Pattern> excludedPackageNamePatterns;
private Set<String> excludedPackageNames;
private Set<Class<?>> excludedPackageExemptClasses;
private Set<Class<?>> devModeExcludedClasses;
private Set<Pattern> devModeExcludedPackageNamePatterns;
private Set<String> devModeExcludedPackageNames;
private Set<Class<?>> devModeExcludedPackageExemptClasses;
private Container container;
private boolean allowStaticFieldAccess = true;
@@ -112,10 +125,12 @@ public class OgnlUtil {
excludedClasses = Collections.unmodifiableSet(new HashSet<>());
excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>());
excludedPackageNames = Collections.unmodifiableSet(new HashSet<>());
excludedPackageExemptClasses = Collections.unmodifiableSet(new HashSet<>());
devModeExcludedClasses = Collections.unmodifiableSet(new HashSet<>());
devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>());
devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>());
devModeExcludedPackageExemptClasses = Collections.unmodifiableSet(new HashSet<>());
this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache();
this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache();
@@ -159,7 +174,7 @@ public class OgnlUtil {
protected void setExcludedClasses(String commaDelimitedClasses) {
Set<Class<?>> excludedClasses = new HashSet<>();
excludedClasses.addAll(this.excludedClasses);
excludedClasses.addAll(parseExcludedClasses(commaDelimitedClasses));
excludedClasses.addAll(parseClasses(commaDelimitedClasses));
this.excludedClasses = Collections.unmodifiableSet(excludedClasses);
}
@@ -167,11 +182,11 @@ public class OgnlUtil {
protected void setDevModeExcludedClasses(String commaDelimitedClasses) {
Set<Class<?>> excludedClasses = new HashSet<>();
excludedClasses.addAll(this.devModeExcludedClasses);
excludedClasses.addAll(parseExcludedClasses(commaDelimitedClasses));
excludedClasses.addAll(parseClasses(commaDelimitedClasses));
this.devModeExcludedClasses = Collections.unmodifiableSet(excludedClasses);
}
private Set<Class<?>> parseExcludedClasses(String commaDelimitedClasses) {
private Set<Class<?>> parseClasses(String commaDelimitedClasses) {
Set<String> classNames = TextParseUtil.commaDelimitedStringToSet(commaDelimitedClasses);
Set<Class<?>> classes = new HashSet<>();
@@ -179,7 +194,7 @@ public class OgnlUtil {
try {
classes.add(Class.forName(className));
} catch (ClassNotFoundException e) {
throw new ConfigurationException("Cannot load excluded class: " + className, e);
throw new ConfigurationException("Cannot load class for exclusion/exemption configuration: " + className, e);
}
}
@@ -229,6 +244,22 @@ public class OgnlUtil {
this.devModeExcludedPackageNames = Collections.unmodifiableSet(excludedPackageNames);
}
@Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
public void setExcludedPackageExemptClasses(String commaDelimitedClasses) {
Set<Class<?>> excludedPackageExemptClasses = new HashSet<>();
excludedPackageExemptClasses.addAll(this.excludedPackageExemptClasses);
excludedPackageExemptClasses.addAll(parseClasses(commaDelimitedClasses));
this.excludedPackageExemptClasses = Collections.unmodifiableSet(excludedPackageExemptClasses);
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
public void setDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) {
Set<Class<?>> excludedPackageExemptClasses = new HashSet<>();
excludedPackageExemptClasses.addAll(this.devModeExcludedPackageExemptClasses);
excludedPackageExemptClasses.addAll(parseClasses(commaDelimitedClasses));
this.devModeExcludedPackageExemptClasses = Collections.unmodifiableSet(excludedPackageExemptClasses);
}
private Set<String> parseExcludedPackageNames(String commaDelimitedPackageNames) {
return TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackageNames);
}
@@ -245,6 +276,10 @@ public class OgnlUtil {
return excludedPackageNames;
}
public Set<Class<?>> getExcludedPackageExemptClasses() {
return excludedPackageExemptClasses;
}
@Inject
protected void setContainer(Container container) {
this.container = container;
@@ -847,10 +882,12 @@ public class OgnlUtil {
memberAccess.setExcludedClasses(devModeExcludedClasses);
memberAccess.setExcludedPackageNamePatterns(devModeExcludedPackageNamePatterns);
memberAccess.setExcludedPackageNames(devModeExcludedPackageNames);
memberAccess.setExcludedPackageExemptClasses(devModeExcludedPackageExemptClasses);
} else {
memberAccess.setExcludedClasses(excludedClasses);
memberAccess.setExcludedPackageNamePatterns(excludedPackageNamePatterns);
memberAccess.setExcludedPackageNames(excludedPackageNames);
memberAccess.setExcludedPackageExemptClasses(excludedPackageExemptClasses);
}
return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter);
@@ -29,7 +29,12 @@ import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.MemberAccessValueStack;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import ognl.*;
import ognl.MethodFailedException;
import ognl.NoSuchPropertyException;
import ognl.Ognl;
import ognl.OgnlContext;
import ognl.OgnlException;
import ognl.PropertyAccessor;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -87,6 +92,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS
securityMemberAccess.setExcludedClasses(ognlUtil.getExcludedClasses());
securityMemberAccess.setExcludedPackageNamePatterns(ognlUtil.getExcludedPackageNamePatterns());
securityMemberAccess.setExcludedPackageNames(ognlUtil.getExcludedPackageNames());
securityMemberAccess.setExcludedPackageExemptClasses(ognlUtil.getExcludedPackageExemptClasses());
securityMemberAccess.setDisallowProxyMemberAccess(ognlUtil.isDisallowProxyMemberAccess());
}
@@ -47,6 +47,7 @@ public class SecurityMemberAccess implements MemberAccess {
private Set<Class<?>> excludedClasses = Collections.emptySet();
private Set<Pattern> excludedPackageNamePatterns = Collections.emptySet();
private Set<String> excludedPackageNames = Collections.emptySet();
private Set<Class<?>> excludedPackageExemptClasses = Collections.emptySet();
private boolean disallowProxyMemberAccess;
/**
@@ -117,23 +118,23 @@ public class SecurityMemberAccess implements MemberAccess {
}
final Class<?> memberClass = member.getDeclaringClass();
// target can be null in case of accessing static fields, since OGNL 3.2.8
final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
if (isClassExcluded(memberClass)) {
LOG.warn("Declaring class of member type [{}] is excluded!", member);
return false;
}
// target can be null in case of accessing static fields, since OGNL 3.2.8
final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
if (isPackageExcluded(targetClass.getPackage(), memberClass.getPackage())) {
LOG.warn("Package [{}] of target class [{}] of target [{}] or package [{}] of member [{}] are excluded!", targetClass.getPackage(), targetClass,
target, memberClass.getPackage(), member);
if (isClassExcluded(targetClass)) {
LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
return false;
}
if (isClassExcluded(targetClass)) {
LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
if (!isClassExcludedPackageExempt(targetClass) && !isClassExcludedPackageExempt(memberClass)
&& isPackageExcluded(targetClass.getPackage(), memberClass.getPackage())) {
LOG.warn("Package [{}] of target class [{}] of target [{}] or package [{}] of member [{}] are excluded!",
targetClass.getPackage(), targetClass, target, memberClass.getPackage(), member);
return false;
}
@@ -234,6 +235,15 @@ public class SecurityMemberAccess implements MemberAccess {
return false;
}
protected boolean isClassExcludedPackageExempt(Class<?> clazz) {
for (Class<?> excludedPackageExemptClass : excludedPackageExemptClasses) {
if (clazz.isAssignableFrom(excludedPackageExemptClass)) {
return true;
}
}
return false;
}
protected boolean isAcceptableProperty(String name) {
return name == null || ((!isExcluded(name)) && isAccepted(name));
}
@@ -287,6 +297,10 @@ public class SecurityMemberAccess implements MemberAccess {
this.excludedPackageNames = excludedPackageNames;
}
public void setExcludedPackageExemptClasses(Set<Class<?>> excludedPackageExemptClasses) {
this.excludedPackageExemptClasses = excludedPackageExemptClasses;
}
public void setDisallowProxyMemberAccess(boolean disallowProxyMemberAccess) {
this.disallowProxyMemberAccess = disallowProxyMemberAccess;
}
@@ -414,11 +414,14 @@ public final class StrutsConstants {
public static final String STRUTS_EXCLUDED_CLASSES = "struts.excludedClasses";
public static final String STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS = "struts.excludedPackageNamePatterns";
public static final String STRUTS_EXCLUDED_PACKAGE_NAMES = "struts.excludedPackageNames";
public static final String STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES = "struts.excludedPackageExemptClasses";
/** Comma delimited set of excluded classes and package names which cannot be accessed via expressions in devMode */
public static final String STRUTS_DEV_MODE_EXCLUDED_CLASSES = "struts.devMode.excludedClasses";
public static final String STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS = "struts.devMode.excludedPackageNamePatterns";
public static final String STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES = "struts.devMode.excludedPackageNames";
public static final String STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES = "struts.devMode.excludedPackageExemptClasses";
/** Dedicated services to check if passed string is excluded/accepted */
public static final String STRUTS_EXCLUDED_PATTERNS_CHECKER = "struts.excludedPatterns.checker";
@@ -47,7 +47,7 @@ public class SecurityMemberAccessTest extends TestCase {
SecurityMemberAccess sma = new SecurityMemberAccess(true);
String propertyName = "stringField";
Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
Member member = FooBar.class.getMethod(formGetterName(propertyName));
// when
boolean accessible = sma.isAccessible(context, target, member, propertyName);
@@ -61,7 +61,7 @@ public class SecurityMemberAccessTest extends TestCase {
SecurityMemberAccess sma = new SecurityMemberAccess(true);
String propertyName = "stringField";
Member member = FooBar.class.getDeclaredMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
Member member = FooBar.class.getDeclaredMethod(formGetterName(propertyName));
Set<Class<?>> excluded = new HashSet<>();
excluded.add(FooBar.class);
@@ -138,6 +138,24 @@ public class SecurityMemberAccessTest extends TestCase {
assertTrue("fooLogic() from FooInterface isn't accessible!!!", accessible);
}
public void testMiddleOfInheritanceExclusion2() throws Exception {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(true);
String propertyName = "barLogic";
Member member = BarInterface.class.getMethod(propertyName);
Set<Class<?>> excluded = new HashSet<>();
excluded.add(BarInterface.class);
sma.setExcludedClasses(excluded);
// when
boolean accessible = sma.isAccessible(context, target, member, propertyName);
// then
assertFalse("barLogic() from BarInterface is accessible!!!", accessible);
}
public void testMiddleOfInheritanceExclusion3() throws Exception {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(true);
@@ -145,6 +163,10 @@ public class SecurityMemberAccessTest extends TestCase {
String propertyName = "barLogic";
Member member = BarInterface.class.getMethod(propertyName);
Set<Class<?>> excluded = new HashSet<>();
excluded.add(FooInterface.class);
sma.setExcludedClasses(excluded);
// when
boolean accessible = sma.isAccessible(context, target, member, propertyName);
@@ -179,7 +201,7 @@ public class SecurityMemberAccessTest extends TestCase {
sma.setExcludedPackageNamePatterns(excluded);
String propertyName = "stringField";
Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
Member member = FooBar.class.getMethod(formGetterName(propertyName));
// when
boolean actual = sma.isAccessible(context, target, member, propertyName);
@@ -188,6 +210,28 @@ public class SecurityMemberAccessTest extends TestCase {
assertFalse("stringField is accessible!", actual);
}
public void testPackageExclusionExemption() throws Exception {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(false);
Set<Pattern> excluded = new HashSet<>();
excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*"));
sma.setExcludedPackageNamePatterns(excluded);
Set<Class<?>> allowed = new HashSet<>();
allowed.add(FooBar.class);
sma.setExcludedPackageExemptClasses(allowed);
String propertyName = "stringField";
Member member = FooBar.class.getMethod(formGetterName(propertyName));
// when
boolean actual = sma.isAccessible(context, target, member, propertyName);
// then
assertTrue("stringField isn't accessible!", actual);
}
public void testPackageNameExclusion() throws Exception {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(true);
@@ -197,7 +241,7 @@ public class SecurityMemberAccessTest extends TestCase {
sma.setExcludedPackageNames(excluded);
String propertyName = "stringField";
Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
Member member = FooBar.class.getMethod(formGetterName(propertyName));
// when
boolean actual = sma.isAccessible(context, target, member, propertyName);
@@ -206,6 +250,51 @@ public class SecurityMemberAccessTest extends TestCase {
assertFalse("stringField is accessible!", actual);
}
public void testPackageNameExclusionExemption() throws Exception {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(false);
Set<String> excluded = new HashSet<>();
excluded.add(FooBar.class.getPackage().getName());
sma.setExcludedPackageNames(excluded);
Set<Class<?>> allowed = new HashSet<>();
allowed.add(FooBar.class);
sma.setExcludedPackageExemptClasses(allowed);
String propertyName = "stringField";
Member member = FooBar.class.getMethod(formGetterName(propertyName));
// when
boolean actual = sma.isAccessible(context, target, member, propertyName);
// then
assertTrue("stringField isn't accessible!", actual);
}
public void testPackageNameExclusionExemptionInheritance() throws Exception {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(false);
Set<String> excluded = new HashSet<>();
excluded.add(FooBar.class.getPackage().getName());
sma.setExcludedPackageNames(excluded);
Set<Class<?>> allowed = new HashSet<>();
allowed.add(BarInterface.class);
sma.setExcludedPackageExemptClasses(allowed);
String propertyName = "barLogic";
Member member = BarInterface.class.getMethod(propertyName);
// when
boolean actual = sma.isAccessible(context, target, member, propertyName);
// then
assertTrue("barLogic isn't accessible!", actual);
}
public void testDefaultPackageExclusion() {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(true);
@@ -429,7 +518,7 @@ public class SecurityMemberAccessTest extends TestCase {
sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.,ognl,javax"));
String propertyName = "intField";
Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
Member member = FooBar.class.getMethod(formGetterName(propertyName));
// when
boolean accessible = sma.isAccessible(context, target, member, propertyName);
@@ -473,7 +562,7 @@ public class SecurityMemberAccessTest extends TestCase {
// given
propertyName = "intField";
member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
member = FooBar.class.getMethod(formGetterName(propertyName));
// when
accessible = sma.isAccessible(context, target, member, propertyName);
@@ -482,7 +571,7 @@ public class SecurityMemberAccessTest extends TestCase {
// given
propertyName = "doubleField";
member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
member = FooBar.class.getMethod(formGetterName(propertyName));
// when
accessible = sma.isAccessible(context, target, member, propertyName);
@@ -547,7 +636,6 @@ public class SecurityMemberAccessTest extends TestCase {
// given
SecurityMemberAccess sma = new SecurityMemberAccess(true);
sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang."));
// when
@@ -558,6 +646,9 @@ public class SecurityMemberAccessTest extends TestCase {
assertTrue("package java.lang. is accessible!", actual);
}
private static String formGetterName(String propertyName) {
return "get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1);
}
}
class FooBar implements FooBarInterface {