This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config in repository https://gitbox.apache.org/repos/asf/struts.git
commit 32ceb19246cd905d7b685b084f0625c78abad4a8 Author: Lukasz Lenart <[email protected]> AuthorDate: Fri Aug 14 14:11:32 2026 +0200 WW-5675 feat(ognl): add a container-singleton OGNL security config bean Co-Authored-By: Claude Opus 5 <[email protected]> --- .../struts2/ognl/SecurityMemberAccessConfig.java | 227 +++++++++++++++++++++ .../ognl/SecurityMemberAccessConfigTest.java | 143 +++++++++++++ 2 files changed, 370 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java new file mode 100644 index 000000000..600973830 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java @@ -0,0 +1,227 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.ognl; + +import org.apache.commons.lang3.BooleanUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.inject.Inject; +import org.apache.struts2.inject.Initializable; + +import java.util.Set; +import java.util.regex.Pattern; + +import static java.util.Collections.emptySet; +import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES; +import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES; +import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet; +import static org.apache.struts2.util.ConfigParseUtil.toClassesSet; +import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet; +import static org.apache.struts2.util.ConfigParseUtil.toNewPackageNamesSet; +import static org.apache.struts2.util.ConfigParseUtil.toNewPatternsSet; +import static org.apache.struts2.util.ConfigParseUtil.toPackageNamesSet; +import static org.apache.struts2.util.DebugUtils.logWarningForFirstOccurrence; + +/** + * Holds the parsed OGNL security configuration for one container. + * <p> + * {@link SecurityMemberAccess} is a {@code Scope.PROTOTYPE} bean, constructed once per value stack and + * again for each OGNL context. Parsing the roughly ninety configuration entries on every one of those + * was the dominant cost identified by WW-5667. This bean is a {@code Scope.SINGLETON}, so the parsing + * happens once per container and each {@code SecurityMemberAccess} merely copies immutable references. + * <p> + * Dev-mode is resolved in {@link #init()} rather than in a setter, because the container iterates + * {@code getDeclaredMethods()}, whose order the JDK leaves unspecified. If {@code init()} never runs, + * the normal production exclusions stay in force, which fails closed. + * + * @since Struts 7.4.0 + */ +public class SecurityMemberAccessConfig implements Initializable { + + private static final Logger LOG = LogManager.getLogger(SecurityMemberAccessConfig.class); + + private boolean allowStaticFieldAccess = true; + + private Set<String> excludedClasses = Set.of(Object.class.getName()); + private Set<Pattern> excludedPackageNamePatterns = emptySet(); + private Set<String> excludedPackageNames = emptySet(); + private Set<String> excludedPackageExemptClasses = emptySet(); + + private boolean isDevMode; + private Set<String> devModeExcludedClasses = Set.of(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(); + private Set<String> allowlistPackageNames = emptySet(); + + private boolean disallowProxyObjectAccess = false; + private boolean disallowProxyMemberAccess = false; + private boolean disallowDefaultPackageAccess = false; + + @Override + public void init() { + if (!isDevMode) { + return; + } + logWarningForFirstOccurrence("devMode", LOG, + "DevMode enabled, using DevMode excluded classes and packages for OGNL security enforcement!"); + excludedClasses = devModeExcludedClasses; + excludedPackageNamePatterns = devModeExcludedPackageNamePatterns; + excludedPackageNames = devModeExcludedPackageNames; + excludedPackageExemptClasses = devModeExcludedPackageExemptClasses; + } + + @Inject(value = StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, required = false) + public void useAllowStaticFieldAccess(String allowStaticFieldAccess) { + this.allowStaticFieldAccess = BooleanUtils.toBoolean(allowStaticFieldAccess); + if (!this.allowStaticFieldAccess) { + useExcludedClasses(Class.class.getName()); + } + } + + @Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES, required = false) + public void useExcludedClasses(String commaDelimitedClasses) { + this.excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses); + } + + @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) + public void useExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { + this.excludedPackageNamePatterns = toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns); + } + + @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, required = false) + public void useExcludedPackageNames(String commaDelimitedPackageNames) { + this.excludedPackageNames = toNewPackageNamesSet(excludedPackageNames, commaDelimitedPackageNames); + } + + @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) + public void useExcludedPackageExemptClasses(String commaDelimitedClasses) { + this.excludedPackageExemptClasses = toClassesSet(commaDelimitedClasses); + } + + @Inject(value = StrutsConstants.STRUTS_ALLOWLIST_ENABLE, required = false) + public void useEnforceAllowlistEnabled(String enforceAllowlistEnabled) { + this.enforceAllowlistEnabled = BooleanUtils.toBoolean(enforceAllowlistEnabled); + if (!this.enforceAllowlistEnabled) { + String msg = "OGNL allowlist is disabled!" + + " We strongly recommend keeping it enabled to protect against critical vulnerabilities." + + " Set the configuration `{}=true` to enable it." + + " Please refer to the Struts 7.0 migration guide and security documentation for further information."; + logWarningForFirstOccurrence("allowlist", LOG, msg, StrutsConstants.STRUTS_ALLOWLIST_ENABLE); + } + } + + @Inject(value = STRUTS_ALLOWLIST_CLASSES, required = false) + public void useAllowlistClasses(String commaDelimitedClasses) { + this.allowlistClasses = toClassObjectsSet(commaDelimitedClasses); + } + + @Inject(value = STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false) + public void useAllowlistPackageNames(String commaDelimitedPackageNames) { + this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames); + } + + @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, required = false) + public void useDisallowProxyObjectAccess(String disallowProxyObjectAccess) { + this.disallowProxyObjectAccess = BooleanUtils.toBoolean(disallowProxyObjectAccess); + } + + @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, required = false) + public void useDisallowProxyMemberAccess(String disallowProxyMemberAccess) { + this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess); + } + + @Inject(value = StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, required = false) + public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) { + this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess); + } + + @Inject(StrutsConstants.STRUTS_DEVMODE) + public 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); + } + + public boolean isAllowStaticFieldAccess() { + return allowStaticFieldAccess; + } + + public Set<String> getExcludedClasses() { + return excludedClasses; + } + + public Set<Pattern> getExcludedPackageNamePatterns() { + return excludedPackageNamePatterns; + } + + public Set<String> getExcludedPackageNames() { + return excludedPackageNames; + } + + public Set<String> getExcludedPackageExemptClasses() { + return excludedPackageExemptClasses; + } + + public boolean isEnforceAllowlistEnabled() { + return enforceAllowlistEnabled; + } + + public Set<Class<?>> getAllowlistClasses() { + return allowlistClasses; + } + + public Set<String> getAllowlistPackageNames() { + return allowlistPackageNames; + } + + public boolean isDisallowProxyObjectAccess() { + return disallowProxyObjectAccess; + } + + public boolean isDisallowProxyMemberAccess() { + return disallowProxyMemberAccess; + } + + public boolean isDisallowDefaultPackageAccess() { + return disallowDefaultPackageAccess; + } +} diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java new file mode 100644 index 000000000..e95e99e08 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.ognl; + +import org.junit.Test; + +import java.util.Set; +import java.util.regex.Pattern; + +import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class SecurityMemberAccessConfigTest { + + /** + * Frozen oracle: the accumulation SecurityMemberAccess performed before WW-5675. + * Never delete this, and never make it delegate to production code. + */ + private static Set<String> legacyExcludedClassAccumulation(boolean allowStaticFieldAccess, String configured) { + Set<String> excludedClasses = Set.of(Object.class.getName()); + if (!allowStaticFieldAccess) { + excludedClasses = toNewClassesSet(excludedClasses, Class.class.getName()); + } + return toNewClassesSet(excludedClasses, configured); + } + + private SecurityMemberAccessConfig configWith(boolean devMode, String excludedClasses, String devModeExcludedClasses) { + SecurityMemberAccessConfig config = new SecurityMemberAccessConfig(); + config.useDevMode(String.valueOf(devMode)); + config.useExcludedClasses(excludedClasses); + config.useDevModeExcludedClasses(devModeExcludedClasses); + config.init(); + return config; + } + + @Test + public void excludedClassesMatchLegacyAccumulation() { + SecurityMemberAccessConfig config = new SecurityMemberAccessConfig(); + config.useExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder"); + config.init(); + + assertEquals(legacyExcludedClassAccumulation(true, "java.lang.Runtime,java.lang.ProcessBuilder"), + config.getExcludedClasses()); + } + + @Test + public void disallowingStaticFieldAccessAddsClassToExclusions() { + SecurityMemberAccessConfig config = new SecurityMemberAccessConfig(); + config.useAllowStaticFieldAccess("false"); + config.useExcludedClasses("java.lang.Runtime"); + config.init(); + + assertFalse(config.isAllowStaticFieldAccess()); + assertEquals(legacyExcludedClassAccumulation(false, "java.lang.Runtime"), config.getExcludedClasses()); + } + + /** + * The container iterates getDeclaredMethods(), whose order the JDK leaves unspecified. + * The accumulation must therefore be commutative, as it was before WW-5675. + */ + @Test + public void setterOrderDoesNotAffectExcludedClasses() { + SecurityMemberAccessConfig forward = new SecurityMemberAccessConfig(); + forward.useAllowStaticFieldAccess("false"); + forward.useExcludedClasses("java.lang.Runtime"); + forward.init(); + + SecurityMemberAccessConfig reverse = new SecurityMemberAccessConfig(); + reverse.useExcludedClasses("java.lang.Runtime"); + reverse.useAllowStaticFieldAccess("false"); + reverse.init(); + + assertEquals(forward.getExcludedClasses(), reverse.getExcludedClasses()); + } + + @Test + public void devModeDisabledPublishesNormalExclusions() { + SecurityMemberAccessConfig config = configWith(false, "java.lang.Runtime", "java.lang.ProcessBuilder"); + + assertTrue(config.getExcludedClasses().contains("java.lang.Runtime")); + assertFalse(config.getExcludedClasses().contains("java.lang.ProcessBuilder")); + } + + @Test + public void devModeEnabledPublishesDevModeExclusions() { + SecurityMemberAccessConfig config = configWith(true, "java.lang.Runtime", "java.lang.ProcessBuilder"); + + assertTrue(config.getExcludedClasses().contains("java.lang.ProcessBuilder")); + assertFalse(config.getExcludedClasses().contains("java.lang.Runtime")); + } + + @Test + public void packageNamesAreStrippedOfDots() { + SecurityMemberAccessConfig config = new SecurityMemberAccessConfig(); + config.useExcludedPackageNames("java.io.,.java.net"); + config.init(); + + assertTrue(config.getExcludedPackageNames().contains("java.io")); + assertTrue(config.getExcludedPackageNames().contains("java.net")); + } + + @Test + public void patternsAreCompiledOnce() { + SecurityMemberAccessConfig config = new SecurityMemberAccessConfig(); + config.useExcludedPackageNamePatterns("^java\\.lang\\..*"); + config.init(); + + Set<Pattern> patterns = config.getExcludedPackageNamePatterns(); + assertEquals(1, patterns.size()); + assertTrue(patterns.iterator().next().matcher("java.lang.Runtime").matches()); + } + + /** + * A missing init() must fail closed: production exclusions, never the dev-mode ones. + */ + @Test + public void withoutInitTheNormalExclusionsApply() { + SecurityMemberAccessConfig config = new SecurityMemberAccessConfig(); + config.useDevMode("true"); + config.useExcludedClasses("java.lang.Runtime"); + config.useDevModeExcludedClasses("java.lang.ProcessBuilder"); + + assertTrue(config.getExcludedClasses().contains("java.lang.Runtime")); + } +}
