Skip to content
This repository was archived by the owner on Nov 14, 2024. It is now read-only.

Commit 18da19e

Browse files
johnstcnjohnstcn
authored
feature: use SelfSubjectRulesReview for RBAC, fallback to SelfSubjectAccessRequest (#11)
* feat: use SSRR for RBAC instead of SSAR by default, fallback to SSAR * chore: add api.WarnResult * chore: Makefile: add test target
1 parent e9134d0 commit 18da19e

File tree

8 files changed

+515
-10
lines changed

8 files changed

+515
-10
lines changed

‎Makefile

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,11 @@ lint/shellcheck: $(shell scripts/depfind/sh.sh)
2727

2828
lint: lint/go lint/shellcheck
2929
.PHONY: lint
30+
31+
test: test/go
32+
.PHONY: test
33+
34+
test/go:
35+
@echo "--- go test"
36+
go test -parallel=$(shell nproc) ./...
37+
.PHONY: test/go

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ require (
1111
k8s.io/apimachinery v0.19.14
1212
k8s.io/client-go v0.19.14
1313
k8s.io/klog/v2 v2.10.0 // indirect
14-
k8s.io/utils v0.0.0-20210305010621-2afb4311ab10 // indirect
14+
k8s.io/kubectl v0.19.14
1515
)

‎go.sum

Lines changed: 148 additions & 2 deletions
Large diffs are not rendered by default.

‎internal/api/warn.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package api
2+
3+
// WarnResult returns a CheckResult when a warning occurs.
4+
func WarnResult(name string, summary string) *CheckResult {
5+
return &CheckResult{
6+
Name: name,
7+
State: StateWarning,
8+
Summary: summary,
9+
}
10+
}

‎internal/api/warn_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package api_test
2+
3+
import (
4+
"testing"
5+
6+
"cdr.dev/slog/sloggers/slogtest/assert"
7+
"github.com/cdr/coder-doctor/internal/api"
8+
)
9+
10+
func TestWarnResult(t *testing.T) {
11+
t.Parallel()
12+
res := api.WarnResult("check-name", "something bad happened")
13+
14+
assert.Equal(t, "name matches", "check-name", res.Name)
15+
assert.Equal(t, "state matches", api.StateWarning, res.State)
16+
assert.Equal(t, "summary matches", "something bad happened", res.Summary)
17+
}

‎internal/checks/kube/rbac.go

Lines changed: 162 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,164 @@ package kube
33
import (
44
"context"
55
"fmt"
6+
"sort"
67
"strings"
78

89
"golang.org/x/xerrors"
910

11+
"cdr.dev/slog"
12+
1013
"github.com/Masterminds/semver/v3"
1114

1215
"github.com/cdr/coder-doctor/internal/api"
1316

1417
authorizationv1 "k8s.io/api/authorization/v1"
18+
rbacv1 "k8s.io/api/rbac/v1"
1519
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1620
authorizationclientv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
21+
rbacutil "k8s.io/kubectl/pkg/util/rbac"
22+
"k8s.io/kubectl/pkg/util/slice"
1723
)
1824

25+
// CheckRBAC checks the cluster for the RBAC permissions required by Coder.
26+
// It will attempt to first use a SelfSubjectRulesReview to determine the capabilities
27+
// of the user. If this fails (notably on GKE), fall back to using SelfSubjectAccessRequests
28+
// which is slower but is more likely to work.
1929
func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult {
30+
if ssrrResults := k.checkRBACDefault(ctx); ssrrResults != nil {
31+
return ssrrResults
32+
}
33+
34+
k.log.Warn(ctx, "SelfSubjectRulesReview response incomplete, falling back to SelfSubjectAccessRequests (slow)")
35+
return k.checkRBACFallback(ctx)
36+
}
37+
38+
func (k *KubernetesChecker) checkRBACDefault(ctx context.Context) []*api.CheckResult {
39+
const checkName = "kubernetes-rbac-ssrr"
40+
authClient := k.client.AuthorizationV1()
41+
results := make([]*api.CheckResult, 0)
42+
43+
sar := &authorizationv1.SelfSubjectRulesReview{
44+
Spec: authorizationv1.SelfSubjectRulesReviewSpec{
45+
Namespace: k.namespace,
46+
},
47+
}
48+
49+
response, err := authClient.SelfSubjectRulesReviews().Create(ctx, sar, metav1.CreateOptions{})
50+
if err != nil {
51+
return []*api.CheckResult{api.SkippedResult(checkName, "unable to create SelfSubjectRulesReview request: "+err.Error())}
52+
}
53+
54+
if response.Status.Incomplete {
55+
return nil // In this case, we should fall back to using SelfSubjectAccessRequests.
56+
}
57+
58+
// convert the list of rules from the server to PolicyRules and dedupe/compact
59+
breakdownRules := []rbacv1.PolicyRule{}
60+
for _, rule := range convertToPolicyRule(response.Status) {
61+
breakdownRules = append(breakdownRules, rbacutil.BreakdownRule(rule)...)
62+
}
63+
64+
compactRules, _ := rbacutil.CompactRules(breakdownRules) //nolint:errcheck - err is always nil
65+
sort.Stable(rbacutil.SortableRuleSlice(compactRules))
66+
for _, r := range compactRules {
67+
k.log.Debug(ctx, "Got SSRR PolicyRule", slog.F("rule", r))
68+
}
69+
70+
// TODO: optimise this
71+
for req, reqVerbs := range k.reqs.ResourceRequirements {
72+
if err := satisfies(req, reqVerbs, compactRules); err != nil {
73+
summary := fmt.Sprintf("resource %s: %s", req.Resource, err)
74+
results = append(results, api.ErrorResult(checkName, summary, err))
75+
continue
76+
}
77+
resourceName := req.Resource
78+
if req.Group != "" {
79+
resourceName = req.Group + "/" + req.Resource
80+
}
81+
summary := fmt.Sprintf("resource %s: can %s", resourceName, strings.Join(reqVerbs, ", "))
82+
results = append(results, api.PassResult(checkName, summary))
83+
}
84+
85+
// TODO: remove this when the helm chart is fixed
86+
for req, reqVerbs := range k.reqs.RoleOnlyResourceRequirements {
87+
if err := satisfies(req, reqVerbs, compactRules); err != nil {
88+
summary := fmt.Sprintf("resource %s: %s", req.Resource, err)
89+
results = append(results, api.ErrorResult(checkName, summary, err))
90+
continue
91+
}
92+
resourceName := req.Resource
93+
if req.Group != "" {
94+
resourceName = req.Group + "/" + req.Resource
95+
}
96+
summary := fmt.Sprintf("resource %s: can %s", resourceName, strings.Join(reqVerbs, ", "))
97+
results = append(results, api.PassResult(checkName, summary))
98+
}
99+
100+
return results
101+
}
102+
103+
func satisfies(req *ResourceRequirement, verbs ResourceVerbs, rules []rbacv1.PolicyRule) error {
104+
for _, rule := range rules {
105+
if apiGroupsMatch(req.Group, rule.APIGroups) &&
106+
apiResourceMatch(req.Resource, rule.Resources) &&
107+
verbsMatch(verbs, rule.Verbs) {
108+
return nil
109+
}
110+
}
111+
return xerrors.Errorf("not satisfied: group:%q resource:%q version:%q verbs:%+v", req.Group, req.Resource, req.Version, verbs)
112+
}
113+
114+
// The below adapted from k8s.io/pkg/apis/rbac/v1/evaluation_helpers.go
115+
func verbsMatch(want, have ResourceVerbs) bool {
116+
if slice.ContainsString(have, "*", nil) {
117+
return true
118+
}
119+
120+
for _, v := range want {
121+
if !slice.ContainsString(have, v, nil) {
122+
return false
123+
}
124+
}
125+
return true
126+
}
127+
128+
func apiGroupsMatch(want string, have []string) bool {
129+
for _, g := range have {
130+
if g == "*" {
131+
return true
132+
}
133+
134+
if g == want {
135+
return true
136+
}
137+
}
138+
return false
139+
}
140+
141+
func apiResourceMatch(want string, have []string) bool {
142+
for _, r := range have {
143+
if r == "*" {
144+
return true
145+
}
146+
147+
if r == want {
148+
return true
149+
}
150+
}
151+
return false
152+
}
153+
154+
// checkRBACFallback uses a SelfSubjectAccessRequest to check the cluster for the required
155+
// accesses. This requires a number of checks and is relatively slow.
156+
func (k *KubernetesChecker) checkRBACFallback(ctx context.Context) []*api.CheckResult {
20157
const checkName = "kubernetes-rbac"
21158
authClient := k.client.AuthorizationV1()
22159
results := make([]*api.CheckResult, 0)
23160

24161
for req, reqVerbs := range k.reqs.ResourceRequirements {
25162
resName := fmt.Sprintf("%s-%s", checkName, req.Resource)
26-
if err := k.checkOneRBAC(ctx, authClient, req, reqVerbs); err != nil {
163+
if err := k.checkOneRBACSSAR(ctx, authClient, req, reqVerbs); err != nil {
27164
summary := fmt.Sprintf("missing permissions on resource %s: %s", req.Resource, err)
28165
results = append(results, api.ErrorResult(resName, summary, err))
29166
continue
@@ -37,7 +174,7 @@ func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult {
37174
// that don't exist.
38175
for req, reqVerbs := range k.reqs.RoleOnlyResourceRequirements {
39176
resName := fmt.Sprintf("%s-%s", checkName, req.Resource)
40-
if err := k.checkOneRBAC(ctx, authClient, req, reqVerbs); err != nil {
177+
if err := k.checkOneRBACSSAR(ctx, authClient, req, reqVerbs); err != nil {
41178
summary := fmt.Sprintf("missing permissions on resource %s: %s", req.Resource, err)
42179
results = append(results, api.ErrorResult(resName, summary, err))
43180
continue
@@ -50,7 +187,7 @@ func (k *KubernetesChecker) CheckRBAC(ctx context.Context) []*api.CheckResult {
50187
return results
51188
}
52189

53-
func (k *KubernetesChecker) checkOneRBAC(ctx context.Context, authClient authorizationclientv1.AuthorizationV1Interface, req *ResourceRequirement, reqVerbs ResourceVerbs) error {
190+
func (k *KubernetesChecker) checkOneRBACSSAR(ctx context.Context, authClient authorizationclientv1.AuthorizationV1Interface, req *ResourceRequirement, reqVerbs ResourceVerbs) error {
54191
have := make([]string, 0, len(reqVerbs))
55192
for _, verb := range reqVerbs {
56193
sar := &authorizationv1.SelfSubjectAccessReview{
@@ -92,3 +229,25 @@ func findClosestVersionRequirements(v *semver.Version) *VersionedResourceRequire
92229
}
93230
return nil
94231
}
232+
233+
// lifted from kubectl/pkg/cmd/auth/cani.go
234+
func convertToPolicyRule(status authorizationv1.SubjectRulesReviewStatus) []rbacv1.PolicyRule {
235+
ret := []rbacv1.PolicyRule{}
236+
for _, resource := range status.ResourceRules {
237+
ret = append(ret, rbacv1.PolicyRule{
238+
Verbs: resource.Verbs,
239+
APIGroups: resource.APIGroups,
240+
Resources: resource.Resources,
241+
ResourceNames: resource.ResourceNames,
242+
})
243+
}
244+
245+
for _, nonResource := range status.NonResourceRules {
246+
ret = append(ret, rbacv1.PolicyRule{
247+
Verbs: nonResource.Verbs,
248+
NonResourceURLs: nonResource.NonResourceURLs,
249+
})
250+
}
251+
252+
return ret
253+
}

0 commit comments

Comments
 (0)