Block updates to log level for restricted loggers if less specific th…

…an INFO (elastic#105020)

To prevent leaking sensitive information such as credentials and keys in logs, this 
commit prevents configuring some restricted loggers (currently `org.apache.http` 
and `com.amazonaws.request`) at high verbosity unless the NetworkTraceFlag 
(`es.insecure_network_trace_enabled`) is enabled.

docs/reference/setup/logging-config.asciidoc

@@ -150,7 +150,9 @@ update settings API>> to change the related logger's log level. Each logger
 accepts Log4j 2's built-in log levels, from least to most verbose: `OFF`,
 `FATAL`, `ERROR`, `WARN`, `INFO`, `DEBUG`, and `TRACE`. The default log level is
 `INFO`. Messages logged at higher verbosity levels (`DEBUG` and `TRACE`) are
-only intended for expert use.
+only intended for expert use. To prevent leaking sensitive information in logs,
+{es} will reject setting certain loggers to higher verbosity levels unless
+<<http-rest-request-tracer,insecure network trace logging>> is enabled.
 
 [source,console]
 ----
@@ -227,7 +229,7 @@ to `OFF` in `log4j2.properties` :
 ----
 logger.deprecation.level = OFF
 ----
-Alternatively, you can change the logging level dynamically: 
+Alternatively, you can change the logging level dynamically:
 
 [source,console]
 ----

docs/reference/snapshot-restore/repository-s3.asciidoc

@@ -564,7 +564,15 @@ is usually simplest to collect these logs and provide them to the supplier of
 your storage system for further analysis. If the incompatibility is not clear
 from the logs emitted by the storage system, configure {es} to log every
 request it makes to the S3 API by <<configuring-logging-levels,setting the
-logging level>> of the `com.amazonaws.request` logger to `DEBUG`:
+logging level>> of the `com.amazonaws.request` logger to `DEBUG`.
+
+To prevent leaking sensitive information such as credentials and keys in logs,
+{es} rejects configuring this logger at high verbosity unless
+<<http-rest-request-tracer,insecure network trace logging>> is enabled.
+To do so, you must explicitly enable it on each node by setting the system
+property `es.insecure_network_trace_enabled` to `true`.
+
+Once enabled, you can configure the `com.amazonaws.request` logger:
 
 [source,console]
 ----
@@ -585,8 +593,9 @@ https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/java-dg-logging.html
 documentation for further information, including details about other loggers
 that can be used to obtain even more verbose logs. When you have finished
 collecting the logs needed by your supplier, set the logger settings back to
-`null` to return to the default logging configuration. See <<cluster-logger>>
-and <<cluster-update-settings>> for more information.
+`null` to return to the default logging configuration and disable insecure network
+trace logging again. See <<cluster-logger>> and <<cluster-update-settings>> for
+more information.
 
 [[repository-s3-linearizable-registers]]
 ==== Linearizable register implementation

qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerConfigurationTests.java

@@ -26,6 +26,7 @@
 import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.notNullValue;
 
@@ -144,7 +145,13 @@ public void testLoggingLevelsFromSettings() throws IOException, UserException {
         final LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
         final Configuration config = ctx.getConfiguration();
         final Map<String, LoggerConfig> loggerConfigs = config.getLoggers();
-        assertThat(loggerConfigs.size(), equalTo(5));
+
+        if (rootLevel.isMoreSpecificThan(Level.INFO)) {
+            assertThat(loggerConfigs.size(), equalTo(5));
+        } else {
+            // below INFO restricted loggers will be set in addition
+            assertThat(loggerConfigs.size(), greaterThan(5));
+        }
         assertThat(loggerConfigs, hasKey(""));
         assertThat(loggerConfigs.get("").getLevel(), equalTo(rootLevel));
         assertThat(loggerConfigs, hasKey("foo"));

qa/restricted-loggers/build.gradle

@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+apply plugin: 'elasticsearch.standalone-test'
+
+dependencies {
+  testImplementation project(":test:framework")
+}
+
+tasks.named("test").configure {
+  // do not enable TRACE_ENABLED
+  systemProperties.remove('es.insecure_network_trace_enabled')
+}

qa/restricted-loggers/src/test/java/org/elasticsearch/common/logging/LoggersTests.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.common.logging;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.List;
+import java.util.Map;
+
+import static org.elasticsearch.common.logging.Loggers.checkRestrictedLoggers;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+
+public class LoggersTests extends ESTestCase {
+
+    public void testClusterUpdateSettingsRequestValidationForLoggers() {
+        assertThat(Loggers.RESTRICTED_LOGGERS, hasSize(greaterThan(0)));
+
+        ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
+        for (String logger : Loggers.RESTRICTED_LOGGERS) {
+            var validation = request.persistentSettings(Map.of("logger." + logger, org.elasticsearch.logging.Level.DEBUG)).validate();
+            assertNotNull(validation);
+            assertThat(validation.validationErrors(), contains("Level [DEBUG] is not permitted for logger [" + logger + "]"));
+            // INFO is permitted
+            assertNull(request.persistentSettings(Map.of("logger." + logger, org.elasticsearch.logging.Level.INFO)).validate());
+        }
+    }
+
+    public void testCheckRestrictedLoggers() {
+        assertThat(Loggers.RESTRICTED_LOGGERS, hasSize(greaterThan(0)));
+
+        Settings settings;
+        for (String restricted : Loggers.RESTRICTED_LOGGERS) {
+            for (String suffix : List.of("", ".xyz")) {
+                String logger = restricted + suffix;
+                for (Level level : List.of(Level.ALL, Level.TRACE, Level.DEBUG)) {
+                    settings = Settings.builder().put("logger." + logger, level).build();
+                    List<String> errors = checkRestrictedLoggers(settings);
+                    assertThat(errors, contains("Level [" + level + "] is not permitted for logger [" + logger + "]"));
+                }
+                for (Level level : List.of(Level.ERROR, Level.WARN, Level.INFO)) {
+                    settings = Settings.builder().put("logger." + logger, level).build();
+                    assertThat(checkRestrictedLoggers(settings), hasSize(0));
+                }
+
+                settings = Settings.builder().put("logger." + logger, "INVALID").build();
+                assertThat(checkRestrictedLoggers(settings), hasSize(0));
+
+                settings = Settings.builder().put("logger." + logger, (String) null).build();
+                assertThat(checkRestrictedLoggers(settings), hasSize(0));
+            }
+        }
+    }
+
+    public void testSetLevelWithRestrictions() {
+        assertThat(Loggers.RESTRICTED_LOGGERS, hasSize(greaterThan(0)));
+
+        for (String restricted : Loggers.RESTRICTED_LOGGERS) {
+
+            // 'org.apache.http' is an example of a restricted logger,
+            // a restricted component logger would be `org.apache.http.client.HttpClient` for instance,
+            // and the parent logger is `org.apache`.
+            Logger restrictedLogger = LogManager.getLogger(restricted);
+            Logger restrictedComponent = LogManager.getLogger(restricted + ".component");
+            Logger parentLogger = LogManager.getLogger(restricted.substring(0, restricted.lastIndexOf('.')));
+
+            Loggers.setLevel(restrictedLogger, Level.INFO);
+            assertHasINFO(restrictedLogger, restrictedComponent);
+
+            for (Logger log : List.of(restrictedComponent, restrictedLogger)) {
+                // DEBUG is rejected due to restriction
+                Loggers.setLevel(log, Level.DEBUG);
+                assertHasINFO(restrictedComponent, restrictedLogger);
+            }
+
+            // OK for parent `org.apache`, but restriction is enforced for restricted descendants
+            Loggers.setLevel(parentLogger, Level.DEBUG);
+            assertEquals(Level.DEBUG, parentLogger.getLevel());
+            assertHasINFO(restrictedComponent, restrictedLogger);
+
+            // Inheriting DEBUG of parent `org.apache` is rejected
+            Loggers.setLevel(restrictedLogger, (Level) null);
+            assertHasINFO(restrictedComponent, restrictedLogger);
+
+            // DEBUG of root logger isn't propagated to restricted loggers
+            Loggers.setLevel(LogManager.getRootLogger(), Level.DEBUG);
+            assertEquals(Level.DEBUG, LogManager.getRootLogger().getLevel());
+            assertHasINFO(restrictedComponent, restrictedLogger);
+        }
+    }
+
+    private static void assertHasINFO(Logger... loggers) {
+        for (Logger log : loggers) {
+            assertThat("Unexpected log level for [" + log.getName() + "]", log.getLevel(), is(Level.INFO));
+        }
+    }
+}

server/build.gradle

@@ -152,7 +152,7 @@ if (BuildParams.isSnapshotBuild() == false) {
 }
 
 tasks.named("test").configure {
-    systemProperty 'es.insecure_network_trace_enabled', 'true'
+  systemProperty 'es.insecure_network_trace_enabled', 'true'
 }
 
 tasks.named("thirdPartyAudit").configure {

server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.action.support.master.AcknowledgedRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.xcontent.ObjectParser;
 import org.elasticsearch.xcontent.ParseField;
@@ -62,6 +63,13 @@ public ActionRequestValidationException validate() {
         if (transientSettings.isEmpty() && persistentSettings.isEmpty()) {
             validationException = addValidationError("no settings to update", validationException);
         }
+        // for bwc we have to reject logger settings on the REST level instead of using a validator
+        for (String error : Loggers.checkRestrictedLoggers(transientSettings)) {
+            validationException = addValidationError(error, validationException);
+        }
+        for (String error : Loggers.checkRestrictedLoggers(persistentSettings)) {
+            validationException = addValidationError(error, validationException);
+        }
         return validationException;
     }
 

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

@@ -16,11 +16,17 @@
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.Configurator;
 import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.transport.NetworkTraceFlag;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.stream.Stream;
 
@@ -29,8 +35,18 @@
  */
 public class Loggers {
 
+    private Loggers() {};
+
     public static final String SPACE = " ";
 
+    /**
+     * Restricted loggers can't be set to a level less specific than INFO.
+     * For some loggers this might be permitted if {@link NetworkTraceFlag#TRACE_ENABLED} is enabled.
+     */
+    static final List<String> RESTRICTED_LOGGERS = NetworkTraceFlag.TRACE_ENABLED
+        ? Collections.emptyList()
+        : List.of("org.apache.http", "com.amazonaws.request");
+
     public static final Setting<Level> LOG_DEFAULT_LEVEL_SETTING = new Setting<>(
         "logger.level",
         Level.INFO.name(),
@@ -42,6 +58,30 @@ public class Loggers {
         (key) -> new Setting<>(key, Level.INFO.name(), Level::valueOf, Setting.Property.Dynamic, Setting.Property.NodeScope)
     );
 
+    public static List<String> checkRestrictedLoggers(Settings settings) {
+        return checkRestrictedLoggers(settings, RESTRICTED_LOGGERS);
+    }
+
+    // visible for testing only
+    static List<String> checkRestrictedLoggers(Settings settings, List<String> restrictions) {
+        List<String> errors = null;
+        for (String key : settings.keySet()) {
+            if (LOG_LEVEL_SETTING.match(key)) {
+                Level level = Level.toLevel(settings.get(key), null);
+                if (level != null) {
+                    String logger = key.substring("logger.".length());
+                    if (level.intLevel() > Level.INFO.intLevel() && restrictions.stream().anyMatch(r -> isSameOrDescendantOf(logger, r))) {
+                        if (errors == null) {
+                            errors = new ArrayList<>(2);
+                        }
+                        errors.add(Strings.format("Level [%s] is not permitted for logger [%s]", level, logger));
+                    }
+                }
+            }
+        }
+        return errors == null ? Collections.emptyList() : errors;
+    }
+
     public static Logger getLogger(Class<?> clazz, ShardId shardId, String... prefixes) {
         return getLogger(
             clazz,
@@ -100,33 +140,83 @@ private static String formatPrefix(String... prefixes) {
      * level.
      */
     public static void setLevel(Logger logger, String level) {
-        final Level l;
-        if (level == null) {
-            l = null;
-        } else {
-            l = Level.valueOf(level);
-        }
-        setLevel(logger, l);
+        setLevel(logger, level == null ? null : Level.valueOf(level), RESTRICTED_LOGGERS);
     }
 
+    /**
+     * Set the level of the logger. If the new level is null, the logger will inherit it's level from its nearest ancestor with a non-null
+     * level.
+     */
     public static void setLevel(Logger logger, Level level) {
-        if (LogManager.ROOT_LOGGER_NAME.equals(logger.getName()) == false) {
-            Configurator.setLevel(logger.getName(), level);
-        } else {
+        setLevel(logger, level, RESTRICTED_LOGGERS);
+    }
+
+    // visible for testing only
+    static void setLevel(Logger logger, Level level, List<String> restrictions) {
+        // If configuring an ancestor / root, the restriction has to be explicitly set afterward.
+        boolean setRestriction = false;
+
+        if (isRootLogger(logger.getName())) {
+            assert level != null : "Log level is required when configuring the root logger";
             final LoggerContext ctx = LoggerContext.getContext(false);
             final Configuration config = ctx.getConfiguration();
             final LoggerConfig loggerConfig = config.getLoggerConfig(logger.getName());
             loggerConfig.setLevel(level);
             ctx.updateLoggers();
+            setRestriction = level.intLevel() > Level.INFO.intLevel();
+        } else {
+            Level actual = level != null ? level : parentLoggerLevel(logger);
+            if (actual.intLevel() > Level.INFO.intLevel()) {
+                for (String restricted : restrictions) {
+                    if (isSameOrDescendantOf(logger.getName(), restricted)) {
+                        LogManager.getLogger(Loggers.class)
+                            .warn("Level [{}/{}] not permitted for logger [{}], skipping.", level, actual, logger.getName());
+                        return;
+                    }
+                    if (isDescendantOf(restricted, logger.getName())) {
+                        setRestriction = true;
+                    }
+                }
+            }
+            Configurator.setLevel(logger.getName(), level);
         }
 
         // we have to descend the hierarchy
         final LoggerContext ctx = LoggerContext.getContext(false);
         for (final LoggerConfig loggerConfig : ctx.getConfiguration().getLoggers().values()) {
-            if (LogManager.ROOT_LOGGER_NAME.equals(logger.getName()) || loggerConfig.getName().startsWith(logger.getName() + ".")) {
+            if (isDescendantOf(loggerConfig.getName(), logger.getName())) {
                 Configurator.setLevel(loggerConfig.getName(), level);
             }
         }
+
+        if (setRestriction) {
+            // if necessary, after setting the level of an ancestor, enforce restriction again
+            for (String restricted : restrictions) {
+                if (isDescendantOf(restricted, logger.getName())) {
+                    setLevel(LogManager.getLogger(restricted), Level.INFO, Collections.emptyList());
+                }
+            }
+        }
+    }
+
+    private static Level parentLoggerLevel(Logger logger) {
+        int idx = logger.getName().lastIndexOf('.');
+        if (idx != -1) {
+            return LogManager.getLogger(logger.getName().substring(0, idx)).getLevel();
+        }
+        return LogManager.getRootLogger().getLevel();
+    }
+
+    private static boolean isRootLogger(String name) {
+        return LogManager.ROOT_LOGGER_NAME.equals(name);
+    }
+
+    private static boolean isDescendantOf(String candidate, String ancestor) {
+        return isRootLogger(ancestor) || candidate.startsWith(ancestor + ".");
+    }
+
+    private static boolean isSameOrDescendantOf(String candidate, String ancestor) {
+        return candidate.equals(ancestor) || isDescendantOf(candidate, ancestor);
     }
 
     public static void addAppender(final Logger logger, final Appender appender) {