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

Update PSS Kyverno policies and others #241

Merged
merged 36 commits into from
Jan 30, 2022
Merged

Update PSS Kyverno policies and others #241

merged 36 commits into from
Jan 30, 2022

Conversation

chipzoller
Copy link
Contributor

@chipzoller chipzoller commented Jan 28, 2022

Related issue

Closes #239
Closes #229
Closes #189
Closes #171
Closes #175
Closes #195
Closes #238
Closes #218
Closes #231
Closes #51
Closes #191
Closes #192
Closes #193
Closes #196
Closes #197
Closes #213
Closes #228
Closes #226

Proposed Changes

This PR brings the current state of Kubernetes Pod Security Standards implemented as Kyverno policies in addition to a number of fixes, enhancements, and clean-up as well as net new policies.

New Policies

  • Restrict Secrets by Name
  • Require Requests and Limits for emptyDir
  • Block Ephemeral Containers
  • Add Default Resources
  • Enforce Resources as Ratio
  • Restrict Service Port Range
  • Check supplementalGroups
  • Restrict Adding Capabilities
  • Allowed Annotations
  • Advanced Restrict Image Registries
  • Ingress Host Match TLS
  • Check Node for CVE-2022-0185
  • Resolve Image to Digest
  • Block Images with Volumes
  • Block Large Images
  • Only Trustworthy Registries Set Root
  • Check NVIDIA GPUs
  • Require Image Source

Other

  • Updates the PR template to be more appropriate to kyverno/policies rather than a copy-and-paste from kyverno/kyverno
  • Converts issue templates into GitHub issue forms

Checklist

  • I have read the contributing guidelines.
  • I have added tests that prove my fix is effective or that my feature works.
  • [] My PR contains new or altered behavior to Kyverno and
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.

Further Comments

Signed-off-by: Chip Zoller <chipzoller@gmail.com>
…een removed from upstream PSS list.

Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
@chipzoller chipzoller added the sample Sample policy label Jan 28, 2022
@chipzoller chipzoller added this to the 1.6.0 milestone Jan 28, 2022
@chipzoller chipzoller linked an issue Jan 28, 2022 that may be closed by this pull request
@chipzoller
Copy link
Contributor Author

CI tests failing because tests being done with latest release and not latest CLI code from main. Need to fix this so that PRs to main in kyverno/policies are tested by code in kyverno/kyverno main and vice versa.

Signed-off-by: Chip Zoller <chipzoller@gmail.com>
@chipzoller
Copy link
Contributor Author

@zeborg all tests pass now. Thank you!

@chipzoller
Copy link
Contributor Author

@JimBugwadia CI is fixed and this is ready to merge

@treydock
Copy link
Contributor

@chipzoller Any reason not using match.any? I ask because one thing I was going to do in Helm was integrate kyverno/kyverno#3051 into the PSP pull request to avoid a huge merge conflict issue and I discovered (though maybe not issue with 1.6) that matching match.resources and exclude.any produces errors so one thing I was doing was moving everything to match.any.

@chipzoller
Copy link
Contributor Author

@chipzoller Any reason not using match.any? I ask because one thing I was going to do in Helm was integrate kyverno/kyverno#3051 into the PSP pull request to avoid a huge merge conflict issue and I discovered (though maybe not issue with 1.6) that matching match.resources and exclude.any produces errors so one thing I was doing was moving everything to match.any.

No good reason other than at the time I was writing them it wasn't an issue. Is the suggestion here to just standardize on that now going forward, for anything under match or exclude?

@treydock
Copy link
Contributor

Is the suggestion here to just standardize on that now going forward, for anything under match or exclude?

I think it makes sense, and base on docs, resources directly under match and exclude is deprecated: https://kyverno.io/docs/writing-policies/match-exclude/

Specifying resource filters directly under match and exclude has been marked for deprecation and will be removed in a future release. It is highly recommended you specify them under any or all blocks.

The match.any and in particular the exclude.any is very robust and if users using Helm can exclude multiple things, my experience has been the match must not be match.resources but I haven't tested if that's still the case with 1.6, only tested that with 1.5.x.

@chipzoller
Copy link
Contributor Author

chipzoller commented Jan 29, 2022

You're right, this should have been done. I will go through all PSS policies and update them.

@treydock
Copy link
Contributor

One minor nitpick as I've been going through these one-by-one for the Helm chart, I noticed so far many in baseline have 2 spaces indenting list elements but like disallow-host-process does not use 2 spaces to indent a list. The current Helm chart suffers from this inconsistent indentation. It does not affect functionality but something I noticed as using nindent function in Helm can vary between different values based on a policies particular indentation standard.

Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
@chipzoller
Copy link
Contributor Author

I just pushed the match.any changes and also tried to normalize some of the indentation. Take a look now.

@chipzoller
Copy link
Contributor Author

Also, one thing to be aware of when porting these to the Helm chart. In baseline, the "disallow-host-ports-range" should NOT be wrapped in the chart, only "disallow-host-ports". They are mutually exclusive and the more strict one (the latter) should be used.

@treydock
Copy link
Contributor

I am also copying in require-non-root-groups from other because that is security related and was already in Helm charts.

Also one minor issue, the filename and directory for disallow-proc-mount does not match the policy name. This caused some issues when templating the Helm charts since the policy name was very important for some of the templating.

Signed-off-by: Chip Zoller <chipzoller@gmail.com>
@chipzoller
Copy link
Contributor Author

I am also copying in require-non-root-groups from other because that is security related and was already in Helm charts.

I would recommend dropping it because it was also dropped from the upstream PSS definitions. That's why I moved it to the other folder and changed the annotation. It's no longer considered "official".

Also one minor issue, the filename and directory for disallow-proc-mount does not match the policy name. This caused some issues when templating the Helm charts since the policy name was very important for some of the templating.

Thank you, I just fixed that.

@treydock
Copy link
Contributor

For require-non-root-groups would it be better to allow supplemental groups but just not allow GID 0 (ie root)?

    - name: check-supplementalGroups
      match:
        any:
          - resources:
              kinds:
                - Pod
      preconditions:
      - key: "{{`{{ request.object.spec.securityContext.supplementalGroups.length(@) }}`}}"
        operator: GreaterThan
        value: 0
      validate:
        message: >-
          Disallow the the root group in spec.securityContext.supplementalGroups.
        deny:
          conditions:
            all:
              - key: "{{`{{ request.object.spec.securityContext.supplementalGroups[*].to_string(@) }}`}}"
                operator: NotIn
                value:
                  - '0'

…label" to audit

Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
@chipzoller chipzoller added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 30, 2022
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
Signed-off-by: Chip Zoller <chipzoller@gmail.com>
@chipzoller
Copy link
Contributor Author

Ready for final review.

@chipzoller chipzoller merged commit 47ed252 into kyverno:main Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment