elastic / elastic-agent

Elastic Agent - single, unified way to add monitoring for logs, metrics, and other types of data to a host.
Other
124 stars 134 forks source link

`--insecure` flag should not be required during enroll/install because we have an `http` Fleet URL #4896

Open pchila opened 3 months ago

pchila commented 3 months ago

In the current elastic-agent implementation if an http Fleet URL is specified the install/enroll errors out as it requires --insecure to be specified as well https://github.com/elastic/elastic-agent/blob/e8fefd741526c79069a7901418df8edc9ed0ed8d/internal/pkg/agent/cmd/enroll_cmd.go#L135-L137

--insecure has bigger implications than just allowing an http:// URL for fleet as it also disables any SSL/TLS verification https://github.com/elastic/elastic-agent/blob/e8fefd741526c79069a7901418df8edc9ed0ed8d/internal/pkg/agent/cmd/enroll_cmd.go#L146-L148

While writing of test for PR #4770 it became quite apparent that it's impossible to specify an http fleet URL while going through an https proxy and have the agent validate the proxy certificate.

I am not sure why 2 concepts are depending on the same flag but it would be more logical that:

elasticmachine commented 3 months ago

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

elasticmachine commented 3 months ago

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

pchila commented 3 months ago

/cc @ycombinator @AndersonQ

blakerouse commented 3 months ago

A flag should be required to use http://. http:// should be heavily discouraged and so we should ensure that a flag (even if different) is required.

ycombinator commented 3 months ago

A test was added in https://github.com/elastic/elastic-agent/pull/4770 that needed to be skipped until this issue is resolved.

@pchila would you mind editing this comment and mentioning the name of the test? That way, whenever we get around to resolving this issue, we can tell very easily which test should be un-skipped. Thanks!

nimarezainia commented 3 months ago

--insecure flag is not recommended for production environments. We stipulate this in public docs as well.

pchila commented 3 months ago

A flag should be required to use http://. http:// should be heavily discouraged and so we should ensure that a flag (even if different) is required.

@blakerouse I understand that we may want to be explicit about discouraging http fleet endpoints, I am not sure that requiring disabling all TLS validation is the correct way to do it by requiring --insecure flag on the command line.

--insecure flag is not recommended for production environments. We stipulate this in public docs as well.

@nimarezainia At this point in time we have to specify --insecure to be able to use a mock fleet server that exposes the endpoint over http. I agree that this should not be used for production environments but I still don't see why we disable every TLS check when we have an http fleet server URL...