diff --git a/api/types/role.go b/api/types/role.go index 5b51424f1f65a..e40229d78bb23 100644 --- a/api/types/role.go +++ b/api/types/role.go @@ -891,24 +891,11 @@ func (r *RoleV6) CheckAndSetDefaults() error { r.Spec.Allow.DatabaseLabels = Labels{Wildcard: []string{Wildcard}} } - if len(r.Spec.Allow.KubernetesResources) == 0 { - r.Spec.Allow.KubernetesResources = []KubernetesResource{ - { - Kind: KindKubePod, - Namespace: Wildcard, - Name: Wildcard, - }, - } - } else { - if err := validateRoleSpecKubeResources(r.Spec); err != nil { - return trace.Wrap(err) - } - } + fallthrough case V4, V5: // Labels default to nil/empty for v4+ roles - // Allow unrestricted access to all pods. - if len(r.Spec.Allow.KubernetesResources) == 0 { + if len(r.Spec.Allow.KubernetesResources) == 0 && len(r.Spec.Allow.KubernetesLabels) > 0 { r.Spec.Allow.KubernetesResources = []KubernetesResource{ { Kind: KindKubePod, @@ -916,11 +903,12 @@ func (r *RoleV6) CheckAndSetDefaults() error { Name: Wildcard, }, } - } else { - if err := validateRoleSpecKubeResources(r.Spec); err != nil { - return trace.Wrap(err) - } } + + if err := validateRoleSpecKubeResources(r.Spec); err != nil { + return trace.Wrap(err) + } + case V6: if err := validateRoleSpecKubeResources(r.Spec); err != nil { return trace.Wrap(err) diff --git a/integrations/operator/controllers/resources/role_controller_test.go b/integrations/operator/controllers/resources/role_controller_test.go index 270913f4c3113..d5744415c9018 100644 --- a/integrations/operator/controllers/resources/role_controller_test.go +++ b/integrations/operator/controllers/resources/role_controller_test.go @@ -94,13 +94,6 @@ allow: expectedSpec: &types.RoleSpecV6{ Allow: types.RoleConditions{ Logins: []string{"ubuntu", "root"}, - KubernetesResources: []types.KubernetesResource{ - { - Kind: types.KindKubePod, - Namespace: types.Wildcard, - Name: types.Wildcard, - }, - }, }, }, }, @@ -117,13 +110,6 @@ allow: NodeLabels: map[string]apiutils.Strings{ "*": {"*"}, }, - KubernetesResources: []types.KubernetesResource{ - { - Kind: types.KindKubePod, - Namespace: types.Wildcard, - Name: types.Wildcard, - }, - }, }, }, }, @@ -140,13 +126,6 @@ allow: NodeLabels: map[string]apiutils.Strings{ "*": {"*"}, }, - KubernetesResources: []types.KubernetesResource{ - { - Kind: types.KindKubePod, - Namespace: types.Wildcard, - Name: types.Wildcard, - }, - }, }, }, }, diff --git a/lib/kube/proxy/constants.go b/lib/kube/proxy/constants.go index b2d2ea1a29b72..caf968bfe19d9 100644 --- a/lib/kube/proxy/constants.go +++ b/lib/kube/proxy/constants.go @@ -75,3 +75,9 @@ const ( // PortForwardProtocolV1Name is the subprotocol "portforward.k8s.io" is used for port forwarding PortForwardProtocolV1Name = "portforward.k8s.io" ) + +const ( + // kubernetesResourcesKey is the key used to store the kubernetes resources + // in the role. + kubernetesResourcesKey = "kubernetes_resources" +) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index 8a8ba0b4addf8..def4e0e008ef1 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -322,6 +322,7 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) { router.GET("/api/:ver/pods", fwd.withAuth(fwd.listPods)) router.GET("/api/:ver/namespaces/:podNamespace/pods", fwd.withAuth(fwd.listPods)) + router.POST("/apis/authorization.k8s.io/:ver/selfsubjectaccessreviews", fwd.withAuth(fwd.selfSubjectAccessReviews)) router.DELETE("/api/:ver/namespaces/:podNamespace/pods", fwd.withAuth(fwd.deletePodsCollection)) router.POST("/api/:ver/namespaces/:podNamespace/pods", fwd.withAuth( func(ctx *authContext, w http.ResponseWriter, r *http.Request, _ httprouter.Params) (any, error) { @@ -2834,7 +2835,7 @@ func (f *Forwarder) handleDeleteCollectionReq(req *http.Request, authCtx *authCo const internalErrStatus = http.StatusInternalServerError // get content-type value - contentType := responsewriters.GetContentHeader(memWriter.Header()) + contentType := responsewriters.GetContentTypeHeader(memWriter.Header()) encoder, decoder, err := newEncoderAndDecoderForContentType(contentType, newClientNegotiator()) if err != nil { return internalErrStatus, trace.Wrap(err) @@ -2970,7 +2971,9 @@ func kubeResourceDeniedAccessMsg(user, method string, kubeResource *types.Kubern apiGroup := "" // "" is forbidden: User "" cannot create resource "" in API group "" in the namespace "" return fmt.Sprintf( - "%s %q is forbidden: User %q cannot %s resource %q in API group %q in the namespace %q", + "%s %q is forbidden: User %q cannot %s resource %q in API group %q in the namespace %q\n"+ + "Ask your Teleport admin to ensure that your Teleport role includes access to the pod in %q field.\n"+ + "Check by running: kubectl auth can-i %s %s/%s --namespace %s ", resource, kubeResource.Name, user, @@ -2978,6 +2981,11 @@ func kubeResourceDeniedAccessMsg(user, method string, kubeResource *types.Kubern resource, apiGroup, kubeResource.Namespace, + kubernetesResourcesKey, + getRequestVerb(method), + resource, + kubeResource.Name, + kubeResource.Namespace, ) } diff --git a/lib/kube/proxy/pod_filters.go b/lib/kube/proxy/pod_filters.go index 5f2e4e6a47917..658d869bba3aa 100644 --- a/lib/kube/proxy/pod_filters.go +++ b/lib/kube/proxy/pod_filters.go @@ -327,7 +327,7 @@ func filterBuffer(filterWrapper responsewriters.FilterWrapper, src *responsewrit return nil } - filter, err := filterWrapper(responsewriters.GetContentHeader(src.Header()), src.Status()) + filter, err := filterWrapper(responsewriters.GetContentTypeHeader(src.Header()), src.Status()) if err != nil { return trace.Wrap(err) } diff --git a/lib/kube/proxy/pod_rbac_test.go b/lib/kube/proxy/pod_rbac_test.go index 9bbf4e73d560b..01dd9b99d388f 100644 --- a/lib/kube/proxy/pod_rbac_test.go +++ b/lib/kube/proxy/pod_rbac_test.go @@ -321,7 +321,13 @@ func TestListPodRBAC(t *testing.T) { testPodName, metav1.GetOptions{}, ) - require.Equal(t, tt.want.getTestPodResult, err) + + if tt.want.getTestPodResult == nil { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tt.want.getTestPodResult.Error()) + } }) } } diff --git a/lib/kube/proxy/responsewriters/filters.go b/lib/kube/proxy/responsewriters/filters.go index da34cc5a95de0..73ff1d3992182 100644 --- a/lib/kube/proxy/responsewriters/filters.go +++ b/lib/kube/proxy/responsewriters/filters.go @@ -16,6 +16,7 @@ package responsewriters import ( "io" + "mime" "net/http" "k8s.io/apimachinery/pkg/runtime" @@ -54,12 +55,23 @@ type Filter interface { // - allowedPods: excluded if (namespace,name) not match a single entry. type FilterWrapper func(contentType string, responseCode int) (Filter, error) -// GetContentHeader checks for the presence of the "Content-Type" header and +// GetContentTypeHeader checks for the presence of the "Content-Type" header and // returns its value or returns the default content-type: "application/json". -func GetContentHeader(header http.Header) string { +func GetContentTypeHeader(header http.Header) string { contentType := header.Get(ContentTypeHeader) if len(contentType) > 0 { return contentType } return DefaultContentType } + +// SetContentTypeHeader checks for the presence of the "Content-Type" header and +// sets its media type value or sets the default content-type: "application/json". +func SetContentTypeHeader(w http.ResponseWriter, header http.Header) { + contentType := header.Get(ContentTypeHeader) + if mediaType, _, err := mime.ParseMediaType(contentType); err == nil { + w.Header().Set(ContentTypeHeader, mediaType) + return + } + w.Header().Set(ContentTypeHeader, DefaultContentType) +} diff --git a/lib/kube/proxy/responsewriters/watcher.go b/lib/kube/proxy/responsewriters/watcher.go index 3e5fad48008d4..ef6cbcc7115f9 100644 --- a/lib/kube/proxy/responsewriters/watcher.go +++ b/lib/kube/proxy/responsewriters/watcher.go @@ -107,7 +107,7 @@ func (w *WatcherResponseWriter) Header() http.Header { func (w *WatcherResponseWriter) WriteHeader(code int) { w.status = code w.target.WriteHeader(code) - contentType := GetContentHeader(w.Header()) + contentType := GetContentTypeHeader(w.Header()) w.group.Go( func() error { switch { diff --git a/lib/kube/proxy/self_subject_reviews.go b/lib/kube/proxy/self_subject_reviews.go new file mode 100644 index 0000000000000..33556e11a1a14 --- /dev/null +++ b/lib/kube/proxy/self_subject_reviews.go @@ -0,0 +1,264 @@ +// Copyright 2023 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package proxy + +import ( + "bytes" + "errors" + "fmt" + "io" + "net/http" + "strings" + + "github.com/gravitational/trace" + "github.com/julienschmidt/httprouter" + semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + oteltrace "go.opentelemetry.io/otel/trace" + "golang.org/x/exp/slices" + authv1 "k8s.io/api/authorization/v1" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/httplib" + "github.com/gravitational/teleport/lib/kube/proxy/responsewriters" + "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/utils" +) + +// selfSubjectAccessReviews intercepts self subject access reviews requests and pre-validates +// them by applying the kubernetes resources RBAC rules to the request. +// If the self subject access review is allowed, the request is forwarded to the +// kubernetes API server for final validation. +func (f *Forwarder) selfSubjectAccessReviews(authCtx *authContext, w http.ResponseWriter, req *http.Request, p httprouter.Params) (resp any, err error) { + ctx, span := f.cfg.tracer.Start( + req.Context(), + "kube.Forwarder/selfSubjectAccessReviews", + oteltrace.WithSpanKind(oteltrace.SpanKindServer), + oteltrace.WithAttributes( + semconv.RPCServiceKey.String(f.cfg.KubeServiceType), + semconv.RPCMethodKey.String("selfSubjectAccessReviews"), + semconv.RPCSystemKey.String("kube"), + ), + ) + req = req.WithContext(ctx) + defer span.End() + + // only allow self subject access reviews for the local teleport cluster + // and not for remote clusters + if !authCtx.teleportCluster.isRemote { + if err := f.validateSelfSubjectAccessReview(authCtx, w, req); trace.IsAccessDenied(err) { + return nil, nil + } else if err != nil { + return nil, trace.Wrap(err) + } + } + sess, err := f.newClusterSession(req.Context(), *authCtx) + if err != nil { + // This error goes to kubernetes client and is not visible in the logs + // of the teleport server if not logged here. + f.log.Errorf("Failed to create cluster session: %v.", err) + return nil, trace.Wrap(err) + } + // sess.Close cancels the connection monitor context to release it sooner. + // When the server is under heavy load it can take a while to identify that + // the underlying connection is gone. This change prevents that and releases + // the resources as soon as we know the session is no longer active. + defer sess.close() + + sess.upgradeToHTTP2 = true + sess.forwarder, err = f.makeSessionForwarder(sess) + if err != nil { + return nil, trace.Wrap(err) + } + + if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil { + // This error goes to kubernetes client and is not visible in the logs + // of the teleport server if not logged here. + f.log.Errorf("Failed to set up forwarding headers: %v.", err) + return nil, trace.Wrap(err) + } + rw := httplib.NewResponseStatusRecorder(w) + sess.forwarder.ServeHTTP(rw, req) + + f.emitAuditEvent(authCtx, req, sess, rw.Status()) + + return nil, nil +} + +// validateSelfSubjectAccessReview validates the self subject access review +// request by applying the kubernetes resources RBAC rules to the request. +func (f *Forwarder) validateSelfSubjectAccessReview(actx *authContext, w http.ResponseWriter, req *http.Request) error { + negotiator := newClientNegotiator() + encoder, decoder, err := newEncoderAndDecoderForContentType(responsewriters.GetContentTypeHeader(req.Header), negotiator) + if err != nil { + return trace.Wrap(err) + } + accessReview, err := parseSelfSubjectAccessReviewRequest(decoder, req) + if err != nil { + return trace.Wrap(err) + } + + if accessReview.Spec.ResourceAttributes == nil { + return nil + } + + namespace := accessReview.Spec.ResourceAttributes.Namespace + resource := depluralizeResource(accessReview.Spec.ResourceAttributes.Resource) + name := accessReview.Spec.ResourceAttributes.Name + // If the request is for a resource that Teleport does not support, return + // nil and let the Kubernetes API server handle the request. + if !slices.Contains(types.KubernetesResourcesKinds, resource) { + return nil + } + + authPref, err := f.cfg.CachingAuthClient.GetAuthPreference(req.Context()) + if err != nil { + return trace.Wrap(err) + } + + state := actx.GetAccessState(authPref) + switch err := actx.Checker.CheckAccess( + actx.kubeCluster, + state, + services.RoleMatchers{ + // Append a matcher that validates if the Kubernetes resource is allowed + // by the roles that satisfy the Kubernetes Cluster. + &kubernetesResourceMatcher{ + types.KubernetesResource{ + Kind: resource, + Name: name, + Namespace: namespace, + }, + }, + }...); { + case errors.Is(err, services.ErrTrustedDeviceRequired): + return trace.Wrap(err) + case err != nil: + accessReview.Status = authv1.SubjectAccessReviewStatus{ + Allowed: false, + Denied: true, + Reason: fmt.Sprintf( + "access to %s %s/%s denied by Teleport: please ensure that %q field in your Teleport role defines access to the desired resource.\n\n"+ + "Valid example:\n"+ + "kubernetes_resources:\n"+ + "- kind: %s\n"+ + " name: %s\n"+ + " namespace: %s\n", resource, namespace, name, kubernetesResourcesKey, resource, emptyOrWildcard(name), emptyOrWildcard(namespace)), + } + + responsewriters.SetContentTypeHeader(w, req.Header) + if err := encoder.Encode(accessReview, w); err != nil { + return trace.Wrap(err) + } + return trace.Wrap(err) + } + return nil +} + +// emptyOrWildcard returns the string s if it is not empty, otherwise it returns +// '*'. +func emptyOrWildcard(s string) string { + if s == "" { + return fmt.Sprintf("'%s'", types.Wildcard) + } + return s +} + +// parseSelfSubjectAccessReviewRequest parses the request body into a SelfSubjectAccessReview object +// and replaces the body so it can be read again. +func parseSelfSubjectAccessReviewRequest(decoder runtime.Decoder, req *http.Request) (*authv1.SelfSubjectAccessReview, error) { + payload, err := io.ReadAll(req.Body) + if err != nil { + return nil, trace.Wrap(err) + } + req.Body.Close() + + req.Body = io.NopCloser(bytes.NewReader(payload)) + obj, err := decodeAndSetGVK(decoder, payload) + if err != nil { + return nil, trace.Wrap(err) + } + + switch o := obj.(type) { + case *authv1.SelfSubjectAccessReview: + return o, nil + default: + return nil, trace.BadParameter("unexpected object type: %T", obj) + } +} + +// depluralizeResource returns the singular form of the resource if it is plural. +func depluralizeResource(resource string) string { + if strings.HasSuffix(resource, "s") { + return resource[:len(resource)-1] + } + return resource +} + +// kubernetesResourceMatcher matches a role against a Kubernetes Resource. +// This matcher is different form services.KubernetesResourceMatcher because +// if skips some validations if the user doesn't ask for a specific resource. +// If name and namespace are empty, it means that the user wants to match all +// resources of the specified kind for which the user might have access to. +// If the user asks for name="", namespace="" and the role has a matcher +// with name="foo", namespace="bar", the matcher will match but the user +// might not be able to see any resource if the resource does not exist +// in the cluster. +// This matcher assumes the role's kubernetes_resources configured eventually +// match with resources that exist in the cluster. +type kubernetesResourceMatcher struct { + resource types.KubernetesResource +} + +// Match matches a Kubernetes Resource against provided role and condition. +func (m *kubernetesResourceMatcher) Match(role types.Role, condition types.RoleConditionType) (bool, error) { + resources := role.GetKubeResources(condition) + if len(resources) == 0 { + return false, nil + } + for _, resource := range resources { + // TODO(tigrato): evaluate if we should support wildcards as well + // for future compatibility. + if m.resource.Kind != resource.Kind { + continue + } + // If the resource name and namespace are empty, it means that the + // user wants to match all resources of the specified kind. + // We can return true immediately because the user is allowed to get resources + // of the specified kind but might not be able to see any if the matchers do not + // match with any resource. + if m.resource.Name == "" && m.resource.Namespace == "" { + return true, nil + } + if m.resource.Name != "" { + switch ok, err := utils.SliceMatchesRegex(m.resource.Name, []string{resource.Name}); { + case err != nil: + return false, trace.Wrap(err) + case !ok: + continue + } + } + if ok, err := utils.SliceMatchesRegex(m.resource.Namespace, []string{resource.Namespace}); err != nil || ok || m.resource.Namespace == "" { + return ok || m.resource.Namespace == "", trace.Wrap(err) + } + } + + return false, nil +} + +// String returns the matcher's string representation. +func (m *kubernetesResourceMatcher) String() string { + return fmt.Sprintf("kubernetesResourceMatcher(Resource=%v)", m.resource) +} diff --git a/lib/kube/proxy/self_subject_reviews_test.go b/lib/kube/proxy/self_subject_reviews_test.go new file mode 100644 index 0000000000000..a9bc815ff02e6 --- /dev/null +++ b/lib/kube/proxy/self_subject_reviews_test.go @@ -0,0 +1,228 @@ +// Copyright 2023 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package proxy + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + authv1 "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/gravitational/teleport/api/types" + testingkubemock "github.com/gravitational/teleport/lib/kube/proxy/testing/kube_server" +) + +func TestSelfSubjectAccessReviewsRBAC(t *testing.T) { + t.Parallel() + // kubeMock is a Kubernetes API mock for the session tests. + // Once a new session is created, this mock will write to + // stdout and stdin (if available) the pod name, followed + // by copying the contents of stdin into both streams. + kubeMock, err := testingkubemock.NewKubeAPIMock() + require.NoError(t, err) + t.Cleanup(func() { kubeMock.Close() }) + + // creates a Kubernetes service with a configured cluster pointing to mock api server + testCtx := SetupTestContext( + context.Background(), + t, + TestConfig{ + Clusters: []KubeClusterConfig{{Name: kubeCluster, APIEndpoint: kubeMock.URL}}, + }, + ) + // close tests + t.Cleanup(func() { require.NoError(t, testCtx.Close()) }) + + type args struct { + name string + namespace string + kind string + resources []types.KubernetesResource + } + + tests := []struct { + name string + args args + want bool + }{ + { + name: "user with full access to kubernetes resources", + args: args{ + name: "", + namespace: "", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Namespace: types.Wildcard, + Name: types.Wildcard, + }, + }, + }, + want: true, + }, + { + name: "user with full access to kubernetes resources to namespace=namespace-1, pod=pod-1", + args: args{ + name: "pod-1", + namespace: "namespace-1", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Namespace: types.Wildcard, + Name: types.Wildcard, + }, + }, + }, + want: true, + }, + { + name: "user with full access to kubernetes resources to pod=pod-1", + args: args{ + name: "pod-1", + namespace: "", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Namespace: types.Wildcard, + Name: types.Wildcard, + }, + }, + }, + want: true, + }, + { + name: "user with no access to kubernetes resources to namespace=namespace-1, pod=pod-1", + args: args{ + name: "pod-1", + namespace: "namespace-1", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Name: "pod-2", + Namespace: "namespace-1", + }, + }, + }, + want: false, + }, + { + name: "user with access to kubernetes resources to namespace=namespace-1, pod=pod-1", + args: args{ + name: "pod-1", + namespace: "namespace-1", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Name: "pod-2", + Namespace: "namespace-1", + }, + { + Kind: types.KindKubePod, + Name: "pod-1", + Namespace: "namespace-1", + }, + }, + }, + want: true, + }, + { + name: "user with access to kubernetes resources to namespace=namespace-1, pod=pod-2", + args: args{ + name: "", + namespace: "namespace-1", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Name: "pod-2", + Namespace: "namespace-1", + }, + }, + }, + want: true, + }, + { + name: "user without access to kubernetes resources to namespace=namespace-2", + args: args{ + name: "", + namespace: "namespace-2", + kind: types.KindKubePod, + resources: []types.KubernetesResource{ + { + Kind: types.KindKubePod, + Name: "pod-2", + Namespace: "namespace-1", + }, + }, + }, + want: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + // create a user with full access to kubernetes Pods. + // (kubernetes_user and kubernetes_groups specified) + userID := uuid.New().String() + user, _ := testCtx.CreateUserAndRole( + testCtx.Context, + t, + userID, + RoleSpec{ + Name: userID, + KubeUsers: roleKubeUsers, + KubeGroups: roleKubeGroups, + + SetupRoleFunc: func(r types.Role) { + r.SetKubeResources(types.Allow, tt.args.resources) + }, + }, + ) + // generate a kube client with user certs for auth + client, _ := testCtx.GenTestKubeClientTLSCert( + t, + user.GetName(), + kubeCluster, + ) + + rsp, err := client.AuthorizationV1().SelfSubjectAccessReviews().Create( + context.TODO(), + &authv1.SelfSubjectAccessReview{ + Spec: authv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Resource: pluralize(tt.args.kind), + Name: tt.args.name, + Namespace: tt.args.namespace, + Verb: "list", + }, + }, + }, + metav1.CreateOptions{}, + ) + require.NoError(t, err) + require.Equal(t, tt.want, rsp.Status.Allowed) + }) + } +} diff --git a/lib/services/role_test.go b/lib/services/role_test.go index 07354b15ecf60..a7d50e2ad2f71 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -297,22 +297,116 @@ func TestRoleParse(t *testing.T) { }, Allow: types.RoleConditions{ Namespaces: []string{apidefaults.Namespace}, - KubernetesResources: []types.KubernetesResource{ + }, + Deny: types.RoleConditions{ + Namespaces: []string{apidefaults.Namespace}, + }, + }, + }, + error: nil, + }, + { + name: "full valid role v6", + in: `{ + "kind": "role", + "version": "v6", + "metadata": {"name": "name1", "labels": {"a-b": "c"}}, + "spec": { + "options": { + "cert_format": "standard", + "max_session_ttl": "20h", + "port_forwarding": true, + "client_idle_timeout": "17m", + "disconnect_expired_cert": "yes", + "enhanced_recording": ["command", "network"], + "desktop_clipboard": true, + "desktop_directory_sharing": true, + "ssh_file_copy" : false + }, + "allow": { + "node_labels": {"a": "b", "c-d": "e"}, + "app_labels": {"a": "b", "c-d": "e"}, + "group_labels": {"a": "b", "c-d": "e"}, + "kubernetes_labels": {"a": "b", "c-d": "e"}, + "db_labels": {"a": "b", "c-d": "e"}, + "db_names": ["postgres"], + "db_users": ["postgres"], + "namespaces": ["default"], + "rules": [ + { + "resources": ["role"], + "verbs": ["read", "list"], + "where": "contains(user.spec.traits[\"groups\"], \"prod\")", + "actions": [ + "log(\"info\", \"log entry\")" + ] + } + ] + }, + "deny": { + "logins": ["c"] + } + } + }`, + role: types.RoleV6{ + Kind: types.KindRole, + Version: types.V6, + Metadata: types.Metadata{ + Name: "name1", + Namespace: apidefaults.Namespace, + Labels: map[string]string{"a-b": "c"}, + }, + Spec: types.RoleSpecV6{ + Options: types.RoleOptions{ + CertificateFormat: constants.CertificateFormatStandard, + MaxSessionTTL: types.NewDuration(20 * time.Hour), + PortForwarding: types.NewBoolOption(true), + RecordSession: &types.RecordSession{ + Default: constants.SessionRecordingModeBestEffort, + Desktop: types.NewBoolOption(true), + }, + ClientIdleTimeout: types.NewDuration(17 * time.Minute), + DisconnectExpiredCert: types.NewBool(true), + BPF: apidefaults.EnhancedEvents(), + DesktopClipboard: types.NewBoolOption(true), + DesktopDirectorySharing: types.NewBoolOption(true), + CreateDesktopUser: types.NewBoolOption(false), + CreateHostUser: types.NewBoolOption(false), + SSHFileCopy: types.NewBoolOption(false), + IDP: &types.IdPOptions{ + SAML: &types.IdPSAMLOptions{ + Enabled: types.NewBoolOption(true), + }, + }, + }, + Allow: types.RoleConditions{ + NodeLabels: types.Labels{"a": []string{"b"}, "c-d": []string{"e"}}, + AppLabels: types.Labels{"a": []string{"b"}, "c-d": []string{"e"}}, + GroupLabels: types.Labels{"a": []string{"b"}, "c-d": []string{"e"}}, + KubernetesLabels: types.Labels{"a": []string{"b"}, "c-d": []string{"e"}}, + DatabaseLabels: types.Labels{"a": []string{"b"}, "c-d": []string{"e"}}, + DatabaseNames: []string{"postgres"}, + DatabaseUsers: []string{"postgres"}, + Namespaces: []string{"default"}, + Rules: []types.Rule{ { - Kind: types.KindKubePod, - Namespace: types.Wildcard, - Name: types.Wildcard, + Resources: []string{types.KindRole}, + Verbs: []string{types.VerbRead, types.VerbList}, + Where: "contains(user.spec.traits[\"groups\"], \"prod\")", + Actions: []string{ + "log(\"info\", \"log entry\")", + }, }, }, }, Deny: types.RoleConditions{ Namespaces: []string{apidefaults.Namespace}, + Logins: []string{"c"}, }, }, }, error: nil, }, - { name: "full valid role", in: `{ diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index a726f7584ad90..2d9f00bd17b98 100644 --- a/tool/tctl/common/resource_command.go +++ b/tool/tctl/common/resource_command.go @@ -397,6 +397,7 @@ func (rc *ResourceCommand) createRole(ctx context.Context, client auth.ClientI, return trace.Wrap(err) } + warnAboutKubernetesResources(rc.config.Log, role) roleName := role.GetName() _, err = client.GetRole(ctx, roleName) if err != nil && !trace.IsNotFound(err) { @@ -413,6 +414,26 @@ func (rc *ResourceCommand) createRole(ctx context.Context, client auth.ClientI, return nil } +// warnAboutKubernetesResources warns about kubernetes resources +// if kubernetes_labels are set but kubernetes_resources are not. +func warnAboutKubernetesResources(logger utils.Logger, r types.Role) { + role, ok := r.(*types.RoleV6) + // only warn about kubernetes resources for v6 roles + if !ok || role.Version != types.V6 { + return + } + if len(role.Spec.Allow.KubernetesLabels) > 0 && len(role.Spec.Allow.KubernetesResources) == 0 { + logger.Warningf("role %q has allow.kubernetes_labels set but no allow.kubernetes_resources, this is probably a mistake. Teleport will restrict access to pods.", role.Metadata.Name) + } + if len(role.Spec.Allow.KubernetesLabels) == 0 && len(role.Spec.Allow.KubernetesResources) > 0 { + logger.Warningf("role %q has allow.kubernetes_resources set but no allow.kubernetes_labels, this is probably a mistake. kubernetes_resources won't be effective.", role.Metadata.Name) + } + + if len(role.Spec.Deny.KubernetesLabels) > 0 && len(role.Spec.Deny.KubernetesResources) > 0 { + logger.Warningf("role %q has deny.kubernetes_labels set but also has deny.kubernetes_resources set, this is probably a mistake. deny.kubernetes_resources won't be effective.", role.Metadata.Name) + } +} + // createUser implements `tctl create user.yaml` command. func (rc *ResourceCommand) createUser(ctx context.Context, client auth.ClientI, raw services.UnknownResource) error { user, err := services.UnmarshalUser(raw.Raw)