Merge pull request #979 from apache/WW-5439-fix-dev-mode

WW-5439 Move DevMode security configuration to SecurityMemberAccess
This commit is contained in:
Kusal Kithul-Godage
2024-07-08 20:18:40 +10:00
committed by GitHub
3 changed files with 86 additions and 41 deletions
@@ -47,7 +47,6 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
import static com.opensymphony.xwork2.util.ConfigParseUtil.toClassesSet;
@@ -68,9 +67,6 @@ public class OgnlUtil {
private static final Logger LOG = LogManager.getLogger(OgnlUtil.class);
// Flag used to reduce flooding logs with WARNs about using DevMode excluded packages
private final AtomicBoolean warnReported = new AtomicBoolean(false);
private final OgnlCache<String, Object> expressionCache;
private final OgnlCache<Class<?>, BeanInfo> beanInfoCache;
private TypeConverter defaultConverter;
@@ -80,11 +76,6 @@ public class OgnlUtil {
private boolean enableExpressionCache = true;
private boolean enableEvalExpression;
private String devModeExcludedClasses = "";
private String devModeExcludedPackageNamePatterns = "";
private String devModeExcludedPackageNames = "";
private String devModeExcludedPackageExemptClasses = "";
private Container container;
/**
@@ -164,9 +155,12 @@ public class OgnlUtil {
// Must be set directly on SecurityMemberAccess
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false)
/**
* @deprecated since 6.5.0, no replacement.
*/
@Deprecated
protected void setDevModeExcludedClasses(String commaDelimitedClasses) {
this.devModeExcludedClasses = commaDelimitedClasses;
// Must be set directly on SecurityMemberAccess
}
/**
@@ -177,9 +171,12 @@ public class OgnlUtil {
// Must be set directly on SecurityMemberAccess
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
/**
* @deprecated since 6.5.0, no replacement.
*/
@Deprecated
protected void setDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
this.devModeExcludedPackageNamePatterns = commaDelimitedPackagePatterns;
// Must be set directly on SecurityMemberAccess
}
/**
@@ -190,9 +187,12 @@ public class OgnlUtil {
// Must be set directly on SecurityMemberAccess
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false)
/**
* @deprecated since 6.5.0, no replacement.
*/
@Deprecated
protected void setDevModeExcludedPackageNames(String commaDelimitedPackageNames) {
this.devModeExcludedPackageNames = commaDelimitedPackageNames;
// Must be set directly on SecurityMemberAccess
}
/**
@@ -203,9 +203,12 @@ public class OgnlUtil {
// Must be set directly on SecurityMemberAccess
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
/**
* @deprecated since 6.5.0, no replacement.
*/
@Deprecated
public void setDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) {
this.devModeExcludedPackageExemptClasses = commaDelimitedClasses;
// Must be set directly on SecurityMemberAccess
}
/**
@@ -856,6 +859,11 @@ public class OgnlUtil {
return createDefaultContext(root, null);
}
/**
* Note that the allowlist capability is not enforced by the {@link OgnlContext} returned by this method. Currently,
* this context is only leveraged by some public methods on {@link OgnlUtil} which are called by
* {@link OgnlReflectionProvider}.
*/
protected Map<String, Object> createDefaultContext(Object root, ClassResolver resolver) {
if (resolver == null) {
resolver = container.getInstance(RootAccessor.class);
@@ -867,17 +875,6 @@ public class OgnlUtil {
SecurityMemberAccess memberAccess = container.getInstance(SecurityMemberAccess.class);
memberAccess.useEnforceAllowlistEnabled(Boolean.FALSE.toString());
if (devMode) {
if (!warnReported.get()) {
warnReported.set(true);
LOG.warn("Working in devMode, using devMode excluded classes and packages!");
}
memberAccess.useExcludedClasses(devModeExcludedClasses);
memberAccess.useExcludedPackageNamePatterns(devModeExcludedPackageNamePatterns);
memberAccess.useExcludedPackageNames(devModeExcludedPackageNames);
memberAccess.useExcludedPackageExemptClasses(devModeExcludedPackageExemptClasses);
}
return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter);
}
@@ -90,7 +90,12 @@ public class SecurityMemberAccess implements MemberAccess {
private Set<String> excludedPackageNames = emptySet();
private Set<String> excludedPackageExemptClasses = emptySet();
private volatile boolean isDevModeInit;
private boolean isDevMode;
private Set<String> devModeExcludedClasses = unmodifiableSet(new HashSet<>(singletonList(Object.class.getName())));
private Set<Pattern> devModeExcludedPackageNamePatterns = emptySet();
private Set<String> devModeExcludedPackageNames = emptySet();
private Set<String> devModeExcludedPackageExemptClasses = emptySet();
private boolean enforceAllowlistEnabled = false;
private Set<Class<?>> allowlistClasses = emptySet();
@@ -296,6 +301,7 @@ public class SecurityMemberAccess implements MemberAccess {
* @return {@code true} if member access is allowed
*/
protected boolean checkExclusionList(Object target, Member member) {
useDevModeConfiguration();
Class<?> memberClass = member.getDeclaringClass();
if (isClassExcluded(memberClass)) {
LOG.warn("Declaring class of member type [{}] is excluded!", memberClass);
@@ -520,4 +526,36 @@ public class SecurityMemberAccess implements MemberAccess {
protected void useDevMode(String devMode) {
this.isDevMode = BooleanUtils.toBoolean(devMode);
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false)
public void useDevModeExcludedClasses(String commaDelimitedClasses) {
this.devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses);
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
public void useDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
this.devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns);
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false)
public void useDevModeExcludedPackageNames(String commaDelimitedPackageNames) {
this.devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames);
}
@Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
public void useDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) {
this.devModeExcludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
}
private void useDevModeConfiguration() {
if (!isDevMode || isDevModeInit) {
return;
}
isDevModeInit = true;
LOG.warn("Working in devMode, using devMode excluded classes and packages!");
excludedClasses = devModeExcludedClasses;
excludedPackageNamePatterns = devModeExcludedPackageNamePatterns;
excludedPackageNames = devModeExcludedPackageNames;
excludedPackageExemptClasses = devModeExcludedPackageExemptClasses;
}
}
@@ -80,6 +80,11 @@ public class OgnlUtilTest extends XWorkTestCase {
ognlUtil = container.getInstance(OgnlUtil.class);
}
private void resetOgnlUtil(Map<String, ?> properties) {
loadButSet(properties);
ognlUtil = container.getInstance(OgnlUtil.class);
}
public void testCanSetADependentObject() {
String dogName = "fido";
@@ -1152,8 +1157,8 @@ public class OgnlUtilTest extends XWorkTestCase {
Exception expected = null;
try {
ognlUtil.setExcludedClasses(Object.class.getName());
ognlUtil.setValue("class.classLoader.defaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true);
// Object.class is excluded by default
ognlUtil.setValue("class.classLoader", ognlUtil.createDefaultContext(foo), foo, true);
fail();
} catch (OgnlException e) {
expected = e;
@@ -1166,9 +1171,11 @@ public class OgnlUtilTest extends XWorkTestCase {
public void testAllowCallingMethodsOnObjectClassInDevModeTrue() {
Exception expected = null;
try {
ognlUtil.setExcludedClasses(Foo.class.getName());
ognlUtil.setDevModeExcludedClasses("");
ognlUtil.setDevMode(Boolean.TRUE.toString());
Map<String, String> properties = new HashMap<>();
properties.put(StrutsConstants.STRUTS_EXCLUDED_CLASSES, Foo.class.getName());
properties.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, "");
properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.TRUE.toString());
resetOgnlUtil(properties);
Foo foo = new Foo();
String result = (String) ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class);
@@ -1180,14 +1187,18 @@ public class OgnlUtilTest extends XWorkTestCase {
}
public void testExclusionListDevModeOnOff() throws Exception {
ognlUtil.setDevModeExcludedClasses(Foo.class.getName());
Foo foo = new Foo();
ognlUtil.setDevMode(Boolean.TRUE.toString());
Map<String, String> properties = new HashMap<>();
properties.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, Foo.class.getName());
properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.TRUE.toString());
resetOgnlUtil(properties);
OgnlException e = assertThrows(OgnlException.class, () -> ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class));
assertThat(e).hasMessageContaining("com.opensymphony.xwork2.util.Foo.toString");
ognlUtil.setDevMode(Boolean.FALSE.toString());
properties.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE.toString());
resetOgnlUtil(properties);
assertEquals("Foo", (String) ognlUtil.getValue("toString", ognlUtil.createDefaultContext(foo), foo, String.class));
}
@@ -1196,7 +1207,7 @@ public class OgnlUtilTest extends XWorkTestCase {
Exception expected = null;
try {
ognlUtil.setExcludedClasses(Object.class.getName());
// Object.class is excluded by default
ognlUtil.setValue("Class.ClassLoader.DefaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true);
fail();
} catch (OgnlException e) {
@@ -1212,7 +1223,7 @@ public class OgnlUtilTest extends XWorkTestCase {
Exception expected = null;
try {
ognlUtil.setExcludedClasses(Object.class.getName());
// Object.class is excluded by default
ognlUtil.setValue("class['classLoader']['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true);
fail();
} catch (OgnlException e) {
@@ -1243,7 +1254,7 @@ public class OgnlUtilTest extends XWorkTestCase {
Exception expected = null;
try {
ognlUtil.setExcludedClasses(Object.class.getName());
// Object.class is excluded by default
ognlUtil.setValue("class[\"classLoader\"]['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true);
fail();
} catch (OgnlException e) {
@@ -1284,12 +1295,11 @@ public class OgnlUtilTest extends XWorkTestCase {
assertEquals(expected.getMessage(), "Inappropriate OGNL expression: toString()");
}
public void testAvoidCallingSomeClasses() {
public void testStaticMethodBlocked() {
Foo foo = new Foo();
Exception expected = null;
try {
ognlUtil.setExcludedClasses(Runtime.class.getName());
ognlUtil.setValue("@java.lang.Runtime@getRuntime().exec('mate')", ognlUtil.createDefaultContext(foo), foo, true);
fail();
} catch (OgnlException e) {