Skip to content

Commit

Permalink
Cherry-pick elastic#19678 to 7.x: [Elastic Agent] Harden permissions …
Browse files Browse the repository at this point in the history
…and remove encrypted store (elastic#19707)

* [Elastic Agent] Harden permissions and remove encrypted store (elastic#19678)

* Harden permissions, remove encrypted store.

* Add changelog.

* Fix issues with DiskStore error handling.

(cherry picked from commit b8c49e0)

* Run mage fmt
  • Loading branch information
blakerouse authored Jul 9, 2020
1 parent 6ad98a3 commit 044c3fe
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 127 deletions.
1 change: 1 addition & 0 deletions x-pack/elastic-agent/CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
==== Breaking changes
- Rename agent to elastic-agent {pull}17391[17391]
- Change fleet.yml structure, causes upgraded agent to register as new agent {pull}19248[19248]
- Remove obfuscation of fleet.yml, causes re-enroll of agent to Fleet {pull}19678[19678]

==== Bugfixes

Expand Down
9 changes: 8 additions & 1 deletion x-pack/elastic-agent/pkg/agent/application/action_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@ func newActionStore(log *logger.Logger, store storeLoad) (*actionStore, error) {
var action actionConfigChangeSerializer

dec := yaml.NewDecoder(reader)
if err := dec.Decode(&action); err != nil {
err = dec.Decode(&action)
if err == io.EOF {
return &actionStore{
log: log,
store: store,
}, nil
}
if err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion x-pack/elastic-agent/pkg/agent/application/enroll_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func NewEnrollCmd(
store := storage.NewReplaceOnSuccessStore(
configPath,
DefaultAgentFleetConfig,
storage.NewEncryptedDiskStore(info.AgentConfigFile(), []byte("")),
storage.NewDiskStore(info.AgentConfigFile()),
)

return NewEnrollCmdWithStore(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func generateAgentID() (string, error) {

func loadAgentInfo(forceUpdate bool) (*persistentAgentInfo, error) {
agentConfigFile := AgentConfigFile()
s := storage.NewEncryptedDiskStore(agentConfigFile, []byte(""))
s := storage.NewDiskStore(agentConfigFile)

agentinfo, err := getInfoFromStore(s)
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions x-pack/elastic-agent/pkg/agent/application/managed_mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ func newManaged(

path := info.AgentConfigFile()

// TODO(ph): Define the encryption password.
store := storage.NewEncryptedDiskStore(path, []byte(""))
store := storage.NewDiskStore(path)
reader, err := store.Load()
if err != nil {
return nil, errors.New(err, "could not initialize config store",
Expand Down
83 changes: 3 additions & 80 deletions x-pack/elastic-agent/pkg/agent/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ import (

"github.com/elastic/beats/v7/libbeat/common/file"
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors"
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/crypto"
)

const perms = 0600
const perms os.FileMode = 0600

type store interface {
Save(io.Reader) error
Expand Down Expand Up @@ -76,9 +75,6 @@ func NewReplaceOnSuccessStore(target string, replaceWith []byte, wrapped store)

// Save will replace a target file with new content if the wrapped store is successful.
func (r *ReplaceOnSuccessStore) Save(in io.Reader) error {
// get original permission
s, err := os.Stat(r.target)

// Ensure we can read the target files before delegating any call to the wrapped store.
target, err := ioutil.ReadFile(r.target)
if err != nil {
Expand Down Expand Up @@ -111,7 +107,7 @@ func (r *ReplaceOnSuccessStore) Save(in io.Reader) error {
errors.M(errors.MetaKeyPath, r.target))
}

fd, err := os.OpenFile(r.target, os.O_CREATE|os.O_WRONLY, s.Mode())
fd, err := os.OpenFile(r.target, os.O_CREATE|os.O_WRONLY, perms)
if err != nil {
// Rollback on any errors to minimize non working state.
if err := file.SafeFileRotate(r.target, backFilename); err != nil {
Expand Down Expand Up @@ -187,85 +183,12 @@ func (d *DiskStore) Save(in io.Reader) error {

// Load return a io.ReadCloser for the target file.
func (d *DiskStore) Load() (io.ReadCloser, error) {
return os.OpenFile(d.target, os.O_RDONLY, perms)
}

// EncryptedDiskStore save the persisted configuration and encrypt the data on disk.
type EncryptedDiskStore struct {
target string
password []byte
}

// NewEncryptedDiskStore creates an encrypted disk store.
func NewEncryptedDiskStore(target string, password []byte) *EncryptedDiskStore {
return &EncryptedDiskStore{target: target, password: password}
}

// Save accepts a persistedConfig, encrypt it and saved it to a target file, to do so we will
// make a temporary files if the write is successful we are replacing the target file with the
// original content.
func (d *EncryptedDiskStore) Save(in io.Reader) error {
const perms = 0600

tmpFile := d.target + ".tmp"

fd, err := os.OpenFile(tmpFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, perms)
if err != nil {
return errors.New(err,
fmt.Sprintf("could not save to %s", tmpFile),
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, tmpFile))
}
// Always clean up the temporary file and ignore errors.
defer os.Remove(tmpFile)

w, err := crypto.NewWriterWithDefaults(fd, d.password)
if err != nil {
return errors.New(err, "could not encrypt the data to disk",
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, tmpFile))
}

if _, err := io.Copy(w, in); err != nil {
return errors.New(err, "could not save content on disk",
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, tmpFile))
}

if err := fd.Close(); err != nil {
return errors.New(err, "could not close temporary file",
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, tmpFile))
}

if err := file.SafeFileRotate(d.target, tmpFile); err != nil {
return errors.New(err,
fmt.Sprintf("could not replace target file %s", d.target),
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, d.target))
}

return nil
}

// Load return a io.ReadCloser that will take care on unencrypting the data.
func (d *EncryptedDiskStore) Load() (io.ReadCloser, error) {
fd, err := os.OpenFile(d.target, os.O_RDONLY|os.O_CREATE, perms)
if err != nil {
return nil, errors.New(err,
fmt.Sprintf("could not open %s", d.target),
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, d.target))
}

r, err := crypto.NewReaderWithDefaults(fd, d.password)
if err != nil {
fd.Close()
return nil, errors.New(err,
fmt.Sprintf("could not decode file %s", d.target),
errors.TypeFilesystem,
errors.M(errors.MetaKeyPath, d.target))
}

return r, nil
return fd, nil
}
66 changes: 25 additions & 41 deletions x-pack/elastic-agent/pkg/agent/storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ func TestReplaceOrRollbackStore(t *testing.T) {

require.True(t, bytes.Equal(writtenContent, replaceWith))
requireFilesCount(t, dir, 2)

info, err := os.Stat(target)
require.NoError(t, err)

require.Equal(t, perms, info.Mode())
})

t.Run("when save is not successful", func(t *testing.T) {
Expand Down Expand Up @@ -97,6 +102,11 @@ func TestReplaceOrRollbackStore(t *testing.T) {

require.True(t, bytes.Equal(writtenContent, replaceWith))
requireFilesCount(t, dir, 1)

info, err := os.Stat(target)
require.NoError(t, err)

require.Equal(t, perms, info.Mode())
})

t.Run("when target file do not exist", func(t *testing.T) {
Expand All @@ -115,7 +125,7 @@ func TestDiskStore(t *testing.T) {
target, err := genFile([]byte("hello world"))
require.NoError(t, err)
defer os.Remove(target)
d := &DiskStore{target: target}
d := NewDiskStore(target)

msg := []byte("bonjour la famille")
err = d.Save(bytes.NewReader(msg))
Expand All @@ -125,6 +135,11 @@ func TestDiskStore(t *testing.T) {
require.NoError(t, err)

require.Equal(t, msg, content)

info, err := os.Stat(target)
require.NoError(t, err)

require.Equal(t, perms, info.Mode())
})

t.Run("when the target do no exist", func(t *testing.T) {
Expand All @@ -133,7 +148,7 @@ func TestDiskStore(t *testing.T) {
defer os.Remove(dir)

target := filepath.Join(dir, "hello.txt")
d := &DiskStore{target: target}
d := NewDiskStore(target)

msg := []byte("bonjour la famille")
err = d.Save(bytes.NewReader(msg))
Expand All @@ -143,62 +158,31 @@ func TestDiskStore(t *testing.T) {
require.NoError(t, err)

require.Equal(t, msg, content)

info, err := os.Stat(target)
require.NoError(t, err)

require.Equal(t, perms, info.Mode())
})

t.Run("return an io.ReadCloser to the target file", func(t *testing.T) {
msg := []byte("bonjour la famille")
target, err := genFile(msg)
require.NoError(t, err)

d := &DiskStore{target: target}
d := NewDiskStore(target)
r, err := d.Load()
require.NoError(t, err)
defer r.Close()

content, err := ioutil.ReadAll(r)
require.NoError(t, err)
require.Equal(t, msg, content)
})
}

func TestEncryptedDiskStore(t *testing.T) {
t.Run("when the target file already exists", func(t *testing.T) {
target, err := genFile([]byte("hello world"))
info, err := os.Stat(target)
require.NoError(t, err)
defer os.Remove(target)
d := &EncryptedDiskStore{target: target}

msg := []byte("bonjour la famille")
err = d.Save(bytes.NewReader(msg))
require.NoError(t, err)

// lets read the file
nd := &EncryptedDiskStore{target: target}
r, err := nd.Load()
require.NoError(t, err)

content, err := ioutil.ReadAll(r)
require.NoError(t, err)

require.Equal(t, msg, content)
})

t.Run("when the target do not exist", func(t *testing.T) {
dir, err := ioutil.TempDir("", "configs")
require.NoError(t, err)
defer os.Remove(dir)

target := filepath.Join(dir, "hello.txt")
d := &DiskStore{target: target}

msg := []byte("bonjour la famille")
err = d.Save(bytes.NewReader(msg))
require.NoError(t, err)

content, err := ioutil.ReadFile(target)
require.NoError(t, err)

require.Equal(t, msg, content)
require.Equal(t, perms, info.Mode())
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (b *Monitor) EnrichArgs(process, pipelineID string, args []string, isSideca
"-E", "logging.files.path="+loggingPath,
"-E", "logging.files.name="+logFile,
"-E", "logging.files.keepfiles=7",
"-E", "logging.files.permission=0644",
"-E", "logging.files.permission=0640",
"-E", "logging.files.interval=1h",
)
}
Expand Down

0 comments on commit 044c3fe

Please sign in to comment.