Skip to content

Commit

Permalink
Mask password discovered via module autodiscover hint (elastic#15616)
Browse files Browse the repository at this point in the history
* Mask password is string representation of config

* Rename method

* Adding unit test

* Use const for module config password setting name

* Using common.DebugString

* Simplifying

* Removing now-invalid unit test

* Removing now-unnecessary const

* Refactoring: moving debug-related var and func to common file

* Refactoring: rename from black list to mask list

* Implement fmt.Formatter for common.MapStr

* Reintroduce debug statement

* Make MarshalLogObject always filter MapStr object for logging purposes

* Refactoring: renaming to be bit more generic

* Forgot to add license header to new file

* Fixing verb syntax
  • Loading branch information
ycombinator committed Jan 17, 2020
1 parent 37a276b commit 104756c
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 43 deletions.
40 changes: 2 additions & 38 deletions libbeat/common/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,6 @@ const (
selectorConfigWithPassword = "config-with-passwords"
)

var debugBlacklist = MakeStringSet(
"password",
"passphrase",
"key_passphrase",
"pass",
"proxy_url",
"url",
"urls",
"host",
"hosts",
)

// make hasSelector and configDebugf available for unit testing
var hasSelector = logp.HasSelector
var configDebugf = logp.Debug
Expand Down Expand Up @@ -369,7 +357,7 @@ func DebugString(c *Config, filterPrivate bool) string {
return fmt.Sprintf("<config error> %v", err)
}
if filterPrivate {
filterDebugObject(content)
applyLoggingMask(content)
}
j, _ := json.MarshalIndent(content, "", " ")
bufs = append(bufs, string(j))
Expand All @@ -380,7 +368,7 @@ func DebugString(c *Config, filterPrivate bool) string {
return fmt.Sprintf("<config error> %v", err)
}
if filterPrivate {
filterDebugObject(content)
applyLoggingMask(content)
}
j, _ := json.MarshalIndent(content, "", " ")
bufs = append(bufs, string(j))
Expand All @@ -392,30 +380,6 @@ func DebugString(c *Config, filterPrivate bool) string {
return strings.Join(bufs, "\n")
}

func filterDebugObject(c interface{}) {
switch cfg := c.(type) {
case map[string]interface{}:
for k, v := range cfg {
if debugBlacklist.Has(k) {
if arr, ok := v.([]interface{}); ok {
for i := range arr {
arr[i] = "xxxxx"
}
} else {
cfg[k] = "xxxxx"
}
} else {
filterDebugObject(v)
}
}

case []interface{}:
for _, elem := range cfg {
filterDebugObject(elem)
}
}
}

// OwnerHasExclusiveWritePerms asserts that the current user or root is the
// owner of the config file and that the config file is (at most) writable by
// the owner or root (e.g. group and other cannot have write access).
Expand Down
54 changes: 54 additions & 0 deletions libbeat/common/logging.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package common

var maskList = MakeStringSet(
"password",
"passphrase",
"key_passphrase",
"pass",
"proxy_url",
"url",
"urls",
"host",
"hosts",
)

func applyLoggingMask(c interface{}) {
switch cfg := c.(type) {
case map[string]interface{}:
for k, v := range cfg {
if maskList.Has(k) {
if arr, ok := v.([]interface{}); ok {
for i := range arr {
arr[i] = "xxxxx"
}
} else {
cfg[k] = "xxxxx"
}
} else {
applyLoggingMask(v)
}
}

case []interface{}:
for _, elem := range cfg {
applyLoggingMask(elem)
}
}
}
23 changes: 20 additions & 3 deletions libbeat/common/mapstr.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package common
import (
"encoding/json"
"fmt"
"io"
"sort"
"strings"

Expand Down Expand Up @@ -218,13 +219,16 @@ func (m MapStr) MarshalLogObject(enc zapcore.ObjectEncoder) error {
return nil
}

keys := make([]string, 0, len(m))
for k := range m {
debugM := m.Clone()
applyLoggingMask(map[string]interface{}(debugM))

keys := make([]string, 0, len(debugM))
for k := range debugM {
keys = append(keys, k)
}
sort.Strings(keys)
for _, k := range keys {
v := m[k]
v := debugM[k]
if inner, ok := tryToMapStr(v); ok {
enc.AddObject(k, inner)
continue
Expand All @@ -234,6 +238,19 @@ func (m MapStr) MarshalLogObject(enc zapcore.ObjectEncoder) error {
return nil
}

// Format implements fmt.Formatter
func (m MapStr) Format(f fmt.State, c rune) {
if f.Flag('+') || f.Flag('#') {
io.WriteString(f, m.String())
return
}

debugM := m.Clone()
applyLoggingMask(map[string]interface{}(debugM))

io.WriteString(f, debugM.String())
}

// Flatten flattens the given MapStr and returns a flat MapStr.
//
// Example:
Expand Down
23 changes: 23 additions & 0 deletions libbeat/common/mapstr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,3 +1002,26 @@ func BenchmarkWalkMap(b *testing.B) {
}
})
}

func TestFormat(t *testing.T) {
input := MapStr{
"foo": "bar",
"password": "SUPER_SECURE",
}

tests := map[string]string{
"%v": `{"foo":"bar","password":"xxxxx"}`,
"%+v": `{"foo":"bar","password":"SUPER_SECURE"}`,
"%#v": `{"foo":"bar","password":"SUPER_SECURE"}`,
"%s": `{"foo":"bar","password":"xxxxx"}`,
"%+s": `{"foo":"bar","password":"SUPER_SECURE"}`,
"%#s": `{"foo":"bar","password":"SUPER_SECURE"}`,
}

for verb, expected := range tests {
t.Run(verb, func(t *testing.T) {
actual := fmt.Sprintf(verb, input)
assert.Equal(t, expected, actual)
})
}
}
4 changes: 2 additions & 2 deletions metricbeat/autodiscover/builder/hints/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,14 @@ func (m *metricHints) CreateConfig(event bus.Event) []*common.Config {
moduleConfig["password"] = password
}

logp.Debug("hints.builder", "generated config: %v", moduleConfig.String())
logp.Debug("hints.builder", "generated config: %v", moduleConfig)

// Create config object
cfg, err := common.NewConfigFrom(moduleConfig)
if err != nil {
logp.Debug("hints.builder", "config merge failed with error: %v", err)
}
logp.Debug("hints.builder", "generated config: +%v", *cfg)
logp.Debug("hints.builder", "generated config: %+v", common.DebugString(cfg, true))
config = append(config, cfg)

// Apply information in event to the template to generate the final config
Expand Down

0 comments on commit 104756c

Please sign in to comment.