fix: metrics should be protected behind authZ (#1895)

Signed-off-by: Alexei Dodon <adodon@cisco.com>

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) getAuthnMiddlewareContext(authnType string, request
 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,9 +203,7 @@ func (ac *AccessController) isPermitted(userGroups []string, username, action st
 			if common.Contains(p.Actions, action) {
 				for _, group := range p.Groups {
 					if common.Contains(userGroups, group) {
-						result = true
-
-						return result
+						return true
 					}
 				}
 			}
@@ -217,20 +211,16 @@ func (ac *AccessController) isPermitted(userGroups []string, username, action st
 	}
 
 	// 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 DistSpecAuthzHandler(ctlr *Controller) mux.MiddlewareFunc {
 		})
 	}
 }
+
+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)