diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e6e923e85..8ae27eae9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -66,6 +66,22 @@ See the following links: Or go directly here for an exhaustive list: https://github.com/elastic/logstash/contribute +Sometimes during the development of Logstash core or plugins, configuration parameters for sensitive data such as password, API keys or SSL keyphrases need to be provided to the user. +To protect sensitive information leaking into the logs, follow the best practices below: +- Logstash core and plugin should flag any settings that may contain secrets. If your source changes are in Ruby, apply `:password` validation or wrap the object with `Logstash::Util::Password` object. +- A setting marked as secret should not disclose its value unless retrieved in a non-obvious way. If you are introducing a new class for sensitive object (check `Password.java` and `SecretVariable.java` classes before doing so), make sure to mask the sensitive info by overriding get value (or `toString()` of Java class) default methods. +- Logstash should not log secrets on any log level by default. Make sure you double-check the loggers if they are touching the objects carrying sensitive info. +- Logstash has a dedicated flag, disabled by default, that the raw configuration text be logged for debugging purposes only. Use of `config.debug` will log/display sensitive info in the debug logs, so beware of using it in Produciton. + +As an example, suppose you have `my_auth` config which carries the sensitive key. Defining with `:password` validated protects `my_auth` being leaked in Logstash core logics. +``` +:my_auth => { :validate => :password }, +``` +In the plugin level, make sure to wrap `my_auth` with `Password` object. +``` +::LogStash::Util::Password.new(my_auth) +``` + Using IntelliJ? See a detailed getting started guide [here](https://docs.google.com/document/d/1kqunARvYMrlfTEOgMpYHig0U-ZqCcMJfhvTtGt09iZg/pub). ## Breaking Changes @@ -90,6 +106,8 @@ After a pull request is marked as a "breaking change," it becomes necessary to e Check our [documentation](https://www.elastic.co/guide/en/logstash/current/contributing-to-logstash.html) on how to contribute to plugins or write your own! +Check the _Contributing Documentation and Code Changes_ section to prevent plugin sensitive info from leaking in the debug logs. + ### Logstash Plugin Changelog Guidelines This document provides guidelines on editing a logstash plugin's CHANGELOG file. diff --git a/logstash-core/src/main/java/org/logstash/config/ir/CompiledPipeline.java b/logstash-core/src/main/java/org/logstash/config/ir/CompiledPipeline.java index 6db3afc12..a1a69e64b 100644 --- a/logstash-core/src/main/java/org/logstash/config/ir/CompiledPipeline.java +++ b/logstash-core/src/main/java/org/logstash/config/ir/CompiledPipeline.java @@ -19,8 +19,6 @@ package org.logstash.config.ir; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.jruby.RubyArray; import org.jruby.RubyHash; import org.jruby.runtime.builtin.IRubyObject; @@ -36,7 +34,6 @@ import org.logstash.config.ir.compiler.DatasetCompiler; import org.logstash.config.ir.compiler.EventCondition; import org.logstash.config.ir.compiler.RubyIntegration; import org.logstash.config.ir.compiler.SplitDataset; -import org.logstash.config.ir.expression.*; import org.logstash.config.ir.graph.SeparatorVertex; import org.logstash.config.ir.graph.IfVertex; import org.logstash.config.ir.graph.PluginVertex; @@ -47,9 +44,14 @@ import org.logstash.ext.JrubyEventExtLibrary.RubyEvent; import org.logstash.plugins.ConfigVariableExpander; import org.logstash.secret.store.SecretStore; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -64,8 +66,6 @@ import static org.logstash.config.ir.compiler.Utils.copyNonCancelledEvents; */ public final class CompiledPipeline { - private static final Logger LOGGER = LogManager.getLogger(CompiledPipeline.class); - /** * Compiler for conditional expressions that turn {@link IfVertex} into {@link EventCondition}. */ @@ -433,7 +433,6 @@ public final class CompiledPipeline { flatten(datasets, vertex), filters.get(vertexId) ); - LOGGER.debug("Compiled filter\n {} \n into \n {}", vertex, prepared); plugins.put(vertexId, prepared.instantiate()); } @@ -458,7 +457,6 @@ public final class CompiledPipeline { outputs.get(vertexId), outputs.size() == 1 ); - LOGGER.debug("Compiled output\n {} \n into \n {}", vertex, prepared); plugins.put(vertexId, prepared.instantiate()); } @@ -489,7 +487,6 @@ public final class CompiledPipeline { if (conditional == null) { final ComputeStepSyntaxElement prepared = DatasetCompiler.splitDataset(dependencies, condition); - LOGGER.debug("Compiled conditional\n {} \n into \n {}", vertex, prepared); conditional = prepared.instantiate(); iffs.put(vertexId, conditional); diff --git a/logstash-core/src/main/java/org/logstash/plugins/ConfigurationImpl.java b/logstash-core/src/main/java/org/logstash/plugins/ConfigurationImpl.java index 345cb7a2f..fc3bf5dc8 100644 --- a/logstash-core/src/main/java/org/logstash/plugins/ConfigurationImpl.java +++ b/logstash-core/src/main/java/org/logstash/plugins/ConfigurationImpl.java @@ -24,8 +24,6 @@ import co.elastic.logstash.api.Configuration; import co.elastic.logstash.api.Password; import co.elastic.logstash.api.PluginConfigSpec; import co.elastic.logstash.api.Codec; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.jruby.RubyObject; import org.logstash.config.ir.compiler.RubyIntegration; import org.logstash.plugins.factory.RubyCodecDelegator; @@ -39,7 +37,6 @@ import java.util.Map; * Configuration for Logstash Java plugins. */ public final class ConfigurationImpl implements Configuration { - private static final Logger LOGGER = LogManager.getLogger(ConfigurationImpl.class); private final RubyIntegration.PluginFactory pluginFactory; private final Map rawSettings;