Added feature gate and tests

pkg/features/kube_features.go

@@ -74,6 +74,13 @@ const (
 	// Requires AuthorizeWithSelectors to be enabled.
 	AuthorizeNodeWithSelectors featuregate.Feature = "AuthorizeNodeWithSelectors"
 
+	// owner: @seans3
+	// kep: http://kep.k8s.io/4006
+	//
+	// Forces authorization of the "create" verb for pod subresources like exec, attach, and portforward.
+	// See: https://github.com/kubernetes/kubernetes/issues/133515
+	AuthorizePodWebsocketUpgradeCreatePermission featuregate.Feature = "AuthorizePodWebsocketUpgradeCreatePermission"
+
 	// owner: @szuecs
 	//
 	// Enable nodes to change CPUCFSQuotaPeriod
@@ -1078,6 +1085,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 		{Version: version.MustParse("1.34"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.37
 	},
 
+	AuthorizePodWebsocketUpgradeCreatePermission: {
+		{Version: version.MustParse("1.35"), Default: true, PreRelease: featuregate.Beta},
+	},
+
 	CPUCFSQuotaPeriod: {
 		{Version: version.MustParse("1.12"), Default: false, PreRelease: featuregate.Alpha},
 	},
@@ -2016,6 +2027,8 @@ var defaultKubernetesFeatureGateDependencies = map[featuregate.Feature][]feature
 
 	AuthorizeNodeWithSelectors: {genericfeatures.AuthorizeWithSelectors},
 
+	AuthorizePodWebsocketUpgradeCreatePermission: {},
+
 	CPUCFSQuotaPeriod: {},
 
 	CPUManagerPolicyAlphaOptions: {},

pkg/registry/core/pod/rest/authorize.go

@@ -28,6 +28,13 @@ import (
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 )
 
+// Pod subresources differs on the REST verbs depending on the protocol used
+// SPDY uses POST that at the authz layer is translated to "create".
+// Websockets uses GET that is translated to "get".
+// Since the defaulting to websocket for kubectl in KEP-4006 this caused an
+// unexpected side effect and in order to keep existing policies backwards
+// compatible we always check that the "create" verb is allowed.
+// Ref: https://issues.k8s.io/133515
 func ensureAuthorizedForVerb(ctx context.Context, a authorizer.Authorizer, verb string) error {
 	requestInfo, ok := genericapirequest.RequestInfoFrom(ctx)
 	if !ok {

pkg/registry/core/pod/rest/authorize_test.go

@@ -0,0 +1,118 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+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 rest
+
+import (
+	"context"
+	"errors"
+	"reflect"
+	"testing"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+)
+
+// mockAuthorizer provides a mock implementation of the authorizer.Interface.
+type mockAuthorizer struct {
+	decision authorizer.Decision
+	reason   string
+	err      error
+}
+
+func (a *mockAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+	return a.decision, a.reason, a.err
+}
+
+func TestEnsureAuthorizedForVerb(t *testing.T) {
+	tests := []struct {
+		name          string
+		ctx           context.Context
+		authorizer    authorizer.Authorizer
+		verb          string
+		expectErr     string
+		expectErrType interface{}
+	}{
+		{
+			name:          "no request info in context",
+			ctx:           context.Background(),
+			verb:          "create",
+			expectErr:     `Internal error occurred: no request info in context`,
+			expectErrType: &apierrors.StatusError{},
+		},
+		{
+			name: "verb already matches",
+			ctx: genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{
+				Verb: "create",
+			}),
+			verb: "create",
+		},
+		{
+			name: "nil authorizer",
+			ctx: genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{
+				Verb: "get",
+			}),
+			authorizer:    nil,
+			verb:          "create",
+			expectErr:     `Internal error occurred: no authorizer available`,
+			expectErrType: &apierrors.StatusError{},
+		},
+		{
+			name:       "authorizer returns error",
+			ctx:        genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{Verb: "get"}),
+			authorizer: &mockAuthorizer{err: errors.New("auth error")},
+			verb:       "create",
+			expectErr:  "auth error",
+		},
+		{
+			name:          "authorizer denies",
+			ctx:           genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{Verb: "get", Resource: "pods", Name: "my-pod"}),
+			authorizer:    &mockAuthorizer{decision: authorizer.DecisionDeny, reason: "no reason"},
+			verb:          "create",
+			expectErr:     `pods "my-pod" is forbidden: no reason`,
+			expectErrType: &apierrors.StatusError{},
+		},
+		{
+			name:       "authorizer allows",
+			ctx:        genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{Verb: "get"}),
+			authorizer: &mockAuthorizer{decision: authorizer.DecisionAllow},
+			verb:       "create",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := ensureAuthorizedForVerb(tc.ctx, tc.authorizer, tc.verb)
+
+			if len(tc.expectErr) > 0 {
+				if err == nil {
+					t.Fatalf("expected error %q, got nil", tc.expectErr)
+				}
+				if err.Error() != tc.expectErr {
+					t.Errorf("expected error %q, got %q", tc.expectErr, err.Error())
+				}
+				if tc.expectErrType != nil {
+					if reflect.TypeOf(err) != reflect.TypeOf(tc.expectErrType) {
+						t.Errorf("expected error type %T, got %T", tc.expectErrType, err)
+					}
+				}
+			} else if err != nil {
+				t.Fatalf("expected no error, got %v", err)
+			}
+		})
+	}
+}

pkg/registry/core/pod/rest/subresources.go

@@ -111,8 +111,12 @@ func (r *AttachREST) Destroy() {
 
 // Connect returns a handler for the pod exec proxy
 func (r *AttachREST) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) {
-	if err := ensureAuthorizedForVerb(ctx, r.Authorizer, "create"); err != nil {
-		return nil, err
+	// Forces a authz check for "create", if feature gate enabled.
+	// See: https://github.com/kubernetes/kubernetes/issues/133515
+	if utilfeature.DefaultFeatureGate.Enabled(features.AuthorizePodWebsocketUpgradeCreatePermission) {
+		if err := ensureAuthorizedForVerb(ctx, r.Authorizer, "create"); err != nil {
+			return nil, err
+		}
 	}
 
 	attachOpts, ok := opts.(*api.PodAttachOptions)
@@ -173,8 +177,12 @@ func (r *ExecREST) Destroy() {
 
 // Connect returns a handler for the pod exec proxy
 func (r *ExecREST) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) {
-	if err := ensureAuthorizedForVerb(ctx, r.Authorizer, "create"); err != nil {
-		return nil, err
+	// Forces a authz check for "create", if feature gate enabled.
+	// See: https://github.com/kubernetes/kubernetes/issues/133515
+	if utilfeature.DefaultFeatureGate.Enabled(features.AuthorizePodWebsocketUpgradeCreatePermission) {
+		if err := ensureAuthorizedForVerb(ctx, r.Authorizer, "create"); err != nil {
+			return nil, err
+		}
 	}
 
 	execOpts, ok := opts.(*api.PodExecOptions)
@@ -246,8 +254,12 @@ func (r *PortForwardREST) ConnectMethods() []string {
 
 // Connect returns a handler for the pod portforward proxy
 func (r *PortForwardREST) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) {
-	if err := ensureAuthorizedForVerb(ctx, r.Authorizer, "create"); err != nil {
-		return nil, err
+	// Forces a authz check for "create", if feature gate enabled.
+	// See: https://github.com/kubernetes/kubernetes/issues/133515
+	if utilfeature.DefaultFeatureGate.Enabled(features.AuthorizePodWebsocketUpgradeCreatePermission) {
+		if err := ensureAuthorizedForVerb(ctx, r.Authorizer, "create"); err != nil {
+			return nil, err
+		}
 	}
 
 	portForwardOpts, ok := opts.(*api.PodPortForwardOptions)

test/compatibility_lifecycle/reference/versioned_feature_list.yaml

@@ -153,6 +153,12 @@
     lockToDefault: true
     preRelease: GA
     version: "1.34"
+- name: AuthorizePodWebsocketUpgradeCreatePermission
+  versionedSpecs:
+  - default: true
+    lockToDefault: false
+    preRelease: Beta
+    version: "1.35"
 - name: AuthorizeWithSelectors
   versionedSpecs:
   - default: false

test/integration/apiserver/subresource_auth_test.go

@@ -0,0 +1,177 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+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 apiserver
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+	"k8s.io/kubernetes/cmd/kube-apiserver/app/options"
+	"k8s.io/kubernetes/test/integration/framework"
+	"k8s.io/kubernetes/test/utils/ktesting"
+)
+
+// TestPodSubresourceAuth tests that the synthetic authorization check for pod subresources is working correctly.
+func TestPodSubresourceAuth(t *testing.T) {
+	tCtx := ktesting.Init(t)
+	_, clientConfig, tearDownFn := framework.StartTestServer(tCtx, t, framework.TestServerSetup{
+		ModifyServerRunOptions: func(opts *options.ServerRunOptions) {
+			opts.Authorization.Modes = []string{"RBAC"}
+		},
+	})
+	defer tearDownFn()
+
+	adminConfig := rest.CopyConfig(clientConfig)
+	adminClientset, err := kubernetes.NewForConfig(adminConfig)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	ns := "test-pod-subresource-auth"
+	if _, err := adminClientset.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}}, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+
+	sa := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
+	if _, err := adminClientset.CoreV1().ServiceAccounts(ns).Create(context.TODO(), sa, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+
+	pod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-pod"},
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{
+				{
+					Name:  "test-container",
+					Image: "test-image",
+				},
+			},
+		},
+	}
+	if _, err := adminClientset.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+
+	// User with only 'get' permissions
+	podGetterUsername := "pod-getter"
+	podGetterRole := &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{Name: "pod-getter-role"},
+		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups: []string{""},
+				Resources: []string{"pods/exec", "pods/attach", "pods/portforward"},
+				Verbs:     []string{"get"},
+			},
+		},
+	}
+	if _, err := adminClientset.RbacV1().Roles(ns).Create(context.TODO(), podGetterRole, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+	podGetterRoleBinding := &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{Name: "pod-getter-binding"},
+		Subjects:   []rbacv1.Subject{{Kind: "User", Name: podGetterUsername}},
+		RoleRef:    rbacv1.RoleRef{Kind: "Role", Name: "pod-getter-role"},
+	}
+	if _, err := adminClientset.RbacV1().RoleBindings(ns).Create(context.TODO(), podGetterRoleBinding, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+
+	podGetterConfig := rest.CopyConfig(clientConfig)
+	podGetterConfig.Impersonate = rest.ImpersonationConfig{UserName: podGetterUsername}
+	podGetterClient, err := kubernetes.NewForConfig(podGetterConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// User with 'get' and 'create' permissions on pods subresources.
+	podCreatorUsername := "pod-creator"
+	podCreatorRole := &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{Name: "pod-creator-role"},
+		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups: []string{""},
+				Resources: []string{"pods/exec", "pods/attach", "pods/portforward"},
+				Verbs:     []string{"get"},
+			},
+			{
+				APIGroups: []string{""},
+				Resources: []string{"pods/exec", "pods/attach", "pods/portforward"},
+				Verbs:     []string{"create"},
+			},
+		},
+	}
+	if _, err := adminClientset.RbacV1().Roles(ns).Create(context.TODO(), podCreatorRole, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+	podCreatorRoleBinding := &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{Name: "pod-creator-binding"},
+		Subjects:   []rbacv1.Subject{{Kind: "User", Name: podCreatorUsername}},
+		RoleRef:    rbacv1.RoleRef{Kind: "Role", Name: "pod-creator-role"},
+	}
+	if _, err := adminClientset.RbacV1().RoleBindings(ns).Create(context.TODO(), podCreatorRoleBinding, metav1.CreateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+
+	podCreatorConfig := rest.CopyConfig(clientConfig)
+	podCreatorConfig.Impersonate = rest.ImpersonationConfig{UserName: podCreatorUsername}
+	podCreatorClient, err := kubernetes.NewForConfig(podCreatorConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	subresources := []string{"exec", "attach", "portforward"}
+	for _, subresource := range subresources {
+		t.Run(fmt.Sprintf("subresource=%s", subresource), func(t *testing.T) {
+			// User with only 'get' permissions should be denied.
+			// GET method, since that is the method for WebSocket upgrade requests.
+			err := podGetterClient.CoreV1().RESTClient().Get().
+				Namespace(ns).
+				Resource("pods").
+				Name("test-pod").
+				SubResource(subresource).
+				Do(context.TODO()).
+				Error()
+			if !errors.IsForbidden(err) {
+				t.Errorf("expected forbidden error for user with only 'get' permissions, but got: %v", err)
+			}
+
+			// User with 'get' and 'create' permissions should be allowed.
+			// GET method, since that is the method for WebSocket upgrade requests.
+			err = podCreatorClient.CoreV1().RESTClient().Get().
+				Namespace(ns).
+				Resource("pods").
+				Name("test-pod").
+				SubResource(subresource).
+				Do(context.TODO()).
+				Error()
+			// Absence of "Forbidden" is proof of success; the integration test
+			// server doesn't have a real Kubelet running for the pod, so the
+			// proxying/streaming connection ultimately fails after the authorization
+			// has already succeeded (hence "Bad Request" error).
+			if err != nil && !errors.IsBadRequest(err) {
+				t.Errorf("expected nil error for user with 'create' permissions, but got: %v", err)
+			}
+		})
+	}
+}