From 87bc3d4ef3701915e695f5c779b4d235c38b8df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Mon, 25 Mar 2024 10:30:37 +0100 Subject: [PATCH 01/15] shared/api: Fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- shared/api/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/api/auth.go b/shared/api/auth.go index 505d85900981..8eb63fc11096 100644 --- a/shared/api/auth.go +++ b/shared/api/auth.go @@ -21,7 +21,7 @@ const ( // IdentityTypeCertificateMetrics represents identities that may only view metrics. IdentityTypeCertificateMetrics = "Metrics certificate" - // IdentityTypeOIDCClient represents an identity that authenticates with OIDC.. + // IdentityTypeOIDCClient represents an identity that authenticates with OIDC. IdentityTypeOIDCClient = "OIDC client" ) From 9f3bb63782186dad1193ed20e156b364eb4e1a02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Mon, 25 Mar 2024 14:37:30 +0100 Subject: [PATCH 02/15] lxd/api_metrics: Check individual project permissions if set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/api_metrics.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lxd/api_metrics.go b/lxd/api_metrics.go index 990bf0cf1b65..73ada240eb35 100644 --- a/lxd/api_metrics.go +++ b/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 From d3b5ab7b66a7313da508e9876c0ec9d65e7770f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 11:51:18 +0100 Subject: [PATCH 03/15] lxd/metrics: Use label aware permission check when filtering samples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/metrics/metrics.go | 24 +++++------------------- lxd/metrics/metrics_test.go | 13 ++++++------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/lxd/metrics/metrics.go b/lxd/metrics/metrics.go index b422a24e6659..37307f6c0a4a 100644 --- a/lxd/metrics/metrics.go +++ b/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) } } diff --git a/lxd/metrics/metrics_test.go b/lxd/metrics/metrics_test.go index 9c2c03cafd06..85db4173b887 100644 --- a/lxd/metrics/metrics_test.go +++ b/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]) From d94e0ec7daf01aa5bd12b5901b0f6566c326b193 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Mon, 25 Mar 2024 14:38:47 +0100 Subject: [PATCH 04/15] lxd/api_metrics: Filter metrics by looping only once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This way we can apply multiple filters on internal server and project metrics at once. Additionally this ensures that entity URLs are always called with the right type. Otherwise this will raise a warning for every sample. Signed-off-by: Julian Pelizäus --- lxd/api_metrics.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lxd/api_metrics.go b/lxd/api_metrics.go index 73ada240eb35..ad85bcfdaa96 100644 --- a/lxd/api_metrics.go +++ b/lxd/api_metrics.go @@ -331,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()) } From dc1de11d73c796854ac27677f9e439163fc94605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Mon, 25 Mar 2024 14:40:28 +0100 Subject: [PATCH 05/15] lxd/auth/driver_tls: Allow viewing metrics for unrestricted metrics certs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/auth/driver_tls.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lxd/auth/driver_tls.go b/lxd/auth/driver_tls.go index 6e796ed903d5..59d344bbcd30 100644 --- a/lxd/auth/driver_tls.go +++ b/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 } From 1f2c8d2c057a98c3eda4e6fe4a03431488444d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 11:55:59 +0100 Subject: [PATCH 06/15] lxd/db/cluster: Add identityTypeCertificateMetricsRestricted and identityTypeCertificateMetricsUnrestricted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This effectively replaces identityTypeCertificateMetrics with identityTypeCertificateMetricsRestricted and adds the new type identityTypeCertificateMetricsUnrestricted. By replacing identityTypeCertificateMetrics with identityTypeCertificateMetricsRestricted we ensure that restricted metrics certificates that were falsely moved to identityTypeCertificateMetrics are still considered to be secure. On the other side this marks unrestricted certificates as restricted by default. This has to be manually modified by unrestricting the respective metrics certficiate. Signed-off-by: Julian Pelizäus --- lxd/db/cluster/identities.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lxd/db/cluster/identities.go b/lxd/db/cluster/identities.go index 26f01917f847..9e097b7ccab2 100644 --- a/lxd/db/cluster/identities.go +++ b/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 } From 60e497187abdebcf482d081d3362418aac9cda33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Thu, 28 Mar 2024 18:07:02 +0100 Subject: [PATCH 07/15] lxd/db/cluster/identities: Handle unrestricted metrics certificates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/db/cluster/identities.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lxd/db/cluster/identities.go b/lxd/db/cluster/identities.go index 9e097b7ccab2..db64117353fa 100644 --- a/lxd/db/cluster/identities.go +++ b/lxd/db/cluster/identities.go @@ -251,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, From 72661bb09f735904787d5f9959a227a6327a26f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 12:01:37 +0100 Subject: [PATCH 08/15] shared/api/auth: Replace IdentityTypeCertificateMetrics with a restricted and unrestricted type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- shared/api/auth.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shared/api/auth.go b/shared/api/auth.go index 8eb63fc11096..935ffa680395 100644 --- a/shared/api/auth.go +++ b/shared/api/auth.go @@ -18,8 +18,11 @@ 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)" + + // 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" From 35f9a3aca340f253e4ddd18ce4aae776a4375cb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 11:59:03 +0100 Subject: [PATCH 09/15] lxd/daemon: Use IdentityTypeCertificateMetricsRestricted and IdentityTypeCertificateMetricsUnrestricted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/daemon.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd/daemon.go b/lxd/daemon.go index d2f0de7520b5..da64dcb93704 100644 --- a/lxd/daemon.go +++ b/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 } From b7054548cbe9c454e89fade09bb4440955059dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 12:00:26 +0100 Subject: [PATCH 10/15] lxd/db/cluster/certificates: Use IdentityTypeCertificateMetricsRestricted and IdentityTypeCertificateMetricsUnrestricted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/db/cluster/certificates.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lxd/db/cluster/certificates.go b/lxd/db/cluster/certificates.go index 63ac8315d880..f1bd1a143bb8 100644 --- a/lxd/db/cluster/certificates.go +++ b/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. From 525ea8fc92463dedd835d8af018d05bd65e8bc2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 12:00:55 +0100 Subject: [PATCH 11/15] lxd/identity: Use IdentityTypeCertificateMetricsRestricted and IdentityTypeCertificateMetricsUnrestricted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- lxd/identity/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd/identity/util.go b/lxd/identity/util.go index d7e715ef3a69..99e8f9959fe1 100644 --- a/lxd/identity/util.go +++ b/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 From ad57cca7062a26d17485efd3b6fc17015c0a40c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 12:03:26 +0100 Subject: [PATCH 12/15] lxd/auth/openfga: Extend can_view_metrics entitlement to projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows granting the can_view_metrics entitlement on specific projects so that requests to the metrics endpoint containing the project query parameter only return the respective project's metrics. Signed-off-by: Julian Pelizäus --- lxd/auth/authorization_types.go | 2 +- lxd/auth/driver_openfga_model.openfga | 1 + lxd/auth/util.go | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lxd/auth/authorization_types.go b/lxd/auth/authorization_types.go index 729975b096f2..9a9bad9f9eb9 100644 --- a/lxd/auth/authorization_types.go +++ b/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. diff --git a/lxd/auth/driver_openfga_model.openfga b/lxd/auth/driver_openfga_model.openfga index c450d98c2035..9ec9e19ccd9c 100644 --- a/lxd/auth/driver_openfga_model.openfga +++ b/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] diff --git a/lxd/auth/util.go b/lxd/auth/util.go index 59daa72925f4..3a7be6b31ce0 100644 --- a/lxd/auth/util.go +++ b/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{ From 22a307623c7fe00d0539900079f9719559fae894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 12:05:43 +0100 Subject: [PATCH 13/15] lxd/db/cluster/update: Fix updateFromV69 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure that restricted certificates are mapped to IdentityTypeCertificateMetricsRestricted and vice versa. Signed-off-by: Julian Pelizäus --- lxd/db/cluster/update.go | 3 ++- lxd/db/cluster/update_test.go | 14 +++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go index bea9bcce4fbb..f94be4168c3f 100644 --- a/lxd/db/cluster/update.go +++ b/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; diff --git a/lxd/db/cluster/update_test.go b/lxd/db/cluster/update_test.go index 01d378218f12..ee9315b3fe6f 100644 --- a/lxd/db/cluster/update_test.go +++ b/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) From d1ba04ae12b4b88ffcbcec3fec1c7943268c1033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Tue, 26 Mar 2024 12:56:52 +0100 Subject: [PATCH 14/15] test/suites/auth: Update test to account for can_view_metrics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julian Pelizäus --- test/suites/auth.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/suites/auth.sh b/test/suites/auth.sh index fd07641c3928..2a079e7b6a31 100644 --- a/test/suites/auth.sh +++ b/test/suites/auth.sh @@ -119,7 +119,7 @@ EOF echo "${list_output}" | grep -Fq 'server,/1.0,"admin,can_create_groups,can_create_identities,can_create_projects,can_create_storage_pools,can_delete_groups,can_delete_identities,can_delete_projects,can_delete_storage_pools,can_edit,can_edit_groups,can_edit_identities,can_edit_projects,can_edit_storage_pools,can_override_cluster_target_restriction,can_view,can_view_groups,can_view_identities,can_view_metrics,can_view_permissions,can_view_privileged_events,can_view_projects,can_view_resources,can_view_warnings,permission_manager,project_manager,storage_pool_manager,viewer"' list_output="$(lxc auth permission list entity_type=project --format csv --max-entitlements 0)" - echo "${list_output}" | grep -Fq 'project,/1.0/projects/default,"can_create_image_aliases,can_create_images,can_create_instances,can_create_network_acls,can_create_network_zones,can_create_networks,can_create_profiles,can_create_storage_buckets,can_create_storage_volumes,can_delete,can_delete_image_aliases,can_delete_images,can_delete_instances,can_delete_network_acls,can_delete_network_zones,can_delete_networks,can_delete_profiles,can_delete_storage_buckets,can_delete_storage_volumes,can_edit,can_edit_image_aliases,can_edit_images,can_edit_instances,can_edit_network_acls,can_edit_network_zones,can_edit_networks,can_edit_profiles,can_edit_storage_buckets,can_edit_storage_volumes,can_operate_instances,can_view,can_view_events,can_view_image_aliases,can_view_images,can_view_instances,can_view_network_acls,can_view_network_zones,can_view_networks,can_view_operations,can_view_profiles,can_view_storage_buckets,can_view_storage_volumes,image_alias_manager,image_manager,instance_manager,network_acl_manager,network_manager,network_zone_manager,operator,profile_manager,storage_bucket_manager,storage_volume_manager,viewer"' + echo "${list_output}" | grep -Fq 'project,/1.0/projects/default,"can_create_image_aliases,can_create_images,can_create_instances,can_create_network_acls,can_create_network_zones,can_create_networks,can_create_profiles,can_create_storage_buckets,can_create_storage_volumes,can_delete,can_delete_image_aliases,can_delete_images,can_delete_instances,can_delete_network_acls,can_delete_network_zones,can_delete_networks,can_delete_profiles,can_delete_storage_buckets,can_delete_storage_volumes,can_edit,can_edit_image_aliases,can_edit_images,can_edit_instances,can_edit_network_acls,can_edit_network_zones,can_edit_networks,can_edit_profiles,can_edit_storage_buckets,can_edit_storage_volumes,can_operate_instances,can_view,can_view_events,can_view_image_aliases,can_view_images,can_view_instances,can_view_metrics,can_view_network_acls,can_view_network_zones,can_view_networks,can_view_operations,can_view_profiles,can_view_storage_buckets,can_view_storage_volumes,image_alias_manager,image_manager,instance_manager,network_acl_manager,network_manager,network_zone_manager,operator,profile_manager,storage_bucket_manager,storage_volume_manager,viewer"' # Test max entitlements flag doesn't apply to entitlements that are assigned. lxc auth group permission add test-group server viewer From 65035608013849ae7c5d1727397b5d833fe6d167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Peliz=C3=A4us?= Date: Wed, 27 Mar 2024 12:46:50 +0100 Subject: [PATCH 15/15] test/suites/metrics: Add restricted and unrestricted certificate tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additionally check the responses if the endpoint gets queried using the project parameter as this is producing different results for restricted and unrestricted metrics certificates. Signed-off-by: Julian Pelizäus --- test/suites/metrics.sh | 57 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/suites/metrics.sh b/test/suites/metrics.sh index 490a321fff0d..514cabc7be34 100644 --- a/test/suites/metrics.sh +++ b/test/suites/metrics.sh @@ -7,12 +7,23 @@ test_metrics() { lxc launch testimage c1 lxc init testimage c2 + # create another container in the non default project + lxc project create foo -c features.images=false -c features.profiles=false + lxc init testimage c3 --project foo + # c1 metrics should show as the container is running lxc query "/1.0/metrics" | grep "name=\"c1\"" + lxc query "/1.0/metrics?project=default" | grep "name=\"c1\"" # c2 metrics should show the container as stopped lxc query "/1.0/metrics" | grep "name=\"c2\"" + lxc query "/1.0/metrics?project=default" | grep "name=\"c2\"" lxc query "/1.0/metrics" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + lxc query "/1.0/metrics?project=default" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + + # c3 metrics from another project also show up for non metrics unrestricted certificate + lxc query "/1.0/metrics" | grep "name=\"c3\"" + lxc query "/1.0/metrics?project=foo" | grep "name=\"c3\"" # create new certificate gen_cert_and_key "${TEST_DIR}/metrics.key" "${TEST_DIR}/metrics.crt" "metrics.local" @@ -22,12 +33,23 @@ test_metrics() { # trust newly created certificate for metrics only lxc config trust add "${TEST_DIR}/metrics.crt" --type=metrics + lxc config trust show "$(openssl x509 -in "${TEST_DIR}/metrics.crt" -outform der | sha256sum | head -c12)" | grep -xF "restricted: false" # c1 metrics should show as the container is running curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics" | grep "name=\"c1\"" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=default" | grep "name=\"c1\"" # c2 metrics should show the container as stopped curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=default" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + + # c3 metrics from another project should show the container as stopped for unrestricted certificate + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics" | grep "name=\"c3\"" | grep "state=\"STOPPED\"" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=foo" | grep "name=\"c3\"" | grep "state=\"STOPPED\"" + + # internal server metrics should be shown as the certificate is not restricted + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics" | grep -E "^lxd_warnings_total [0-9]+$" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=default" | grep -E "^lxd_warnings_total [0-9]+$" # make sure nothing else can be done with this certificate curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/instances" | grep "\"error_code\":403" @@ -38,20 +60,55 @@ test_metrics() { # c1 metrics should show as the container is running curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/metrics" | grep "name=\"c1\"" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/metrics?project=default" | grep "name=\"c1\"" # c2 metrics should show the container as stopped curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/metrics" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/metrics?project=default" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + + # c3 metrics from another project should show the container as stopped for unrestricted metrics certificate + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics" | grep "name=\"c3\"" | grep "state=\"STOPPED\"" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=foo" | grep "name=\"c3\"" | grep "state=\"STOPPED\"" + + # internal server metrics should be shown as the certificate is not restricted + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/metrics" | grep -E "^lxd_warnings_total [0-9]+$" + curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/metrics?project=default" | grep -E "^lxd_warnings_total [0-9]+$" # make sure no other endpoint is available curl -k -s --cert "${TEST_DIR}/metrics.crt" --key "${TEST_DIR}/metrics.key" -X GET "https://${metrics_addr}/1.0/instances" | grep "\"error_code\":404" + # create new certificate + gen_cert_and_key "${TEST_DIR}/metrics-restricted.key" "${TEST_DIR}/metrics-restricted.crt" "metrics-restricted.local" + + # trust newly created certificate for metrics only and mark it as restricted for the foo project + lxc config trust add "${TEST_DIR}/metrics-restricted.crt" --type=metrics --restricted --projects foo + lxc config trust show "$(openssl x509 -in "${TEST_DIR}/metrics-restricted.crt" -outform der | sha256sum | head -c12)" | grep -xF "restricted: true" + + # c3 metrics should show the container as stopped for restricted metrics certificate + curl -k -s --cert "${TEST_DIR}/metrics-restricted.crt" --key "${TEST_DIR}/metrics-restricted.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=foo" | grep "name=\"c3\"" | grep "state=\"STOPPED\"" + + # c3 metrics for the stopped container cannot be viewed via the generic metrics endpoint if the certificate is restricted + ! curl -k -s --cert "${TEST_DIR}/metrics-restricted.crt" --key "${TEST_DIR}/metrics-restricted.key" -X GET "https://${LXD_ADDR}/1.0/metrics" + + # other projects metrics aren't visible as they aren't allowed for the restricted certificate + ! curl -k -s --cert "${TEST_DIR}/metrics-restricted.crt" --key "${TEST_DIR}/metrics-restricted.key" -X GET "https://${LXD_ADDR}/1.0/metrics?project=default" + + # c1 and c2 metrics are not visible as they are in another project + ! curl -k -s --cert "${TEST_DIR}/metrics-restricted.crt" --key "${TEST_DIR}/metrics-restricted.key" -X GET "https://${metrics_addr}/1.0/metrics?project=foo" | grep "name=\"c1\"" + ! curl -k -s --cert "${TEST_DIR}/metrics-restricted.crt" --key "${TEST_DIR}/metrics-restricted.key" -X GET "https://${metrics_addr}/1.0/metrics?project=foo" | grep "name=\"c2\"" | grep "state=\"STOPPED\"" + # test unauthenticated connections ! curl -k -s -X GET "https://${metrics_addr}/1.0/metrics" | grep "name=\"c1\"" || false + ! curl -k -s -X GET "https://${metrics_addr}/1.0/metrics?project=default" | grep "name=\"c1\"" || false lxc config set core.metrics_authentication=false curl -k -s -X GET "https://${metrics_addr}/1.0/metrics" | grep "name=\"c1\"" + curl -k -s -X GET "https://${metrics_addr}/1.0/metrics?project=default" | grep "name=\"c1\"" # Filesystem metrics should contain instance type curl -k -s -X GET "https://${metrics_addr}/1.0/metrics" | grep "lxd_filesystem_avail_bytes" | grep "type=\"container\"" + curl -k -s -X GET "https://${metrics_addr}/1.0/metrics?project=default" | grep "lxd_filesystem_avail_bytes" | grep "type=\"container\"" lxc delete -f c1 c2 + lxc delete -f c3 --project foo + lxc project rm foo }