Skip to content

Commit 4cc56de

Browse files
authored
Tolerate unprivileged log4j getClassLoaders calls (#81840)
Tolerate unprivileged log4j getClassLoaders calls, as if (but not exactly) like they were wrapped in doPriv. This is precautionary step as security permission exceptions have been observed during testing. This change also reverts changes to tests in the log4j 2.15 Upgrade #81709, as they should no longer be needed, given this change and changes in #81851. No explicit new test has been added in this PR, but the code in question is exercised extensively by existing tests, since the policy is set in the test framework. The test reverts mentioned above confirm that the changes are working as expected. This change is a workaround to the issue raised in log4j: https://issues.apache.org/jira/browse/LOG4J2-3236
1 parent 0763439 commit 4cc56de

File tree

5 files changed

+26
-20
lines changed

5 files changed

+26
-20
lines changed

server/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java

+25
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import java.security.Permissions;
2121
import java.security.Policy;
2222
import java.security.ProtectionDomain;
23+
import java.util.Arrays;
2324
import java.util.Collections;
2425
import java.util.Map;
26+
import java.util.Optional;
2527
import java.util.function.Predicate;
2628

2729
/** custom policy for union of static and dynamic permissions */
@@ -58,6 +60,25 @@ final class ESPolicy extends Policy {
5860
this.plugins = plugins;
5961
}
6062

63+
private static final Predicate<StackTraceElement> JDK_BOOT = f -> f.getClassLoaderName() == null;
64+
private static final Predicate<StackTraceElement> ES_BOOTSTRAP = f -> f.getClassName().startsWith("org.elasticsearch.bootstrap");
65+
private static final Predicate<StackTraceElement> IS_LOG4J = f -> "org.apache.logging.log4j.util.LoaderUtil".equals(f.getClassName())
66+
&& "getClassLoaders".equals(f.getMethodName());
67+
68+
/**
69+
* Returns true if the top of the call stack has:
70+
* 1) Only frames belonging from the JDK's boot loader or org.elasticsearch.bootstrap, followed directly by
71+
* 2) org.apache.logging.log4j.util.LoaderUtil.getClassLoaders
72+
*/
73+
private static boolean isLoaderUtilGetClassLoaders() {
74+
Optional<StackTraceElement> frame = Arrays.stream(Thread.currentThread().getStackTrace())
75+
.dropWhile(JDK_BOOT.or(ES_BOOTSTRAP))
76+
.limit(1)
77+
.findFirst()
78+
.filter(IS_LOG4J);
79+
return frame.isPresent();
80+
}
81+
6182
@Override
6283
@SuppressForbidden(reason = "fast equals check is desired")
6384
public boolean implies(ProtectionDomain domain, Permission permission) {
@@ -101,6 +122,10 @@ public boolean implies(ProtectionDomain domain, Permission permission) {
101122
return true;
102123
}
103124

125+
if (permission instanceof RuntimePermission && "getClassLoader".equals(permission.getName()) && isLoaderUtilGetClassLoaders()) {
126+
return true;
127+
}
128+
104129
// otherwise defer to template + dynamic file permissions
105130
return template.implies(domain, permission) || dynamic.implies(permission) || system.implies(domain, permission);
106131
}

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@
129129
import java.nio.charset.StandardCharsets;
130130
import java.nio.file.Files;
131131
import java.nio.file.Path;
132-
import java.security.AccessController;
133-
import java.security.PrivilegedAction;
134132
import java.time.Clock;
135133
import java.util.ArrayList;
136134
import java.util.Arrays;
@@ -249,12 +247,7 @@ public static void lookupPatternLayout() throws Exception {
249247
assertThat(properties.getProperty("appender.audit_rolling.layout.type"), is("PatternLayout"));
250248
final String patternLayoutFormat = properties.getProperty("appender.audit_rolling.layout.pattern");
251249
assertThat(patternLayoutFormat, is(notNullValue()));
252-
patternLayout = AccessController.doPrivileged(
253-
(PrivilegedAction<PatternLayout>) () -> PatternLayout.newBuilder()
254-
.withPattern(patternLayoutFormat)
255-
.withCharset(StandardCharsets.UTF_8)
256-
.build()
257-
);
250+
patternLayout = PatternLayout.newBuilder().withPattern(patternLayoutFormat).withCharset(StandardCharsets.UTF_8).build();
258251
customAnonymousUsername = randomAlphaOfLength(8);
259252
reservedRealmEnabled = randomBoolean();
260253
}

x-pack/plugin/sql/qa/server/multi-node/build.gradle

-5
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,3 @@ testClusters.matching { it.name == "integTest" }.configureEach {
1212
setting 'xpack.license.self_generated.type', 'trial'
1313
plugin ':x-pack:qa:freeze-plugin'
1414
}
15-
16-
tasks.named("integTest").configure {
17-
// Disabled because of log4j Security Manager permission issues in CLI tools
18-
systemProperty 'tests.security.manager', 'false'
19-
}

x-pack/plugin/sql/qa/server/security/build.gradle

-3
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,6 @@ subprojects {
5454
"${-> testClusters.integTest.singleNode().getAuditLog()}"
5555
nonInputProperties.systemProperty 'tests.audit.yesterday.logfile',
5656
"${-> testClusters.integTest.singleNode().getAuditLog().getParentFile()}/integTest_audit-${new Date().format('yyyy-MM-dd')}-1.json.gz"
57-
58-
// Disabled because of log4j Security Manager permission issues in CLI tools
59-
systemProperty 'tests.security.manager', 'false'
6057
}
6158

6259
tasks.named("testingConventions").configure { enabled = false }

x-pack/plugin/sql/qa/server/single-node/build.gradle

-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,3 @@ testClusters.matching { it.name == "integTest" }.configureEach {
55
plugin ':x-pack:qa:freeze-plugin'
66
}
77

8-
tasks.named("integTest").configure {
9-
// Disabled because of log4j Security Manager permission issues in CLI tools
10-
systemProperty 'tests.security.manager', 'false'
11-
}

0 commit comments

Comments
 (0)