From 6fcebbbd40b04a346029e04db84846b3720d6d6c Mon Sep 17 00:00:00 2001 From: Simon Murray Date: Tue, 5 Nov 2024 10:12:23 +0000 Subject: [PATCH] Migrate to New Security Model Having access to the compute service should not require direct access to the region service endpoints. Move things so privilege is escalated in the controller/API and not granted to the user directly. --- .../compute/templates/server/deployment.yaml | 2 + go.mod | 6 +-- go.sum | 8 ++-- .../managers/cluster/provisioner.go | 4 +- pkg/server/handler/handler.go | 42 +++++++++++++++---- pkg/server/server.go | 6 ++- 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/charts/compute/templates/server/deployment.yaml b/charts/compute/templates/server/deployment.yaml index f88ac6a..4d6c2ee 100644 --- a/charts/compute/templates/server/deployment.yaml +++ b/charts/compute/templates/server/deployment.yaml @@ -23,6 +23,8 @@ spec: {{- include "unikorn.otlp.flags" . | nindent 8 }} {{- include "unikorn.identity.flags" . | nindent 8 }} {{- include "unikorn.region.flags" . | nindent 8 }} + - --client-certificate-namespace={{ .Release.Namespace }} + - --client-certificate-name=unikorn-compute-client-certificate ports: - name: http containerPort: 6080 diff --git a/go.mod b/go.mod index eee5514..4771479 100644 --- a/go.mod +++ b/go.mod @@ -8,11 +8,12 @@ require ( github.com/oapi-codegen/runtime v1.1.1 github.com/spf13/pflag v1.0.5 github.com/unikorn-cloud/core v0.1.77 - github.com/unikorn-cloud/identity v0.2.42 - github.com/unikorn-cloud/region v0.1.44 + github.com/unikorn-cloud/identity v0.2.44 + github.com/unikorn-cloud/region v0.1.45 go.opentelemetry.io/otel/sdk v1.31.0 k8s.io/api v0.31.1 k8s.io/apimachinery v0.31.1 + k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 sigs.k8s.io/controller-runtime v0.19.0 ) @@ -88,7 +89,6 @@ require ( k8s.io/client-go v0.31.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20241009091222-67ed5848f094 // indirect - k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect diff --git a/go.sum b/go.sum index 7d645a2..27a2086 100644 --- a/go.sum +++ b/go.sum @@ -137,10 +137,10 @@ github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65E github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= github.com/unikorn-cloud/core v0.1.77 h1:DHxIOuS5RAZylzxHF/kUt955TdmE5kJiY/Z2yqC0/e0= github.com/unikorn-cloud/core v0.1.77/go.mod h1:S9AF4PwTQljImb9w0P2jKjzRe8fLM+rx+ZbxrAHw/yE= -github.com/unikorn-cloud/identity v0.2.42 h1:9amEcydDq23RZYO4rTtxOhVgw/BH1mdXQgq0fWT+RM0= -github.com/unikorn-cloud/identity v0.2.42/go.mod h1:JMbS6iTYzt0OVt5AkqZys3WVnpLabGvUl8kGWcxzFZI= -github.com/unikorn-cloud/region v0.1.44 h1:GJnUHFBkxnOAssHd7NBXY9Zpva6lB5ozKPesHnORVzo= -github.com/unikorn-cloud/region v0.1.44/go.mod h1:N5wS4Js49JR5WARnRwzeVXRL//M+WVKZBAiqQVA7Yao= +github.com/unikorn-cloud/identity v0.2.44 h1:tXV/qsJ77Dkx8ba8gnBFXHWUgBNsJ2oo/5TjnyhkH7U= +github.com/unikorn-cloud/identity v0.2.44/go.mod h1:JMbS6iTYzt0OVt5AkqZys3WVnpLabGvUl8kGWcxzFZI= +github.com/unikorn-cloud/region v0.1.45 h1:qpUwB+s/SEZNHZqwHTYovtWUVdJB2AKEl06NbiIwnOw= +github.com/unikorn-cloud/region v0.1.45/go.mod h1:QqWLEfB8bNRIUAU7h5JjkQsjyJdTV+2ltDYksRjKMds= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/pkg/provisioners/managers/cluster/provisioner.go b/pkg/provisioners/managers/cluster/provisioner.go index 9a243f2..bb5af8c 100644 --- a/pkg/provisioners/managers/cluster/provisioner.go +++ b/pkg/provisioners/managers/cluster/provisioner.go @@ -112,14 +112,14 @@ func (p *Provisioner) getRegionClient(ctx context.Context, traceName string) (co tokenIssuer := identityclient.NewTokenIssuer(cli, p.options.identityOptions, &p.options.clientOptions, constants.Application, constants.Version) - ctx, err = tokenIssuer.Context(ctx, traceName) + token, err := tokenIssuer.Issue(ctx, traceName) if err != nil { return nil, nil, err } getter := regionclient.New(cli, p.options.regionOptions, &p.options.clientOptions) - client, err := getter.Client(ctx) + client, err := getter.Client(ctx, token) if err != nil { return nil, nil, err } diff --git a/pkg/server/handler/handler.go b/pkg/server/handler/handler.go index 04987dd..d763dcf 100644 --- a/pkg/server/handler/handler.go +++ b/pkg/server/handler/handler.go @@ -18,6 +18,7 @@ limitations under the License. package handler import ( + "context" "net/http" "slices" @@ -25,9 +26,11 @@ import ( "github.com/unikorn-cloud/compute/pkg/server/handler/cluster" "github.com/unikorn-cloud/core/pkg/server/errors" "github.com/unikorn-cloud/core/pkg/server/util" + identityclient "github.com/unikorn-cloud/identity/pkg/client" identityapi "github.com/unikorn-cloud/identity/pkg/openapi" "github.com/unikorn-cloud/identity/pkg/rbac" regionclient "github.com/unikorn-cloud/region/pkg/client" + regionapi "github.com/unikorn-cloud/region/pkg/openapi" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -42,21 +45,40 @@ type Handler struct { // options allows behaviour to be defined on the CLI. options *Options + // issuer provides privilge escallation for the API so the end user doesn't + // have to be granted unnecessary privilige. + issuer *identityclient.TokenIssuer + // region is a client to access regions. region *regionclient.Client } -func New(client client.Client, namespace string, options *Options, region *regionclient.Client) (*Handler, error) { +func New(client client.Client, namespace string, options *Options, issuer *identityclient.TokenIssuer, region *regionclient.Client) (*Handler, error) { h := &Handler{ client: client, namespace: namespace, options: options, + issuer: issuer, region: region, } return h, nil } +func (h *Handler) regionClient(ctx context.Context) (*regionapi.ClientWithResponses, error) { + token, err := h.issuer.Issue(ctx, "kubernetes-api") + if err != nil { + return nil, err + } + + region, err := h.region.Client(ctx, token) + if err != nil { + return nil, err + } + + return region, nil +} + /* func (h *Handler) setCacheable(w http.ResponseWriter) { w.Header().Add("Cache-Control", fmt.Sprintf("max-age=%d", h.options.CacheMaxAge/time.Second)) @@ -69,7 +91,7 @@ func (h *Handler) setUncacheable(w http.ResponseWriter) { } func (h *Handler) GetApiV1OrganizationsOrganizationIDClusters(w http.ResponseWriter, r *http.Request, organizationID openapi.OrganizationIDParameter) { - region, err := h.region.Client(r.Context()) + region, err := h.regionClient(r.Context()) if err != nil { errors.HandleError(w, r, err) return @@ -81,8 +103,10 @@ func (h *Handler) GetApiV1OrganizationsOrganizationIDClusters(w http.ResponseWri return } + ctx := r.Context() + result = slices.DeleteFunc(result, func(resource openapi.ComputeClusterRead) bool { - return rbac.AllowProjectScope(r.Context(), "computeclusters", identityapi.Read, organizationID, resource.Metadata.ProjectId) != nil + return rbac.AllowProjectScope(ctx, "compute:clusters", identityapi.Read, organizationID, resource.Metadata.ProjectId) != nil }) h.setUncacheable(w) @@ -90,7 +114,7 @@ func (h *Handler) GetApiV1OrganizationsOrganizationIDClusters(w http.ResponseWri } func (h *Handler) PostApiV1OrganizationsOrganizationIDProjectsProjectIDClusters(w http.ResponseWriter, r *http.Request, organizationID openapi.OrganizationIDParameter, projectID openapi.ProjectIDParameter) { - if err := rbac.AllowProjectScope(r.Context(), "computeclusters", identityapi.Create, organizationID, projectID); err != nil { + if err := rbac.AllowProjectScope(r.Context(), "compute:clusters", identityapi.Create, organizationID, projectID); err != nil { errors.HandleError(w, r, err) return } @@ -102,7 +126,7 @@ func (h *Handler) PostApiV1OrganizationsOrganizationIDProjectsProjectIDClusters( return } - region, err := h.region.Client(r.Context()) + region, err := h.regionClient(r.Context()) if err != nil { errors.HandleError(w, r, err) return @@ -119,12 +143,12 @@ func (h *Handler) PostApiV1OrganizationsOrganizationIDProjectsProjectIDClusters( } func (h *Handler) DeleteApiV1OrganizationsOrganizationIDProjectsProjectIDClustersClusterID(w http.ResponseWriter, r *http.Request, organizationID openapi.OrganizationIDParameter, projectID openapi.ProjectIDParameter, clusterID openapi.ClusterIDParameter) { - if err := rbac.AllowProjectScope(r.Context(), "computeclusters", identityapi.Delete, organizationID, projectID); err != nil { + if err := rbac.AllowProjectScope(r.Context(), "compute:clusters", identityapi.Delete, organizationID, projectID); err != nil { errors.HandleError(w, r, err) return } - region, err := h.region.Client(r.Context()) + region, err := h.regionClient(r.Context()) if err != nil { errors.HandleError(w, r, err) return @@ -140,7 +164,7 @@ func (h *Handler) DeleteApiV1OrganizationsOrganizationIDProjectsProjectIDCluster } func (h *Handler) PutApiV1OrganizationsOrganizationIDProjectsProjectIDClustersClusterID(w http.ResponseWriter, r *http.Request, organizationID openapi.OrganizationIDParameter, projectID openapi.ProjectIDParameter, clusterID openapi.ClusterIDParameter) { - if err := rbac.AllowProjectScope(r.Context(), "computeclusters", identityapi.Update, organizationID, projectID); err != nil { + if err := rbac.AllowProjectScope(r.Context(), "compute:clusters", identityapi.Update, organizationID, projectID); err != nil { errors.HandleError(w, r, err) return } @@ -152,7 +176,7 @@ func (h *Handler) PutApiV1OrganizationsOrganizationIDProjectsProjectIDClustersCl return } - region, err := h.region.Client(r.Context()) + region, err := h.regionClient(r.Context()) if err != nil { errors.HandleError(w, r, err) return diff --git a/pkg/server/server.go b/pkg/server/server.go index 3c9c161..4bb304e 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -152,9 +152,13 @@ func (s *Server) GetServer(client client.Client) (*http.Server, error) { }, } + // NOTE: any clients that are used, must issue new tokens as this service to + // prevent the user having to be granted excessive privilege. + issuer := identityclient.NewTokenIssuer(client, s.IdentityOptions, &s.ClientOptions, constants.Application, constants.Version) + region := regionclient.New(client, s.RegionOptions, &s.ClientOptions) - handlerInterface, err := handler.New(client, s.Options.Namespace, &s.HandlerOptions, region) + handlerInterface, err := handler.New(client, s.Options.Namespace, &s.HandlerOptions, issuer, region) if err != nil { return nil, err }