This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch 7.0.x/WW-5454-log-security-warnings
in repository https://gitbox.apache.org/repos/asf/struts.git
commit f41fe80d803590699e1609d8dd8d25c7f3e7ee3e
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Sat Aug 3 20:24:29 2024 +1000
WW-5454 Log warnings on startup for disabled critical security features
---
.../opensymphony/xwork2/ognl/SecurityMemberAccess.java | 15 ++++++++++-----
.../java/com/opensymphony/xwork2/util/DebugUtils.java | 13 +++++++++++++
.../interceptor/parameter/ParametersInterceptor.java | 8 ++++++++
3 files changed, 31 insertions(+), 5 deletions(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
index 8223d86ad..4cd62292d 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -46,6 +46,7 @@ import static
com.opensymphony.xwork2.util.ConfigParseUtil.toNewClassesSet;
import static
com.opensymphony.xwork2.util.ConfigParseUtil.toNewPackageNamesSet;
import static com.opensymphony.xwork2.util.ConfigParseUtil.toNewPatternsSet;
import static com.opensymphony.xwork2.util.ConfigParseUtil.toPackageNamesSet;
+import static
com.opensymphony.xwork2.util.DebugUtils.logWarningForFirstOccurrence;
import static java.text.MessageFormat.format;
import static java.util.Collections.emptySet;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
@@ -87,7 +88,6 @@ public class SecurityMemberAccess implements MemberAccess {
private Set<String> excludedPackageNames = emptySet();
private Set<String> excludedPackageExemptClasses = emptySet();
- private static volatile boolean isDevModeLogged = false;
private volatile boolean isDevModeInit;
private boolean isDevMode;
private Set<String> devModeExcludedClasses =
Set.of(Object.class.getName());
@@ -459,6 +459,13 @@ public class SecurityMemberAccess implements MemberAccess {
@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)
@@ -515,11 +522,9 @@ public class SecurityMemberAccess implements MemberAccess {
if (!isDevMode || isDevModeInit) {
return;
}
+ logWarningForFirstOccurrence("devMode", LOG,
+ "DevMode enabled, using DevMode excluded classes and packages
for OGNL security enforcement!");
isDevModeInit = true;
- if (!isDevModeLogged) {
- LOG.warn("Working in devMode, using devMode excluded classes and
packages!");
- isDevModeLogged = true;
- }
excludedClasses = devModeExcludedClasses;
excludedPackageNamePatterns = devModeExcludedPackageNamePatterns;
excludedPackageNames = devModeExcludedPackageNames;
diff --git a/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java
b/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java
index 13dc78b38..d0f35af05 100644
--- a/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java
+++ b/core/src/main/java/com/opensymphony/xwork2/util/DebugUtils.java
@@ -22,11 +22,16 @@ import com.opensymphony.xwork2.TextProvider;
import com.opensymphony.xwork2.interceptor.ValidationAware;
import org.apache.logging.log4j.Logger;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
/**
* @since 6.5.0
*/
public final class DebugUtils {
+ private static final Set<String> IS_LOGGED = ConcurrentHashMap.newKeySet();
+
public static void notifyDeveloperOfError(Logger log, Object action,
String message) {
if (action instanceof TextProvider tp) {
message = tp.getText("devmode.notification", "Developer
Notification:\n{0}", new String[]{message});
@@ -37,4 +42,12 @@ public final class DebugUtils {
}
}
+ /**
+ * @since 7.0
+ */
+ public static void logWarningForFirstOccurrence(String key, Logger log,
String msg, Object... args) {
+ if (IS_LOGGED.add(key)) {
+ log.warn(msg, args);
+ }
+ }
}
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
index c294361af..353f3cb82 100644
---
a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
+++
b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
@@ -64,6 +64,7 @@ import java.util.regex.Pattern;
import static
com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS;
import static
com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS_STR;
+import static
com.opensymphony.xwork2.util.DebugUtils.logWarningForFirstOccurrence;
import static com.opensymphony.xwork2.util.DebugUtils.notifyDeveloperOfError;
import static java.lang.String.format;
import static java.util.Collections.unmodifiableSet;
@@ -115,6 +116,13 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
@Inject(value = StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS,
required = false)
public void setRequireAnnotations(String requireAnnotations) {
this.requireAnnotations = BooleanUtils.toBoolean(requireAnnotations);
+ if (!this.requireAnnotations) {
+ String msg = "@StrutsParameter annotation requirement 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("strutsParameter", LOG, msg,
StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS);
+ }
}
/**
