Read and apply mTLS config from policy #4398

AndersonQ · 2024-03-11T15:49:23Z

What does this PR do?

Receive and apply:

fleet.ssl.certificate_authorities
fleet.ssl.certificate
fleet.ssl.key
fleet.ssl.key_passphrase
fleet.ssl.key_passphrase_path

The policy provided config has precedence over the CLI parameters and an empty or absent config on a policy is ignored.

Why is it important?

The Elastic Agent should support mTLS for communicating with fleet-server.

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~~

How to test this PR locally

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?
...

elasticmachine · 2024-03-11T15:49:26Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

AndersonQ · 2024-03-11T15:50:12Z

dev-tools/notice/overrides.json

@@ -1,5 +1,6 @@
 {"name": "github.com/elastic/beats/v7", "licenceType": "Elastic"}
 {"name": "github.com/elastic/elastic-agent-client/v7", "licenceType": "Elastic"}
+{"name": "github.com/AndersonQ/elastic-agent-libs", "licenceType": "Elastic"}


Waiting elastic/elastic-agent-libs#189 to be merged

AndersonQ · 2024-03-11T15:50:23Z

go.mod

@@ -282,3 +282,5 @@ replace (

 // Exclude this version because the version has an invalid checksum.
 exclude github.com/docker/distribution v2.8.0+incompatible
+
+replace github.com/elastic/elastic-agent-libs => github.com/AndersonQ/elastic-agent-libs v0.0.0-20240307144511-b45f19396c67


Waiting elastic/elastic-agent-libs#189 to be merged

.

mergify · 2024-03-19T16:17:17Z

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 2247-mtls upstream/2247-mtls
git merge upstream/main
git push upstream 2247-mtls

AndersonQ · 2024-03-19T16:18:52Z

dev-tools/notice/overrides.json

@@ -1,5 +1,6 @@
 {"name": "github.com/elastic/beats/v7", "licenceType": "Elastic"}
 {"name": "github.com/elastic/elastic-agent-client/v7", "licenceType": "Elastic"}
+{"name": "github.com/AndersonQ/elastic-agent-libs", "licenceType": "Apache-2.0"}


Waiting elastic/elastic-agent-libs#189 to be merged

cmacknz · 2024-03-20T20:41:26Z

internal/pkg/agent/application/actions/handlers/handler_action_policy_change.go

 return false
 }
 }

+ // should it ignore empty headers?


Empty headers from Fleet? This should have the same precedence rules as everything else.

yes, empty proxy headers. Thinking better bout it, if there is a new or non-nil/non-empty proxy config, if the new has no headers, we need to override them. So I think it's correct as it's right now

cmacknz · 2024-03-20T20:53:55Z

internal/pkg/agent/application/actions/handlers/handler_action_policy_change.go

+ if cfg.Transport.TLS.Certificate == emptyCertificate {
+ h.log.Debug("TLS certificate/key from fleet are empty or null, the TLS config will not be changed")
+ } else {
+ h.config.Fleet.Client.Transport.TLS.Certificate = cfg.Transport.TLS.Certificate


Is the Certificate configuration from Fleet always complete? Does it or could it ever allow updating only individual fields?

If it does this unintentionally clears the ones that weren't set, for example if someone sets only the Certificate but not the Key and Key were set previously.

I suppose if this happened unintentionally and broke things it would be reverted, but it would be hard for a user to debug.

We either need to make debugging that failure easier or just prevent it from happening entirely.

Is the Certificate configuration from Fleet always complete?
Well, I don't see how one would change a certificate and not the key and the key's passphrase.

Besides again we're in the problem of "remove it" or "no change". Changing it all atomically seems to me the best and less error prone approach.

…247-mtls

AndersonQ · 2024-03-21T15:22:46Z

@cmacknz I did the changes or replied to your comments. Could you re-review

cmacknz · 2024-03-21T19:13:59Z

Was this tested against a real instance of Fleet (or Fleet Server)?

Is there any way we can integration test this? If anything here is broken in an unexpected way Fleet connectivity gets completely broken so lets be as thorough as we can.

These questions aren't answered yet :)

If you are testing this manually, the instructions need to be the PR description.

AndersonQ · 2024-03-22T06:34:53Z

my bad, I missed them.

Was this tested against a real instance of Fleet (or Fleet Server)?

No, but I can test

Is there any way we can integration test this? If anything here is broken in an unexpected way Fleet connectivity gets completely broken so lets be as thorough as we can.

I don't think we can with our current setup. It cannot be tested with Elastic Cloud, but we could modify the mock fleet-server to have TLS or the mock proxy. Either would do

…247-mtls

elastic-sonarqube · 2024-04-03T06:45:04Z

Quality Gate passed

The SonarQube Quality Gate passed, but some issues were introduced.

3 New issues
0 Security Hotspots
68.0% 68.0% Coverage on New Code
0.0% 0.0% Duplication on New Code

See analysis details on SonarQube

michalpristas · 2024-04-03T07:29:17Z

internal/pkg/agent/application/actions/handlers/handler_action_policy_change.go

+ defaultcfg := configuration.DefaultFleetAgentConfig()
+
+ if cfg.Protocol != defaultcfg.Client.Protocol ||
+ cfg.Host != defaultcfg.Client.Host ||


what if you're changing from localhost:5602 to localhost:5601

michalpristas · 2024-04-03T07:33:54Z

internal/pkg/agent/application/actions/handlers/handler_action_policy_change.go

+ "new_proxy_url", cfg.Transport.Proxy.URL.String())
+ } else {
+ h.log.Debugw("received proxy from fleet, applying it",
+ "old_proxy_url", "nil",


can be simplified with string variable oldProxy = h.config.Fleet.Client.Transport.Proxy.URL != nil ? h.config.Fleet.Client.Transport.Proxy.URL.String() : "nil"
c# syntax but you get the idea

mergify · 2024-04-15T15:19:53Z

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 2247-mtls upstream/2247-mtls
git merge upstream/main
git push upstream 2247-mtls

ycombinator · 2024-05-10T21:12:11Z

I don't think we can with our current setup. It cannot be tested with Elastic Cloud, but we could modify the mock fleet-server to have TLS or the mock proxy. Either would do

#4497

AndersonQ added Team:Elastic-Agent Label for the Agent team backport-skip labels Mar 11, 2024

AndersonQ self-assigned this Mar 11, 2024

AndersonQ requested a review from a team as a code owner March 11, 2024 15:49

AndersonQ requested review from michalpristas and blakerouse March 11, 2024 15:49

AndersonQ commented Mar 11, 2024

View reviewed changes

AndersonQ marked this pull request as draft March 13, 2024 13:36

AndersonQ added 16 commits March 19, 2024 17:16

wip - so far so good. Still missing several tests

e56c7b8

.

add elastic-agent-libs override

c90d31d

.

.

ac843e1

added CA tests

b4e7023

go mod tidy

10078f3

certificate/key code and test

201fdcd

unify SSL tests and add a proxy subtest

df1419a

better log when test fails

b886e79

fixes and adjustments

2077e3b

go mod tidy

e41f4d6

revert magefile change

878a230

fix override

424cbdf

fix overrides

7411b93

fix panic

6736f89

fix CA ext keyusage

713b7b2

add test for encrypted certificate key

74c21c2

fix go.sum after rebase

9db4742

AndersonQ force-pushed the 2247-mtls branch from 74b5181 to 9db4742 Compare March 19, 2024 16:17

AndersonQ commented Mar 19, 2024

View reviewed changes

cmacknz reviewed Mar 20, 2024

View reviewed changes

AndersonQ added 6 commits March 21, 2024 08:49

Merge branch 'main' into 2247-mtls

b66b67e

Merge branch 'main' of github.com:elastic/elastic-agent into 2247-mtls

cf264db

Merge branch 'main' into 2247-mtls

c7bdf50

use a released version of elastic-agent-libs

2296c98

Merge branch '2247-mtls' of github.com:AndersonQ/elastic-agent into 2…

4964255

…247-mtls

PR changes

89fd117

AndersonQ requested a review from cmacknz March 21, 2024 15:21

Merge branch 'main' into 2247-mtls

6c9b6a8

AndersonQ added 2 commits March 22, 2024 07:18

Merge branch 'main' into 2247-mtls

e3fcadc

fix panic in debug log

e4a6a81

AndersonQ added 6 commits March 22, 2024 07:36

Merge branch '2247-mtls' of github.com:AndersonQ/elastic-agent into 2…

cbc9209

…247-mtls

Merge branch 'main' of github.com:elastic/elastic-agent into 2247-mtls

e279691

add cchangelog

c1c9289

Merge branch 'main' into 2247-mtls

d878942

making linter happy

5d688b3

Merge branch '2247-mtls' of github.com:AndersonQ/elastic-agent into 2…

cd3222b

…247-mtls

michalpristas reviewed Apr 3, 2024

View reviewed changes

AndersonQ marked this pull request as draft April 12, 2024 13:19

This was referenced May 10, 2024

Load fleet.ssl.certificate_authorities from agent policy #2247

Open

Load fleet.ssl.certificate and fleet.ssl.key from agent policy #2248

Open

pchila mentioned this pull request May 16, 2024

Apply mTLS config from policy #4770

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Read and apply mTLS config from policy #4398

Read and apply mTLS config from policy #4398

AndersonQ commented Mar 11, 2024 •

edited

elasticmachine commented Mar 11, 2024

AndersonQ Mar 11, 2024

AndersonQ Mar 11, 2024

mergify bot commented Mar 19, 2024

AndersonQ Mar 19, 2024

cmacknz Mar 20, 2024

AndersonQ Mar 21, 2024

cmacknz Mar 20, 2024

AndersonQ Mar 21, 2024

AndersonQ commented Mar 21, 2024

cmacknz commented Mar 21, 2024

AndersonQ commented Mar 22, 2024

elastic-sonarqube bot commented Apr 3, 2024

michalpristas Apr 3, 2024

michalpristas Apr 3, 2024 •

edited

mergify bot commented Apr 15, 2024

ycombinator commented May 10, 2024

Read and apply mTLS config from policy #4398

Are you sure you want to change the base?

Read and apply mTLS config from policy #4398

Conversation

AndersonQ commented Mar 11, 2024 • edited

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

Questions to ask yourself

elasticmachine commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Mar 19, 2024

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

AndersonQ commented Mar 21, 2024

cmacknz commented Mar 21, 2024

AndersonQ commented Mar 22, 2024

elastic-sonarqube bot commented Apr 3, 2024

Quality Gate passed

Choose a reason for hiding this comment

michalpristas Apr 3, 2024 • edited

Choose a reason for hiding this comment

mergify bot commented Apr 15, 2024

ycombinator commented May 10, 2024

AndersonQ commented Mar 11, 2024 •

edited

michalpristas Apr 3, 2024 •

edited