Browse Source

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
Chris Hegarty 3 years ago
parent
commit
4cc56dea06

+ 25 - 0
server/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java

@@ -20,8 +20,10 @@ import java.security.PermissionCollection;
 import java.security.Permissions;
 import java.security.Policy;
 import java.security.ProtectionDomain;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Predicate;
 
 /** custom policy for union of static and dynamic permissions */
@@ -58,6 +60,25 @@ final class ESPolicy extends Policy {
         this.plugins = plugins;
     }
 
+    private static final Predicate<StackTraceElement> JDK_BOOT = f -> f.getClassLoaderName() == null;
+    private static final Predicate<StackTraceElement> ES_BOOTSTRAP = f -> f.getClassName().startsWith("org.elasticsearch.bootstrap");
+    private static final Predicate<StackTraceElement> IS_LOG4J = f -> "org.apache.logging.log4j.util.LoaderUtil".equals(f.getClassName())
+        && "getClassLoaders".equals(f.getMethodName());
+
+    /**
+     *  Returns true if the top of the call stack has:
+     *   1) Only frames belonging from the JDK's boot loader or org.elasticsearch.bootstrap, followed directly by
+     *   2) org.apache.logging.log4j.util.LoaderUtil.getClassLoaders
+     */
+    private static boolean isLoaderUtilGetClassLoaders() {
+        Optional<StackTraceElement> frame = Arrays.stream(Thread.currentThread().getStackTrace())
+            .dropWhile(JDK_BOOT.or(ES_BOOTSTRAP))
+            .limit(1)
+            .findFirst()
+            .filter(IS_LOG4J);
+        return frame.isPresent();
+    }
+
     @Override
     @SuppressForbidden(reason = "fast equals check is desired")
     public boolean implies(ProtectionDomain domain, Permission permission) {
@@ -101,6 +122,10 @@ final class ESPolicy extends Policy {
             return true;
         }
 
+        if (permission instanceof RuntimePermission && "getClassLoader".equals(permission.getName()) && isLoaderUtilGetClassLoaders()) {
+            return true;
+        }
+
         // otherwise defer to template + dynamic file permissions
         return template.implies(domain, permission) || dynamic.implies(permission) || system.implies(domain, permission);
     }

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

@@ -129,8 +129,6 @@ import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.time.Clock;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -249,12 +247,7 @@ public class LoggingAuditTrailTests extends ESTestCase {
         assertThat(properties.getProperty("appender.audit_rolling.layout.type"), is("PatternLayout"));
         final String patternLayoutFormat = properties.getProperty("appender.audit_rolling.layout.pattern");
         assertThat(patternLayoutFormat, is(notNullValue()));
-        patternLayout = AccessController.doPrivileged(
-            (PrivilegedAction<PatternLayout>) () -> PatternLayout.newBuilder()
-                .withPattern(patternLayoutFormat)
-                .withCharset(StandardCharsets.UTF_8)
-                .build()
-        );
+        patternLayout = PatternLayout.newBuilder().withPattern(patternLayoutFormat).withCharset(StandardCharsets.UTF_8).build();
         customAnonymousUsername = randomAlphaOfLength(8);
         reservedRealmEnabled = randomBoolean();
     }

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

@@ -12,8 +12,3 @@ testClusters.matching { it.name == "integTest" }.configureEach {
   setting 'xpack.license.self_generated.type', 'trial'
   plugin ':x-pack:qa:freeze-plugin'
 }
-
-tasks.named("integTest").configure {
-  // Disabled because of log4j Security Manager permission issues in CLI tools
-  systemProperty 'tests.security.manager', 'false'
-}

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

@@ -54,9 +54,6 @@ subprojects {
       "${-> testClusters.integTest.singleNode().getAuditLog()}"
     nonInputProperties.systemProperty 'tests.audit.yesterday.logfile',
       "${-> testClusters.integTest.singleNode().getAuditLog().getParentFile()}/integTest_audit-${new Date().format('yyyy-MM-dd')}-1.json.gz"
-
-    // Disabled because of log4j Security Manager permission issues in CLI tools
-    systemProperty 'tests.security.manager', 'false'
   }
 
   tasks.named("testingConventions").configure { enabled = false }

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

@@ -5,7 +5,3 @@ testClusters.matching { it.name == "integTest" }.configureEach {
   plugin ':x-pack:qa:freeze-plugin'
 }
 
-tasks.named("integTest").configure {
-  // Disabled because of log4j Security Manager permission issues in CLI tools
-  systemProperty 'tests.security.manager', 'false'
-}