From 45d70bb88f3dda2c0fc7e93383f0c114477dd347 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 17 Feb 2017 16:12:27 -0500 Subject: [PATCH] Handle security exception on getting class loader When logging a stack trace, Log4j attempts to get the class loader for the caller class. If it does not have permissions to get this class loader, a security exception will be thrown. Log4j does not handle this exception and this exception unwinds to the caller. This causes the original exception to be lost, and can even unwind the JVM. This commit fixes this by treating the class loader as unavailable in this case. This preserves the original exception. --- .../logging/log4j/core/impl/ThrowableProxy.java | 8 +- .../log4j/core/impl/ThrowableProxyTest.java | 95 +++++++++++++--------- 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java index 4dd19682c..4cff083e5 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java @@ -557,7 +557,7 @@ public class ThrowableProxy implements Serializable { private Class loadClass(final String className) { try { - return Loader.loadClass(className,this.getClass().getClassLoader()); + return Loader.loadClass(className, this.getClass().getClassLoader()); } catch (final ClassNotFoundException | NoClassDefFoundError | SecurityException e) { return null; } @@ -602,7 +602,11 @@ public class ThrowableProxy implements Serializable { version = ver; } } - lastLoader = callerClass.getClassLoader(); + try { + lastLoader = callerClass.getClassLoader(); + } catch (final SecurityException e) { + lastLoader = null; + } } return new CacheEntry(new ExtendedClassInfo(exact, location, version), lastLoader); } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java index 9c0b86131..87b975fe0 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ThrowableProxyTest.java @@ -29,16 +29,17 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.net.BindException; import java.net.InetSocketAddress; -import java.net.SocketPermission; import java.nio.channels.ServerSocketChannel; -import java.security.CodeSource; -import java.security.PermissionCollection; -import java.security.Permissions; -import java.security.Policy; +import java.security.Permission; +import java.security.SecureRandom; import java.util.HashMap; import java.util.Map; import java.util.Stack; +import javax.crypto.AEADBadTagException; +import javax.crypto.Cipher; +import javax.crypto.KeyGenerator; +import javax.crypto.spec.GCMParameterSpec; import javax.xml.bind.DatatypeConverter; import org.apache.logging.log4j.LogManager; @@ -133,41 +134,20 @@ public class ThrowableProxyTest { @Test public void testLogStackTraceWithClassThatWillCauseSecurityException() throws IOException { - class SimplePolicy extends Policy { - - private final Permissions permissions; - - public SimplePolicy(final Permissions permissions) { - this.permissions = permissions; - } - - @Override - public PermissionCollection getPermissions(final CodeSource codesource) { - return permissions; - } - - } - final SecurityManager sm = System.getSecurityManager(); try { - final Permissions permissions = new Permissions(); - - // you know, for binding - permissions.add(new SocketPermission("localhost:9300", "listen,resolve")); - - /** - * the JUnit test runner uses reflection to invoke the test; while leaving this - * permission out would display the same issue, it's clearer to grant this - * permission and show the real issue that would arise - */ - // TODO: other JDKs might need a different permission here - permissions.add(new RuntimePermission("accessClassInPackage.sun.reflect")); - - // for restoring the security manager after test execution - permissions.add(new RuntimePermission("setSecurityManager")); - - Policy.setPolicy(new SimplePolicy(permissions)); - System.setSecurityManager(new SecurityManager()); + System.setSecurityManager( + new SecurityManager() { + @Override + public void checkPermission(Permission perm) { + if (perm instanceof RuntimePermission) { + // deny access to the class to trigger the security exception + if ("accessClassInPackage.sun.nio.ch".equals(perm.getName())) { + throw new SecurityException(perm.toString()); + } + } + } + }); ServerSocketChannel.open().socket().bind(new InetSocketAddress("localhost", 9300)); ServerSocketChannel.open().socket().bind(new InetSocketAddress("localhost", 9300)); fail("expected a java.net.BindException"); @@ -179,6 +159,45 @@ public class ThrowableProxyTest { } } + @Test + public void testLogStackTraceWithClassLoaderThatWithCauseSecurityException() throws Exception { + final SecurityManager sm = System.getSecurityManager(); + try { + System.setSecurityManager( + new SecurityManager() { + @Override + public void checkPermission(Permission perm) { + if (perm instanceof RuntimePermission) { + // deny access to the classloader to trigger the security exception + if ("getClassLoader".equals(perm.getName())) { + throw new SecurityException(perm.toString()); + } + } + } + }); + final String algorithm = "AES/GCM/NoPadding"; + final Cipher ec = Cipher.getInstance(algorithm); + final byte[] bytes = new byte[12]; // initialization vector + final SecureRandom secureRandom = new SecureRandom(); + secureRandom.nextBytes(bytes); + final KeyGenerator generator = KeyGenerator.getInstance("AES"); + generator.init(128); + final GCMParameterSpec algorithmParameterSpec = new GCMParameterSpec(128, bytes); + ec.init(Cipher.ENCRYPT_MODE, generator.generateKey(), algorithmParameterSpec, secureRandom); + final byte[] raw = new byte[0]; + final byte[] encrypted = ec.doFinal(raw); + final Cipher dc = Cipher.getInstance(algorithm); + dc.init(Cipher.DECRYPT_MODE, generator.generateKey(), algorithmParameterSpec, secureRandom); + dc.doFinal(encrypted); + fail("expected a javax.crypto.AEADBadTagException"); + } catch (final AEADBadTagException e) { + new ThrowableProxy(e); + } finally { + // restore the existing security manager + System.setSecurityManager(sm); + } + } + // DO NOT REMOVE THIS COMMENT: // UNCOMMENT WHEN GENERATING SERIALIZED THROWABLEPROXY FOR #testSerializationWithUnknownThrowable // public static class DeletedException extends Exception { -- 2.11.1