Skip to content

Commit

Permalink
Move fleetworkspace validation to GlobalRoleResolver
Browse files Browse the repository at this point in the history
  • Loading branch information
raulcabello committed Apr 11, 2024
1 parent 109fe53 commit cf97831
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 122 deletions.
37 changes: 11 additions & 26 deletions pkg/auth/globalrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,32 +41,7 @@ func (g *GlobalRoleResolver) GlobalRulesFromRole(gr *v3.GlobalRole) []rbacv1.Pol
return gr.Rules
}

// FleetWorkspaceRulesFromRole finds all rules which apply to all fleet workspaces except fleet-local.
func (g *GlobalRoleResolver) FleetWorkspaceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule {
// no rules on a nil role
if gr == nil {
return nil
}
return gr.InheritedFleetWorkspacePermissions.ResourceRules
}

// FleetWorkspaceRulesFromRole finds all rules which apply to all fleet workspaces except fleet-local.
func (g *GlobalRoleResolver) FleetWorkspaceVerbsRuleFromRole(gr *v3.GlobalRole, workspaceNames []string) []rbacv1.PolicyRule {
if gr == nil {
return nil
}

return []rbacv1.PolicyRule{
{
Verbs: gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs,
APIGroups: []string{"management.cattle.io"},
Resources: []string{"fleetworkspaces"},
ResourceNames: workspaceNames,
},
}
}

// ClusterRulesFromRole finds all rules which this gr gives on downstream clusters.
// ClusterRulesFromRole finds all rules which this gr gives on downstream clusters and fleet workspaces except local.
func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.PolicyRule, error) {
if gr == nil {
return nil, nil
Expand All @@ -90,6 +65,16 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P
}
rules = append(rules, templateRules...)
}
if gr.InheritedFleetWorkspacePermissions.ResourceRules != nil {
rules = append(rules, gr.InheritedFleetWorkspacePermissions.ResourceRules...)
}
if gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil {
rules = append(rules, rbacv1.PolicyRule{
Verbs: gr.InheritedFleetWorkspacePermissions.WorkspaceVerbs,
APIGroups: []string{"management.cattle.io"},
Resources: []string{"fleetworkspaces"},
})
}
return rules, nil
}

Expand Down
1 change: 0 additions & 1 deletion pkg/codegen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func main() {
v3.Node{},
v3.Project{},
v3.ClusterProxyConfig{},
v3.FleetWorkspace{},
},
},
"provisioning.cattle.io": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Rules without verbs, resources, or apigroups are not permitted. The `rules` incl

Escalation checks are bypassed if a user has the `escalate` verb on the GlobalRole that they are attempting to update or create. This can also be given through a wildcard permission (i.e. the `*` verb also gives `escalate`).

Users can only change GlobalRoles with rights less than or equal to those they currently possess. This is to prevent privilege escalation. This includes the rules in the RoleTemplates referred to in `inheritedClusterRoles`.
Users can only change GlobalRoles with rights less than or equal to those they currently possess. This is to prevent privilege escalation. This includes the rules in the RoleTemplates referred to in `inheritedClusterRoles` and the rules in `inheritedFleetWorkspacePermissions`.

Users can only grant rules in the `NamespacedRules` field with rights less than or equal to those they currently possess. This works on a per namespace basis, meaning that the user must have the permission
in the namespace specified. The `Rules` field apply to every namespace, which means a user can create `NamespacedRules` in any namespace that are equal to or less than the `Rules` they currently possess.
Expand Down
38 changes: 7 additions & 31 deletions pkg/resources/management.cattle.io/v3/globalrole/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"fmt"
"testing"

"k8s.io/apimachinery/pkg/labels"

"github.com/golang/mock/gomock"
v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
"github.com/rancher/webhook/pkg/admission"
Expand Down Expand Up @@ -44,7 +42,7 @@ var (
},
},
}
clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR, readFleetWorkspacesCR}
clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR}

clusterRoleBindings = []*v1.ClusterRoleBinding{
{
Expand Down Expand Up @@ -76,12 +74,6 @@ var (
},
},
},
{
Subjects: []v1.Subject{
{Kind: v1.UserKind, Name: testUser},
},
RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: readFleetWorkspacesCR.Name},
},
}
baseRT = v3.RoleTemplate{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -107,6 +99,12 @@ var (
},
},
InheritedClusterRoles: []string{baseRT.Name},
InheritedFleetWorkspacePermissions: v3.FleetWorkspacePermission{
ResourceRules: []v1.PolicyRule{
ruleReadPods,
},
WorkspaceVerbs: []string{"GET"},
},
}
baseGRB = v3.GlobalRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -136,13 +134,6 @@ var (
APIGroups: []string{"*"},
Resources: []string{"*"},
}
ruleReadFleetWorkspaces = v1.PolicyRule{
Verbs: []string{"GET"},
APIGroups: []string{"management.cattle.io"},
Resources: []string{"fleetworkspaces"},
ResourceNames: []string{"fleet-default"},
}

adminCR = &v1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-role",
Expand All @@ -153,10 +144,6 @@ var (
ObjectMeta: metav1.ObjectMeta{Name: "read-pods"},
Rules: []v1.PolicyRule{ruleReadPods},
}
readFleetWorkspacesCR = &v1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{Name: "read-fleetworkspaces"},
Rules: []v1.PolicyRule{ruleReadFleetWorkspaces},
}

roleTemplate = v3.RoleTemplate{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -191,7 +178,6 @@ type testState struct {
rtCacheMock *fake.MockNonNamespacedCacheInterface[*v3.RoleTemplate]
grCacheMock *fake.MockNonNamespacedCacheInterface[*v3.GlobalRole]
grbCacheMock *fake.MockNonNamespacedCacheInterface[*v3.GlobalRoleBinding]
fwCacheMock *fake.MockNonNamespacedCacheInterface[*v3.FleetWorkspace]
sarMock *k8fake.FakeSubjectAccessReviews
resolver validation.AuthorizationRuleResolver
}
Expand Down Expand Up @@ -279,21 +265,12 @@ func newDefaultState(t *testing.T) testState {
rtCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](ctrl)
grCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRole](ctrl)
grbCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl)
fwCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.FleetWorkspace](ctrl)
grbs := []*v3.GlobalRoleBinding{&baseGRB}
grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes()
grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes()
grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes()
grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes()
rtCacheMock.EXPECT().Get(baseRT.Name).Return(&baseRT, nil).AnyTimes()
fwCacheMock.EXPECT().List(labels.Everything()).Return([]*v3.FleetWorkspace{
{
ObjectMeta: metav1.ObjectMeta{Name: "fleet-default"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "fleet-local"},
},
}, nil).AnyTimes()
k8Fake := &k8testing.Fake{}
fakeSAR := &k8fake.FakeSubjectAccessReviews{Fake: &k8fake.FakeAuthorizationV1{Fake: k8Fake}}

Expand All @@ -302,7 +279,6 @@ func newDefaultState(t *testing.T) testState {
rtCacheMock: rtCacheMock,
grCacheMock: grCacheMock,
grbCacheMock: grbCacheMock,
fwCacheMock: fwCacheMock,
sarMock: fakeSAR,
resolver: resolver,
}
Expand Down
42 changes: 3 additions & 39 deletions pkg/resources/management.cattle.io/v3/globalrole/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@ import (

v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
"github.com/rancher/webhook/pkg/admission"
mgmtv3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3"
objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3"
"github.com/rancher/webhook/pkg/resolvers"
"github.com/rancher/webhook/pkg/resources/common"
admissionv1 "k8s.io/api/admission/v1"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
Expand All @@ -36,13 +34,12 @@ const (

// NewValidator returns a new validator used for validation globalRoles.
func NewValidator(ruleResolver validation.AuthorizationRuleResolver, grbResolver *resolvers.GRBClusterRuleResolver,
sar authorizationv1.SubjectAccessReviewInterface, fwCache mgmtv3.FleetWorkspaceCache) *Validator {
sar authorizationv1.SubjectAccessReviewInterface) *Validator {
return &Validator{
admitter: admitter{
resolver: ruleResolver,
grbResolver: grbResolver,
sar: sar,
fwCache: fwCache,
},
}
}
Expand Down Expand Up @@ -76,7 +73,6 @@ type admitter struct {
resolver validation.AuthorizationRuleResolver
grbResolver *resolvers.GRBClusterRuleResolver
sar authorizationv1.SubjectAccessReviewInterface
fwCache mgmtv3.FleetWorkspaceCache
}

// Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request.
Expand Down Expand Up @@ -132,8 +128,8 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp
nsrPath.Child(index)))
}
// Validate fleet workspace rules
if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil || newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil {
fleetWorkspaceRules := a.grbResolver.GlobalRoleResolver.FleetWorkspaceRulesFromRole(newGR)
if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil {
fleetWorkspaceRules := newGR.InheritedFleetWorkspacePermissions.ResourceRules
fwrPath := fldPath.Child("inheritedFleetWorkspacePermissions").Child("resourceRules")
returnError = errors.Join(returnError, common.ValidateRules(fleetWorkspaceRules, true, fwrPath))
}
Expand Down Expand Up @@ -164,45 +160,13 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp
return admission.ResponseAllowed(), nil
}
}
// Check for escalations in fleet workspace rules
if newGR.InheritedFleetWorkspacePermissions.ResourceRules != nil || newGR.InheritedFleetWorkspacePermissions.WorkspaceVerbs != nil {
fleetWorkspaceRules := a.grbResolver.GlobalRoleResolver.FleetWorkspaceRulesFromRole(newGR)
workspaceNames, err := a.allFleetWorkspaceNamesExceptLocal()
if err != nil {
return nil, fmt.Errorf("unable to fecht fleet workspaces: %w", err)
}
for _, w := range workspaceNames {
returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fleetWorkspaceRules, a.resolver, w))
if escalateChecker.HasVerb() {
return admission.ResponseAllowed(), nil
}
}
fleetWorkspaceVerbs := a.grbResolver.GlobalRoleResolver.FleetWorkspaceVerbsRuleFromRole(newGR, workspaceNames)
returnError = errors.Join(returnError, escalateChecker.IsRulesAllowed(fleetWorkspaceVerbs, a.resolver, ""))
}
if returnError != nil {
return admission.ResponseFailedEscalation(fmt.Sprintf("errors due to escalation: %v", returnError)), nil
}

return admission.ResponseAllowed(), nil
}

// allFleetWorkspaceNamesExceptLocal returns all workspace names except fleet-local
func (a *admitter) allFleetWorkspaceNamesExceptLocal() ([]string, error) {
workspaces, err := a.fwCache.List(labels.Everything())
if err != nil {
return nil, fmt.Errorf("unable to fecht fleet workspaces: %w", err)
}
var workspaceNames []string
for _, w := range workspaces {
if w.Name != "fleet-local" {
workspaceNames = append(workspaceNames, w.Name)
}
}

return workspaceNames, nil
}

// validateDelete checks if a global role can be deleted and returns the appropriate response.
func validateDelete(oldRole *v3.GlobalRole, fldPath *field.Path) (*admissionv1.AdmissionResponse, error) {
if oldRole.Builtin {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ func TestAdmit(t *testing.T) {
test.args.stateSetup(state)
}
grbResolver := state.createBaseGRBResolver()
admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock, state.fwCacheMock).Admitters()
admitters := globalrole.NewValidator(state.resolver, grbResolver, state.sarMock).Admitters()
assert.Len(t, admitters, 1)

req := createGRRequest(t, test)
Expand All @@ -875,7 +875,7 @@ func TestAdmit(t *testing.T) {
func Test_UnexpectedErrors(t *testing.T) {
t.Parallel()
resolver, _ := validation.NewTestRuleResolver(nil, nil, nil, nil)
validator := globalrole.NewValidator(resolver, nil, nil, nil)
validator := globalrole.NewValidator(resolver, nil, nil)
admitters := validator.Admitters()
require.Len(t, admitters, 1, "wanted only one admitter")
test := testCase{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package clusterrole
import (
"fmt"

v1 "k8s.io/api/rbac/v1"

"github.com/rancher/webhook/pkg/admission"
objectsv1 "github.com/rancher/webhook/pkg/generated/objects/rbac.authorization.k8s.io/v1"
"github.com/rancher/webhook/pkg/resources/common"
Expand All @@ -25,7 +27,9 @@ type Validator struct {
// NewValidator returns a new validator for roles.
func NewValidator() *Validator {
return &Validator{
admitter: admitter{},
admitter: admitter{
clusterRoleOldAndNewFromRequest: objectsv1.ClusterRoleOldAndNewFromRequest,
},
}
}

Expand Down Expand Up @@ -64,14 +68,18 @@ func (v *Validator) Admitters() []admission.Admitter {
return []admission.Admitter{&v.admitter}
}

type admitter struct{}
type clusterRoleOldAndNewFromRequest func(request *admissionv1.AdmissionRequest) (*v1.ClusterRole, *v1.ClusterRole, error)

type admitter struct {
clusterRoleOldAndNewFromRequest clusterRoleOldAndNewFromRequest
}

// Admit is the entrypoint for the validator. Admit will return an error if it's unable to process the request.
func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) {
listTrace := trace.New("clusterRoleValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username})
defer listTrace.LogIfLong(admission.SlowTraceDuration)

oldRole, newRole, err := objectsv1.ClusterRoleOldAndNewFromRequest(&request.AdmissionRequest)
oldRole, newRole, err := a.clusterRoleOldAndNewFromRequest(&request.AdmissionRequest)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit cf97831

Please sign in to comment.