allow */subresource in rbac policy rules

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{