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

Add basic auth to /metrics endpoint #2322

Closed
zwass opened this issue Oct 1, 2021 · 6 comments
Closed

Add basic auth to /metrics endpoint #2322

zwass opened this issue Oct 1, 2021 · 6 comments
Assignees
Labels
~risk-reduction Related to improvements that could help reduce risk of outages, security, privacy, or trust issues.
Milestone

Comments

@zwass
Copy link
Member

zwass commented Oct 1, 2021

Goal

Though the /metrics endpoint doesn't contain anything particularly sensitive, it is desirable to keep this behind authentication.

Prometheus supports scraping with basic auth (see https://prometheus.io/docs/prometheus/latest/configuration/configuration/#scrape_config).

Tasks

  • Add a prometheus_username and prometheus_password Server configuration option and use this to authenticate the /metrics endpoint with basic auth.
  • If any of the two configuration options are not-defined/empty, then metrics collection and the /metrics endpoint will be disabled.
@zwass zwass added idea labels Oct 1, 2021
@noahtalerman
Copy link
Member

@gavinelder I'm adding your comment from the separate "[FR] Separate internal / external endpoints" issue here:

The reasoning for /version /metrics was to limit unauthenticated which could expose information about an estate that could be used for reconnaissance purposes but there may be more.

Maybe the first step is to add basic auth for both /metrics and /version? This way, all Fleet endpoints are kept behind basic auth.

Later, Fleet might want to make it easier to allow more granular control over which endpoints are public v. internal.

@mikermcneil mikermcneil removed the idea label Mar 23, 2022
@chiiph chiiph assigned lucasmrod and unassigned chiiph Mar 30, 2022
@chiiph chiiph added this to the 4.13.0 milestone Mar 30, 2022
@lucasmrod
Copy link
Member

If the configuration option is not defined, then the user experiences the current unauthenticated behavior

What about the following three options:

  1. auth'd /metrics by default: fleet errors out if FLEET_PROMETHEUS_METRICS_USERNAME, FLEET_PROMETHEUS_METRICS_PASSWORD credentials are not set.
  2. allow unauth'd /metrics: via explicit setting of FLEET_PROMETHEUS_METRICS_DISABLE_AUTH.
  3. no /metrics: via explicit setting of FLEET_PROMETHEUS_METRICS_ENABLE. (see Allow /metrics route to be enabled/disabled via config #4934)

The above is the secure by default approach but we should discuss because it's sort of a "breaking change".

@zwass
Copy link
Member Author

zwass commented Apr 5, 2022

What if we go with metrics are turned off by default and only enabled if the credentials are provided?

I suspect not many teams are actually using the metrics, so we'll break the fewest installations if we go that way.

@lucasmrod
Copy link
Member

What if we go with metrics are turned off by default and only enabled if the credentials are provided?

Works too, it's a good trade off between security <-> deployment-ease.

@lucasmrod
Copy link
Member

I've updated the issue description.

@gavinelder
Copy link

@gavinelder I'm adding your comment from the separate "[FR] Separate internal / external endpoints" issue here:

The reasoning for /version /metrics was to limit unauthenticated which could expose information about an estate that could be used for reconnaissance purposes but there may be more.

Maybe the first step is to add basic auth for both /metrics and /version? This way, all Fleet endpoints are kept behind basic auth.

Later, Fleet might want to make it easier to allow more granular control over which endpoints are public v. internal.

This would probably close down the linked issue in #3614

@lukeheath lukeheath modified the milestone: 4.13.0 Apr 18, 2022
@GuillaumeRoss GuillaumeRoss added ~risk-reduction Related to improvements that could help reduce risk of outages, security, privacy, or trust issues. security labels May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
~risk-reduction Related to improvements that could help reduce risk of outages, security, privacy, or trust issues.
Development

No branches or pull requests

8 participants