Merge pull request canonical#13214 from roosterfish/fix_metrics_perms

Metrics: Differentiate between restricted and unrestricted certificates

lxd/api_metrics.go

@@ -49,7 +49,15 @@ func allowMetrics(d *Daemon, r *http.Request) response.Response {
 		return response.EmptySyncResponse
 	}
 
-	return allowPermission(entity.TypeServer, auth.EntitlementCanViewMetrics)(d, r)
+	entityType := entity.TypeServer
+
+	// Check for individual permissions on project if set.
+	projectName := request.QueryParam(r, "project")
+	if projectName != "" {
+		entityType = entity.TypeProject
+	}
+
+	return allowPermission(entityType, auth.EntitlementCanViewMetrics)(d, r)
 }
 
 // swagger:operation GET /1.0/metrics metrics metrics_get
@@ -323,31 +331,30 @@ func metricsGet(d *Daemon, r *http.Request) response.Response {
 }
 
 func getFilteredMetrics(s *state.State, r *http.Request, compress bool, metricSet *metrics.MetricSet) response.Response {
+	// Ignore filtering in case the authentication for metrics is disabled.
 	if !s.GlobalConfig.MetricsAuthentication() {
 		return response.SyncResponsePlain(true, compress, metricSet.String())
 	}
 
-	// Get instances the user is allowed to view.
-	userHasPermission, err := s.Authorizer.GetPermissionChecker(r.Context(), r, auth.EntitlementCanView, entity.TypeInstance)
-	if err != nil && !auth.IsDeniedError(err) {
+	userHasProjectPermission, err := s.Authorizer.GetPermissionChecker(r.Context(), r, auth.EntitlementCanViewMetrics, entity.TypeProject)
+	if err != nil {
 		return response.SmartError(err)
-	} else if err != nil {
-		// This is counterintuitive. We are unauthorized to get a permission checker for viewing instances because a metric type certificate
-		// can't view instances. However, in order to get to this point we must already have auth.EntitlementCanViewMetrics. So we can view
-		// the metrics but we can't do any filtering, so just return the metrics.
-		return response.SyncResponsePlain(true, compress, metricSet.String())
 	}
 
-	// Filter by project name and instance name.
-	metricSet.FilterSamples(userHasPermission)
-
-	userHasPermission, err = s.Authorizer.GetPermissionChecker(r.Context(), r, auth.EntitlementCanView, entity.TypeProject)
+	userHasServerPermission, err := s.Authorizer.GetPermissionChecker(r.Context(), r, auth.EntitlementCanViewMetrics, entity.TypeServer)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
-	// Filter by project only.
-	metricSet.FilterSamples(userHasPermission)
+	// Filter all metrics for view permissions.
+	// Internal server metrics without labels are compared against the server entity.
+	metricSet.FilterSamples(func(labels map[string]string) bool {
+		if labels["project"] != "" {
+			return userHasProjectPermission(entity.ProjectURL(labels["project"]))
+		}
+
+		return userHasServerPermission(entity.ServerURL())
+	})
 
 	return response.SyncResponsePlain(true, compress, metricSet.String())
 }

lxd/auth/authorization_types.go

@@ -103,7 +103,7 @@ const (
 	// EntitlementCanViewResources is the `can_view_resources` Entitlement. It applies to entity.TypeServer.
 	EntitlementCanViewResources Entitlement = "can_view_resources"
 
-	// EntitlementCanViewMetrics is the `can_view_metrics` Entitlement. It applies to entity.TypeServer.
+	// EntitlementCanViewMetrics is the `can_view_metrics` Entitlement. It applies to entity.TypeServer and entity.TypeProject.
 	EntitlementCanViewMetrics Entitlement = "can_view_metrics"
 
 	// EntitlementCanViewWarnings is the `can_view_warnings` Entitlement. It applies to entity.TypeServer.

lxd/auth/driver_openfga_model.openfga

@@ -121,6 +121,7 @@ type project
     define can_delete_storage_buckets: [identity, service_account, group#member] or operator or storage_bucket_manager or can_edit_projects from server
     define can_view_operations: [identity, service_account, group#member] or operator or viewer or can_view_projects from server
     define can_view_events: [identity, service_account, group#member] or operator or viewer or can_view_projects from server
+    define can_view_metrics: [identity, service_account, group#member] or operator or viewer or can_view_metrics from server
 type image
   relations
     define project: [project]

lxd/auth/driver_tls.go

@@ -76,7 +76,7 @@ func (t *tls) CheckPermission(ctx context.Context, r *http.Request, entityURL *a
 
 	if !isRestricted {
 		return nil
-	} else if id.IdentityType == api.IdentityTypeCertificateMetrics && entitlement == EntitlementCanViewMetrics {
+	} else if id.IdentityType == api.IdentityTypeCertificateMetricsUnrestricted && entitlement == EntitlementCanViewMetrics {
 		return nil
 	}
 
@@ -166,7 +166,7 @@ func (t *tls) GetPermissionChecker(ctx context.Context, r *http.Request, entitle
 
 	if !isRestricted {
 		return allowFunc(true), nil
-	} else if id.IdentityType == api.IdentityTypeCertificateMetrics && entitlement == EntitlementCanViewMetrics {
+	} else if id.IdentityType == api.IdentityTypeCertificateMetricsUnrestricted && entitlement == EntitlementCanViewMetrics {
 		return allowFunc(true), nil
 	}
 
@@ -178,6 +178,9 @@ func (t *tls) GetPermissionChecker(ctx context.Context, r *http.Request, entitle
 	// Check server level object types
 	switch entityType {
 	case entity.TypeServer:
+		// We have to keep EntitlementCanViewMetrics here for backwards compatibility with older versions of LXD.
+		// Historically when viewing the metrics endpoint for a specific project with a restricted certificate
+		// also the internal server metrics get returned.
 		if entitlement == EntitlementCanView || entitlement == EntitlementCanViewResources || entitlement == EntitlementCanViewMetrics {
 			return allowFunc(true), nil
 		}

lxd/auth/util.go

@@ -152,6 +152,7 @@ func EntitlementsByEntityType(entityType entity.Type) ([]Entitlement, error) {
 			EntitlementCanDeleteStorageBuckets,
 			EntitlementCanViewOperations,
 			EntitlementCanViewEvents,
+			EntitlementCanViewMetrics,
 		}, nil
 	case entity.TypeServer:
 		return []Entitlement{

lxd/daemon.go

@@ -369,7 +369,7 @@ func (d *Daemon) Authenticate(w http.ResponseWriter, r *http.Request) (trusted b
 	// Validate metrics certificates.
 	if r.URL.Path == "/1.0/metrics" {
 		for _, i := range r.TLS.PeerCertificates {
-			trusted, username := util.CheckTrustState(*i, d.identityCache.X509Certificates(api.IdentityTypeCertificateMetrics), d.endpoints.NetworkCert(), trustCACertificates)
+			trusted, username := util.CheckTrustState(*i, d.identityCache.X509Certificates(api.IdentityTypeCertificateMetricsRestricted, api.IdentityTypeCertificateMetricsUnrestricted), d.endpoints.NetworkCert(), trustCACertificates)
 			if trusted {
 				return true, username, api.AuthenticationMethodTLS, nil, nil
 			}

lxd/db/cluster/certificates.go

@@ -58,7 +58,11 @@ func (cert *Certificate) ToIdentityType() (IdentityType, error) {
 	case certificate.TypeServer:
 		return api.IdentityTypeCertificateServer, nil
 	case certificate.TypeMetrics:
-		return api.IdentityTypeCertificateMetrics, nil
+		if cert.Restricted {
+			return api.IdentityTypeCertificateMetricsRestricted, nil
+		}
+
+		return api.IdentityTypeCertificateMetricsUnrestricted, nil
 	}
 
 	return "", fmt.Errorf("Unknown certificate type `%d`", cert.Type)
@@ -178,7 +182,7 @@ func GetCertificates(ctx context.Context, tx *sql.Tx, filters ...CertificateFilt
 			case certificate.TypeServer:
 				identityTypes = append(identityTypes, api.IdentityTypeCertificateServer)
 			case certificate.TypeMetrics:
-				identityTypes = append(identityTypes, api.IdentityTypeCertificateMetrics)
+				identityTypes = append(identityTypes, api.IdentityTypeCertificateMetricsRestricted, api.IdentityTypeCertificateMetricsUnrestricted)
 			}
 		}
 
@@ -278,9 +282,16 @@ func DeleteCertificates(ctx context.Context, tx *sql.Tx, name string, certificat
 		return DeleteIdentitys(ctx, tx, name, api.IdentityTypeCertificateClientUnrestricted)
 	} else if certificateType == certificate.TypeServer {
 		return DeleteIdentitys(ctx, tx, name, api.IdentityTypeCertificateServer)
+	} else if certificateType == certificate.TypeMetrics {
+		err := DeleteIdentitys(ctx, tx, name, api.IdentityTypeCertificateMetricsRestricted)
+		if err != nil {
+			return err
+		}
+
+		return DeleteIdentitys(ctx, tx, name, api.IdentityTypeCertificateMetricsUnrestricted)
 	}
 
-	return DeleteIdentitys(ctx, tx, name, api.IdentityTypeCertificateMetrics)
+	return nil
 }
 
 // UpdateCertificate updates the certificate matching the given key parameters.

lxd/db/cluster/identities.go

@@ -111,11 +111,12 @@ func (a AuthMethod) Value() (driver.Value, error) {
 type IdentityType string
 
 const (
-	identityTypeCertificateClientRestricted   int64 = 1
-	identityTypeCertificateClientUnrestricted int64 = 2
-	identityTypeCertificateServer             int64 = 3
-	identityTypeCertificateMetrics            int64 = 4
-	identityTypeOIDCClient                    int64 = 5
+	identityTypeCertificateClientRestricted    int64 = 1
+	identityTypeCertificateClientUnrestricted  int64 = 2
+	identityTypeCertificateServer              int64 = 3
+	identityTypeCertificateMetricsRestricted   int64 = 4
+	identityTypeOIDCClient                     int64 = 5
+	identityTypeCertificateMetricsUnrestricted int64 = 6
 )
 
 // Scan implements sql.Scanner for IdentityType. This converts the integer value back into the correct API constant or
@@ -142,8 +143,10 @@ func (i *IdentityType) Scan(value any) error {
 		*i = api.IdentityTypeCertificateClientUnrestricted
 	case identityTypeCertificateServer:
 		*i = api.IdentityTypeCertificateServer
-	case identityTypeCertificateMetrics:
-		*i = api.IdentityTypeCertificateMetrics
+	case identityTypeCertificateMetricsRestricted:
+		*i = api.IdentityTypeCertificateMetricsRestricted
+	case identityTypeCertificateMetricsUnrestricted:
+		*i = api.IdentityTypeCertificateMetricsUnrestricted
 	case identityTypeOIDCClient:
 		*i = api.IdentityTypeOIDCClient
 	default:
@@ -162,8 +165,10 @@ func (i IdentityType) Value() (driver.Value, error) {
 		return identityTypeCertificateClientUnrestricted, nil
 	case api.IdentityTypeCertificateServer:
 		return identityTypeCertificateServer, nil
-	case api.IdentityTypeCertificateMetrics:
-		return identityTypeCertificateMetrics, nil
+	case api.IdentityTypeCertificateMetricsRestricted:
+		return identityTypeCertificateMetricsRestricted, nil
+	case api.IdentityTypeCertificateMetricsUnrestricted:
+		return identityTypeCertificateMetricsUnrestricted, nil
 	case api.IdentityTypeOIDCClient:
 		return identityTypeOIDCClient, nil
 	}
@@ -180,7 +185,9 @@ func (i IdentityType) toCertificateType() (certificate.Type, error) {
 		return certificate.TypeClient, nil
 	case api.IdentityTypeCertificateServer:
 		return certificate.TypeServer, nil
-	case api.IdentityTypeCertificateMetrics:
+	case api.IdentityTypeCertificateMetricsRestricted:
+		return certificate.TypeMetrics, nil
+	case api.IdentityTypeCertificateMetricsUnrestricted:
 		return certificate.TypeMetrics, nil
 	}
 
@@ -244,6 +251,13 @@ func (i Identity) ToCertificate() (*Certificate, error) {
 		return nil, fmt.Errorf("Failed to check restricted status of identity: %w", err)
 	}
 
+	// Metrics certificates can be both restricted and unrestricted.
+	// But an unrestricted metrics certificate has still less permissions as an unrestricted client certificate.
+	// So it does not have full access to LXD only the metrics endpoint.
+	if i.Type == api.IdentityTypeCertificateMetricsUnrestricted {
+		isRestricted = false
+	}
+
 	c := &Certificate{
 		ID:          i.ID,
 		Fingerprint: i.Identifier,

lxd/db/cluster/update.go

@@ -228,7 +228,8 @@ CREATE TABLE identities_projects (
 INSERT INTO identities (id, auth_method, type, identifier, name, metadata) SELECT id, 1, 1, fingerprint, name, json_object('cert', certificate) FROM certificates WHERE type = 1 AND restricted = 1;
 INSERT INTO identities (id, auth_method, type, identifier, name, metadata) SELECT id, 1, 2, fingerprint, name, json_object('cert', certificate) FROM certificates WHERE type = 1 AND restricted = 0;
 INSERT INTO identities (id, auth_method, type, identifier, name, metadata) SELECT id, 1, 3, fingerprint, name, json_object('cert', certificate) FROM certificates WHERE type = 2;
-INSERT INTO identities (id, auth_method, type, identifier, name, metadata) SELECT id, 1, 4, fingerprint, name, json_object('cert', certificate) FROM certificates WHERE type = 3;
+INSERT INTO identities (id, auth_method, type, identifier, name, metadata) SELECT id, 1, 4, fingerprint, name, json_object('cert', certificate) FROM certificates WHERE type = 3 AND restricted = 1;
+INSERT INTO identities (id, auth_method, type, identifier, name, metadata) SELECT id, 1, 6, fingerprint, name, json_object('cert', certificate) FROM certificates WHERE type = 3 AND restricted = 0;
 INSERT INTO identities_projects (identity_id, project_id) SELECT certificate_id, project_id FROM certificates_projects;
 
 DROP TABLE certificates;

lxd/db/cluster/update_test.go

@@ -777,14 +777,15 @@ func TestUpdateFromV69(t *testing.T) {
 INSERT INTO certificates (fingerprint, type, name, certificate, restricted) VALUES ('eeef45f0570ce713864c86ec60c8d88f60b4844d3a8849b262c77cb18e88394d', 1, 'restricted-client', ?, 1);
 INSERT INTO certificates (fingerprint, type, name, certificate, restricted) VALUES ('86ec60c8d88f60b4844d3a8849b262c77cb18e88394deeef45f0570ce713864c', 1, 'unrestricted-client', ?, 0);
 INSERT INTO certificates (fingerprint, type, name, certificate, restricted) VALUES ('49b262c77cb18e88394d8e6ec60c8d8eef45f0570ce713864c8f60b4844d3a88', 2, 'server', ?, 0);
-INSERT INTO certificates (fingerprint, type, name, certificate, restricted) VALUES ('60c8d8eef45f0570ce713864c8f60b4844d3a8849b262c77cb18e88394d8e6ec', 3, 'metrics', ?, 0);
+INSERT INTO certificates (fingerprint, type, name, certificate, restricted) VALUES ('60c8d8eef45f0570ce713864c8f60b4844d3a8849b262c77cb18e88394d8e6ec', 3, 'metrics', ?, 1);
+INSERT INTO certificates (fingerprint, type, name, certificate, restricted) VALUES ('47c88da8fd0cb9a8d44768a445e6c27aee44e078ce74cbaec0726de427bac056', 3, 'metrics', ?, 0);
 INSERT INTO projects (name, description) VALUES ('p1', '');
 INSERT INTO projects (name, description) VALUES ('p2', '');
 INSERT INTO projects (name, description) VALUES ('p3', '');
 INSERT INTO certificates_projects (certificate_id, project_id) VALUES (1, 2);
 INSERT INTO certificates_projects (certificate_id, project_id) VALUES (1, 3);
 INSERT INTO certificates_projects (certificate_id, project_id) VALUES (1, 4);
-`, c1, c2, c1, c2)
+`, c1, c2, c1, c2, c2)
 		require.NoError(t, err)
 	})
 	require.NoError(t, err)
@@ -833,7 +834,14 @@ INSERT INTO certificates_projects (certificate_id, project_id) VALUES (1, 4);
 	assert.Equal(t, c1, metadata.Certificate)
 
 	identity = getTLSIdentityByFingerprint("60c8d8eef45f0570ce713864c8f60b4844d3a8849b262c77cb18e88394d8e6ec")
-	assert.Equal(t, api.IdentityTypeCertificateMetrics, string(identity.Type))
+	assert.Equal(t, api.IdentityTypeCertificateMetricsRestricted, string(identity.Type))
+	assert.Equal(t, "metrics", identity.Name)
+	err = json.Unmarshal([]byte(identity.Metadata), &metadata)
+	require.NoError(t, err)
+	assert.Equal(t, c2, metadata.Certificate)
+
+	identity = getTLSIdentityByFingerprint("47c88da8fd0cb9a8d44768a445e6c27aee44e078ce74cbaec0726de427bac056")
+	assert.Equal(t, api.IdentityTypeCertificateMetricsUnrestricted, string(identity.Type))
 	assert.Equal(t, "metrics", identity.Name)
 	err = json.Unmarshal([]byte(identity.Metadata), &metadata)
 	require.NoError(t, err)

lxd/identity/util.go

@@ -22,7 +22,7 @@ func IsRestrictedIdentityType(identityType string) (bool, error) {
 // identity types must correspond to an authentication method. An error is returned if the identity type is not recognised.
 func AuthenticationMethodFromIdentityType(identityType string) (string, error) {
 	switch identityType {
-	case api.IdentityTypeCertificateClientRestricted, api.IdentityTypeCertificateClientUnrestricted, api.IdentityTypeCertificateServer, api.IdentityTypeCertificateMetrics:
+	case api.IdentityTypeCertificateClientRestricted, api.IdentityTypeCertificateClientUnrestricted, api.IdentityTypeCertificateServer, api.IdentityTypeCertificateMetricsRestricted, api.IdentityTypeCertificateMetricsUnrestricted:
 		return api.AuthenticationMethodTLS, nil
 	case api.IdentityTypeOIDCClient:
 		return api.AuthenticationMethodOIDC, nil

lxd/metrics/metrics.go

@@ -7,8 +7,6 @@ import (
 	"strings"
 
 	"github.com/canonical/lxd/shared"
-	"github.com/canonical/lxd/shared/api"
-	"github.com/canonical/lxd/shared/entity"
 )
 
 // NewMetricSet returns a new MetricSet.
@@ -24,26 +22,14 @@ func NewMetricSet(labels map[string]string) *MetricSet {
 	return &out
 }
 
-// FilterSamples filters the existing MetricSet using the given permission checker. Samples not containing the "project" label are skipped.
-func (m *MetricSet) FilterSamples(permissionChecker func(entityURL *api.URL) bool) {
+// FilterSamples filters the existing samples based on the provided check.
+// When checking a sample for permission its labels get passed into the check function
+// so that the samples relation to specific identity types can be used for verification.
+func (m *MetricSet) FilterSamples(permissionCheck func(labels map[string]string) bool) {
 	for metricType, samples := range m.set {
 		allowedSamples := make([]Sample, 0, len(samples))
 		for _, s := range samples {
-			projectName := s.Labels["project"]
-			instanceName := s.Labels["name"]
-			if projectName == "" {
-				continue
-			}
-
-			var hasPermission bool
-
-			if instanceName != "" {
-				hasPermission = permissionChecker(entity.InstanceURL(projectName, instanceName))
-			} else {
-				hasPermission = permissionChecker(entity.ProjectURL(projectName))
-			}
-
-			if hasPermission {
+			if permissionCheck(s.Labels) {
 				allowedSamples = append(allowedSamples, s)
 			}
 		}

lxd/metrics/metrics_test.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/canonical/lxd/shared/api"
 	"github.com/canonical/lxd/shared/entity"
 )
 
@@ -20,21 +19,21 @@ func TestMetricSet_FilterSamples(t *testing.T) {
 	}
 
 	m := newMetricSet()
-	permissionChecker := func(u *api.URL) bool {
-		return u.String() == entity.InstanceURL("default", "jammy").String()
+	filter := func(labels map[string]string) bool {
+		return entity.InstanceURL(labels["project"], labels["name"]).String() == entity.InstanceURL("default", "jammy").String()
 	}
 
-	m.FilterSamples(permissionChecker)
+	m.FilterSamples(filter)
 
 	// Should still contain the sample
 	require.Equal(t, []Sample{{Value: 10, Labels: labels}}, m.set[CPUSecondsTotal])
 
 	m = newMetricSet()
-	permissionChecker = func(u *api.URL) bool {
-		return u.String() == entity.InstanceURL("not-default", "jammy").String()
+	filter = func(labels map[string]string) bool {
+		return entity.InstanceURL(labels["project"], labels["name"]).String() == entity.InstanceURL("not-default", "jammy").String()
 	}
 
-	m.FilterSamples(permissionChecker)
+	m.FilterSamples(filter)
 
 	// Should no longer contain the sample.
 	require.Equal(t, []Sample{}, m.set[CPUSecondsTotal])

shared/api/auth.go

@@ -18,10 +18,13 @@ const (
 	// IdentityTypeCertificateServer represents cluster member authentication.
 	IdentityTypeCertificateServer = "Server certificate"
 
-	// IdentityTypeCertificateMetrics represents identities that may only view metrics.
-	IdentityTypeCertificateMetrics = "Metrics certificate"
+	// IdentityTypeCertificateMetricsRestricted represents identities that may only view metrics and are not privileged.
+	IdentityTypeCertificateMetricsRestricted = "Metrics certificate (restricted)"
 
-	// IdentityTypeOIDCClient represents an identity that authenticates with OIDC..
+	// IdentityTypeCertificateMetricsUnrestricted represents identities that may only view metrics and are privileged.
+	IdentityTypeCertificateMetricsUnrestricted = "Metrics certificate (unrestricted)"
+
+	// IdentityTypeOIDCClient represents an identity that authenticates with OIDC.
 	IdentityTypeOIDCClient = "OIDC client"
 )