Merge pull request #24795 from deads2k/use-all-attributes

Automatic merge from submit-queue

enable resource name and service account cases for impersonation

Adds the resource name check since that attribute was added for authorization.  Also adds a check against a separate resource for service accounts.  Allowing impersonation of service accounts to use a different resource check places control of impersonation with the same users to have the power to get the SA tokens directly.

@kubernetes/kube-iam 
@sgallagher FYI

pkg/apiserver/handlers.go

@@ -410,7 +410,7 @@ func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attrib
 	return &attribs
 }
 
-func WithActingAs(handler http.Handler, requestContextMapper api.RequestContextMapper, a authorizer.Authorizer) http.Handler {
+func WithImpersonation(handler http.Handler, requestContextMapper api.RequestContextMapper, a authorizer.Authorizer) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		requestedSubject := req.Header.Get("Impersonate-User")
 		if len(requestedSubject) == 0 {
@@ -430,13 +430,18 @@ func WithActingAs(handler http.Handler, requestContextMapper api.RequestContextM
 		}
 
 		actingAsAttributes := &authorizer.AttributesRecord{
-			User:     requestor,
+			User:            requestor,
-			Verb:     "impersonate",
+			Verb:            "impersonate",
-			APIGroup: api.GroupName,
+			APIGroup:        api.GroupName,
-			Resource: "users",
+			Resource:        "users",
-			// ResourceName:    requestedSubject,
+			Name:            requestedSubject,
 			ResourceRequest: true,
 		}
+		if namespace, name, err := serviceaccount.SplitUsername(requestedSubject); err == nil {
+			actingAsAttributes.Resource = "serviceaccounts"
+			actingAsAttributes.Namespace = namespace
+			actingAsAttributes.Name = name
+		}
 
 		err := a.Authorize(actingAsAttributes)
 		if err != nil {

pkg/genericapiserver/genericapiserver.go

@@ -475,7 +475,7 @@ func (s *GenericAPIServer) init(c *Config) {
 
 	attributeGetter := apiserver.NewRequestAttributeGetter(s.RequestContextMapper, s.NewRequestInfoResolver())
 	handler = apiserver.WithAuthorizationCheck(handler, attributeGetter, s.authorizer)
-	handler = apiserver.WithActingAs(handler, s.RequestContextMapper, s.authorizer)
+	handler = apiserver.WithImpersonation(handler, s.RequestContextMapper, s.authorizer)
 
 	// Install Authenticator
 	if c.Authenticator != nil {

test/integration/auth_test.go

@@ -47,6 +47,7 @@ import (
 	"k8s.io/kubernetes/pkg/auth/authorizer/abac"
 	"k8s.io/kubernetes/pkg/auth/user"
 	"k8s.io/kubernetes/pkg/master"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/plugin/pkg/admission/admit"
 	"k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/tokentest"
 	"k8s.io/kubernetes/test/integration/framework"
@@ -724,12 +725,22 @@ type impersonateAuthorizer struct{}
 
 // alice can't act as anyone and bob can't do anything but act-as someone
 func (impersonateAuthorizer) Authorize(a authorizer.Attributes) error {
+	// alice can impersonate service accounts and do other actions
+	if a.GetUserName() == "alice" && a.GetVerb() == "impersonate" && a.GetResource() == "serviceaccounts" {
+		return nil
+	}
 	if a.GetUserName() == "alice" && a.GetVerb() != "impersonate" {
 		return nil
 	}
+	// bob can impersonate anyone, but that it
 	if a.GetUserName() == "bob" && a.GetVerb() == "impersonate" {
 		return nil
 	}
+	// service accounts can do everything
+	if strings.HasPrefix(a.GetUserName(), serviceaccount.ServiceAccountUsernamePrefix) {
+		return nil
+	}
+
 	return errors.New("I can't allow that.  Go ask alice.")
 }
 
@@ -752,6 +763,7 @@ func TestImpersonateIsForbidden(t *testing.T) {
 
 	transport := http.DefaultTransport
 
+	// bob can't perform actions himself
 	for _, r := range getTestRequests() {
 		token := BobToken
 		bodyBytes := bytes.NewReader([]byte(r.body))
@@ -776,6 +788,7 @@ func TestImpersonateIsForbidden(t *testing.T) {
 		}()
 	}
 
+	// bob can impersonate alice to do other things
 	for _, r := range getTestRequests() {
 		token := BobToken
 		bodyBytes := bytes.NewReader([]byte(r.body))
@@ -799,6 +812,58 @@ func TestImpersonateIsForbidden(t *testing.T) {
 			}
 		}()
 	}
+
+	// alice can't impersonate bob
+	for _, r := range getTestRequests() {
+		token := AliceToken
+		bodyBytes := bytes.NewReader([]byte(r.body))
+		req, err := http.NewRequest(r.verb, s.URL+r.URL, bodyBytes)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
+		req.Header.Set("Impersonate-User", "bob")
+
+		func() {
+			resp, err := transport.RoundTrip(req)
+			defer resp.Body.Close()
+			if err != nil {
+				t.Logf("case %v", r)
+				t.Fatalf("unexpected error: %v", err)
+			}
+			// Expect all of bob's actions to return Forbidden
+			if resp.StatusCode != http.StatusForbidden {
+				t.Logf("case %v", r)
+				t.Errorf("Expected not status Forbidden, but got %s", resp.Status)
+			}
+		}()
+	}
+
+	// alice can impersonate a service account
+	for _, r := range getTestRequests() {
+		token := BobToken
+		bodyBytes := bytes.NewReader([]byte(r.body))
+		req, err := http.NewRequest(r.verb, s.URL+r.URL, bodyBytes)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
+		req.Header.Set("Impersonate-User", serviceaccount.MakeUsername("default", "default"))
+		func() {
+			resp, err := transport.RoundTrip(req)
+			defer resp.Body.Close()
+			if err != nil {
+				t.Logf("case %v", r)
+				t.Fatalf("unexpected error: %v", err)
+			}
+			// Expect all the requests to be allowed, don't care what they actually do
+			if resp.StatusCode == http.StatusForbidden {
+				t.Logf("case %v", r)
+				t.Errorf("Expected status not %v, but got %v", http.StatusForbidden, resp.StatusCode)
+			}
+		}()
+	}
+
 }
 
 func newAuthorizerWithContents(t *testing.T, contents string) authorizer.Authorizer {