Agent configuration encryption #398

aleksmaus · 2022-05-02T14:00:53Z

What does this PR do?

Implements the key material storage and the agent configuration and state encryption.
Providing the bulletproof key material storage across all the platforms is fairly difficult.
The open source nature of the project is making it even more easier to figure any obfuscation techniques that we could have used to make it difficult to reverse engineer the key protection algorithm.

Opening this PR draft to gather feedback. If we agree on the general approach there are still some TODO items to finalize this change

TODO

Improve unit test coverage
Retest the agent on all platforms (darwin, linux, windows)

Implementation details

Here is the approach that was taken on the different platforms at the moment:

Darwin (Mac OS)

The key material is stored in the System keychain. The value is stored as is without any additional transformations.

Windows

The data is encrypted with DPAPI CryptProtectData with CRYPTPROTECT_LOCAL_MACHINE.
The additional entropy is derived from crypto/rand bytes stored in .seed file.
The data is stored as separate files where the name of the file is SHA256 hash of the key and the content of the file is encrypted with DPAPI data.
The security of entropy data relies on file system permissions. Only the Administrator should be able to access the file.

Linux

The encryption key is derived from crypto/rand bytes that stored in the .seed file after PBKDF2 transformation.
The data is stored as separate files where the name of the file is SHA256 hash of the key and the content of the file is AES256-GSM encrypted.
The security if the key material largely relies on the files system permissions.

The Agent modifications

In this POC the Agent creates the encryption key during enrollment. I would probably need the feedback from the agent team on all the possible use cases, maybe the key should be created in the different place
The new EncryptedDiskStore was implemented as a drop-in replacement for the DiskStore. The EncryptedDiskStore uses already /internal/pkg/crypto Reader and Writer implementation, which uses PBKDF2 for key derivation with AES-GCM encryption. We can change this to whatever else we prefer. Let me know.
The Agent configuration (fleet.yml) is encrypted with the ".enc" file extension (fleet.enc).

The Agent state state.yml is encrypted with the ".enc" file extension (state.enc):

Why is it important?

Address the configuration encryption at rest requirement.
https://github.com/elastic/obs-dc-team/issues/535

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation

How to test this PR locally

Full regression testing of the agent, including enrollment, upgrade etc.

Related issues

Closes https://github.com/elastic/obs-dc-team/issues/535

mergify · 2022-05-02T14:01:00Z

This pull request does not have a backport label. Could you fix it @aleksmaus? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2022-05-02T14:05:21Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-05-23T19:16:04.535+0000
Duration: 17 min 32 sec

Test stats 🧪

Test	Results
Failed	0
Passed	5778
Skipped	23
Total	5801

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages.
run integration tests : Run the Elastic Agent Integration tests.
run end-to-end tests : Generate the packages and run the E2E Tests.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

leehinman

nice

One request, could we add meta data to the keys? Specifically date created would be really nice if we have to do any kind of key rotation.

internal/pkg/agent/storage/encrypted_disk_store.go

leehinman · 2022-05-02T17:52:01Z

internal/pkg/agent/storage/encrypted_disk_store.go

+	// Wrap into crypto writer, reusing already existing crypto writer, open to other suggestions
+	w, err := crypto.NewWriterWithDefaults(fd, d.key)
+	if err != nil {
+		fd.Close()


tmp file not removed in this case, is that expected?

internal/pkg/agent/storage/encrypted_disk_store.go

internal/pkg/agent/vault/aesgcm.go

leehinman · 2022-05-02T18:13:33Z

internal/pkg/agent/vault/aesgcm.go

+	if _, err = rand.Read(nonce); err != nil {
+		return nil, err
+	}
+	ciphertext := aesGCM.Seal(nonce, nonce, data, nil)


is the first nonce really supposed to be the dst?

yes, this way the ciphertext is the concatenation of nonce + encrypted data, thats what is intent here

makes sense. might be worth a comment, on first read it looks a little odd.

leehinman · 2022-05-02T18:18:41Z

internal/pkg/agent/vault/seed.go

+
+func getSeed(path string) ([]byte, error) {
+	fp := filepath.Join(path, seedFile)
+	b, err := ioutil.ReadFile(fp)


might be a good idea to check permissions of seed file and complain if they are too permissive.

added the root owner check. the agent will fail to run in .seed file is not root/root owned

$ sudo ./elastic-agent -e Error: could not read overwrites: could not initialize config store: non-root file owner For help, please see our troubleshooting guide at https://www.elastic.co/guide/en/fleet/8.3/fleet-troubleshooting.html

@nimarezainia, do we have plans to allow users to run the agent as non-root?

@nimarezainia, do we have plans to allow users to run the agent as non-root?

Not at the moment.

internal/pkg/agent/vault/vault_linux.go

aleksmaus · 2022-05-02T19:33:41Z

nice

One request, could we add meta data to the keys? Specifically date created would be really nice if we have to do any kind of key rotation.

sure, can encode that into stored data.

aleksmaus · 2022-05-03T20:56:05Z

nice
One request, could we add meta data to the keys? Specifically date created would be really nice if we have to do any kind of key rotation.

sure, can encode that into stored data.

done now, the value is encoded as JSON

// Secret is the structure that is JSON serialized and stored
type Secret struct {
	Value     []byte    `json:"v"` // binary value
	CreatedOn time.Time `json:"t"` // date/time the secret was created on
}

…tate.yml into fleet.enc and state.enc after the encryption

… sudo, thus don't have access to the system keychain

…(nakedret)

unnecessary conversion (unconvert)

…encrypted storage on any of the platforms

This reverts commit c3d3c4c.

amolnater-qasource · 2022-06-06T12:14:34Z

Hi @jlind23
We have validated this PR on 8.3 BC2 Kibana cloud environment and had below observations:

Installed Windows, Linux and MAC agents with System integration and observed no issues with logs and data under Data Streams tab.
For enhanced testing we have validated the same with Endpoint Security integration and observed no issues there as well.

Further we have observed the files like:

fleet.enc at location Elastic/Agent for all the 3 OS's.
state.enc at Elastic/Agent/data/elastic-agent-8907dd/ for all the 3 OS's.
.seed file at location Elastic/Agent/data/elastic-agent-8907dd/vault for Windows and Linux agents.

We haven't observed vault folder for MAC agent. Could you please confirm if this is expected?

Build details:
VERSION: 8.3.0
BUILD: 53231
COMMIT: 25476b531ba9f32292bde85508d342aa5e1c29eb
Artifact Link: https://staging.elastic.co/8.3.0-fcd43da7/summary-8.3.0.html

NOTE:
Further we are in continuation of agent regression under P1 regression plan and will cover all the related agent scenarios.

Please let us know if we are missing anything here.
Thanks

aleksmaus · 2022-06-06T12:17:51Z

We haven't observed vault folder for MAC agent. Could you please confirm if this is expected?

For Mac OS we use keychain, no vault folder is needed.

amolnater-qasource · 2022-06-07T06:46:09Z

Hi @aleksmaus
Thank you for the update. Could you please confirm we need to run any other test related to this PR on Agents?

Please let us know if we are missing anything here.
cc: @jlind23
Thanks

aleksmaus · 2022-06-07T11:14:46Z

Hi @aleksmaus Thank you for the update. Could you please confirm we need to run any other test related to this PR on Agents?

Please let us know if we are missing anything here. cc: @jlind23 Thanks

Full regression testing of Elastic Agent would be necessary to make sure we didn't miss any code paths when transitioned to the encrypted storage.

amolnater-qasource · 2022-06-08T11:48:22Z

Hi @aleksmaus
Thanks for the feedback.
We are in continuation of agent tests under Fleet Team 8.3 BC2 Regression plan.
Further we have reported agent upgrade failure at #526

Thanks

michalpristas · 2022-06-09T07:44:49Z

@amolnater-qasource have you tried standalone mode with this PR?

amolnater-qasource · 2022-06-09T10:49:21Z

Hi @michalpristas
Earlier we were having issues with standalone agent installation and reported at elastic/kibana#133276

However we had revalidated this issue on latest 8.3 BC2 and found this fixed.
Please let us know if any we need to validate this on any other build.

Thanks

@cmacknz

Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users.

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co>

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co> (cherry picked from commit 7996f31)

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co> (cherry picked from commit 7996f31) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co> (cherry picked from commit 7996f31)

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co> (cherry picked from commit 7996f31) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co>

@cmacknz

* [agent] Add documentation for Agent encryption at rest Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users. * Apply suggestions from code review Thanks for the copy edits Dede! Co-authored-by: DeDe Morton <dede.morton@elastic.co> Co-authored-by: DeDe Morton <dede.morton@elastic.co>

Agent configuration encryption

c10b579

aleksmaus requested a review from scunningham May 2, 2022 14:00

mergify bot assigned aleksmaus May 2, 2022

mergify bot added the backport-skip label May 2, 2022

jlind23 added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label May 2, 2022

ph self-requested a review May 2, 2022 14:46

jlind23 requested review from a team and narph and removed request for a team May 2, 2022 15:21

cmacknz requested a review from leehinman May 2, 2022 15:54

ph requested a review from blakerouse May 2, 2022 18:39

leehinman reviewed May 2, 2022

View reviewed changes

aleksmaus added 4 commits May 2, 2022 16:08

Make linter happy

6afad66

Address code review feedback

17214e0

Make linux linter happy

750d0a4

Encode created on date/time with the vault key/value

e94ccc7

aleksmaus added 2 commits May 3, 2022 18:25

Check .seed file owner is root on load

5922e7b

Update upgrade code paths, rename configuration files fleet.yml and s…

73da02a

…tate.yml into fleet.enc and state.enc after the encryption

aleksmaus force-pushed the feature/vault branch from c53341d to 73da02a Compare May 4, 2022 16:42

aleksmaus added 7 commits May 4, 2022 12:51

Make linter happy again

a00e7f9

Disable storage encryption for Darwin unit tests that do not run with…

0ea9f6c

… sudo, thus don't have access to the system keychain

Make linter happy for the last commit touched files

4e0d215

Make check happy

332cd63

Fix Winderz linter: Error: naked return in func with 7 lines of code …

bfaa47f

…(nakedret)

Fix Winderz linter: internal/pkg/agent/vault/vault_windows.go#L57

05dcce7

unnecessary conversion (unconvert)

Remove .seed file root owner check, makes it impossible to unit test …

2bff2cb

…encrypted storage on any of the platforms

jlind23 added QA:Needs Validation Needs validation by the QA Team and removed QA:Ready For Testing Code is merged and ready for QA to validate labels May 25, 2022

mdelapenya mentioned this pull request Jun 1, 2022

fix: agent directories must belong to root elastic/e2e-testing#2590

Merged

8 tasks

narph mentioned this pull request Jun 1, 2022

Update Elastic Agent docker image for azure containers runtime #82

Closed

joshdover mentioned this pull request Jun 1, 2022

[Fleet]: On fresh 8.3.0-Snapshot kibana environment fleet tab shows 'Enable fleet tab' and fleet tab is not accessible. elastic/fleet-server#1510

Closed

aleksmaus added a commit to aleksmaus/elastic-agent that referenced this pull request Jun 3, 2022

Revert "Agent configuration encryption (elastic#398)"

9680f2e

This reverts commit c3d3c4c.

aleksmaus mentioned this pull request Jun 3, 2022

Revert "Agent configuration encryption (#398)" #511

Closed

2 tasks

cmacknz mentioned this pull request Jun 20, 2022

[Shipper][Design] Generate and securely store an encryption key for the shipper's encrypted disk queue #582

Closed

amolnater-qasource added the QA:Validated Validated by the QA Team label Jun 30, 2022

simitt mentioned this pull request Sep 22, 2022

Support configuring a TLS key passphrase in APM Integration elastic/apm-server#9164

Open

cleydyr mentioned this pull request Nov 14, 2022

Fix installation layout docs elastic/observability-docs#2359

Merged

andrewvc added a commit to andrewvc/observability-docs that referenced this pull request Nov 15, 2022

[agent] Add documentation for Agent encryption at rest

62331d1

Based on elastic/elastic-agent#398 and a discussion with @cmacknz . It's a somewhat common ask from synthetics users.

andrewvc mentioned this pull request Nov 15, 2022

[agent] Add documentation for Agent encryption at rest elastic/observability-docs#2368

Merged

cmacknz mentioned this pull request Feb 10, 2023

Agent does not load fleet.yml after upgrade from 7.17 to 8.x #2249

Closed

ycombinator mentioned this pull request May 9, 2023

[Dev] Clean up deprecation warnings from vault_darwin.c #2639

Closed

cmacknz mentioned this pull request Apr 4, 2024

Fix and refactor the state store #4441

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Agent configuration encryption #398

Agent configuration encryption #398

aleksmaus commented May 2, 2022 •

edited

Loading

mergify bot commented May 2, 2022

elasticmachine commented May 2, 2022 •

edited

Loading

Build stats

Test stats 🧪

leehinman left a comment

leehinman May 2, 2022

aleksmaus May 2, 2022

leehinman May 2, 2022

aleksmaus May 2, 2022

leehinman May 2, 2022

aleksmaus May 2, 2022

leehinman May 2, 2022

aleksmaus May 3, 2022

michel-laterman May 6, 2022

nimarezainia May 16, 2022

aleksmaus commented May 2, 2022

aleksmaus commented May 3, 2022

amolnater-qasource commented Jun 6, 2022

aleksmaus commented Jun 6, 2022

amolnater-qasource commented Jun 7, 2022

aleksmaus commented Jun 7, 2022

amolnater-qasource commented Jun 8, 2022

michalpristas commented Jun 9, 2022

amolnater-qasource commented Jun 9, 2022

Agent configuration encryption #398

Agent configuration encryption #398

Conversation

aleksmaus commented May 2, 2022 • edited Loading

What does this PR do?

TODO

Implementation details

Darwin (Mac OS)

Windows

Linux

The Agent modifications

Why is it important?

Checklist

How to test this PR locally

Related issues

mergify bot commented May 2, 2022

elasticmachine commented May 2, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

leehinman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aleksmaus commented May 2, 2022

aleksmaus commented May 3, 2022

amolnater-qasource commented Jun 6, 2022

aleksmaus commented Jun 6, 2022

amolnater-qasource commented Jun 7, 2022

aleksmaus commented Jun 7, 2022

amolnater-qasource commented Jun 8, 2022

michalpristas commented Jun 9, 2022

amolnater-qasource commented Jun 9, 2022

aleksmaus commented May 2, 2022 •

edited

Loading

elasticmachine commented May 2, 2022 •

edited

Loading