Merge pull request #43888 from liggitt/unsecured-port-user

Automatic merge from submit-queue (batch tested with PRs 43545, 44293, 44221, 43888)

Avoid nil user special-casing in unsecured endpoint

The unsecured handler currently adds no `user.Info` to the request context.  That means that anything that tries to authorize actions in the API server currently has to special case nil users to ensure the unsecured localhost endpoint remains capable of performing all actions. 

This PR changes the unsecured localhost endpoint to be treated as a privileged user internally, so that no special casing is required by code inside the authentication layer

I'm not particularly attached to the username. It doesn't bother me for it to have a slightly uncomfortable sounding name.

pkg/kubeapiserver/server/BUILD

@@ -13,6 +13,7 @@ go_library(
     tags = ["automanaged"],
     deps = [
         "//vendor:github.com/golang/glog",
+        "//vendor:k8s.io/apiserver/pkg/authentication/user",
         "//vendor:k8s.io/apiserver/pkg/endpoints/filters",
         "//vendor:k8s.io/apiserver/pkg/endpoints/request",
         "//vendor:k8s.io/apiserver/pkg/server",

pkg/kubeapiserver/server/insecure_handler.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/golang/glog"
 
+	"k8s.io/apiserver/pkg/authentication/user"
 	genericapifilters "k8s.io/apiserver/pkg/endpoints/filters"
 	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/server"
@@ -35,6 +36,7 @@ import (
 
 func BuildInsecureHandlerChain(apiHandler http.Handler, c *server.Config) http.Handler {
 	handler := genericapifilters.WithAudit(apiHandler, c.RequestContextMapper, c.AuditWriter)
+	handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, insecureSuperuser{}, nil)
 	handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")
 	handler = genericfilters.WithPanicRecovery(handler)
 	handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc)
@@ -111,3 +113,15 @@ func serveInsecurely(insecureServingInfo *InsecureServingInfo, insecureHandler h
 	_, err = server.RunServer(insecureServer, insecureServingInfo.BindNetwork, stopCh)
 	return err
 }
+
+// insecureSuperuser implements authenticator.Request to always return a superuser.
+// This is functionally equivalent to skipping authentication and authorization,
+// but allows apiserver code to stop special-casing a nil user to skip authorization checks.
+type insecureSuperuser struct{}
+
+func (insecureSuperuser) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
+	return &user.DefaultInfo{
+		Name:   "system:unsecured",
+		Groups: []string{user.SystemPrivilegedGroup, user.AllAuthenticated},
+	}, true, nil
+}

pkg/registry/rbac/escalation_check.go

@@ -29,9 +29,7 @@ import (
 func EscalationAllowed(ctx genericapirequest.Context) bool {
 	u, ok := genericapirequest.UserFrom(ctx)
 	if !ok {
-		// the only way to be without a user is to either have no authenticators by explicitly saying that's your preference
-		// or to be connecting via the insecure port, in which case this logically doesn't apply
-		return true
+		return false
 	}
 
 	// system:masters is special because the API server uses it for privileged loopback connections

plugin/pkg/admission/security/podsecuritypolicy/admission.go

@@ -288,8 +288,7 @@ func getMatchingPolicies(lister extensionslisters.PodSecurityPolicyLister, user
 	}
 
 	for _, constraint := range list {
-		// if no user info exists then the API is being hit via the unsecured port. In this case authorize the request.
-		if user == nil || authorizedForPolicy(user, namespace, constraint, authz) || authorizedForPolicy(sa, namespace, constraint, authz) {
+		if authorizedForPolicy(user, namespace, constraint, authz) || authorizedForPolicy(sa, namespace, constraint, authz) {
 			matchedPolicies = append(matchedPolicies, constraint)
 		}
 	}

plugin/pkg/admission/security/podsecuritypolicy/admission_test.go

@@ -1612,25 +1612,34 @@ func TestGetMatchingPolicies(t *testing.T) {
 			},
 			expectedPolicies: sets.NewString("policy1", "policy2", "policy4", "policy5"),
 		},
-		"policies are allowed for nil user info": {
-			user:    nil,
-			sa:      &user.DefaultInfo{Name: "sa"},
-			ns:      "test",
-			allowed: map[string]map[string]map[string]bool{}, // authorizer not consulted
+		"policies are not allowed for nil user info": {
+			user: nil,
+			sa:   &user.DefaultInfo{Name: "sa"},
+			ns:   "test",
+			allowed: map[string]map[string]map[string]bool{
+				"sa": {
+					"test": {"policy1": true},
+				},
+				"user": {
+					"test": {"policy2": true},
+				},
+			},
 			inPolicies: []*extensions.PodSecurityPolicy{
 				policyWithName("policy1"),
 				policyWithName("policy2"),
 				policyWithName("policy3"),
 			},
-			// all policies are allowed regardless of the permissions when user info is nil
-			// (ie. a request hitting the unsecure port)
-			expectedPolicies: sets.NewString("policy1", "policy2", "policy3"),
+			// only the policies for the sa are allowed when user info is nil
+			expectedPolicies: sets.NewString("policy1"),
 		},
 		"policies are not allowed for nil sa info": {
 			user: &user.DefaultInfo{Name: "user"},
 			sa:   nil,
 			ns:   "test",
 			allowed: map[string]map[string]map[string]bool{
+				"sa": {
+					"test": {"policy1": true},
+				},
 				"user": {
 					"test": {"policy2": true},
 				},
@@ -1643,6 +1652,26 @@ func TestGetMatchingPolicies(t *testing.T) {
 			// only the policies for the user are allowed when sa info is nil
 			expectedPolicies: sets.NewString("policy2"),
 		},
+		"policies are not allowed for nil sa and user info": {
+			user: nil,
+			sa:   nil,
+			ns:   "test",
+			allowed: map[string]map[string]map[string]bool{
+				"sa": {
+					"test": {"policy1": true},
+				},
+				"user": {
+					"test": {"policy2": true},
+				},
+			},
+			inPolicies: []*extensions.PodSecurityPolicy{
+				policyWithName("policy1"),
+				policyWithName("policy2"),
+				policyWithName("policy3"),
+			},
+			// no policies are allowed if sa and user are both nil
+			expectedPolicies: sets.NewString(),
+		},
 	}
 	for k, v := range tests {
 		informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc())