From bd508d628c60a7c256bebd0b2ddcba8deab41ae7 Mon Sep 17 00:00:00 2001 From: chrismark Date: Mon, 17 Feb 2020 12:35:16 +0200 Subject: [PATCH 1/9] Add k8s secrets Keystore backend Signed-off-by: chrismark --- .../autodiscover/providers/kubernetes/pod.go | 6 ++ libbeat/autodiscover/template/config.go | 11 +++ libbeat/common/kubernetes/watcher.go | 8 ++ libbeat/keystore/file_keystore.go | 4 +- libbeat/keystore/keystore.go | 41 +++++--- libbeat/keystore/kubernetes_keystore.go | 93 +++++++++++++++++++ 6 files changed, 147 insertions(+), 16 deletions(-) create mode 100644 libbeat/keystore/kubernetes_keystore.go diff --git a/libbeat/autodiscover/providers/kubernetes/pod.go b/libbeat/autodiscover/providers/kubernetes/pod.go index 9cd5094fa5a..f2bb8dc9dd4 100644 --- a/libbeat/autodiscover/providers/kubernetes/pod.go +++ b/libbeat/autodiscover/providers/kubernetes/pod.go @@ -21,6 +21,8 @@ import ( "fmt" "time" + "github.com/elastic/beats/libbeat/keystore" + "github.com/gofrs/uuid" k8s "k8s.io/client-go/kubernetes" @@ -297,6 +299,8 @@ func (p *pod) emitEvents(pod *kubernetes.Pod, flag string, containers []kubernet } kubemeta["annotations"] = annotations + k8sKeystore, _ := keystore.Factoryk8s(pod.Namespace, p.watcher.Getk8sClient()) + // Without this check there would be overlapping configurations with and without ports. if len(c.Ports) == 0 { event := bus.Event{ @@ -308,6 +312,7 @@ func (p *pod) emitEvents(pod *kubernetes.Pod, flag string, containers []kubernet "meta": common.MapStr{ "kubernetes": meta, }, + "keystore": k8sKeystore, } p.publish(event) } @@ -323,6 +328,7 @@ func (p *pod) emitEvents(pod *kubernetes.Pod, flag string, containers []kubernet "meta": common.MapStr{ "kubernetes": meta, }, + "keystore": k8sKeystore, } p.publish(event) } diff --git a/libbeat/autodiscover/template/config.go b/libbeat/autodiscover/template/config.go index c1a2c2b6c51..c1369bc765e 100644 --- a/libbeat/autodiscover/template/config.go +++ b/libbeat/autodiscover/template/config.go @@ -21,6 +21,7 @@ import ( "github.com/elastic/beats/libbeat/common" "github.com/elastic/beats/libbeat/common/bus" "github.com/elastic/beats/libbeat/conditions" + "github.com/elastic/beats/libbeat/keystore" "github.com/elastic/beats/libbeat/logp" "github.com/elastic/go-ucfg" ) @@ -105,6 +106,8 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config) []*common.Co ucfg.ResolveEnv, ucfg.VarExp, } + opts = updateOptsWithk8sSecretsKeystore(event, opts) + for _, config := range configs { c, err := ucfg.NewFrom(config, opts...) if err != nil { @@ -128,3 +131,11 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config) []*common.Co } return result } + +func updateOptsWithk8sSecretsKeystore(event bus.Event, opts []ucfg.Option) []ucfg.Option { + if val, ok := event["keystore"]; ok { + eventKeystore := val.(keystore.Keystore) + opts = append(opts, ucfg.Resolve(keystore.ResolverWrap(eventKeystore))) + } + return opts +} diff --git a/libbeat/common/kubernetes/watcher.go b/libbeat/common/kubernetes/watcher.go index 79f2b862579..2ce96dfc20d 100644 --- a/libbeat/common/kubernetes/watcher.go +++ b/libbeat/common/kubernetes/watcher.go @@ -56,6 +56,9 @@ type Watcher interface { // Store returns the store object for the watcher Store() cache.Store + + // Getk8sClient retrieves the Kubernetes Client with which the Watcher is coupled + Getk8sClient() kubernetes.Interface } // WatchOptions controls watch behaviors @@ -141,6 +144,11 @@ func (w *watcher) Store() cache.Store { return w.store } +// Getk8sClient retrieves the Kubernetes Client with which the Watcher is coupled +func (w *watcher) Getk8sClient() kubernetes.Interface { + return w.client +} + // Start watching pods func (w *watcher) Start() error { go w.informer.Run(w.ctx.Done()) diff --git a/libbeat/keystore/file_keystore.go b/libbeat/keystore/file_keystore.go index 357002cb151..7867e880ee1 100644 --- a/libbeat/keystore/file_keystore.go +++ b/libbeat/keystore/file_keystore.go @@ -69,13 +69,13 @@ type serializableSecureString struct { // NewFileKeystore returns an new File based keystore or an error, currently users cannot set their // own password on the keystore, the default password will be an empty string. When the keystore // is initialized the secrets are automatically loaded into memory. -func NewFileKeystore(keystoreFile string) (Keystore, error) { +func NewFileKeystore(keystoreFile string) (ListingKeystore, error) { return NewFileKeystoreWithPassword(keystoreFile, NewSecureString([]byte(""))) } // NewFileKeystoreWithPassword return a new File based keystore or an error, allow to define what // password to use to create the keystore. -func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (Keystore, error) { +func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (ListingKeystore, error) { keystore := FileKeystore{ Path: keystoreFile, dirty: false, diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index 15937b5163e..574861ebdf1 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -21,6 +21,8 @@ import ( "errors" "fmt" + k8s "k8s.io/client-go/kubernetes" + "github.com/elastic/beats/libbeat/common" ucfg "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/parse" @@ -39,31 +41,37 @@ var ( // to that concept, so we can deal with tokens that has a limited duration or can be revoked by a // remote keystore. type Keystore interface { - // Store add keys to the keystore, wont be persisted until we save. - Store(key string, secret []byte) error - // Retrieve returns a SecureString instance of the searched key or an error. Retrieve(key string) (*SecureString, error) - // Delete removes a specific key from the keystore. - Delete(key string) error - - // List returns the list of keys in the keystore, return an empty list if none is found. - List() ([]string, error) - // GetConfig returns the key value pair in the config format to be merged with other configuration. GetConfig() (*common.Config, error) - // Create Allow to create an empty keystore. - Create(override bool) error - // IsPersisted check if the current keystore is persisted. IsPersisted() bool +} + +type WritableKeystore interface { + Keystore + // Store add keys to the keystore, wont be persisted until we save. + Store(key string, secret []byte) error + + // Delete removes a specific key from the keystore. + Delete(key string) error + + // Create Allow to create an empty keystore. + Create(override bool) error // Save persist the changes to the keystore. Save() error } +type ListingKeystore interface { + WritableKeystore + // List returns the list of keys in the keystore, return an empty list if none is found. + List() ([]string, error) +} + // Packager defines a keystore that we can read the raw bytes and be packaged in an artifact. type Packager interface { Package() ([]byte, error) @@ -71,7 +79,7 @@ type Packager interface { } // Factory Create the right keystore with the configured options. -func Factory(cfg *common.Config, defaultPath string) (Keystore, error) { +func Factory(cfg *common.Config, defaultPath string) (ListingKeystore, error) { config := defaultConfig if cfg == nil { @@ -91,10 +99,15 @@ func Factory(cfg *common.Config, defaultPath string) (Keystore, error) { return keystore, err } +// Factoryk8s Create the right keystore with the configured options. +func Factoryk8s(keystoreNamespace string, ks8client k8s.Interface) (Keystore, error) { + keystore, err := NewKubernetesSecretsKeystore(keystoreNamespace, ks8client) + return keystore, err +} + // ResolverFromConfig create a resolver from a configuration. func ResolverFromConfig(cfg *common.Config, dataPath string) (func(string) (string, parse.Config, error), error) { keystore, err := Factory(cfg, dataPath) - if err != nil { return nil, err } diff --git a/libbeat/keystore/kubernetes_keystore.go b/libbeat/keystore/kubernetes_keystore.go new file mode 100644 index 00000000000..f5b24f51ead --- /dev/null +++ b/libbeat/keystore/kubernetes_keystore.go @@ -0,0 +1,93 @@ +// 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 keystore + +import ( + "fmt" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8s "k8s.io/client-go/kubernetes" + + "github.com/elastic/beats/libbeat/common" +) + +type KubernetesKeystores map[string]KubernetesSecretsKeystore + +var kubernetesKeystoresRegistry = KubernetesKeystores{} + +// KubernetesSecretsKeystore Allows to retrieve passwords from Kubernetes secrets for a given namespace. +type KubernetesSecretsKeystore struct { + namespace string + client k8s.Interface +} + +// NewKubernetesSecretsKeystore returns an new k8s Keystore +func NewKubernetesSecretsKeystore(keystoreNamespace string, ks8client k8s.Interface) (Keystore, error) { + // search for in the keystore registry to find if there is keystore already for that namespace + storedKeystore, err := lookupForKeystore(keystoreNamespace) + if err != nil { + return nil, err + } + if storedKeystore != nil { + return storedKeystore, nil + } + + keystore := KubernetesSecretsKeystore{ + namespace: keystoreNamespace, + client: ks8client, + } + kubernetesKeystoresRegistry[keystoreNamespace] = keystore + return &keystore, nil +} + +func lookupForKeystore(keystoreNamespace string) (Keystore, error) { + if keystore, ok := kubernetesKeystoresRegistry[keystoreNamespace]; ok { + return &keystore, nil + } + return nil, nil +} + +// Retrieve return a SecureString instance that will contains both the key and the secret. +func (k *KubernetesSecretsKeystore) Retrieve(key string) (*SecureString, error) { + // key = "kubernetes:somenamespace:somesecret:value" + toks := strings.Split(key, ":") + ns := toks[1] + secretName := toks[2] + secretVar := toks[3] + if ns != k.namespace { + return nil, fmt.Errorf("cannot access Kubernetes secrets from a different namespace than: %v", ns) + } + secret, err := k.client.CoreV1().Secrets(ns).Get(secretName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + secretString := secret.Data[secretVar] + return NewSecureString(secretString), nil +} + +// GetConfig returns common.Config representation of the key / secret pair to be merged with other +// loaded configuration. +func (k *KubernetesSecretsKeystore) GetConfig() (*common.Config, error) { + return nil, nil +} + +// IsPersisted return if the keystore is physically persisted on disk. +func (k *KubernetesSecretsKeystore) IsPersisted() bool { + return true +} From 8fc096b1857ebfcb61d08561c3da4694ddea8ea2 Mon Sep 17 00:00:00 2001 From: chrismark Date: Thu, 20 Feb 2020 11:45:27 +0200 Subject: [PATCH 2/9] Keep only keystore refactoring Signed-off-by: chrismark --- .../autodiscover/providers/kubernetes/pod.go | 6 -- libbeat/autodiscover/template/config.go | 10 -- libbeat/common/kubernetes/watcher.go | 8 -- libbeat/keystore/keystore.go | 10 +- libbeat/keystore/kubernetes_keystore.go | 93 ------------------- 5 files changed, 1 insertion(+), 126 deletions(-) delete mode 100644 libbeat/keystore/kubernetes_keystore.go diff --git a/libbeat/autodiscover/providers/kubernetes/pod.go b/libbeat/autodiscover/providers/kubernetes/pod.go index f2bb8dc9dd4..9cd5094fa5a 100644 --- a/libbeat/autodiscover/providers/kubernetes/pod.go +++ b/libbeat/autodiscover/providers/kubernetes/pod.go @@ -21,8 +21,6 @@ import ( "fmt" "time" - "github.com/elastic/beats/libbeat/keystore" - "github.com/gofrs/uuid" k8s "k8s.io/client-go/kubernetes" @@ -299,8 +297,6 @@ func (p *pod) emitEvents(pod *kubernetes.Pod, flag string, containers []kubernet } kubemeta["annotations"] = annotations - k8sKeystore, _ := keystore.Factoryk8s(pod.Namespace, p.watcher.Getk8sClient()) - // Without this check there would be overlapping configurations with and without ports. if len(c.Ports) == 0 { event := bus.Event{ @@ -312,7 +308,6 @@ func (p *pod) emitEvents(pod *kubernetes.Pod, flag string, containers []kubernet "meta": common.MapStr{ "kubernetes": meta, }, - "keystore": k8sKeystore, } p.publish(event) } @@ -328,7 +323,6 @@ func (p *pod) emitEvents(pod *kubernetes.Pod, flag string, containers []kubernet "meta": common.MapStr{ "kubernetes": meta, }, - "keystore": k8sKeystore, } p.publish(event) } diff --git a/libbeat/autodiscover/template/config.go b/libbeat/autodiscover/template/config.go index c1369bc765e..8262f226f88 100644 --- a/libbeat/autodiscover/template/config.go +++ b/libbeat/autodiscover/template/config.go @@ -21,7 +21,6 @@ import ( "github.com/elastic/beats/libbeat/common" "github.com/elastic/beats/libbeat/common/bus" "github.com/elastic/beats/libbeat/conditions" - "github.com/elastic/beats/libbeat/keystore" "github.com/elastic/beats/libbeat/logp" "github.com/elastic/go-ucfg" ) @@ -106,7 +105,6 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config) []*common.Co ucfg.ResolveEnv, ucfg.VarExp, } - opts = updateOptsWithk8sSecretsKeystore(event, opts) for _, config := range configs { c, err := ucfg.NewFrom(config, opts...) @@ -131,11 +129,3 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config) []*common.Co } return result } - -func updateOptsWithk8sSecretsKeystore(event bus.Event, opts []ucfg.Option) []ucfg.Option { - if val, ok := event["keystore"]; ok { - eventKeystore := val.(keystore.Keystore) - opts = append(opts, ucfg.Resolve(keystore.ResolverWrap(eventKeystore))) - } - return opts -} diff --git a/libbeat/common/kubernetes/watcher.go b/libbeat/common/kubernetes/watcher.go index 2ce96dfc20d..79f2b862579 100644 --- a/libbeat/common/kubernetes/watcher.go +++ b/libbeat/common/kubernetes/watcher.go @@ -56,9 +56,6 @@ type Watcher interface { // Store returns the store object for the watcher Store() cache.Store - - // Getk8sClient retrieves the Kubernetes Client with which the Watcher is coupled - Getk8sClient() kubernetes.Interface } // WatchOptions controls watch behaviors @@ -144,11 +141,6 @@ func (w *watcher) Store() cache.Store { return w.store } -// Getk8sClient retrieves the Kubernetes Client with which the Watcher is coupled -func (w *watcher) Getk8sClient() kubernetes.Interface { - return w.client -} - // Start watching pods func (w *watcher) Start() error { go w.informer.Run(w.ctx.Done()) diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index 574861ebdf1..fee0548ffee 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -21,10 +21,8 @@ import ( "errors" "fmt" - k8s "k8s.io/client-go/kubernetes" - "github.com/elastic/beats/libbeat/common" - ucfg "github.com/elastic/go-ucfg" + "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/parse" ) @@ -99,12 +97,6 @@ func Factory(cfg *common.Config, defaultPath string) (ListingKeystore, error) { return keystore, err } -// Factoryk8s Create the right keystore with the configured options. -func Factoryk8s(keystoreNamespace string, ks8client k8s.Interface) (Keystore, error) { - keystore, err := NewKubernetesSecretsKeystore(keystoreNamespace, ks8client) - return keystore, err -} - // ResolverFromConfig create a resolver from a configuration. func ResolverFromConfig(cfg *common.Config, dataPath string) (func(string) (string, parse.Config, error), error) { keystore, err := Factory(cfg, dataPath) diff --git a/libbeat/keystore/kubernetes_keystore.go b/libbeat/keystore/kubernetes_keystore.go deleted file mode 100644 index f5b24f51ead..00000000000 --- a/libbeat/keystore/kubernetes_keystore.go +++ /dev/null @@ -1,93 +0,0 @@ -// 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 keystore - -import ( - "fmt" - "strings" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - k8s "k8s.io/client-go/kubernetes" - - "github.com/elastic/beats/libbeat/common" -) - -type KubernetesKeystores map[string]KubernetesSecretsKeystore - -var kubernetesKeystoresRegistry = KubernetesKeystores{} - -// KubernetesSecretsKeystore Allows to retrieve passwords from Kubernetes secrets for a given namespace. -type KubernetesSecretsKeystore struct { - namespace string - client k8s.Interface -} - -// NewKubernetesSecretsKeystore returns an new k8s Keystore -func NewKubernetesSecretsKeystore(keystoreNamespace string, ks8client k8s.Interface) (Keystore, error) { - // search for in the keystore registry to find if there is keystore already for that namespace - storedKeystore, err := lookupForKeystore(keystoreNamespace) - if err != nil { - return nil, err - } - if storedKeystore != nil { - return storedKeystore, nil - } - - keystore := KubernetesSecretsKeystore{ - namespace: keystoreNamespace, - client: ks8client, - } - kubernetesKeystoresRegistry[keystoreNamespace] = keystore - return &keystore, nil -} - -func lookupForKeystore(keystoreNamespace string) (Keystore, error) { - if keystore, ok := kubernetesKeystoresRegistry[keystoreNamespace]; ok { - return &keystore, nil - } - return nil, nil -} - -// Retrieve return a SecureString instance that will contains both the key and the secret. -func (k *KubernetesSecretsKeystore) Retrieve(key string) (*SecureString, error) { - // key = "kubernetes:somenamespace:somesecret:value" - toks := strings.Split(key, ":") - ns := toks[1] - secretName := toks[2] - secretVar := toks[3] - if ns != k.namespace { - return nil, fmt.Errorf("cannot access Kubernetes secrets from a different namespace than: %v", ns) - } - secret, err := k.client.CoreV1().Secrets(ns).Get(secretName, metav1.GetOptions{}) - if err != nil { - return nil, err - } - secretString := secret.Data[secretVar] - return NewSecureString(secretString), nil -} - -// GetConfig returns common.Config representation of the key / secret pair to be merged with other -// loaded configuration. -func (k *KubernetesSecretsKeystore) GetConfig() (*common.Config, error) { - return nil, nil -} - -// IsPersisted return if the keystore is physically persisted on disk. -func (k *KubernetesSecretsKeystore) IsPersisted() bool { - return true -} From 5b03a561d59ad0d167d43d08993a86167897033c Mon Sep 17 00:00:00 2001 From: chrismark Date: Thu, 20 Feb 2020 12:03:54 +0200 Subject: [PATCH 3/9] left overs Signed-off-by: chrismark --- libbeat/autodiscover/template/config.go | 1 - libbeat/keystore/keystore.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/libbeat/autodiscover/template/config.go b/libbeat/autodiscover/template/config.go index 8262f226f88..c1a2c2b6c51 100644 --- a/libbeat/autodiscover/template/config.go +++ b/libbeat/autodiscover/template/config.go @@ -105,7 +105,6 @@ func ApplyConfigTemplate(event bus.Event, configs []*common.Config) []*common.Co ucfg.ResolveEnv, ucfg.VarExp, } - for _, config := range configs { c, err := ucfg.NewFrom(config, opts...) if err != nil { diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index fee0548ffee..f2f3e81e680 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -100,6 +100,7 @@ func Factory(cfg *common.Config, defaultPath string) (ListingKeystore, error) { // ResolverFromConfig create a resolver from a configuration. func ResolverFromConfig(cfg *common.Config, dataPath string) (func(string) (string, parse.Config, error), error) { keystore, err := Factory(cfg, dataPath) + if err != nil { return nil, err } From 5100deb862721d7c4f2df51332de0378cf77a5b9 Mon Sep 17 00:00:00 2001 From: chrismark Date: Tue, 25 Feb 2020 16:34:37 +0200 Subject: [PATCH 4/9] fixes Signed-off-by: chrismark --- libbeat/keystore/file_keystore.go | 28 ++++++++++++++++++++++-- libbeat/keystore/keystore.go | 36 ------------------------------- 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/libbeat/keystore/file_keystore.go b/libbeat/keystore/file_keystore.go index 7867e880ee1..7bb760f6fea 100644 --- a/libbeat/keystore/file_keystore.go +++ b/libbeat/keystore/file_keystore.go @@ -53,6 +53,9 @@ var version = []byte("v1") // FileKeystore Allows to store key / secrets pair securely into an encrypted local file. type FileKeystore struct { + Keystore + WritableKeystore + ListingKeystore sync.RWMutex Path string secrets map[string]serializableSecureString @@ -66,16 +69,37 @@ type serializableSecureString struct { Value []byte `json:"value"` } +// Factory Create the right keystore with the configured options. +func Factory(cfg *common.Config, defaultPath string) (Keystore, error) { + config := defaultConfig + + if cfg == nil { + cfg = common.NewConfig() + } + err := cfg.Unpack(&config) + + if err != nil { + return nil, fmt.Errorf("could not read keystore configuration, err: %v", err) + } + + if config.Path == "" { + config.Path = defaultPath + } + + keystore, err := NewFileKeystore(config.Path) + return keystore, err +} + // NewFileKeystore returns an new File based keystore or an error, currently users cannot set their // own password on the keystore, the default password will be an empty string. When the keystore // is initialized the secrets are automatically loaded into memory. -func NewFileKeystore(keystoreFile string) (ListingKeystore, error) { +func NewFileKeystore(keystoreFile string) (Keystore, error) { return NewFileKeystoreWithPassword(keystoreFile, NewSecureString([]byte(""))) } // NewFileKeystoreWithPassword return a new File based keystore or an error, allow to define what // password to use to create the keystore. -func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (ListingKeystore, error) { +func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (Keystore, error) { keystore := FileKeystore{ Path: keystoreFile, dirty: false, diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index f2f3e81e680..f407a20ff19 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -19,8 +19,6 @@ package keystore import ( "errors" - "fmt" - "github.com/elastic/beats/libbeat/common" "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/parse" @@ -50,7 +48,6 @@ type Keystore interface { } type WritableKeystore interface { - Keystore // Store add keys to the keystore, wont be persisted until we save. Store(key string, secret []byte) error @@ -65,7 +62,6 @@ type WritableKeystore interface { } type ListingKeystore interface { - WritableKeystore // List returns the list of keys in the keystore, return an empty list if none is found. List() ([]string, error) } @@ -76,38 +72,6 @@ type Packager interface { ConfiguredPath() string } -// Factory Create the right keystore with the configured options. -func Factory(cfg *common.Config, defaultPath string) (ListingKeystore, error) { - config := defaultConfig - - if cfg == nil { - cfg = common.NewConfig() - } - err := cfg.Unpack(&config) - - if err != nil { - return nil, fmt.Errorf("could not read keystore configuration, err: %v", err) - } - - if config.Path == "" { - config.Path = defaultPath - } - - keystore, err := NewFileKeystore(config.Path) - return keystore, err -} - -// ResolverFromConfig create a resolver from a configuration. -func ResolverFromConfig(cfg *common.Config, dataPath string) (func(string) (string, parse.Config, error), error) { - keystore, err := Factory(cfg, dataPath) - - if err != nil { - return nil, err - } - - return ResolverWrap(keystore), nil -} - // ResolverWrap wrap a config resolver around an existing keystore. func ResolverWrap(keystore Keystore) func(string) (string, parse.Config, error) { return func(keyName string) (string, parse.Config, error) { From d3790735d0ad8a9489130877bb5d0cfe40709256 Mon Sep 17 00:00:00 2001 From: chrismark Date: Fri, 28 Feb 2020 13:53:37 +0200 Subject: [PATCH 5/9] fixes Signed-off-by: chrismark --- libbeat/cmd/keystore.go | 35 ++++++++++++++++++++------ libbeat/keystore/file_keystore.go | 24 ++++++++++++------ libbeat/keystore/file_keystore_test.go | 3 ++- libbeat/keystore/keystore.go | 7 +----- x-pack/libbeat/management/enroll.go | 20 ++++++++++----- 5 files changed, 61 insertions(+), 28 deletions(-) diff --git a/libbeat/cmd/keystore.go b/libbeat/cmd/keystore.go index b2cffe2b7eb..b053e592d8b 100644 --- a/libbeat/cmd/keystore.go +++ b/libbeat/cmd/keystore.go @@ -130,10 +130,15 @@ func createKeystore(settings instance.Settings, force bool) error { return err } + wKeystore, ok := store.(keystore.WritableKeystore) + if !ok { + return fmt.Errorf("the configured keystore is not writable") + } + if store.IsPersisted() == true && force == false { response := terminal.PromptYesNo("A keystore already exists, Overwrite?", false) if response == true { - err := store.Create(true) + err := wKeystore.Create(true) if err != nil { return fmt.Errorf("error creating the keystore: %s", err) } @@ -142,7 +147,7 @@ func createKeystore(settings instance.Settings, force bool) error { return nil } } else { - err := store.Create(true) + err := wKeystore.Create(true) if err != nil { return fmt.Errorf("Error creating the keystore: %s", err) } @@ -160,6 +165,11 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error { return fmt.Errorf("could not create secret for: %s, you can only provide one key per invocation", keys) } + wKeystore, ok := store.(keystore.WritableKeystore) + if !ok { + return fmt.Errorf("the configured keystore is not writable") + } + if store.IsPersisted() == false { if force == false { answer := terminal.PromptYesNo("The keystore does not exist. Do you want to create it?", false) @@ -167,7 +177,7 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error { return errors.New("exiting without creating keystore") } } - err := store.Create(true) + err := wKeystore.Create(true) if err != nil { return fmt.Errorf("could not create keystore, error: %s", err) } @@ -202,10 +212,10 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error { return fmt.Errorf("could not read value from the input, error: %s", err) } } - if err = store.Store(key, keyValue); err != nil { + if err = wKeystore.Store(key, keyValue); err != nil { return fmt.Errorf("could not add the key in the keystore, error: %s", err) } - if err = store.Save(); err != nil { + if err = wKeystore.Save(); err != nil { return fmt.Errorf("fail to save the keystore: %s", err) } else { fmt.Println("Successfully updated the keystore") @@ -218,6 +228,11 @@ func removeKey(store keystore.Keystore, keys []string) error { return errors.New("you must supply at least one key to remove") } + wKeystore, ok := store.(keystore.WritableKeystore) + if !ok { + return fmt.Errorf("the configured keystore is not writable") + } + if store.IsPersisted() == false { return errors.New("the keystore doesn't exist. Use the 'create' command to create one") } @@ -229,8 +244,8 @@ func removeKey(store keystore.Keystore, keys []string) error { return fmt.Errorf("could not find key '%v' in the keystore", key) } - store.Delete(key) - err = store.Save() + wKeystore.Delete(key) + err = wKeystore.Save() if err != nil { return fmt.Errorf("could not update the keystore with the changes, key: %s, error: %v", key, err) } @@ -240,7 +255,11 @@ func removeKey(store keystore.Keystore, keys []string) error { } func list(store keystore.Keystore) error { - keys, err := store.List() + lKeystore, ok := store.(keystore.ListingKeystore) + if !ok { + return fmt.Errorf("the configured keystore is not writable") + } + keys, err := lKeystore.List() if err != nil { return fmt.Errorf("could not read values from the keystore, error: %s", err) } diff --git a/libbeat/keystore/file_keystore.go b/libbeat/keystore/file_keystore.go index 7bb760f6fea..53314f8d566 100644 --- a/libbeat/keystore/file_keystore.go +++ b/libbeat/keystore/file_keystore.go @@ -51,11 +51,21 @@ const ( // Version of the keystore format, will be added at the beginning of the file. var version = []byte("v1") -// FileKeystore Allows to store key / secrets pair securely into an encrypted local file. -type FileKeystore struct { +// Packager defines a keystore that we can read the raw bytes and be packaged in an artifact. +type Packager interface { + Package() ([]byte, error) + ConfiguredPath() string +} + +// FileKeystoreIn defines a keystore implements Keystore, WritableKeystore, ListingKeystore. +type FileKeystoreIn interface { Keystore WritableKeystore ListingKeystore +} + +// FileKeystore Allows to store key / secrets pair securely into an encrypted local file. +type FileKeystore struct { sync.RWMutex Path string secrets map[string]serializableSecureString @@ -87,19 +97,19 @@ func Factory(cfg *common.Config, defaultPath string) (Keystore, error) { } keystore, err := NewFileKeystore(config.Path) - return keystore, err + return &keystore, err } // NewFileKeystore returns an new File based keystore or an error, currently users cannot set their // own password on the keystore, the default password will be an empty string. When the keystore // is initialized the secrets are automatically loaded into memory. -func NewFileKeystore(keystoreFile string) (Keystore, error) { +func NewFileKeystore(keystoreFile string) (FileKeystore, error) { return NewFileKeystoreWithPassword(keystoreFile, NewSecureString([]byte(""))) } // NewFileKeystoreWithPassword return a new File based keystore or an error, allow to define what // password to use to create the keystore. -func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (Keystore, error) { +func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (FileKeystore, error) { keystore := FileKeystore{ Path: keystoreFile, dirty: false, @@ -109,10 +119,10 @@ func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (K err := keystore.load() if err != nil { - return nil, err + return FileKeystore{}, err } - return &keystore, nil + return keystore, nil } // Retrieve return a SecureString instance that will contains both the key and the secret. diff --git a/libbeat/keystore/file_keystore_test.go b/libbeat/keystore/file_keystore_test.go index 42eb36f2c4e..8b9e3b0a620 100644 --- a/libbeat/keystore/file_keystore_test.go +++ b/libbeat/keystore/file_keystore_test.go @@ -39,6 +39,7 @@ func TestCanCreateAKeyStore(t *testing.T) { keystore, err := NewFileKeystore(path) assert.NoError(t, err) + assert.Nil(t, keystore.Store(keyValue, secretValue)) assert.Nil(t, keystore.Save()) } @@ -289,7 +290,7 @@ func createAndReadKeystoreWithPassword(t *testing.T, password []byte) { // CreateAnExistingKeystore creates a keystore with an existing key /// `output.elasticsearch.password` with the value `secret`. -func CreateAnExistingKeystore(path string) Keystore { +func CreateAnExistingKeystore(path string) FileKeystore { keystore, err := NewFileKeystore(path) // Fail fast in the test suite if err != nil { diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index f407a20ff19..b0b471ccec7 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -19,6 +19,7 @@ package keystore import ( "errors" + "github.com/elastic/beats/libbeat/common" "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/parse" @@ -66,12 +67,6 @@ type ListingKeystore interface { List() ([]string, error) } -// Packager defines a keystore that we can read the raw bytes and be packaged in an artifact. -type Packager interface { - Package() ([]byte, error) - ConfiguredPath() string -} - // ResolverWrap wrap a config resolver around an existing keystore. func ResolverWrap(keystore Keystore) func(string) (string, parse.Config, error) { return func(keyName string) (string, parse.Config, error) { diff --git a/x-pack/libbeat/management/enroll.go b/x-pack/libbeat/management/enroll.go index 49db0d9e342..96f9b85b132 100644 --- a/x-pack/libbeat/management/enroll.go +++ b/x-pack/libbeat/management/enroll.go @@ -9,6 +9,8 @@ import ( "os" "time" + "github.com/elastic/beats/libbeat/keystore" + "github.com/pkg/errors" "github.com/elastic/beats/libbeat/cfgfile" @@ -87,16 +89,22 @@ func Enroll( } func storeAccessToken(beat *instance.Beat, accessToken string) error { - keystore := beat.Keystore() - if !keystore.IsPersisted() { - if err := keystore.Create(false); err != nil { + keyStore := beat.Keystore() + + wKeystore, ok := keyStore.(keystore.WritableKeystore) + if !ok { + return fmt.Errorf("the configured keystore is not writable") + } + + if !keyStore.IsPersisted() { + + if err := wKeystore.Create(false); err != nil { return errors.Wrap(err, "error creating keystore") } } - - if err := keystore.Store(accessTokenKey, []byte(accessToken)); err != nil { + if err := wKeystore.Store(accessTokenKey, []byte(accessToken)); err != nil { return errors.Wrap(err, "error storing the access token") } - return keystore.Save() + return wKeystore.Save() } From 91f306255751c4a4bb91e14c52010152b8ad781d Mon Sep 17 00:00:00 2001 From: chrismark Date: Wed, 4 Mar 2020 11:58:36 +0200 Subject: [PATCH 6/9] review fixes Signed-off-by: chrismark --- libbeat/keystore/file_keystore.go | 17 ++--- libbeat/keystore/file_keystore_test.go | 91 +++++++++++++++++--------- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/libbeat/keystore/file_keystore.go b/libbeat/keystore/file_keystore.go index 53314f8d566..7d819ffe0ff 100644 --- a/libbeat/keystore/file_keystore.go +++ b/libbeat/keystore/file_keystore.go @@ -57,13 +57,6 @@ type Packager interface { ConfiguredPath() string } -// FileKeystoreIn defines a keystore implements Keystore, WritableKeystore, ListingKeystore. -type FileKeystoreIn interface { - Keystore - WritableKeystore - ListingKeystore -} - // FileKeystore Allows to store key / secrets pair securely into an encrypted local file. type FileKeystore struct { sync.RWMutex @@ -97,19 +90,19 @@ func Factory(cfg *common.Config, defaultPath string) (Keystore, error) { } keystore, err := NewFileKeystore(config.Path) - return &keystore, err + return keystore, err } // NewFileKeystore returns an new File based keystore or an error, currently users cannot set their // own password on the keystore, the default password will be an empty string. When the keystore // is initialized the secrets are automatically loaded into memory. -func NewFileKeystore(keystoreFile string) (FileKeystore, error) { +func NewFileKeystore(keystoreFile string) (Keystore, error) { return NewFileKeystoreWithPassword(keystoreFile, NewSecureString([]byte(""))) } // NewFileKeystoreWithPassword return a new File based keystore or an error, allow to define what // password to use to create the keystore. -func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (FileKeystore, error) { +func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (Keystore, error) { keystore := FileKeystore{ Path: keystoreFile, dirty: false, @@ -119,10 +112,10 @@ func NewFileKeystoreWithPassword(keystoreFile string, password *SecureString) (F err := keystore.load() if err != nil { - return FileKeystore{}, err + return nil, err } - return keystore, nil + return &keystore, nil } // Retrieve return a SecureString instance that will contains both the key and the secret. diff --git a/libbeat/keystore/file_keystore_test.go b/libbeat/keystore/file_keystore_test.go index 8b9e3b0a620..4c7e4384a72 100644 --- a/libbeat/keystore/file_keystore_test.go +++ b/libbeat/keystore/file_keystore_test.go @@ -37,11 +37,14 @@ func TestCanCreateAKeyStore(t *testing.T) { path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore, err := NewFileKeystore(path) + keyStore, err := NewFileKeystore(path) assert.NoError(t, err) - assert.Nil(t, keystore.Store(keyValue, secretValue)) - assert.Nil(t, keystore.Save()) + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) + + assert.Nil(t, wKeystore.Store(keyValue, secretValue)) + assert.Nil(t, wKeystore.Save()) } func TestCanReadAnExistingKeyStoreWithEmptyString(t *testing.T) { @@ -67,15 +70,18 @@ func TestCanDeleteAKeyFromTheStoreAndPersistChanges(t *testing.T) { CreateAnExistingKeystore(path) - keystore, _ := NewFileKeystore(path) - _, err := keystore.Retrieve(keyValue) + keyStore, _ := NewFileKeystore(path) + _, err := keyStore.Retrieve(keyValue) assert.NoError(t, err) - keystore.Delete(keyValue) - _, err = keystore.Retrieve(keyValue) + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) + + wKeystore.Delete(keyValue) + _, err = keyStore.Retrieve(keyValue) assert.Error(t, err) - _ = keystore.Save() + _ = wKeystore.Save() newKeystore, err := NewFileKeystore(path) _, err = newKeystore.Retrieve(keyValue) assert.Error(t, err) @@ -114,10 +120,14 @@ func TestFilePermissionOnUpdate(t *testing.T) { path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore := CreateAnExistingKeystore(path) - err := keystore.Store("newkey", []byte("newsecret")) + keyStore := CreateAnExistingKeystore(path) + + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) + + err := wKeystore.Store("newkey", []byte("newsecret")) assert.NoError(t, err) - err = keystore.Save() + err = wKeystore.Save() assert.NoError(t, err) stats, err := os.Stat(path) assert.NoError(t, err) @@ -153,9 +163,12 @@ func TestReturnsUsedKeysInTheStore(t *testing.T) { path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore := CreateAnExistingKeystore(path) + keyStore := CreateAnExistingKeystore(path) + + wKeystore, ok := keyStore.(ListingKeystore) + assert.True(t, ok) - keys, err := keystore.List() + keys, err := wKeystore.List() assert.NoError(t, err) assert.Equal(t, len(keys), 1) @@ -166,9 +179,13 @@ func TestCannotDecryptKeyStoreWithWrongPassword(t *testing.T) { path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore, err := NewFileKeystoreWithPassword(path, NewSecureString([]byte("password"))) - keystore.Store("hello", []byte("world")) - keystore.Save() + keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString([]byte("password"))) + + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) + + wKeystore.Store("hello", []byte("world")) + wKeystore.Save() _, err = NewFileKeystoreWithPassword(path, NewSecureString([]byte("wrongpassword"))) if assert.Error(t, err, "should fail to decrypt the keystore") { @@ -200,13 +217,16 @@ func TestGetConfig(t *testing.T) { path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore := CreateAnExistingKeystore(path) + keyStore := CreateAnExistingKeystore(path) + + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) // Add a bit more data of different type - keystore.Store("super.nested", []byte("hello")) - keystore.Save() + wKeystore.Store("super.nested", []byte("hello")) + wKeystore.Save() - cfg, err := keystore.GetConfig() + cfg, err := keyStore.GetConfig() assert.NotNil(t, cfg) assert.NoError(t, err) @@ -259,11 +279,14 @@ func createAndReadKeystoreSecret(t *testing.T, password []byte, key string, valu path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) + keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) assert.Nil(t, err) - keystore.Store(key, value) - keystore.Save() + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) + + wKeystore.Store(key, value) + wKeystore.Save() newStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) s, _ := newStore.Retrieve(key) @@ -275,11 +298,14 @@ func createAndReadKeystoreWithPassword(t *testing.T, password []byte) { path := GetTemporaryKeystoreFile() defer os.Remove(path) - keystore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) + keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) assert.NoError(t, err) - keystore.Store("hello", []byte("world")) - keystore.Save() + wKeystore, ok := keyStore.(WritableKeystore) + assert.True(t, ok) + + wKeystore.Store("hello", []byte("world")) + wKeystore.Save() newStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) s, _ := newStore.Retrieve("hello") @@ -290,15 +316,18 @@ func createAndReadKeystoreWithPassword(t *testing.T, password []byte) { // CreateAnExistingKeystore creates a keystore with an existing key /// `output.elasticsearch.password` with the value `secret`. -func CreateAnExistingKeystore(path string) FileKeystore { - keystore, err := NewFileKeystore(path) +func CreateAnExistingKeystore(path string) Keystore { + keyStore, err := NewFileKeystore(path) // Fail fast in the test suite if err != nil { panic(err) } - keystore.Store(keyValue, secretValue) - keystore.Save() - return keystore + + wKeystore, _ := keyStore.(WritableKeystore) + + wKeystore.Store(keyValue, secretValue) + wKeystore.Save() + return keyStore } // GetTemporaryKeystoreFile create a temporary file on disk to save the keystore. From fcaadea8abaf2d060b2f25ed02165b1affbd4040 Mon Sep 17 00:00:00 2001 From: chrismark Date: Fri, 27 Mar 2020 18:00:41 +0200 Subject: [PATCH 7/9] fix import Signed-off-by: chrismark --- x-pack/libbeat/management/enroll.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/libbeat/management/enroll.go b/x-pack/libbeat/management/enroll.go index 9020c4d2901..e6b27f639af 100644 --- a/x-pack/libbeat/management/enroll.go +++ b/x-pack/libbeat/management/enroll.go @@ -9,14 +9,13 @@ import ( "os" "time" - "github.com/elastic/beats/libbeat/keystore" - "github.com/pkg/errors" "github.com/elastic/beats/v7/libbeat/cfgfile" "github.com/elastic/beats/v7/libbeat/cmd/instance" "github.com/elastic/beats/v7/libbeat/common/cfgwarn" "github.com/elastic/beats/v7/libbeat/common/file" + "github.com/elastic/beats/v7/libbeat/keystore" "github.com/elastic/beats/v7/libbeat/kibana" "github.com/elastic/beats/v7/x-pack/libbeat/management/api" ) From 77ad58cef4321fe15b0573710659062538621cd4 Mon Sep 17 00:00:00 2001 From: chrismark Date: Wed, 1 Apr 2020 16:45:10 +0300 Subject: [PATCH 8/9] Code improvements Signed-off-by: chrismark --- libbeat/cmd/keystore.go | 40 +++++++-------- libbeat/keystore/file_keystore_test.go | 71 ++++++++++++++------------ libbeat/keystore/keystore.go | 26 ++++++++++ x-pack/libbeat/management/enroll.go | 6 +-- 4 files changed, 86 insertions(+), 57 deletions(-) diff --git a/libbeat/cmd/keystore.go b/libbeat/cmd/keystore.go index 8986e15d13a..1b635d95ba2 100644 --- a/libbeat/cmd/keystore.go +++ b/libbeat/cmd/keystore.go @@ -130,15 +130,15 @@ func createKeystore(settings instance.Settings, force bool) error { return err } - wKeystore, ok := store.(keystore.WritableKeystore) - if !ok { - return fmt.Errorf("the configured keystore is not writable") + writableKeystore, err := keystore.AsWritableKeystore(store) + if err != nil { + return fmt.Errorf("error creating the keystore: %s", err) } if store.IsPersisted() == true && force == false { response := terminal.PromptYesNo("A keystore already exists, Overwrite?", false) if response == true { - err := wKeystore.Create(true) + err := writableKeystore.Create(true) if err != nil { return fmt.Errorf("error creating the keystore: %s", err) } @@ -147,7 +147,7 @@ func createKeystore(settings instance.Settings, force bool) error { return nil } } else { - err := wKeystore.Create(true) + err := writableKeystore.Create(true) if err != nil { return fmt.Errorf("Error creating the keystore: %s", err) } @@ -165,9 +165,9 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error { return fmt.Errorf("could not create secret for: %s, you can only provide one key per invocation", keys) } - wKeystore, ok := store.(keystore.WritableKeystore) - if !ok { - return fmt.Errorf("the configured keystore is not writable") + writableKeystore, err := keystore.AsWritableKeystore(store) + if err != nil { + return fmt.Errorf("error creating the keystore: %s", err) } if store.IsPersisted() == false { @@ -177,7 +177,7 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error { return errors.New("exiting without creating keystore") } } - err := wKeystore.Create(true) + err := writableKeystore.Create(true) if err != nil { return fmt.Errorf("could not create keystore, error: %s", err) } @@ -212,10 +212,10 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error { return fmt.Errorf("could not read value from the input, error: %s", err) } } - if err = wKeystore.Store(key, keyValue); err != nil { + if err = writableKeystore.Store(key, keyValue); err != nil { return fmt.Errorf("could not add the key in the keystore, error: %s", err) } - if err = wKeystore.Save(); err != nil { + if err = writableKeystore.Save(); err != nil { return fmt.Errorf("fail to save the keystore: %s", err) } else { fmt.Println("Successfully updated the keystore") @@ -228,9 +228,9 @@ func removeKey(store keystore.Keystore, keys []string) error { return errors.New("you must supply at least one key to remove") } - wKeystore, ok := store.(keystore.WritableKeystore) - if !ok { - return fmt.Errorf("the configured keystore is not writable") + writableKeystore, err := keystore.AsWritableKeystore(store) + if err != nil { + return fmt.Errorf("error deleting the keystore: %s", err) } if store.IsPersisted() == false { @@ -244,8 +244,8 @@ func removeKey(store keystore.Keystore, keys []string) error { return fmt.Errorf("could not find key '%v' in the keystore", key) } - wKeystore.Delete(key) - err = wKeystore.Save() + writableKeystore.Delete(key) + err = writableKeystore.Save() if err != nil { return fmt.Errorf("could not update the keystore with the changes, key: %s, error: %v", key, err) } @@ -255,11 +255,11 @@ func removeKey(store keystore.Keystore, keys []string) error { } func list(store keystore.Keystore) error { - lKeystore, ok := store.(keystore.ListingKeystore) - if !ok { - return fmt.Errorf("the configured keystore is not writable") + listingKeystore, err := keystore.AsListingKeystore(store) + if err != nil { + return fmt.Errorf("error listing the keystore: %s", err) } - keys, err := lKeystore.List() + keys, err := listingKeystore.List() if err != nil { return fmt.Errorf("could not read values from the keystore, error: %s", err) } diff --git a/libbeat/keystore/file_keystore_test.go b/libbeat/keystore/file_keystore_test.go index 384598b60a7..63c25afdadf 100644 --- a/libbeat/keystore/file_keystore_test.go +++ b/libbeat/keystore/file_keystore_test.go @@ -40,11 +40,11 @@ func TestCanCreateAKeyStore(t *testing.T) { keyStore, err := NewFileKeystore(path) assert.NoError(t, err) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) - assert.Nil(t, wKeystore.Store(keyValue, secretValue)) - assert.Nil(t, wKeystore.Save()) + assert.Nil(t, writableKeystore.Store(keyValue, secretValue)) + assert.Nil(t, writableKeystore.Save()) } func TestCanReadAnExistingKeyStoreWithEmptyString(t *testing.T) { @@ -74,14 +74,14 @@ func TestCanDeleteAKeyFromTheStoreAndPersistChanges(t *testing.T) { _, err := keyStore.Retrieve(keyValue) assert.NoError(t, err) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) - wKeystore.Delete(keyValue) + writableKeystore.Delete(keyValue) _, err = keyStore.Retrieve(keyValue) assert.Error(t, err) - _ = wKeystore.Save() + _ = writableKeystore.Save() newKeystore, err := NewFileKeystore(path) _, err = newKeystore.Retrieve(keyValue) assert.Error(t, err) @@ -122,12 +122,12 @@ func TestFilePermissionOnUpdate(t *testing.T) { keyStore := CreateAnExistingKeystore(path) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) - err := wKeystore.Store("newkey", []byte("newsecret")) + err = writableKeystore.Store("newkey", []byte("newsecret")) assert.NoError(t, err) - err = wKeystore.Save() + err = writableKeystore.Save() assert.NoError(t, err) stats, err := os.Stat(path) assert.NoError(t, err) @@ -165,10 +165,10 @@ func TestReturnsUsedKeysInTheStore(t *testing.T) { keyStore := CreateAnExistingKeystore(path) - wKeystore, ok := keyStore.(ListingKeystore) - assert.True(t, ok) + listingKeystore, err := AsListingKeystore(keyStore) + assert.NoError(t, err) - keys, err := wKeystore.List() + keys, err := listingKeystore.List() assert.NoError(t, err) assert.Equal(t, len(keys), 1) @@ -181,11 +181,11 @@ func TestCannotDecryptKeyStoreWithWrongPassword(t *testing.T) { keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString([]byte("password"))) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) - wKeystore.Store("hello", []byte("world")) - wKeystore.Save() + writableKeystore.Store("hello", []byte("world")) + writableKeystore.Save() _, err = NewFileKeystoreWithPassword(path, NewSecureString([]byte("wrongpassword"))) if assert.Error(t, err, "should fail to decrypt the keystore") { @@ -219,12 +219,12 @@ func TestGetConfig(t *testing.T) { keyStore := CreateAnExistingKeystore(path) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) // Add a bit more data of different type - wKeystore.Store("super.nested", []byte("hello")) - wKeystore.Save() + writableKeystore.Store("super.nested", []byte("hello")) + writableKeystore.Save() cfg, err := keyStore.GetConfig() assert.NotNil(t, cfg) @@ -282,11 +282,11 @@ func createAndReadKeystoreSecret(t *testing.T, password []byte, key string, valu keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) assert.Nil(t, err) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) - wKeystore.Store(key, value) - wKeystore.Save() + writableKeystore.Store(key, value) + writableKeystore.Save() newStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) s, _ := newStore.Retrieve(key) @@ -301,11 +301,11 @@ func createAndReadKeystoreWithPassword(t *testing.T, password []byte) { keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) assert.NoError(t, err) - wKeystore, ok := keyStore.(WritableKeystore) - assert.True(t, ok) + writableKeystore, err := AsWritableKeystore(keyStore) + assert.NoError(t, err) - wKeystore.Store("hello", []byte("world")) - wKeystore.Save() + writableKeystore.Store("hello", []byte("world")) + writableKeystore.Save() newStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password)) s, _ := newStore.Retrieve("hello") @@ -323,10 +323,13 @@ func CreateAnExistingKeystore(path string) Keystore { panic(err) } - wKeystore, _ := keyStore.(WritableKeystore) + writableKeystore, err := AsWritableKeystore(keyStore) + if err != nil { + panic(err) + } - wKeystore.Store(keyValue, secretValue) - wKeystore.Save() + writableKeystore.Store(keyValue, secretValue) + writableKeystore.Save() return keyStore } diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index 19b58f8235c..45f1d4d83ba 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -31,6 +31,12 @@ var ( // ErrKeyDoesntExists is returned when the key doesn't exist in the store ErrKeyDoesntExists = errors.New("cannot retrieve the key") + + // ErrNotWritable is returned when the keystore is not writable + ErrNotWritable = errors.New("the configured keystore is not writable") + + // ErrNotWritable is returned when the keystore is not writable + ErrNotListing = errors.New("the configured keystore is not listing") ) // Keystore implement a way to securely saves and retrieves secrets to be used in the configuration @@ -89,3 +95,23 @@ func ResolverWrap(keystore Keystore) func(string) (string, parse.Config, error) return string(v), parse.DefaultConfig, nil } } + +// AsWritableKeystore casts a keystore to WritableKeystore, returning an error if the given keystore does not implement +// WritableKeystore interface +func AsWritableKeystore(store Keystore) (WritableKeystore, error) { + w, ok := store.(WritableKeystore) + if !ok { + return nil, ErrNotWritable + } + return w, nil +} + +// AsListingKeystore casts a keystore to ListingKeystore, returning an error if the given keystore does not implement +// ListingKeystore interface +func AsListingKeystore(store Keystore) (ListingKeystore, error) { + w, ok := store.(ListingKeystore) + if !ok { + return nil, ErrNotListing + } + return w, nil +} diff --git a/x-pack/libbeat/management/enroll.go b/x-pack/libbeat/management/enroll.go index e6b27f639af..407808ec7ae 100644 --- a/x-pack/libbeat/management/enroll.go +++ b/x-pack/libbeat/management/enroll.go @@ -90,9 +90,9 @@ func Enroll( func storeAccessToken(beat *instance.Beat, accessToken string) error { keyStore := beat.Keystore() - wKeystore, ok := keyStore.(keystore.WritableKeystore) - if !ok { - return fmt.Errorf("the configured keystore is not writable") + wKeystore, err := keystore.AsWritableKeystore(keyStore) + if err != nil { + return err } if !keyStore.IsPersisted() { From 4f06d7575dc97261ccd961eeffdbbcb93a79ab0b Mon Sep 17 00:00:00 2001 From: chrismark Date: Wed, 1 Apr 2020 18:11:25 +0300 Subject: [PATCH 9/9] Improve function docstrings Signed-off-by: chrismark --- libbeat/keystore/keystore.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libbeat/keystore/keystore.go b/libbeat/keystore/keystore.go index 45f1d4d83ba..340b83eb416 100644 --- a/libbeat/keystore/keystore.go +++ b/libbeat/keystore/keystore.go @@ -96,7 +96,7 @@ func ResolverWrap(keystore Keystore) func(string) (string, parse.Config, error) } } -// AsWritableKeystore casts a keystore to WritableKeystore, returning an error if the given keystore does not implement +// AsWritableKeystore casts a keystore to WritableKeystore, returning an ErrNotWritable error if the given keystore does not implement // WritableKeystore interface func AsWritableKeystore(store Keystore) (WritableKeystore, error) { w, ok := store.(WritableKeystore) @@ -106,7 +106,7 @@ func AsWritableKeystore(store Keystore) (WritableKeystore, error) { return w, nil } -// AsListingKeystore casts a keystore to ListingKeystore, returning an error if the given keystore does not implement +// AsListingKeystore casts a keystore to ListingKeystore, returning an ErrNotListing error if the given keystore does not implement // ListingKeystore interface func AsListingKeystore(store Keystore) (ListingKeystore, error) { w, ok := store.(ListingKeystore)