Skip to content

Enable runAsNonRoot by default#4036

Merged
jetstack-bot merged 18 commits intocert-manager:masterfrom
wallrj:3875-pod-security
May 21, 2021
Merged

Enable runAsNonRoot by default#4036
jetstack-bot merged 18 commits intocert-manager:masterfrom
wallrj:3875-pod-security

Conversation

@wallrj
Copy link
Copy Markdown
Member

@wallrj wallrj commented May 20, 2021

Picking up #3875 and modifying the E2E test scripts to install Kyverno which is the policy tool that @mikebryant used to identify the original problem.

I install the restrictive Kyverno pod security policy.

And I've updated some of the E2E addon components to satisfy this policy,
others I couldn't get working, so I configured Kyverno to ignore the resources in those namespaces.

Release note:

runAsNonRoot is now enabled by default in the securityContext values. If you're using custom containers with the chart that run as root, you will need to set this back to false.

Fixes: #3721

/kind feature

@jetstack-bot jetstack-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/deploy Indicates a PR modifies deployment configuration labels May 20, 2021
wallrj added 3 commits May 20, 2021 10:14
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
When running kyverno using https://kyverno.io/policies/pod-security/restricted/, some checks failed. This enables more secure policy by default

Signed-off-by: Mike Bryant <mikebryant@bulb.co.uk>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@wallrj wallrj force-pushed the 3875-pod-security branch from 6c7e51b to 7d02b40 Compare May 20, 2021 09:29
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@jetstack-bot jetstack-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 20, 2021
wallrj added 2 commits May 20, 2021 17:53
… policy

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@wallrj wallrj force-pushed the 3875-pod-security branch from c6c69a6 to 991995d Compare May 20, 2021 16:59
--install \
--wait \
--version 3.15.2 \
--version 3.31.0 \
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I attempted to configure ingress-nginx for non-root operation, but the containerPort values in the Helm chart don't seem to change the nginx listening port. See helm/charts#14605

So I've configured kyverno to ignore the ingress-nginx namespace.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll move this change to a separate PR, since it's not necessary here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See #4046

@wallrj wallrj changed the title WIP: Enable runAsNonRoot by default Enable runAsNonRoot by default May 20, 2021
@jetstack-bot jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2021
@wallrj wallrj requested a review from irbekrm May 20, 2021 17:09
@jetstack-bot jetstack-bot added the area/testing Issues relating to testing label May 20, 2021
@wallrj wallrj force-pushed the 3875-pod-security branch from af17c86 to 55c2abe Compare May 20, 2021 19:55
@wallrj wallrj force-pushed the 3875-pod-security branch from 55c2abe to 2294e44 Compare May 20, 2021 20:42
@wallrj
Copy link
Copy Markdown
Member Author

wallrj commented May 21, 2021

/test pull-cert-manager-e2e-v1-16

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@wallrj wallrj force-pushed the 3875-pod-security branch from 5466a58 to 6d0d650 Compare May 21, 2021 14:05
wallrj added 5 commits May 21, 2021 15:07
There's no need to also set it by default on the container.

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
…E test namespaces

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@wallrj wallrj force-pushed the 3875-pod-security branch from 6d0d650 to a28bff3 Compare May 21, 2021 14:07
The NET_BIND_SERVICE privilege is only needed when binding to privileged ports.

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@jetstack-bot jetstack-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 21, 2021
wallrj added 2 commits May 21, 2021 17:18
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@wallrj wallrj force-pushed the 3875-pod-security branch from 6089866 to 6873aa7 Compare May 21, 2021 16:19
@wallrj
Copy link
Copy Markdown
Member Author

wallrj commented May 21, 2021

/test pull-cert-manager-e2e-v1-16

@wallrj
Copy link
Copy Markdown
Member Author

wallrj commented May 21, 2021

/test pull-cert-manager-e2e-v1-21

Copy link
Copy Markdown
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

Looks great to me- glad we will have this check + I had a chance to learn about Kyverno!

users.
policies.kyverno.io/severity: medium
policies.kyverno.io/subject: Pod
name: require-run-as-non-root
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍🏼 so this policy would have been triggered if we didn't have securityContext.runAsNonRoot

@jetstack-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm, wallrj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@irbekrm
Copy link
Copy Markdown
Contributor

irbekrm commented May 21, 2021

This one is a known flake afair

/test pull-cert-manager-e2e-v1-21

@irbekrm
Copy link
Copy Markdown
Contributor

irbekrm commented May 21, 2021

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/acme/http01 Indicates a PR modifies ACME HTTP01 provider code area/acme Indicates a PR directly modifies the ACME Issuer code area/deploy Indicates a PR modifies deployment configuration area/testing Issues relating to testing dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run acmesolver container as non-root

4 participants