Skip to content

Commit

Permalink
Logstash is logging unnecessary pipeline configuration. (#14779)
Browse files Browse the repository at this point in the history
* Remove unnecessary IR graph loggings.

* Add the guides for protecting sensitive info leaking in the logs.

  `CONTRIBUTING.md` now contains the guide to prevent sensitive info leaking in the debug logs.

  Fixes: #14778
  Pull-request: #14779

* Add sample source codes to the `CONTRIBUTING.md`.

* Apply suggestions from code review

Contribution guide for using sensitive data in Logstash improved.

Co-authored-by: João Duarte <jsvd@users.noreply.github.com>

Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
  • Loading branch information
mashhurs and jsvd authored Dec 7, 2022
1 parent 720996f commit 43d1c8c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 14 deletions.
18 changes: 18 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,7 +34,6 @@
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;
Expand All @@ -47,9 +44,14 @@
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;

Expand All @@ -64,8 +66,6 @@
*/
public final class CompiledPipeline {

private static final Logger LOGGER = LogManager.getLogger(CompiledPipeline.class);

/**
* Compiler for conditional expressions that turn {@link IfVertex} into {@link EventCondition}.
*/
Expand Down Expand Up @@ -433,7 +433,6 @@ private Dataset filterDataset(final Vertex vertex, final Collection<Dataset> dat
flatten(datasets, vertex),
filters.get(vertexId)
);
LOGGER.debug("Compiled filter\n {} \n into \n {}", vertex, prepared);

plugins.put(vertexId, prepared.instantiate());
}
Expand All @@ -458,7 +457,6 @@ private Dataset outputDataset(final Vertex vertex, final Collection<Dataset> dat
outputs.get(vertexId),
outputs.size() == 1
);
LOGGER.debug("Compiled output\n {} \n into \n {}", vertex, prepared);

plugins.put(vertexId, prepared.instantiate());
}
Expand Down Expand Up @@ -489,7 +487,6 @@ private SplitDataset split(
if (conditional == null) {
final ComputeStepSyntaxElement<SplitDataset> prepared =
DatasetCompiler.splitDataset(dependencies, condition);
LOGGER.debug("Compiled conditional\n {} \n into \n {}", vertex, prepared);

conditional = prepared.instantiate();
iffs.put(vertexId, conditional);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
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;
Expand All @@ -39,7 +37,6 @@
* 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<String, Object> rawSettings;
Expand Down

0 comments on commit 43d1c8c

Please sign in to comment.