Skip to content

Commit 778b077

Browse files
committed
Core: Handle security manager permission for deprecation log rolling
When the deprecation log is written to within scripting support code like ScriptDocValues, it runs under the reduces privileges of scripts. Sometimes this can trigger log rolling, which then causes uncaught security errors, as was handled in elastic#28485. While doing individual deprecation handling within each deprecation scripting location is possible, there are a growing number of deprecations in scripts. This commit wraps the logging call within the deprecation logger use a doPrivileged block, just was we would within individual logging call sites for scripting utilities.
1 parent 722b850 commit 778b077

File tree

2 files changed

+67
-3
lines changed

2 files changed

+67
-3
lines changed

server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.elasticsearch.common.util.concurrent.ThreadContext;
2828

2929
import java.nio.charset.Charset;
30+
import java.security.AccessController;
31+
import java.security.PrivilegedAction;
3032
import java.time.ZoneId;
3133
import java.time.ZonedDateTime;
3234
import java.time.format.DateTimeFormatter;
@@ -318,7 +320,10 @@ void deprecated(final Set<ThreadContext> threadContexts, final String message, f
318320
}
319321

320322
if (log) {
321-
logger.warn(message, params);
323+
AccessController.doPrivileged((PrivilegedAction<Void>)() -> {
324+
logger.warn(message, params);
325+
return null;
326+
});
322327
}
323328
}
324329

server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,46 @@
1919
package org.elasticsearch.common.logging;
2020

2121
import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator;
22-
2322
import org.apache.logging.log4j.LogManager;
23+
import org.apache.logging.log4j.Logger;
24+
import org.apache.logging.log4j.simple.SimpleLoggerContext;
25+
import org.apache.logging.log4j.simple.SimpleLoggerContextFactory;
26+
import org.apache.logging.log4j.spi.ExtendedLogger;
27+
import org.apache.logging.log4j.spi.LoggerContext;
28+
import org.apache.logging.log4j.spi.LoggerContextFactory;
2429
import org.elasticsearch.common.settings.Settings;
2530
import org.elasticsearch.common.util.concurrent.ThreadContext;
2631
import org.elasticsearch.test.ESTestCase;
2732
import org.elasticsearch.test.hamcrest.RegexMatcher;
2833
import org.hamcrest.core.IsSame;
2934

3035
import java.io.IOException;
36+
import java.net.URI;
37+
import java.nio.charset.StandardCharsets;
38+
import java.security.AccessControlContext;
39+
import java.security.AccessController;
40+
import java.security.Permissions;
41+
import java.security.PrivilegedAction;
42+
import java.security.ProtectionDomain;
3143
import java.util.Collections;
3244
import java.util.HashSet;
3345
import java.util.List;
3446
import java.util.Locale;
3547
import java.util.Map;
3648
import java.util.Set;
49+
import java.util.concurrent.atomic.AtomicBoolean;
3750
import java.util.stream.IntStream;
38-
import java.nio.charset.StandardCharsets;
3951

4052
import static org.elasticsearch.common.logging.DeprecationLogger.WARNING_HEADER_PATTERN;
4153
import static org.elasticsearch.test.hamcrest.RegexMatcher.matches;
4254
import static org.hamcrest.Matchers.containsString;
4355
import static org.hamcrest.Matchers.equalTo;
4456
import static org.hamcrest.Matchers.hasSize;
4557
import static org.hamcrest.Matchers.not;
58+
import static org.hamcrest.core.Is.is;
59+
import static org.mockito.Mockito.doAnswer;
60+
import static org.mockito.Mockito.mock;
61+
import static org.mockito.Mockito.when;
4662

4763
/**
4864
* Tests {@link DeprecationLogger}
@@ -303,6 +319,49 @@ public void testWarningHeaderSizeSetting() throws IOException{
303319
}
304320
}
305321

322+
public void testLogPermissions() {
323+
AtomicBoolean supplierCalled = new AtomicBoolean(false);
324+
325+
// mocking the logger used inside DeprecationLogger requires heavy hacking...
326+
Logger parentLogger = mock(Logger.class);
327+
when(parentLogger.getName()).thenReturn("logger");
328+
ExtendedLogger mockLogger = mock(ExtendedLogger.class);
329+
doAnswer(invocationOnMock -> {
330+
supplierCalled.set(true);
331+
createTempDir(); // trigger file permission, like rolling logs would
332+
return null;
333+
}).when(mockLogger).warn("foo", new Object[] {"bar"});
334+
final LoggerContext context = new SimpleLoggerContext() {
335+
@Override
336+
public ExtendedLogger getLogger(String name) {
337+
return mockLogger;
338+
}
339+
};
340+
341+
final LoggerContextFactory originalFactory = LogManager.getFactory();
342+
try {
343+
LogManager.setFactory(new SimpleLoggerContextFactory() {
344+
@Override
345+
public LoggerContext getContext(String fqcn, ClassLoader loader, Object externalContext, boolean currentContext,
346+
URI configLocation, String name) {
347+
return context;
348+
}
349+
});
350+
DeprecationLogger deprecationLogger = new DeprecationLogger(parentLogger);
351+
352+
AccessControlContext noPermissionsAcc = new AccessControlContext(
353+
new ProtectionDomain[]{new ProtectionDomain(null, new Permissions())}
354+
);
355+
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
356+
deprecationLogger.deprecated("foo", "bar");
357+
return null;
358+
}, noPermissionsAcc);
359+
assertThat("supplier called", supplierCalled.get(), is(true));
360+
} finally {
361+
LogManager.setFactory(originalFactory);
362+
}
363+
}
364+
306365
private String range(int lowerInclusive, int upperInclusive) {
307366
return IntStream
308367
.range(lowerInclusive, upperInclusive + 1)

0 commit comments

Comments
 (0)