fix: metrics should be protected behind authZ

examples/config-metrics-authz.json

@@ -0,0 +1,39 @@
+{
+    "distSpecVersion": "1.1.0-dev",
+    "storage": {
+        "rootDirectory": "/tmp/zot"
+    },
+    "http": {
+        "address": "127.0.0.1",
+        "port": "8080",
+        "auth": {
+            "htpasswd": {
+              "path": "test/data/htpasswd"
+            }
+        },
+        "accessControl": {
+            "metrics":{
+                "users": ["metrics"]
+            },
+            "repositories": {
+                "**": {
+                    "anonymousPolicy": [
+                        "read"
+                    ],
+                    "defaultPolicy": ["read","create"]
+                }
+            }
+        }
+    },
+    "log": {
+        "level": "debug"
+    },
+    "extensions": {
+        "metrics": {
+            "enable": true,
+            "prometheus": {
+                "path": "/metrics"
+            }
+        }
+    }
+}

pkg/api/authn.go

@@ -57,7 +57,7 @@ func AuthHandler(ctlr *Controller) mux.MiddlewareFunc {
 		return bearerAuthHandler(ctlr)
 	}
 
-	return authnMiddleware.TryAuthnHandlers(ctlr)
+	return authnMiddleware.tryAuthnHandlers(ctlr)
 }
 
 func (amw *AuthnMiddleware) sessionAuthn(ctlr *Controller, userAc *reqCtx.UserAccessControl,
@@ -247,7 +247,7 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce
 	return false, nil
 }
 
-func (amw *AuthnMiddleware) TryAuthnHandlers(ctlr *Controller) mux.MiddlewareFunc { //nolint: gocyclo
+func (amw *AuthnMiddleware) tryAuthnHandlers(ctlr *Controller) mux.MiddlewareFunc { //nolint: gocyclo
 	// no password based authN, if neither LDAP nor HTTP BASIC is enabled
 	if !ctlr.Config.IsBasicAuthnEnabled() {
 		return noPasswdAuth(ctlr)

pkg/api/authz.go

@@ -191,14 +191,10 @@
 func (ac *AccessController) isPermitted(userGroups []string, username, action string,
 	policyGroup config.PolicyGroup,
 ) bool {
-	var result bool
-
 	// check repo/system based policies
 	for _, p := range policyGroup.Policies {
 		if common.Contains(p.Users, username) && common.Contains(p.Actions, action) {
-			result = true
-
-			return result
+			return true
 		}
 	}
 
@@ -207,30 +203,24 @@
 			if common.Contains(p.Actions, action) {
 				for _, group := range p.Groups {
 					if common.Contains(userGroups, group) {
-						result = true
-
-						return result
+						return true
 					}
 				}
 			}
 		}
 	}
 
 	// check defaultPolicy
-	if !result {
-		if common.Contains(policyGroup.DefaultPolicy, action) && username != "" {
-			result = true
-		}
+	if common.Contains(policyGroup.DefaultPolicy, action) && username != "" {
+		return true
 	}
 
 	// check anonymousPolicy
-	if !result {
-		if common.Contains(policyGroup.AnonymousPolicy, action) && username == "" {
-			result = true
-		}
+	if common.Contains(policyGroup.AnonymousPolicy, action) && username == "" {
+		return true
 	}
 
-	return result
+	return false
 }
 
 func BaseAuthzHandler(ctlr *Controller) mux.MiddlewareFunc {
@@ -343,3 +333,40 @@
 		})
 	}
 }
+
+func MetricsAuthzHandler(ctlr *Controller) mux.MiddlewareFunc {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+			if ctlr.Config.HTTP.AccessControl == nil {
+				// allow access to authenticated user as anonymous policy does not exist
+				next.ServeHTTP(response, request)
+
+				return
+			}
+			if len(ctlr.Config.HTTP.AccessControl.Metrics.Users) == 0 {
+				log := ctlr.Log
+				log.Warn().Msg("auth is enabled but no metrics users in accessControl: /metrics is unaccesible")
+				common.AuthzFail(response, request, "", ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay)
+
+				return
+			}
+
+			// get access control context made in authn.go
+			userAc, err := reqCtx.UserAcFromContext(request.Context())
+			if err != nil { // should never happen
+				common.AuthzFail(response, request, "", ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay)
+
+				return
+			}
+
+			username := userAc.GetUsername()
+			if !common.Contains(ctlr.Config.HTTP.AccessControl.Metrics.Users, username) {
+				common.AuthzFail(response, request, username, ctlr.Config.HTTP.Realm, ctlr.Config.HTTP.Auth.FailDelay)
+
+				return
+			}
+
+			next.ServeHTTP(response, request) //nolint:contextcheck
+		})
+	}
+}

pkg/api/config/config.go

@@ -131,6 +131,7 @@ type AccessControlConfig struct {
 	Repositories Repositories `json:"repositories" mapstructure:"repositories"`
 	AdminPolicy  Policy
 	Groups       Groups
+	Metrics      Metrics
 }
 
 func (config *AccessControlConfig) AnonymousPolicyExists() bool {
@@ -168,6 +169,10 @@ type Policy struct {
 	Groups  []string
 }
 
+type Metrics struct {
+	Users []string
+}
+
 type Config struct {
 	DistSpecVersion string `json:"distSpecVersion" mapstructure:"distSpecVersion"`
 	GoVersion       string

pkg/api/routes.go

@@ -183,7 +183,7 @@ func (rh *RouteHandler) SetupRoutes() {
 	pprof.SetupPprofRoutes(rh.c.Config, prefixedRouter, authHandler, rh.c.Log)
 
 	// Preconditions for enabling the actual extension routes are part of extensions themselves
-	ext.SetupMetricsRoutes(rh.c.Config, rh.c.Router, authHandler, rh.c.Log, rh.c.Metrics)
+	ext.SetupMetricsRoutes(rh.c.Config, rh.c.Router, authHandler, MetricsAuthzHandler(rh.c), rh.c.Log, rh.c.Metrics)
 	ext.SetupSearchRoutes(rh.c.Config, prefixedRouter, rh.c.StoreController, rh.c.MetaDB, rh.c.CveScanner,
 		rh.c.Log)
 	ext.SetupImageTrustRoutes(rh.c.Config, prefixedRouter, rh.c.MetaDB, rh.c.Log)

pkg/extensions/README.md

@@ -30,16 +30,16 @@ package extensions
     IsAdmin         bool
     Username        string
     Groups          []string
-    } 
+    }
     ```
-  This data can then be accessed from the request context so that <b>every extension can apply its own authorization logic, if needed </b>. 
+  This data can then be accessed from the request context so that <b>every extension can apply its own authorization logic, if needed </b>.
 
 - when a new extension comes out, the developer should also write some blackbox tests, where a binary that contains the new extension should be tested in a real usage scenario. See [test/blackbox](test/blackbox/sync.bats) folder for multiple extensions examples.
 
 - newly added blackbox tests should have targets in Makefile. You should also add them as Github Workflows, in [.github/workflows/ecosystem-tools.yaml](.github/workflows/ecosystem-tools.yaml)
 
 - with every new extension, you should modify the EXTENSIONS variable in Makefile by adding the new extension. The EXTENSIONS variable represents all extensions and is used in Make targets that require them all (e.g make test).
 
-- the available extensions that can be used at the moment are: <b>sync, scrub, metrics, search </b>.
+- the available extensions that can be used at the moment are: <b>sync, search, scrub, metrics, lint, ui, mgmt, userprefs, imagetrust </b>.
 NOTE: When multiple extensions are used, they should be listed in the above presented order.
 

pkg/extensions/extension_metrics.go

@@ -26,13 +26,14 @@ func EnableMetricsExtension(config *config.Config, log log.Logger, rootDir strin
 }
 
 func SetupMetricsRoutes(config *config.Config, router *mux.Router,
-	authFunc mux.MiddlewareFunc, log log.Logger, metrics monitoring.MetricServer,
+	authnFunc, authzFunc mux.MiddlewareFunc, log log.Logger, metrics monitoring.MetricServer,
 ) {
 	log.Info().Msg("setting up metrics routes")
 
 	if config.IsMetricsEnabled() {
 		extRouter := router.PathPrefix(config.Extensions.Metrics.Prometheus.Path).Subrouter()
-		extRouter.Use(authFunc)
+		extRouter.Use(authnFunc)
+		extRouter.Use(authzFunc)
 		extRouter.Methods("GET").Handler(promhttp.Handler())
 	}
 }

pkg/extensions/extension_metrics_disabled.go

@@ -22,13 +22,14 @@ func EnableMetricsExtension(config *config.Config, log log.Logger, rootDir strin
 
 // SetupMetricsRoutes ...
 func SetupMetricsRoutes(conf *config.Config, router *mux.Router,
-	authFunc mux.MiddlewareFunc, log log.Logger, metrics monitoring.MetricServer,
+	authnFunc, authzFunc mux.MiddlewareFunc, log log.Logger, metrics monitoring.MetricServer,
 ) {
 	getMetrics := func(w http.ResponseWriter, r *http.Request) {
 		m := metrics.ReceiveMetrics()
 		zcommon.WriteJSON(w, http.StatusOK, m)
 	}
 
-	router.Use(authFunc)
+	router.Use(authnFunc)
+	router.Use(authzFunc)
 	router.HandleFunc("/metrics", getMetrics).Methods("GET")
 }