Skip to content

Commit

Permalink
Revert the validation on dotted key in the keystore and add a cyclic …
Browse files Browse the repository at this point in the history
…reference test case (elastic#6098)

* Revert "Refuse to store dotted keys to prevent cyclic reference in our configuration. (elastic#6077)"

This reverts commit 0cdcf4f.

* Keystore adding a system env test

Adding a specific system env test to target a cyclic reference in the
configuration.

* Update go-ucfg

This update allow us to support top level key reference when the top
level doesn't already exist in the YAML document and it allow us to use
cyclic reference in a yaml document if the key exist in other resolvers.

Ref: elastic/go-ucfg#97

* Update changelog
  • Loading branch information
ph authored and Steffen Siering committed Jan 23, 2018
1 parent 6d58741 commit 34b6a46
Show file tree
Hide file tree
Showing 16 changed files with 233 additions and 44 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
- Update the command line library cobra and add support for zsh completion {pull}5761[5761]
- The log format may differ due to logging library changes. {pull}5901[5901]
- Adding a local keystore to allow user to obfuscate password {pull}5687[5687]
- Update go-ucfg library to support top level key reference and cyclic key reference for the
keystore {pull}6098[6098]

*Auditbeat*

Expand Down
3 changes: 2 additions & 1 deletion NOTICE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,8 @@ Apache License 2.0

--------------------------------------------------------------------
Dependency: github.com/elastic/go-ucfg
Revision: ec8488a52542c0c51e42e8ea204dcaff400bc644
Version: v0.5.0
Revision: bda09c7b9afc6263a3fd592fcd7063d03f6acf0f
License type (autodetected): Apache-2.0
./vendor/github.com/elastic/go-ucfg/LICENSE:
--------------------------------------------------------------------
Expand Down
12 changes: 0 additions & 12 deletions libbeat/keystore/file_keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"os"
"path/filepath"
"runtime"
"strings"
"sync"

"golang.org/x/crypto/pbkdf2"
Expand Down Expand Up @@ -89,9 +88,6 @@ func (k *FileKeystore) Retrieve(key string) (*SecureString, error) {

// Store add the key pair to the secret store and mark the store as dirty.
func (k *FileKeystore) Store(key string, value []byte) error {
if err := k.validateKey(key); err != nil {
return err
}
k.Lock()
defer k.Unlock()

Expand Down Expand Up @@ -386,11 +382,3 @@ func (k *FileKeystore) checkPermissions(f string) error {
func (k *FileKeystore) hashPassword(password, salt []byte) []byte {
return pbkdf2.Key(password, salt, iterationsCount, keyLength, sha512.New)
}

func (k *FileKeystore) validateKey(key string) error {
if strings.IndexAny(key, ".") != -1 {
return fmt.Errorf("invalid key format. '.' in keys are not supported yet. key: %s", key)
}

return nil
}
20 changes: 4 additions & 16 deletions libbeat/keystore/file_keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,9 @@ import (
"github.com/elastic/beats/libbeat/common"
)

var keyValue = "output_elasticsearch_password"
var keyValue = "output.elasticsearch.password"
var secretValue = []byte("secret")

func TestInvalidKey(t *testing.T) {
path := GetTemporaryKeystoreFile()
defer os.Remove(path)

keystore, err := NewFileKeystore(path)
assert.NoError(t, err)
err = keystore.Store("output.elasticsearch.password", secretValue)
if assert.Error(t, err) {
assert.Equal(t, err.Error(), "invalid key format. '.' in keys are not supported yet. key: output.elasticsearch.password")
}
}

func TestCanCreateAKeyStore(t *testing.T) {
path := GetTemporaryKeystoreFile()
defer os.Remove(path)
Expand Down Expand Up @@ -197,18 +185,18 @@ func TestGetConfig(t *testing.T) {
keystore := CreateAnExistingKeystore(path)

// Add a bit more data of different type
keystore.Store("super_nested", []byte("hello"))
keystore.Store("super.nested", []byte("hello"))
keystore.Save()

cfg, err := keystore.GetConfig()
assert.NotNil(t, cfg)
assert.NoError(t, err)

secret, err := cfg.String("output_elasticsearch_password", 0)
secret, err := cfg.String("output.elasticsearch.password", 0)
assert.NoError(t, err)
assert.Equal(t, secret, "secret")

port, err := cfg.String("super_nested", 0)
port, err := cfg.String("super.nested", 0)
assert.Equal(t, port, "hello")
}

Expand Down
2 changes: 1 addition & 1 deletion libbeat/keystore/keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestResolverWhenTheKeyExist(t *testing.T) {
keystore := CreateAnExistingKeystore(path)

resolver := ResolverWrap(keystore)
v, err := resolver("output_elasticsearch_password")
v, err := resolver("output.elasticsearch.password")
assert.NoError(t, err)
assert.Equal(t, v, "secret")
}
21 changes: 21 additions & 0 deletions libbeat/tests/system/test_keystore.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,24 @@ def test_keystore_with_key_not_present(self):
assert self.log_contains(
"missing field accessing 'output.elasticsearch.hosts'")
assert exit_code == 1

def test_keystore_with_nested_key(self):
"""
test that we support nested key
"""

key = "output.elasticsearch.hosts.0"
secret = "myeleasticsearchsecrethost"

self.render_config_template(keystore_path=self.keystore_path, elasticsearch={
'hosts': "${%s}" % key
})

exit_code = self.run_beat(extra_args=["keystore", "create"])
assert exit_code == 0

self.add_secret(key, secret)
proc = self.start_beat()
self.wait_until(lambda: self.log_contains("no such host"))
assert self.log_contains(secret)
proc.check_kill_and_wait()
11 changes: 9 additions & 2 deletions vendor/github.com/elastic/go-ucfg/CHANGELOG.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 14 additions & 2 deletions vendor/github.com/elastic/go-ucfg/error.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions vendor/github.com/elastic/go-ucfg/errpred.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions vendor/github.com/elastic/go-ucfg/fieldset.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions vendor/github.com/elastic/go-ucfg/opts.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions vendor/github.com/elastic/go-ucfg/reify.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 34b6a46

Please sign in to comment.