From 66e9cfb01fbbc235b9b5293567d5a1d50a45b4c6 Mon Sep 17 00:00:00 2001 From: uaggarwa <140089501+uaggarwa@users.noreply.github.com> Date: Wed, 10 Jun 2026 03:19:28 -0400 Subject: [PATCH] feat(metrics): anonymous access when enabled in accessControl config (#4110) * feat: add anonymouspolicy support in metrics Signed-off-by: uaggarwa * test: add unit tests Signed-off-by: uaggarwa --------- Signed-off-by: uaggarwa --- examples/README.md | 39 ++++ ...config-metrics-authn-anonymous-access.json | 31 +++ pkg/api/authn.go | 33 ++- pkg/api/authz.go | 6 + pkg/api/config/config.go | 19 +- pkg/cli/server/root.go | 4 +- pkg/extensions/monitoring/monitoring_test.go | 207 ++++++++++++++++++ 7 files changed, 334 insertions(+), 5 deletions(-) create mode 100644 examples/config-metrics-authn-anonymous-access.json diff --git a/examples/README.md b/examples/README.md index 850252c2..02436a1c 100644 --- a/examples/README.md +++ b/examples/README.md @@ -895,6 +895,45 @@ Behaviour-based action list } ``` +##### Metrics access control + +The `metrics` key inside `accessControl` controls access to the Prometheus scrape endpoint independently of repository policies. It supports two fields: + +- `users` - list of named authenticated users allowed to scrape. Requires authentication (e.g. htpasswd) to be configured. +- `anonymousPolicy` - set to `["read"]` to allow unauthenticated access to the metrics endpoint when authentication is configured for other routes. + +To restrict scraping to specific named users: + +``` +"accessControl": { + "metrics": { + "users": ["prometheus"] + } +} +``` + +See [config-metrics-authz.json](config-metrics-authz.json) for a complete example combining htpasswd authentication with repository policies. + +When authentication is configured and repositories have non-anonymous policies, `anonymousPolicy` on `metrics` allows unauthenticated scrapers to reach the metrics endpoint while keeping repository routes protected: + +``` +"http": { + "auth": { + "htpasswd": { "path": "test/data/htpasswd" } + }, + "accessControl": { + "metrics": { + "anonymousPolicy": ["read"] + }, + "repositories": { + "**": { "defaultPolicy": ["read", "create"] } + } + } +} +``` + +See [config-metrics-authn-anonymous-access.json](config-metrics-authn-anonymous-access.json) for a complete example. + ##### Conditional access on policies Policy entries can carry an optional list of `conditions`: CEL boolean diff --git a/examples/config-metrics-authn-anonymous-access.json b/examples/config-metrics-authn-anonymous-access.json new file mode 100644 index 00000000..f6159967 --- /dev/null +++ b/examples/config-metrics-authn-anonymous-access.json @@ -0,0 +1,31 @@ +{ + "distSpecVersion": "1.1.1", + "storage": { + "rootDirectory": "/tmp/zot" + }, + "http": { + "address": "127.0.0.1", + "port": "8080", + "auth": { + "htpasswd": { + "path": "/tmp/zot-test-htpasswd" + } + }, + "accessControl": { + "metrics": { + "anonymousPolicy": ["read"] + } + } + }, + "log": { + "level": "debug" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "/metrics" + } + } + } +} diff --git a/pkg/api/authn.go b/pkg/api/authn.go index 4790f21a..abfd5551 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -40,6 +40,7 @@ import ( "zotregistry.dev/zot/v2/pkg/api/constants" apiErr "zotregistry.dev/zot/v2/pkg/api/errors" zcommon "zotregistry.dev/zot/v2/pkg/common" + extconf "zotregistry.dev/zot/v2/pkg/extensions/config" "zotregistry.dev/zot/v2/pkg/log" reqCtx "zotregistry.dev/zot/v2/pkg/requestcontext" ) @@ -432,6 +433,10 @@ func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun accessControlConfig := ctlr.Config.CopyAccessControlConfig() allowAnonymous := accessControlConfig != nil && accessControlConfig.AnonymousPolicyExists() + // Allow anonymous access to the metrics endpoint only if configured. + extensionsConfig := ctlr.Config.CopyExtensionsConfig() + isMetricsRequestedWithAnonymousAccess := isAnonymousMetricsRequest(request, accessControlConfig, extensionsConfig) + // build user access control info userAc := reqCtx.NewUserAccessControl() // if it will not be populated by authn handlers, this represents an anonymous user @@ -461,7 +466,7 @@ func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun // If session authentication fails, but anonymous or management access is allowed, // treat the request as authenticated. This fallback is necessary because the session // header may be present for anonymous or management requests. - authenticated = authenticated || allowAnonymous || isMgmtRequested + authenticated = authenticated || allowAnonymous || isMgmtRequested || isMetricsRequestedWithAnonymousAccess // Try mTLS authentication if client certificates are present case ctlr.Config.IsMTLSAuthEnabled() && request.TLS != nil && len(request.TLS.PeerCertificates) > 0: @@ -471,8 +476,8 @@ func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFun case !authConfig.IsBasicAuthnEnabled() && !ctlr.Config.IsMTLSAuthEnabled(): authenticated = true - // If no credentials provided - check for anonymous / mgmt requests - case allowAnonymous || isMgmtRequested: + // If no credentials provided - check for anonymous / mgmt / metrics requests + case allowAnonymous || isMgmtRequested || isMetricsRequestedWithAnonymousAccess: // Docker workaround: force 401 on /v2/ when anonymous policies coexist with // authenticated-only policies. Otherwise Docker treats 200 on /v2/ as "no auth" // and will not send stored credentials for protected repositories. @@ -582,6 +587,16 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { return } + // Allow anonymous access to the metrics endpoint only if configured. + accessControlConfig := ctlr.Config.CopyAccessControlConfig() + extensionsConfig := ctlr.Config.CopyExtensionsConfig() + + if isAnonymousMetricsRequest(request, accessControlConfig, extensionsConfig) { + next.ServeHTTP(response, request) + + return + } + var requestedAccess *ResourceAction if request.RequestURI != "/v2/" { @@ -977,6 +992,18 @@ func authFail(w http.ResponseWriter, r *http.Request, realm string, delay int) { zcommon.WriteJSON(w, http.StatusUnauthorized, apiErr.NewError(apiErr.UNAUTHORIZED)) } +func isAnonymousMetricsRequest(request *http.Request, accessControlConfig *config.AccessControlConfig, + extensionsConfig *extconf.ExtensionConfig, +) bool { + prometheusConfig := extensionsConfig.GetMetricsPrometheusConfig() + + return isAuthorizationHeaderEmpty(request) && + slices.Contains(accessControlConfig.GetMetrics().AnonymousPolicy, constants.ReadPermission) && + extensionsConfig.IsMetricsEnabled() && + prometheusConfig != nil && + strings.HasPrefix(request.URL.Path, prometheusConfig.Path) +} + func isAuthorizationHeaderEmpty(request *http.Request) bool { header := request.Header.Get("Authorization") diff --git a/pkg/api/authz.go b/pkg/api/authz.go index 2b204604..99efa287 100644 --- a/pkg/api/authz.go +++ b/pkg/api/authz.go @@ -742,6 +742,12 @@ func MetricsAuthzHandler(ctlr *Controller) mux.MiddlewareFunc { } metricsConfig := accessControlConfig.GetMetrics() + if slices.Contains(metricsConfig.AnonymousPolicy, constants.ReadPermission) { + next.ServeHTTP(response, request) + + return + } + if len(metricsConfig.Users) == 0 { log := ctlr.Log log.Warn().Msg("auth is enabled but no metrics users in accessControl: /metrics is unaccesible") diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index 3c4fd3ae..dc1080e3 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -650,6 +650,22 @@ func (config *AccessControlConfig) GetMetrics() Metrics { return config.Metrics } +// ContainsOnlyMetricsAnonymousPolicy identifies an access control configuration +// that contains only an anonymous policy for metrics. +func (config *AccessControlConfig) ContainsOnlyMetricsAnonymousPolicy() bool { + if config == nil { + return false + } + admin := config.GetAdminPolicy() + if len(admin.Actions)+len(admin.Users)+len(admin.Groups) > 0 { + return false + } + + return len(config.GetRepositories()) == 0 && + len(config.GetGroups()) == 0 && + slices.Contains(config.GetMetrics().AnonymousPolicy, "read") +} + // GetGroups safely gets a copy of the groups configuration. func (config *AccessControlConfig) GetGroups() Groups { if config == nil { @@ -739,7 +755,8 @@ type Condition struct { } type Metrics struct { - Users []string + Users []string + AnonymousPolicy []string } type Config struct { diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index 7287bd79..f31f1ada 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -793,8 +793,10 @@ func validateAuthzPolicies(config *config.Config, logger zlog.Logger) error { logger.Info().Msg("checking if anonymous authorization is the only type of authorization policy configured") + // if no authentication is configured, policies must be anonymous-only; if !authConfig.IsBasicAuthnEnabled() && !config.IsMTLSAuthEnabled() && !authConfig.IsBearerAuthEnabled() && - !accessControlConfig.ContainsOnlyAnonymousPolicy() { + !accessControlConfig.ContainsOnlyAnonymousPolicy() && + !accessControlConfig.ContainsOnlyMetricsAnonymousPolicy() { msg := "access control config requires one of htpasswd, ldap, openid or mTLS authentication " + "or using only 'anonymousPolicy' policies" logger.Error().Err(zerr.ErrBadConfig).Msg(msg) diff --git a/pkg/extensions/monitoring/monitoring_test.go b/pkg/extensions/monitoring/monitoring_test.go index 86bac829..3a8b76f3 100644 --- a/pkg/extensions/monitoring/monitoring_test.go +++ b/pkg/extensions/monitoring/monitoring_test.go @@ -9,6 +9,7 @@ import ( "io" "math/rand" "net/http" + "net/url" "os" "path" "testing" @@ -26,10 +27,12 @@ import ( "zotregistry.dev/zot/v2/pkg/scheduler" common "zotregistry.dev/zot/v2/pkg/storage/common" "zotregistry.dev/zot/v2/pkg/storage/gc" + authutils "zotregistry.dev/zot/v2/pkg/test/auth" test "zotregistry.dev/zot/v2/pkg/test/common" . "zotregistry.dev/zot/v2/pkg/test/image-utils" "zotregistry.dev/zot/v2/pkg/test/mocks" ociutils "zotregistry.dev/zot/v2/pkg/test/oci-utils" + tlsutils "zotregistry.dev/zot/v2/pkg/test/tls" ) func TestExtensionMetrics(t *testing.T) { @@ -422,6 +425,179 @@ func TestMetricsAuthorization(t *testing.T) { So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusOK) }) + Convey("with basic auth: metrics.anonymousPolicy=[read] allows unauthenticated scraping", func() { + conf.HTTP.AccessControl = &config.AccessControlConfig{ + Metrics: config.Metrics{ + AnonymousPolicy: []string{"read"}, + }, + } + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(port) + defer cm.StopServer() + + // unauthenticated client should be allowed to scrape /metrics + resp, err := resty.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // wrong credentials should still be rejected at authn + resp, err = resty.R().SetBasicAuth("hacker", "wrongpass").Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + + // valid credentials should also work + client := resty.New() + client.SetBasicAuth(metricsuser, metricspass) + resp, err = client.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // anonymous access to registry endpoints should remain protected + resp, err = resty.R().Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) + Convey("with basic auth: empty metrics.anonymousPolicy blocks unauthenticated scraping", func() { + conf.HTTP.AccessControl = &config.AccessControlConfig{ + Metrics: config.Metrics{ + AnonymousPolicy: nil, + Users: []string{metricsuser}, + }, + } + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(port) + defer cm.StopServer() + + // unauthenticated client should be blocked + resp, err := resty.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + + // authorized user should still have access + client := resty.New() + client.SetBasicAuth(metricsuser, metricspass) + resp, err = client.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + }) + }) + Convey("Make a new controller with bearer auth & metrics enabled", t, func() { + serverCertPath, serverKeyPath := setupMetricsBearerAuthServerCerts(t) + + authTestServer := authutils.MakeAuthTestServer(serverKeyPath, "RS256", "unauthorized-repo") + defer authTestServer.Close() + + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + + authURL, err := url.Parse(authTestServer.URL) + So(err, ShouldBeNil) + + conf.HTTP.Auth = &config.AuthConfig{ + Bearer: &config.BearerConfig{ + Cert: serverCertPath, + Realm: authTestServer.URL + "/auth/token", + Service: authURL.Host, + }, + } + + enabled := true + conf.Extensions = &extconf.ExtensionConfig{ + Metrics: &extconf.MetricsConfig{ + BaseConfig: extconf.BaseConfig{Enable: &enabled}, + Prometheus: &extconf.PrometheusConfig{Path: "/metrics"}, + }, + } + conf.HTTP.AccessControl = &config.AccessControlConfig{ + Metrics: config.Metrics{ + AnonymousPolicy: []string{"read"}, + }, + } + + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(port) + defer cm.StopServer() + + Convey("with bearer auth: metrics.anonymousPolicy=[read] allows unauthenticated scraping", func() { + // unauthenticated client should be allowed to scrape /metrics + resp, err := resty.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // invalid bearer token should still be rejected at authn + resp, err = resty.R().SetHeader("Authorization", "Bearer invalidToken").Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + + // anonymous access to registry endpoints should remain protected + resp, err = resty.R().Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) + }) +} + +func TestMetricsAnonymousAccessNoAuth(t *testing.T) { + Convey("Make a new controller with no auth and metrics.anonymousPolicy=[read]", t, func() { + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + + enabled := true + conf.Extensions = &extconf.ExtensionConfig{ + Metrics: &extconf.MetricsConfig{ + BaseConfig: extconf.BaseConfig{Enable: &enabled}, + Prometheus: &extconf.PrometheusConfig{Path: "/metrics"}, + }, + } + + conf.HTTP.AccessControl = &config.AccessControlConfig{ + Metrics: config.Metrics{ + AnonymousPolicy: []string{"read"}, + }, + } + + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(port) + defer cm.StopServer() + + // unauthenticated client should be allowed to scrape /metrics + resp, err := resty.R().Get(baseURL + "/metrics") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + }) +} + +func TestMetricsAnonymousPolicyNilAccessControl(t *testing.T) { + Convey("Nil access control does not match metrics anonymous policy", t, func() { + var accessControlConfig *config.AccessControlConfig + + So(accessControlConfig.ContainsOnlyMetricsAnonymousPolicy(), ShouldBeFalse) }) } @@ -570,3 +746,34 @@ func generateRandomString() string { return string(randomBytes) } + +func setupMetricsBearerAuthServerCerts(t *testing.T) (string, string) { + t.Helper() + + tempDir := t.TempDir() + caOpts := &tlsutils.CertificateOptions{ + CommonName: "*", + NotAfter: time.Now().AddDate(10, 0, 0), + KeyType: tlsutils.KeyTypeRSA, + } + caCertPEM, caKeyPEM, err := tlsutils.GenerateCACert(caOpts) + if err != nil { + t.Fatalf("failed to generate CA cert: %v", err) + } + + serverCertPath := path.Join(tempDir, "server.cert") + serverKeyPath := path.Join(tempDir, "server.key") + serverOpts := &tlsutils.CertificateOptions{ + Hostname: "127.0.0.1", + CommonName: "*", + OrganizationalUnit: "TestServer", + NotAfter: time.Now().AddDate(10, 0, 0), + KeyType: tlsutils.KeyTypeRSA, + } + err = tlsutils.GenerateServerCertToFile(caCertPEM, caKeyPEM, serverCertPath, serverKeyPath, serverOpts) + if err != nil { + t.Fatalf("failed to generate server cert: %v", err) + } + + return serverCertPath, serverKeyPath +}