Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a policy for deny-default-service-account-bindings #1118

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

fast-n-curious
Copy link

Description

The default service account is automatically mounted into all pods in a namespace unless explicitly overridden. If this account is bound to a Role or ClusterRole that grants extensive permissions, every pod in the namespace using the default service account will inherit these permissions. This setup can lead to unnecessary security risks if a pod is compromised, as an attacker could potentially gain access to other resources within the cluster.For an enhnaced security, using the default service account in RoleBindings is not recommended.

Checklist

  • I have read the policy contribution guidelines.
  • I have added test manifests and resources covering both positive and negative tests that prove this policy works as intended.
  • I have added the artifacthub-pkg.yml file and have verified it is complete and correct.

Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

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

Tests should be separated by folder in accordance with other examples in the library.

Comment on lines +24 to +28
match:
resources:
kinds:
- RoleBinding
- ClusterRoleBinding
Copy link
Contributor

Choose a reason for hiding this comment

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

Rules must use any or all under the match and exclude blocks to be conformant to the current schema.

attacker could potentially gain access to other resources within the cluster.For an enhnaced
security, using the default service account in RoleBindings is not recommended.
spec:
validationFailureAction: enforce
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
validationFailureAction: enforce
validationFailureAction: Audit

Comment on lines +33 to +34
- kind: "ServiceAccount"
name: "!default"
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't tested this, but my feeling is this may not work in all scenarios because subjects[] is an array of objects. If it isn't the first one in the list, it may bypass the policy. I suggest testing on your own to confirm. If confirmed, this would be better written as a deny rule.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @chipzoller,

With the change in policy, there's a new issue that the policy is excluding all the resources. Could you please point me to the issue here:

      validate:
        message: "Using the default service account in RoleBindings is not allowed."
        foreach:
          - list: "request.object.spec.subject[]"
            deny:
              conditions:
                all:
                  - key: "{{ element.kind }}"
                    operator: Equals
                    value: "ServiceAccount"
                  - key: "{{ element.name }}"
                    operator: Equals
                    value: "default"

Following is the result:

 krishna@Krishnas-MacBook-Pro .tests % kyverno test .                                
Loading test  ( kyverno-test.yaml ) ...
  Loading values/variables ...
  Loading policies ...
  Loading resources ...
  Loading exceptions ...
  Applying 1 policy to 4 resources ...
  Checking results ...

│────│───────────────────────────────────────│───────────────────────────────────────│───────────────────────│────────│──────────│
│ ID │ POLICY                                │ RULE                                  │ RESOURCE              │ RESULT │ REASON   │
│────│───────────────────────────────────────│───────────────────────────────────────│───────────────────────│────────│──────────│
│ 1  │ deny-default-service-account-bindings │ deny-default-service-account-bindings │ RoleBinding/goodpod01 │ Pass   │ Excluded │
│ 2  │ deny-default-service-account-bindings │ deny-default-service-account-bindings │ RoleBinding/goodpod02 │ Pass   │ Excluded │
│ 3  │ deny-default-service-account-bindings │ deny-default-service-account-bindings │ RoleBinding/badpod01  │ Pass   │ Excluded │
│ 4  │ deny-default-service-account-bindings │ deny-default-service-account-bindings │ RoleBinding/badpod02  │ Pass   │ Excluded │
│────│───────────────────────────────────────│───────────────────────────────────────│───────────────────────│────────│──────────│


Test Summary: 4 tests passed and 0 tests failed

Copy link
Author

Choose a reason for hiding this comment

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

With the following policy, we see the goodpods start, but badpods get skipped.

     validate:
        message: "Using the default service account in RoleBindings is not allowed."
        deny:
          conditions:
            all: 
            - key: "{{request.object.subject[].kind}}"
              operator: Equals
              value: "ServiceAccount"
            - key: "{{request.object.subject[].name}}"
              operator: Equals
              value: "default"
krishna@Krishnas-MacBook-Pro .kyverno-test % kyverno test .                           
Loading test  ( kyverno-test.yaml ) ...
  Loading values/variables ...
  Loading policies ...
  Loading resources ...
  Loading exceptions ...
  Applying 1 policy to 4 resources ...
  Checking results ...

│────│───────────────────────────────│───────────────────────────────│───────────────│────────│─────────────────────│
│ ID │ POLICY                        │ RULE                          │ RESOURCE      │ RESULT │ REASON              │
│────│───────────────────────────────│───────────────────────────────│───────────────│────────│─────────────────────│
│ 1  │ deny-default-service-accounts │ deny-default-service-accounts │ Pod/goodpod01 │ Pass   │ Ok                  │
│ 2  │ deny-default-service-accounts │ deny-default-service-accounts │ Pod/goodpod02 │ Pass   │ Ok                  │
│ 3  │ deny-default-service-accounts │ deny-default-service-accounts │ Pod/badpod01  │ Pass   │ Want fail, got skip │
│ 4  │ deny-default-service-accounts │ deny-default-service-accounts │ Pod/badpod02  │ Pass   │ Want fail, got skip │
│────│───────────────────────────────│───────────────────────────────│───────────────│────────│─────────────────────│

- name: step-01
try:
- script:
content: kyverno test .
Copy link
Contributor

Choose a reason for hiding this comment

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

Chainsaw tests must not wrap Kyverno CLI.

policies.kyverno.io/category: Other
policies.kyverno.io/subject: Pod
kyverno.io/kyverno-version: 1.11.0
policies.kyverno.io/minversion: 1.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove

policies.kyverno.io/title: Deny binding of default service accounts
policies.kyverno.io/category: Other
policies.kyverno.io/subject: Pod
kyverno.io/kyverno-version: 1.11.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Please test and certify on the latest Kyverno release.

@@ -0,0 +1,22 @@
name: deny-force-delete
Copy link
Contributor

Choose a reason for hiding this comment

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

Name is not accurate here.

@@ -0,0 +1,22 @@
name: deny-force-delete
version: 1.0.0
displayName: Deny Force Deletion of Resources
Copy link
Contributor

Choose a reason for hiding this comment

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

Not accurate.

Copy link
Contributor

Choose a reason for hiding this comment

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

File needs to be updated accordingly including with changes to annotations in the policy.

JimBugwadia and others added 3 commits August 26, 2024 14:53
…ce-account-bindings.yaml

Co-authored-by: Chip Zoller <[email protected]>
Signed-off-by: Jim Bugwadia <[email protected]>
…ce-account-bindings.yaml

Co-authored-by: Chip Zoller <[email protected]>
Signed-off-by: Jim Bugwadia <[email protected]>
…ce-account-bindings.yaml

Co-authored-by: Chip Zoller <[email protected]>
Signed-off-by: Jim Bugwadia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants