Skip to content

Added permission to manager-role ClusterRole to update 'roles'#2078

Merged
Ilyesbdlala merged 1 commit intomainfrom
bugfix/role-permission
Nov 7, 2023
Merged

Added permission to manager-role ClusterRole to update 'roles'#2078
Ilyesbdlala merged 1 commit intomainfrom
bugfix/role-permission

Conversation

@Ilyesbdlala
Copy link
Member

@Ilyesbdlala Ilyesbdlala commented Oct 31, 2023 •
edited
Loading

Description

Logs of Bug previous to fix:

1.6987528214331818e+09    INFO    controllers.execution.Scan    Matching ParseDefinition Found    {"scan_parse": "default/nikto-bodgeit", "ParseDefinition": "nikto-json"}
1.6987528214341886e+09    INFO    controllers.execution.Scan    Role already exists but not in the correct state
1.6987528214381618e+09    ERROR    controllers.execution.Scan    Failed to update Role    {"error": "roles.rbac.authorization.k8s.io \"parser\" is forbidden: User \"system:serviceaccount:securecodebox-system:securecodebox-operator\" cannot update resource \"roles\" in API group \"rbac.authorization.k8s.io\" in the namespace \"default\""}
github.com/secureCodeBox/secureCodeBox/operator/controllers/execution/scans.(*ScanReconciler).startParser
    /workspace/controllers/execution/scans/parse_reconciler.go:109
github.com/secureCodeBox/secureCodeBox/operator/controllers/execution/scans.(*ScanReconciler).Reconcile
    /workspace/controllers/execution/scans/scan_controller.go:102
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile
    /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:121
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
    /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:320
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
    /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:273
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
    /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:234

@J12934 Adding permission to manager-role ClusterRole to update 'roles' fixes the issue. But does that have any unintended consequences ? Like some kind of privilege escalation. AFAIK, it's fine, since you can't give access you don't have.

@Ilyesbdlala Ilyesbdlala added the bug Bugs label Oct 31, 2023
@Ilyesbdlala Ilyesbdlala self-assigned this Oct 31, 2023
@netlify
Copy link

netlify bot commented Oct 31, 2023 •
edited
Loading

✅ Deploy Preview for docs-securecodebox canceled.

Name Link
🔨 Latest commit ecb237a
🔍 Latest deploy log https://app.netlify.com/sites/docs-securecodebox/deploys/654a0fc183ab7200080c9084

@Ilyesbdlala Ilyesbdlala requested a review from J12934 October 31, 2023 14:26
@github-actions
Copy link

github-actions bot commented Oct 31, 2023 •
edited
Loading

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ YAML prettier 1 0 0.29s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@Ilyesbdlala Ilyesbdlala force-pushed the bugfix/role-permission branch from 597a2c9 to ecb237a Compare November 7, 2023 10:21
J12934
J12934 approved these changes Nov 7, 2023
View reviewed changes
Copy link
Member

@J12934 J12934 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from a secruity perspective i don't think this is a issue. it already had the right to create new roles, so allowing it to update roles isn't an issue.
Especially as you already said you can only do it for permission this service account has itself.

@Ilyesbdlala Ilyesbdlala merged commit c6b9b37 into main Nov 7, 2023
@Ilyesbdlala Ilyesbdlala deleted the bugfix/role-permission branch November 7, 2023 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@J12934 J12934 J12934 approved these changes

Assignees

@Ilyesbdlala Ilyesbdlala

Labels

bug Bugs

Projects

secureCodeBox
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Parser service account cannot get resource parsedefinitions

2 participants

@Ilyesbdlala @J12934