Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decrypt client TLS certificate key for Elastic Defend #5542

Merged

Conversation

AndersonQ
Copy link
Member

@AndersonQ AndersonQ commented Sep 13, 2024

What does this PR do?

It adds EndpointTLSComponentModifier which will check if the client certificate key is encrypted, if so, it'll decrypt the key and pass it decrypted to endpoint (Elastic Defend)

Why is it important?

Elastic Defend does not support passphrase-protected certificate key, but the agent does. It'll allow the agent to receive passphrase protected client certificate key and still work when Elastic Defend is installed.

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
  • [ ] I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
  • [ ] I have added an integration test or an E2E test

Author's checklist

Acceptance Criteria:

Disruptive User Impact

  • None

How to test this PR locally

adjust the IPs/hostnames as needed

you might need to build a 8.16 agent out of main:

AGENT_PACKAGE_VERSION=8.16.0-SNAPSHOT BEATS_VERSION=8.16.0-SNAPSHOT DEV=true SNAPSHOT=true EXTERNAL=true PLATFORMS="linux/amd64" PACKAGES="tar.gz" mage package

create 2 TLS certificates

use elastic-agent-libs/testing/certutil/cmd to create the certificates. Make sure to use [email protected]+ which supports generating RSA certificates as Elastic Defend only accepts RSA certificates.

go run main.go --prefix server --pass server --ips 10.80.40.162,127.0.0.1 --rsa
go run main.go --prefix client --pass client --ips 10.80.40.42,127.0.0.1 --rsa

you should have:

-rw------- 1 ainsoph ainsoph  288 Sep 20 18:35 client-ca_key.pem
-rw------- 1 ainsoph ainsoph  899 Sep 20 18:35 client-ca.pem
-rw------- 1 ainsoph ainsoph  399 Sep 20 18:35 client-localhost_enc-key.pem
-rw------- 1 ainsoph ainsoph  288 Sep 20 18:35 client-localhost_key.pem
-rw------- 1 ainsoph ainsoph    6 Sep 20 18:35 client-localhost-passphrase
-rw------- 1 ainsoph ainsoph  916 Sep 20 18:35 client-localhost.pem

-rw------- 1 ainsoph ainsoph  288 Sep 20 18:27 server-ca_key.pem
-rw------- 1 ainsoph ainsoph  904 Sep 20 18:27 server-ca.pem
-rw------- 1 ainsoph ainsoph  399 Sep 20 18:27 server-localhost_enc-key.pem
-rw------- 1 ainsoph ainsoph  288 Sep 20 18:27 server-localhost_key.pem
-rw------- 1 ainsoph ainsoph    6 Sep 20 18:27 server-localhost-passphrase
-rw------- 1 ainsoph ainsoph  916 Sep 20 18:27 server-localhost.pem

start a elastic stack (considering elastic-cloud)

add a fleet server with mTLS

elastic-agent install -nf \
--url=https://10.80.40.162:8220 \
--fleet-server-es=https://fleet.elastic-cloud.com:443 \
--fleet-server-service-token=a-token \
--fleet-server-policy=fleet-server-policy \
--certificate-authorities=/root/certs/server-ca.pem,/root/certs/client-ca.pem,/etc/ssl/certs/ca-certificates.crt \
--fleet-server-cert=/root/certs/server-localhost.pem \
--fleet-server-cert-key=/root/certs/server-localhost_enc-key.pem \
--fleet-server-cert-key-passphrase=/root/certs/server-localhost-passphrase \
--elastic-agent-cert=/root/certs/client-localhost.pem \
--elastic-agent-cert-key=/root/certs/client-localhost_enc-key.pem \
--elastic-agent-cert-key-passphrase=/root/certs/client-localhost-passphrase \
--fleet-server-client-auth=required \
--fleet-server-port=8220

create a policy with Elastic Defend

add an agent to that policy

elastic-agent install -nf \
--url=https://10.80.40.162:8220 \
--enrollment-token=a-token
--certificate-authorities=/root/certs/server-ca.pem,/etc/ssl/certs/ca-certificates.crt \
--elastic-agent-cert=/root/certs/client-localhost.pem \
--elastic-agent-cert-key=/root/certs/client-localhost_enc-key.pem \
--elastic-agent-cert-key-passphrase=/root/certs/client-localhost-passphrase

Related issues

Questions to ask yourself

  • How are we going to support this in production?
  • How are we going to measure its adoption?
  • How are we going to debug this?
  • What are the metrics I should take care of?
  • ...

@AndersonQ AndersonQ added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label Sep 13, 2024
@AndersonQ AndersonQ self-assigned this Sep 13, 2024
Copy link
Contributor

mergify bot commented Sep 13, 2024

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

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

Copy link
Contributor

mergify bot commented Sep 13, 2024

backport-v8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Sep 13, 2024
@AndersonQ AndersonQ force-pushed the 5490-pass-decrypted-cert-key-to-defend branch 4 times, most recently from 716f0a9 to e00b883 Compare September 19, 2024 13:05
@ycombinator ycombinator force-pushed the 5490-pass-decrypted-cert-key-to-defend branch from e00b883 to d081a2a Compare September 20, 2024 00:33
@AndersonQ AndersonQ force-pushed the 5490-pass-decrypted-cert-key-to-defend branch from ae1e154 to 673335f Compare September 20, 2024 15:24
It adds EndpointTLSComponentModifier which will check if the client certificate key is encrypted, if so, it'll decrypt the key and pass it decrypted to endpoint (Elastic Defend)
@AndersonQ AndersonQ force-pushed the 5490-pass-decrypted-cert-key-to-defend branch from 673335f to 2b6e38f Compare September 27, 2024 15:05
@AndersonQ AndersonQ marked this pull request as ready for review September 27, 2024 15:22
@AndersonQ AndersonQ requested a review from a team as a code owner September 27, 2024 15:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am worried about the performance of this component modifier. Most modifiers should just do some small movements or inject a little bit of extra configration.

This modifier is read a file and decrypting it every time the component model changes, which can be a lot. When using say the kubernetes provider with composable inputs it will recompute the model on every new pod state.

I believe this needs to be changed to only do that work once and only inject the cached content.

@AndersonQ AndersonQ changed the title WIP: decrypt client TLS certificate key for Elastic Defend Decrypt client TLS certificate key for Elastic Defend Sep 30, 2024
@AndersonQ
Copy link
Member Author

I am worried about the performance of this component modifier. Most modifiers should just do some small movements or inject a little bit of extra configration.

This modifier is read a file and decrypting it every time the component model changes, which can be a lot. When using say the kubernetes provider with composable inputs it will recompute the model on every new pod state.

I believe this needs to be changed to only do that work once and only inject the cached content.

good point, even thought I'm not sure if defend would be used on such environments. Anyway I added a local cache for the component modifier and it's also thread-safe as the modifiers might be invoked from the coordinator main goroutine and for diagnostics as well.

@ycombinator ycombinator removed the request for review from pkoutsovasilis October 1, 2024 23:58
@AndersonQ AndersonQ requested a review from blakerouse October 2, 2024 07:14
Copy link
Contributor

mergify bot commented Oct 10, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 5490-pass-decrypted-cert-key-to-defend upstream/5490-pass-decrypted-cert-key-to-defend
git merge upstream/main
git push upstream 5490-pass-decrypted-cert-key-to-defend

@AndersonQ
Copy link
Member Author

/test

@AndersonQ AndersonQ enabled auto-merge (squash) October 14, 2024 12:33
@AndersonQ AndersonQ disabled auto-merge October 15, 2024 09:41
@AndersonQ AndersonQ enabled auto-merge (squash) October 15, 2024 14:45
Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the caching key!

Looks good now.

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube

@pierrehilbert pierrehilbert merged commit 1c041a2 into elastic:main Oct 15, 2024
13 of 14 checks passed
mergify bot pushed a commit that referenced this pull request Oct 15, 2024
* decrypt client mTLS certificate key for Elastic Defend

It adds EndpointTLSComponentModifier which will check if the client certificate key is encrypted, if so, it'll decrypt the key and pass it decrypted to endpoint (Elastic Defend)

* add cache and tests

* fix cache miss test

* fix linter

* update elastic-agent-libs

* better comments

* fix comment

* mage notice

* update elastic-agent-libs

* debug test

* fix test

* Revert "debug test"

This reverts commit b04b42c.

* make cache key from all paths

(cherry picked from commit 1c041a2)
@cmacknz
Copy link
Member

cmacknz commented Oct 15, 2024

Is there anywhere in our logs or diagnistcs .yaml files where the decrypted key could be logged accidentally (expected-config.yaml maybe)? Did we look through the diagnostics to check for this?

The file path likely won't be marked as a secret explicitly because it is obscured by default.

pierrehilbert pushed a commit that referenced this pull request Oct 15, 2024
* decrypt client mTLS certificate key for Elastic Defend

It adds EndpointTLSComponentModifier which will check if the client certificate key is encrypted, if so, it'll decrypt the key and pass it decrypted to endpoint (Elastic Defend)

* add cache and tests

* fix cache miss test

* fix linter

* update elastic-agent-libs

* better comments

* fix comment

* mage notice

* update elastic-agent-libs

* debug test

* fix test

* Revert "debug test"

This reverts commit b04b42c.

* make cache key from all paths

(cherry picked from commit 1c041a2)

Co-authored-by: Anderson Queiroz <[email protected]>
@AndersonQ AndersonQ deleted the 5490-pass-decrypted-cert-key-to-defend branch October 16, 2024 12:24
@AndersonQ
Copy link
Member Author

Is there anywhere in our logs or diagnistcs .yaml files where the decrypted key could be logged accidentally (expected-config.yaml maybe)? Did we look through the diagnostics to check for this?

The file path likely won't be marked as a secret explicitly because it is obscured by default.

Not really, the agent already accepts either a certificate or a filepath in the configuration and everything is correctly handled. The change here does not introduce anything new. So unless something was already leaking, I don't see hot the changes here would create a new leak.

However I must say I haven't done explicit test for that

@cmacknz
Copy link
Member

cmacknz commented Oct 16, 2024

However I must say I haven't done explicit test for that

I don't think we need a test, but a manual spot check searching through diagnostics should be done. Leaked TLS private keys is a bad time for everybody.

@AndersonQ
Copy link
Member Author

a manual spot check searching through diagnostics should be done. Leaked TLS private keys is a bad time for everybody.

we could add it to as a requirement on one of the manual tests for new releases. It'd cover this and any other change we might make

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.x Automated backport to the 8.x branch with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass the decrypted mTLS client certificate key to Elastic Defend
7 participants