allow */subresource in rbac policy rules

api/openapi-spec/swagger.json

@@ -64461,7 +64461,7 @@
       }
      },
      "resources": {
-      "description": "Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  \"*\" means all.",
+      "description": "Resources is a list of resources this rule applies to.  \"*\" means all in the specified apiGroups.\n \"*/foo\" represents the subresource 'foo' for all resources in the specified apiGroups.",
       "type": "array",
       "items": {
        "type": "string"
@@ -64812,7 +64812,7 @@
       }
      },
      "resources": {
-      "description": "Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  \"*\" means all.",
+      "description": "Resources is a list of resources this rule applies to.  \"*\" means all in the specified apiGroups.\n \"*/foo\" represents the subresource 'foo' for all resources in the specified apiGroups.",
       "type": "array",
       "items": {
        "type": "string"
@@ -73180,7 +73180,7 @@
       }
      },
      "resources": {
-      "description": "Resources is a list of resources this rule applies to.  ResourceAll represents all resources.",
+      "description": "Resources is a list of resources this rule applies to.  '*' represents all resources in the specified apiGroups. '*/foo' represents the subresource 'foo' for all resources in the specified apiGroups.",
       "type": "array",
       "items": {
        "type": "string"

api/swagger-spec/authorization.k8s.io_v1.json

@@ -785,7 +785,7 @@
       "items": {
        "type": "string"
       },
-      "description": "Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  \"*\" means all."
+      "description": "Resources is a list of resources this rule applies to.  \"*\" means all in the specified apiGroups.\n \"*/foo\" represents the subresource 'foo' for all resources in the specified apiGroups."
      },
      "resourceNames": {
       "type": "array",

api/swagger-spec/authorization.k8s.io_v1beta1.json

@@ -785,7 +785,7 @@
       "items": {
        "type": "string"
       },
-      "description": "Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  \"*\" means all."
+      "description": "Resources is a list of resources this rule applies to.  \"*\" means all in the specified apiGroups.\n \"*/foo\" represents the subresource 'foo' for all resources in the specified apiGroups."
      },
      "resourceNames": {
       "type": "array",

api/swagger-spec/rbac.authorization.k8s.io_v1beta1.json

@@ -3818,7 +3818,7 @@
       "items": {
        "type": "string"
       },
-      "description": "Resources is a list of resources this rule applies to.  ResourceAll represents all resources."
+      "description": "Resources is a list of resources this rule applies to.  '*' represents all resources in the specified apiGroups. '*/foo' represents the subresource 'foo' for all resources in the specified apiGroups."
      },
      "resourceNames": {
       "type": "array",

docs/api-reference/authorization.k8s.io/v1/definitions.html

@@ -1432,7 +1432,8 @@ <h3 id="_v1_resourcerule">v1.ResourceRule</h3>
 </tr>
 <tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">resources</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  "*" means all.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">Resources is a list of resources this rule applies to.  "<strong>" means all in the specified apiGroups.<br>
+ "</strong>/foo" represents the subresource <em>foo</em> for all resources in the specified apiGroups.</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">string array</p></td>
 <td class="tableblock halign-left valign-top"></td>

docs/api-reference/authorization.k8s.io/v1beta1/definitions.html

@@ -1452,7 +1452,8 @@ <h3 id="_v1beta1_resourcerule">v1beta1.ResourceRule</h3>
 </tr>
 <tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">resources</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  "*" means all.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">Resources is a list of resources this rule applies to.  "<strong>" means all in the specified apiGroups.<br>
+ "</strong>/foo" represents the subresource <em>foo</em> for all resources in the specified apiGroups.</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">string array</p></td>
 <td class="tableblock halign-left valign-top"></td>

docs/api-reference/rbac.authorization.k8s.io/v1beta1/definitions.html

@@ -1682,7 +1682,7 @@ <h3 id="_v1beta1_policyrule">v1beta1.PolicyRule</h3>
 </tr>
 <tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">resources</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">Resources is a list of resources this rule applies to.  ResourceAll represents all resources.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">Resources is a list of resources this rule applies to.  <em><strong></em> represents all resources in the specified apiGroups. <em></strong>/foo</em> represents the subresource <em>foo</em> for all resources in the specified apiGroups.</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">string array</p></td>
 <td class="tableblock halign-left valign-top"></td>

hack/update-generated-protobuf-dockerized.sh

@@ -62,6 +62,7 @@ PACKAGES=(
   k8s.io/api/authentication/v1beta1
   k8s.io/api/rbac/v1alpha1
   k8s.io/api/rbac/v1beta1
+  k8s.io/api/rbac/v1
   k8s.io/api/certificates/v1beta1
   k8s.io/api/imagepolicy/v1alpha1
   k8s.io/api/scheduling/v1alpha1

pkg/apis/authorization/types.go

@@ -205,7 +205,8 @@ type ResourceRule struct {
 	// APIGroups is the name of the APIGroup that contains the resources.  If multiple API groups are specified, any action requested against one of
 	// the enumerated resources in any API group will be allowed.  "*" means all.
 	APIGroups []string
-	// Resources is a list of resources this rule applies to.  ResourceAll represents all resources.  "*" means all.
+	// Resources is a list of resources this rule applies to.  "*" means all in the specified apiGroups.
+	//  "*/foo" represents the subresource 'foo' for all resources in the specified apiGroups.
 	Resources []string
 	// ResourceNames is an optional white list of names that the rule applies to.  An empty set means that everything is allowed.  "*" means all.
 	ResourceNames []string

pkg/apis/rbac/helpers.go

@@ -55,14 +55,29 @@ func APIGroupMatches(rule *PolicyRule, requestedGroup string) bool {
 	return false
 }
 
-func ResourceMatches(rule *PolicyRule, requestedResource string) bool {
+func ResourceMatches(rule *PolicyRule, combinedRequestedResource, requestedSubresource string) bool {
 	for _, ruleResource := range rule.Resources {
+		// if everything is allowed, we match
 		if ruleResource == ResourceAll {
 			return true
 		}
-		if ruleResource == requestedResource {
+		// if we have an exact match, we match
+		if ruleResource == combinedRequestedResource {
 			return true
 		}
+
+		// We can also match a */subresource.
+		// if there isn't a subresource, then continue
+		if len(requestedSubresource) == 0 {
+			continue
+		}
+		// if the rule isn't in the format */subresource, then we don't match, continue
+		if len(ruleResource) == len(requestedSubresource)+2 &&
+			strings.HasPrefix(ruleResource, "*/") &&
+			strings.HasSuffix(ruleResource, requestedSubresource) {
+			return true
+
+		}
 	}
 
 	return false

pkg/apis/rbac/helpers_test.go

@@ -70,3 +70,108 @@ func TestHelpersRoundTrip(t *testing.T) {
 		}
 	}
 }
+
+func TestResourceMatches(t *testing.T) {
+	tests := []struct {
+		name                      string
+		ruleResources             []string
+		combinedRequestedResource string
+		requestedSubresource      string
+		expected                  bool
+	}{
+		{
+			name:                      "all matches 01",
+			ruleResources:             []string{"*"},
+			combinedRequestedResource: "foo",
+			expected:                  true,
+		},
+		{
+			name:                      "checks all rules",
+			ruleResources:             []string{"doesn't match", "*"},
+			combinedRequestedResource: "foo",
+			expected:                  true,
+		},
+		{
+			name:                      "matches exact rule",
+			ruleResources:             []string{"foo/bar"},
+			combinedRequestedResource: "foo/bar",
+			requestedSubresource:      "bar",
+			expected:                  true,
+		},
+		{
+			name:                      "matches exact rule 02",
+			ruleResources:             []string{"foo/bar"},
+			combinedRequestedResource: "foo",
+			expected:                  false,
+		},
+		{
+			name:                      "matches subresource",
+			ruleResources:             []string{"*/scale"},
+			combinedRequestedResource: "foo/scale",
+			requestedSubresource:      "scale",
+			expected:                  true,
+		},
+		{
+			name:                      "doesn't match partial subresource hit",
+			ruleResources:             []string{"foo/bar", "*/other"},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  false,
+		},
+		{
+			name:                      "matches subresource with multiple slashes",
+			ruleResources:             []string{"*/other/segment"},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  true,
+		},
+		{
+			name:                      "doesn't fail on empty",
+			ruleResources:             []string{""},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  false,
+		},
+		{
+			name:                      "doesn't fail on slash",
+			ruleResources:             []string{"/"},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  false,
+		},
+		{
+			name:                      "doesn't fail on missing subresource",
+			ruleResources:             []string{"*/"},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  false,
+		},
+		{
+			name:                      "doesn't match on not star",
+			ruleResources:             []string{"*something/other/segment"},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  false,
+		},
+		{
+			name:                      "doesn't match on something else",
+			ruleResources:             []string{"something/other/segment"},
+			combinedRequestedResource: "foo/other/segment",
+			requestedSubresource:      "other/segment",
+			expected:                  false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			rule := &rbac.PolicyRule{
+				Resources: tc.ruleResources,
+			}
+			actual := rbac.ResourceMatches(rule, tc.combinedRequestedResource, tc.requestedSubresource)
+			if tc.expected != actual {
+				t.Errorf("expected %v, got %v", tc.expected, actual)
+			}
+
+		})
+	}
+}

pkg/apis/rbac/types.go

@@ -48,7 +48,8 @@ type PolicyRule struct {
 	// APIGroups is the name of the APIGroup that contains the resources.
 	// If multiple API groups are specified, any action requested against one of the enumerated resources in any API group will be allowed.
 	APIGroups []string
-	// Resources is a list of resources this rule applies to.  ResourceAll represents all resources.
+	// Resources is a list of resources this rule applies to.  '*' represents all resources in the specified apiGroups.
+	// '*/foo' represents the subresource 'foo' for all resources in the specified apiGroups.
 	Resources []string
 	// ResourceNames is an optional white list of names that the rule applies to.  An empty set means that everything is allowed.
 	ResourceNames []string

pkg/registry/rbac/validation/policy_comparator.go

@@ -105,6 +105,31 @@ func hasAll(set, contains []string) bool {
 	return true
 }
 
+func resourceCoversAll(setResources, coversResources []string) bool {
+	// if we have a star or an exact match on all resources, then we match
+	if has(setResources, rbac.ResourceAll) || hasAll(setResources, coversResources) {
+		return true
+	}
+
+	for _, path := range coversResources {
+		// if we have an exact match, then we match.
+		if has(setResources, path) {
+			continue
+		}
+		// if we're not a subresource, then we definitely don't match.  fail.
+		if !strings.Contains(path, "/") {
+			return false
+		}
+		tokens := strings.SplitN(path, "/", 2)
+		resourceToCheck := "*/" + tokens[1]
+		if !has(setResources, resourceToCheck) {
+			return false
+		}
+	}
+
+	return true
+}
+
 func nonResourceURLsCoversAll(set, covers []string) bool {
 	for _, path := range covers {
 		covered := false
@@ -133,7 +158,7 @@ func nonResourceURLCovers(ownerPath, subPath string) bool {
 func ruleCovers(ownerRule, subRule rbac.PolicyRule) bool {
 	verbMatches := has(ownerRule.Verbs, rbac.VerbAll) || hasAll(ownerRule.Verbs, subRule.Verbs)
 	groupMatches := has(ownerRule.APIGroups, rbac.APIGroupAll) || hasAll(ownerRule.APIGroups, subRule.APIGroups)
-	resourceMatches := has(ownerRule.Resources, rbac.ResourceAll) || hasAll(ownerRule.Resources, subRule.Resources)
+	resourceMatches := resourceCoversAll(ownerRule.Resources, subRule.Resources)
 	nonResourceURLMatches := nonResourceURLsCoversAll(ownerRule.NonResourceURLs, subRule.NonResourceURLs)
 
 	resourceNameMatches := false

pkg/registry/rbac/validation/policy_comparator_test.go

@@ -45,6 +45,20 @@ func TestCoversExactMatch(t *testing.T) {
 	}.test(t)
 }
 
+func TestCoversSubresourceWildcard(t *testing.T) {
+	escalationTest{
+		ownerRules: []rbac.PolicyRule{
+			{APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"*/scale"}},
+		},
+		servantRules: []rbac.PolicyRule{
+			{APIGroups: []string{"v1"}, Verbs: []string{"get"}, Resources: []string{"foo/scale"}},
+		},
+
+		expectedCovered:        true,
+		expectedUncoveredRules: []rbac.PolicyRule{},
+	}.test(t)
+}
+
 func TestCoversMultipleRulesCoveringSingleRule(t *testing.T) {
 	escalationTest{
 		ownerRules: []rbac.PolicyRule{

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go

@@ -156,10 +156,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("get", "list", "watch").Groups(autoscalingGroup).Resources("horizontalpodautoscalers").RuleOrDie(),
 			rbac.NewRule("update").Groups(autoscalingGroup).Resources("horizontalpodautoscalers/status").RuleOrDie(),
-			rbac.NewRule("get", "update").Groups(legacyGroup).Resources("replicationcontrollers/scale").RuleOrDie(),
-			// TODO this should be removable when the HPA contoller is fixed
-			rbac.NewRule("get", "update").Groups(extensionsGroup).Resources("replicationcontrollers/scale").RuleOrDie(),
-			rbac.NewRule("get", "update").Groups(extensionsGroup, appsGroup).Resources("deployments/scale", "replicasets/scale").RuleOrDie(),
+			rbac.NewRule("get", "update").Groups("*").Resources("*/scale").RuleOrDie(),
 			rbac.NewRule("list").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			// TODO: restrict this to the appropriate namespace
 			rbac.NewRule("get").Groups(legacyGroup).Resources("services/proxy").Names("https:heapster:", "http:heapster:").RuleOrDie(),

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml

@@ -445,25 +445,9 @@ items:
     verbs:
     - update
   - apiGroups:
-    - ""
-    resources:
-    - replicationcontrollers/scale
-    verbs:
-    - get
-    - update
-  - apiGroups:
-    - extensions
-    resources:
-    - replicationcontrollers/scale
-    verbs:
-    - get
-    - update
-  - apiGroups:
-    - apps
-    - extensions
+    - '*'
     resources:
-    - deployments/scale
-    - replicasets/scale
+    - '*/scale'
     verbs:
     - get
     - update

plugin/pkg/auth/authorizer/rbac/rbac.go

@@ -174,14 +174,14 @@ func RulesAllow(requestAttributes authorizer.Attributes, rules ...rbac.PolicyRul
 
 func RuleAllows(requestAttributes authorizer.Attributes, rule *rbac.PolicyRule) bool {
 	if requestAttributes.IsResourceRequest() {
-		resource := requestAttributes.GetResource()
+		combinedResource := requestAttributes.GetResource()
 		if len(requestAttributes.GetSubresource()) > 0 {
-			resource = requestAttributes.GetResource() + "/" + requestAttributes.GetSubresource()
+			combinedResource = requestAttributes.GetResource() + "/" + requestAttributes.GetSubresource()
 		}
 
 		return rbac.VerbMatches(rule, requestAttributes.GetVerb()) &&
 			rbac.APIGroupMatches(rule, requestAttributes.GetAPIGroup()) &&
-			rbac.ResourceMatches(rule, resource) &&
+			rbac.ResourceMatches(rule, combinedResource, requestAttributes.GetSubresource()) &&
 			rbac.ResourceNameMatches(rule, requestAttributes.GetName())
 	}
 

plugin/pkg/auth/authorizer/rbac/rbac_test.go

@@ -224,13 +224,19 @@ func TestAuthorizer(t *testing.T) {
 		{
 			// test subresource resolution
 			clusterRoles: []*rbac.ClusterRole{
-				newClusterRole("admin", newRule("*", "*", "pods/status", "*")),
+				newClusterRole("admin",
+					newRule("*", "*", "pods/status", "*"),
+					newRule("*", "*", "*/scale", "*"),
+				),
 			},
 			roleBindings: []*rbac.RoleBinding{
 				newRoleBinding("ns1", "admin", bindToClusterRole, "User:admin", "Group:admins"),
 			},
 			shouldPass: []authorizer.Attributes{
 				&defaultAttributes{"admin", "", "get", "pods", "status", "ns1", ""},
+				&defaultAttributes{"admin", "", "get", "pods", "scale", "ns1", ""},
+				&defaultAttributes{"admin", "", "get", "deployments", "scale", "ns1", ""},
+				&defaultAttributes{"admin", "", "get", "anything", "scale", "ns1", ""},
 			},
 			shouldFail: []authorizer.Attributes{
 				&defaultAttributes{"admin", "", "get", "pods", "", "ns1", ""},