Security Standards for both Windows and Linux by ravisantoshgudimetla · Pull Request #29855 · kubernetes/website

ravisantoshgudimetla · 2021-09-29T13:35:31Z

Some of the current pod security standards may not work for both Windows and Linux pods. This commit is an attempt to audit existing pod security standards and label them based on their relevance to a particular OS.

xref: kubernetes/enhancements#2803

Some of the current pod security standards may not work for both Windows and Linux pods. This commit is an attempt to audit existing pod security standards and label them based on their relevance to a particular OS.

ravisantoshgudimetla · 2021-09-29T13:36:02Z

cc @marosset @jsturtevant @jayunit100 @liggitt

k8s-ci-robot · 2021-09-29T13:36:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign kbhawkey after the PR has been reviewed.
You can assign the PR to them by writing /assign @kbhawkey in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

content/en/OWNERS

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

sftim · 2021-09-29T13:44:20Z

/sig security

netlify · 2021-09-29T13:51:57Z

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: be8a980

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/61546ba595f0c30008fd82d9

😎 Browse the preview: https://deploy-preview-29855--kubernetes-io-main-staging.netlify.app

s-urbaniak · 2021-09-29T14:15:13Z

content/en/docs/concepts/security/pod-security-standards.md

 			<td style="white-space: nowrap">Host Namespaces</td>
 			<td>
-				<p>Sharing the host namespaces must be disallowed.</p>
+				<p>Sharing the host namespaces must be disallowed. This is a Linux specific field and should not be used for Windows Pods.</p>


does it make sense to mention the fact that this will be effectively only possible with the new Windows KEP being in place?

we would not add OS-specific docs to this check... no matter the OS of the pod, this policy would disallow this field

So, what should the admission plugin do? Throw an error saying:

This field is not allowed when baseline policy is enabled?

or

This field is not allowed for Windows pod and cannot be used with baseline profile even for linux pods to give user a more specific error?

or are you saying let's not update the docs for OS specific fields if they are already validated based on values(sometimes nil or undefined etc).

All the checks that forbid specific fields/values can remain unchanged, even if those fields are OS-specific. For example, the baseline policy can forbid privileged: true, regardless of whether the pod is a windows pod or not.

(API validation could additionally complain if you set a linux-specific field on an explicitly windows pod, but that would not be done by the pod security admission plugin)

The checks that currently require specific fields to be set (which are all in the restricted policy) are what we would make conditional on the pod OS.

(API validation could additionally complain if you set a linux-specific field on an explicitly windows pod, but that would not be done by the pod security admission plugin)

I am doing it here - kubernetes/kubernetes#104693

The checks that currently require specific fields to be set (which are all in the restricted policy) are what we would make conditional on the pod OS.

Ok, I'll make changes to restricted profile only then.

liggitt · 2021-09-29T14:19:40Z

content/en/docs/concepts/security/pod-security-standards.md

 			<td style="white-space: nowrap">Privileged Containers</td>
 			<td>
-				<p>Privileged Pods disable most security mechanisms and must be disallowed.</p>
+				<p>Privileged Pods disable most security mechanisms and must be disallowed. This is a Linux specific field and should not be used for Windows Pods.</p>


we would not add OS-specific docs to this check... no matter the OS of the pod, this policy would disallow this field

So, what should the admission plugin do? Throw an error saying:

This field is not allowed when baseline policy is enabled?

or

This field is not allowed for Windows pod and cannot be used with baseline profile even for linux pods to give user a more specific error?

liggitt · 2021-09-29T14:19:48Z

content/en/docs/concepts/security/pod-security-standards.md

 			<td style="white-space: nowrap">Capabilities</td>
 			<td>
-				<p>Adding additional capabilities beyond those listed below must be disallowed.</p>
+				<p>Adding additional capabilities beyond those listed below must be disallowed. This is a Linux specific field and should not be used for Windows Pods.</p>


we would not add OS-specific docs to this check... no matter the OS of the pod, this policy would disallow this field

So, what should the admission plugin do? Throw an error saying:

This field is not allowed when baseline policy is enabled?

or

This field is not allowed for Windows pod and cannot be used with baseline profile even for linux pods to give user a more specific error?

liggitt · 2021-09-29T14:20:54Z

content/en/docs/concepts/security/pod-security-standards.md

 			<td style="white-space: nowrap">AppArmor</td>
 			<td>
-				<p>On supported hosts, the <code>runtime/default</code> AppArmor profile is applied by default. The baseline policy should prevent overriding or disabling the default AppArmor profile, or restrict overrides to an allowed set of profiles.</p>
+				<p>On supported hosts, the <code>runtime/default</code> AppArmor profile is applied by default. The baseline policy should prevent overriding or disabling the default AppArmor profile, or restrict overrides to an allowed set of profiles. This is a Linux specific field and should not be used for Windows Pods.</p>


we would not add OS-specific docs to this check... no matter the OS of the pod, this policy should be enforced (since it permits an undefined value)

So, if the field permits undefined values, we should let the pod security validation pass if there are undefined values? I thought we can outright reject pods if they have mismatched fields(based on OS) in the pod or container security context

liggitt · 2021-09-29T14:21:17Z

content/en/docs/concepts/security/pod-security-standards.md

 			<td style="white-space: nowrap">SELinux</td>
 			<td>
-				<p>Setting the SELinux type is restricted, and setting a custom SELinux user or role option is forbidden.</p>
+				<p>Setting the SELinux type is restricted, and setting a custom SELinux user or role option is forbidden. This is a Linux specific field and should not be used for Windows Pods.</p>


we would not add OS-specific docs to this check... no matter the OS of the pod, this policy should be enforced (since it permits omitting the field)

So, if the field permits undefined values, we should let the pod security validation pass? I thought we can outright reject pods if they have mismatched fields in the pod or container security context

liggitt · 2021-09-29T14:22:28Z

content/en/docs/concepts/security/pod-security-standards.md

  			<td style="white-space: nowrap">Seccomp (v1.19+)</td>
  			<td>
-  				<p>Seccomp profile must be explicitly set to one of the allowed values. Both the <code>Unconfined</code> profile and the <em>absence</em> of a profile are prohibited.</p>
+  				<p>Seccomp profile must be explicitly set to one of the allowed values. Both the <code>Unconfined</code> profile and the <em>absence</em> of a profile are prohibited. This is a Linux specific field and should not be used for Windows Pods.</p>


This check WILL be OS-specific once that support is added to pods, and will not apply to windows pods, but we should not update this doc until that feature lands (planned for alpha in 1.23)

Right. This PR is supposed to merge after the code changes are in place.

liggitt · 2021-09-29T14:22:34Z

content/en/docs/concepts/security/pod-security-standards.md

@@ -422,7 +422,7 @@ fail validation.
 			<td>
 				<p>
 					Containers must drop <code>ALL</code> capabilities, and are only permitted to add back


This check WILL be OS-specific once that support is added to pods, and will not apply to windows pods, but we should not update this doc until that feature lands (planned for alpha in 1.23)

Right. This PR is supposed to merge after the code changes are in place.

See kubernetes/enhancements#2802 (comment) for the request for docs.

sftim · 2021-09-29T15:30:36Z

/sig auth

@ravisantoshgudimetla if you're planning this to document changes for Kubernetes v1.23, please target the dev-1.23 branch. It will also be useful to let the release-docs team know you've opened this (if you haven't already said so).

ravisantoshgudimetla · 2021-09-29T16:22:25Z

@ravisantoshgudimetla if you're planning this to document changes for Kubernetes v1.23, please target the dev-1.23 branch. It will also be useful to let the release-docs team know you've opened this (if you haven't already said so).

Will do that @sftim. Thank you :)

mehabhalodiya · 2021-10-21T04:48:44Z

@ravisantoshgudimetla if you're planning this to document changes for Kubernetes v1.23, please target the dev-1.23 branch. It will also be useful to let the release-docs team know you've opened this (if you haven't already said so).

@ravisantoshgudimetla can you please do this?

marosset · 2021-11-29T17:30:08Z

/sig windows

ravisantoshgudimetla · 2021-11-29T17:59:20Z

@ravisantoshgudimetla can you please do this?

This is not needed for 1.23. @enj can you add appropriate milestone for this(1.25 in this case)

sftim · 2021-11-30T14:12:12Z

If this issue isn't about documenting current Kubernetes behavior, or changes planned for an upcoming release, I suggest transferring the issue to https://github.com/kubernetes/sig-security

I'll highlight this in Slack too.

sftim · 2021-11-30T14:16:23Z

@ravisantoshgudimetla would you be willing to revise the PR description to link to https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/2802-identify-windows-pods-apiserver-admission/ as well ?

jimangel · 2021-12-06T01:40:12Z

/hold
/milestone 1.23
/cc @jlbutler

jimangel · 2021-12-06T01:41:25Z

I should add some context to my comment, I held this PR for after the release and copied Jesse in as the SIG Docs lead to keep an eye on it in the 1.23 milestone that is rapidly approaching. Once 1.23 is out the door, any approvers can feel free to remove the /hold after release day.

k8s-ci-robot · 2021-12-08T11:37:50Z

@ravisantoshgudimetla: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jlbutler · 2021-12-13T20:19:34Z

@ravisantoshgudimetla as this is pointed at main and 1.23 is released, I'm dropping this out of the 1.23 milestone.

k8s-triage-robot · 2022-03-13T21:03:25Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

sftim · 2022-03-14T09:40:16Z

Based on #29855 (review) it doesn't look as if this change would go through as-s. @ravisantoshgudimetla I'm going to close this PR.

You might like to file an issue (against https://github.com/kubernetes/website) to describe the defect you're experiencing or the improvement that you'd like to see.
/close

k8s-ci-robot · 2022-03-14T09:40:36Z

@sftim: Closed this PR.

Details

In response to this:

Based on #29855 (review) it doesn't look as if this change would go through as-s. @ravisantoshgudimetla I'm going to close this PR.

You might like to file an issue (against https://github.com/kubernetes/website) to describe the defect you're experiencing or the improvement that you'd like to see.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Security Standards for both Windows and Linux

be8a980

Some of the current pod security standards may not work for both Windows and Linux pods. This commit is an attempt to audit existing pod security standards and label them based on their relevance to a particular OS.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 29, 2021

k8s-ci-robot requested review from celestehorgan and daminisatya September 29, 2021 13:36

k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Sep 29, 2021

k8s-ci-robot added the sig/security Categorizes an issue or PR as relevant to SIG Security. label Sep 29, 2021

s-urbaniak reviewed Sep 29, 2021

View reviewed changes

liggitt reviewed Sep 29, 2021

View reviewed changes

k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Sep 29, 2021

ravisantoshgudimetla changed the base branch from main to dev-1.23 September 29, 2021 16:21

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 29, 2021

ravisantoshgudimetla changed the base branch from dev-1.23 to main September 29, 2021 16:22

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 29, 2021

k8s-ci-robot added the sig/windows Categorizes an issue or PR as relevant to SIG Windows. label Nov 29, 2021

k8s-ci-robot requested a review from jlbutler December 6, 2021 01:40

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2021

k8s-ci-robot added this to the 1.23 milestone Dec 6, 2021

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2021

jlbutler removed this from the 1.23 milestone Dec 13, 2021

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 13, 2022

k8s-ci-robot closed this Mar 14, 2022

Comments

Conversation

ravisantoshgudimetla commented Sep 29, 2021

Uh oh!

ravisantoshgudimetla commented Sep 29, 2021

Uh oh!

k8s-ci-robot commented Sep 29, 2021

Uh oh!

sftim commented Sep 29, 2021

Uh oh!

netlify bot commented Sep 29, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liggitt Sep 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravisantoshgudimetla Sep 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sftim commented Sep 29, 2021

Uh oh!

ravisantoshgudimetla commented Sep 29, 2021

Uh oh!

mehabhalodiya commented Oct 21, 2021

Uh oh!

marosset commented Nov 29, 2021

Uh oh!

ravisantoshgudimetla commented Nov 29, 2021

Uh oh!

sftim commented Nov 30, 2021

Uh oh!

sftim commented Nov 30, 2021

Uh oh!

jimangel commented Dec 6, 2021

Uh oh!

jimangel commented Dec 6, 2021

Uh oh!

k8s-ci-robot commented Dec 8, 2021

Uh oh!

jlbutler commented Dec 13, 2021

Uh oh!

k8s-triage-robot commented Mar 13, 2022

Uh oh!

sftim commented Mar 14, 2022

Uh oh!

k8s-ci-robot commented Mar 14, 2022

Uh oh!

liggitt Sep 29, 2021 •

edited

Loading

ravisantoshgudimetla Sep 29, 2021 •

edited

Loading