honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Simplify Log4j shutdown hack test 

The Log4j shutdown hack test tests that a hack we have in place to
workaround a bug in Log4j during shutdown is effective. Log4j can use
JMX to control logging levels, but we disable this through the use of a
system property log4j2.disable.jmx (mainly because there is no need for
this feature, but it also means granting additional security
permissions). The bug in Log4j is that during shutdown, it neglects to
check whether or not its usage of JMX is disable and so it attempts to
unregister management beans, leading to a permissions violation. The
test works by attempting to shutdown Log4j and thus triggering the bad
code path. With the Log4j hack in place, we have introduced jar hell so
that its our code running instead of code from the Log4j jar. Our code
correctly checks that the usage of JMX is disabled and thus does not
trip on a permissions violation. The test was a little complicated in
that it attempted to just grant the minimal permissions needed for Log4j
to do its thing, but this can sometimes lead to other unwanted
permissions violations because the permissions put in place are more
restrictive necessary. This commit simplifies this situation by
rewriting the test to only deny Log4j the sole permission needed to
trigger the bug.

Relates #20476
This commit is contained in:
Jason Tedor 2016-09-14 11:25:28 -04:00 committed by GitHub
parent 4ac3b416f2
commit 875387936b
1 changed files with 8 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java
View File

@ -147,26 +147,24 @@ public class EvilLoggerTests extends ESTestCase {
System.setSecurityManager(new SecurityManager() {
@Override
public void checkPermission(Permission perm) {
if (perm instanceof RuntimePermission && "setSecurityManager".equals(perm.getName())) {
// so we can restore the security manager at the end of the test
return;
}
// just grant all permissions to Log4j, except we deny MBeanServerPermission
// "createMBeanServer" as this will trigger the Log4j bug
if (perm instanceof MBeanServerPermission && "createMBeanServer".equals(perm.getName())) {
// without the hack in place, Log4j will try to get an MBean server which we will deny
// with the hack in place, this permission should never be requested by Log4j
denied.set(true);
throw new AccessControlException("denied");
}
super.checkPermission(perm);
}
@Override
public void checkPropertyAccess(String key) {
// so that Log4j can check if its usage of JMX is disabled or not
if ("log4j2.disable.jmx".equals(key)) {
return;
}
super.checkPropertyAccess(key);
/*
* grant access to all properties; this is so that Log4j can check if its usage
* of JMX is disabled or not by reading log4j2.disable.jmx but there are other
* properties that Log4j will try to read as well and its simpler to just grant
* them all
*/
}
});