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

Verifier does not honor proxy settings #4237

Closed
ycombinator opened this issue Feb 9, 2024 · 6 comments · Fixed by #4270
Closed

Verifier does not honor proxy settings #4237

ycombinator opened this issue Feb 9, 2024 · 6 comments · Fixed by #4270
Assignees
Labels
bug Something isn't working Team:Elastic-Agent Label for the Agent team

Comments

@ycombinator
Copy link
Contributor

ycombinator commented Feb 9, 2024

For confirmed bugs, please report:

  • Version: 8.12.0 (although this bug likely exists in older versions as well)
  • Steps to Reproduce:
  1. Enroll an Agent in Fleet.
  2. Setup an HTTP proxy to be used by Agent.
  3. Configure the proxy settings in the Fleet UI (Settings > Proxies).
  4. On the host that Agent is installed on, block network access to artifacts.elastic.co:443 so the Agent is forced to download upgrade artifacts, including the .asc signature file, via the proxy.
  5. Initiate an upgrade to the Agent from the Fleet UI.

Expected behavior:

  • The upgrade completes successfully

Observed behavior:

  • The upgrade does not complete successfuly. Agent logs show that the Agent package is downloaded but the download of the .asc signature file fails.

Relevant implementation details:

This bug is almost certainly being caused because the Verifier code path does not use an HTTP client that's configured with the proxy settings, unlike the Downloader code path, which does.

Downloader code path, showing the use of a custom-configured HTTP client:

resp, err := e.client.Do(req.WithContext(ctx))
if err != nil {
// return path, file already exists and needs to be cleaned up
return fullPath, errors.New(err, "fetching package failed", errors.TypeNetwork, errors.M(errors.MetaKeyURI, sourceURI))
}

Verifier code path, showing the use of the default HTTP client:

// TODO: receive a http.Client
resp, err := http.DefaultClient.Do(req)
if err != nil {
return nil, errors.New(err, "failed loading public key", errors.TypeNetwork, errors.M(errors.MetaKeyURI, sourceURI))
}

@ycombinator ycombinator added bug Something isn't working Team:Elastic-Agent Label for the Agent team labels Feb 9, 2024
@elasticmachine
Copy link
Contributor

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

@mbudge
Copy link
Contributor

mbudge commented Feb 10, 2024

Agent/fleet has gone 889 days since GA release without fully regression tested managed proxy support.

If we had deployed agent to a data centre to find it doesn’t upgrade, which we are about to do, we’d probably move to Splunk as we’ve already delayed the rollout of agent by 18 months.

Using the default Go proxy environment variables is something you don’t do on a production network. It’s not maintainable on 1000’s of endpoints. It’s also a security risk as an attacker changing the value would break log collection.

Customer networks are not for this
https://en.m.wikipedia.org/wiki/Regression_testing

Can you please set up a lab environment with a proxy to test all code changes and new releases, with both elastic/logstash outputs?

Thanks

@pierrehilbert
Copy link
Contributor

@AndersonQ Could you please have a look as you spent time lately on the proxy setting and added tests around it?

@cmacknz
Copy link
Member

cmacknz commented Feb 12, 2024

To make this work properly without relying on the GOPROXY environment variable there are a few things to address as part of this:

  1. The verifier HTTP client should probably be the same one used for the download, the only reason to make them different would be if we supported downloading the signature files from a separate location. This avoids needing to expose even more HTTP client configuration in Fleet and the agent policy because we will just be using the binary download settings.
  2. There is one exception to this, which is the fallback GPG key we can download from Fleet server. This can be handled in a separate issue: Fall back PGP download from Fleet server ignores Fleet server proxy configuration #4241
  3. Add an upgrade test that sets a proxy for the binary download source without relying on the GOPROXY environment variable. This the root cause for why we have missed this for so long.
  4. Stabilize and re-enable the test for the Fleet proxy URL https://github.com/elastic/elastic-agent/blob/main/testing/integration/proxy_url_test.go

@cmacknz
Copy link
Member

cmacknz commented Feb 12, 2024

Can you please set up a lab environment with a proxy to test all code changes and new releases, with both elastic/logstash outputs?

We are working through this. Every commit we make has automation installing the agent on VMs connected to an Elastic Cloud deployment to ensure it works as expected.

There are a lot of scenarios and combinations to cover (proxies, air gapped networks, output types, soon root and non-root installs, etc) so we haven't covered everything yet but we are working on it. Should all of this have been in place earlier? Yes, but we are catching up as quickly as we can.

@AndersonQ
Copy link
Member

@pierrehilbert sorry for the delay. I guess it wasn't caught before because not all tests check the package signature. By now we should be able to check the signature, and generate it when needed, to all your tests. I removed the option to skip the PGP signature check on #3590. However I believe we did not have a test including the check, a proxy and the fleet-server PGP fallback not available. Or at least ensuring it would not download from the fallback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants