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

unable to deploy privileged pod after 1.8 upgrade unless I set allowPrivilegeEscalation true #53437

Closed
jhorwit2 opened this issue Oct 4, 2017 · 7 comments · Fixed by #53443
Closed
Assignees
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Milestone

Comments

@jhorwit2
Copy link
Contributor

jhorwit2 commented Oct 4, 2017

Is this a BUG REPORT or FEATURE REQUEST?:

/kind bug

What happened:

I upgraded a cluster to 1.8 and ran into an issue with pods that use privileged: true and don't set allowPrivilegeEscalation: true.

The PSP I had created and been using prior to 1.8:

apiVersion: extensions/v1beta1
kind: PodSecurityPolicy
metadata:
  name: privileged
spec:
  privileged: true
  hostNetwork: true
  hostPID: true
  hostIPC: true
  hostPorts:
  - min: 1
    max: 32000
  fsGroup:
    rule: RunAsAny
  runAsUser:
    rule: RunAsAny
  seLinux:
    rule: RunAsAny
  supplementalGroups:
    rule: RunAsAny
  volumes:
  - '*'

The error I got when applying an update to the canal daemonset:

  Warning  FailedCreate  10m                 daemon-set  Error creating: Pod "canal-z8dmj" is invalid: [spec.containers[0].securityContext: Invalid value: api.SecurityContext{Capabilities:(*api.Capabilities)(nil), Privileged:(*bool)(0xc424f9c2c0), SELinuxOptions:(*api.SELinuxOptions)(nil), RunAsUser:(*int64)(nil), RunAsNonRoot:(*bool)(nil), ReadOnlyRootFilesystem:(*bool)(nil), AllowPrivilegeEscalation:(*bool)(0xc427e10c38)}: cannot set `allowPrivilegeEscalation` to false and `privileged` to true, spec.containers[2].securityContext: Invalid value: api.SecurityContext{Capabilities:(*api.Capabilities)(nil), Privileged:(*bool)(0xc424f9c2ca), SELinuxOptions:(*api.SELinuxOptions)(nil), RunAsUser:(*int64)(nil), RunAsNonRoot:(*bool)(nil), ReadOnlyRootFilesystem:(*bool)(nil), AllowPrivilegeEscalation:(*bool)(0xc427e10c38)}: cannot set `allowPrivilegeEscalation` to false and `privileged` to true]

What you expected to happen:

I expect the pod to be valid since it was not noted as a breaking change in the release notes for 1.8.

How to reproduce it (as minimally and precisely as possible):

Using the PSP above, attempt to create a pod with privileged: true and no value for allowPrivilegeEscalation.

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version):
Client Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.0", GitCommit:"6e937839ac04a38cac63e6a7a306c5d035fe7b0a", GitTreeState:"clean", BuildDate:"2017-09-28T22:57:57Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.0", GitCommit:"6e937839ac04a38cac63e6a7a306c5d035fe7b0a", GitTreeState:"clean", BuildDate:"2017-09-28T22:46:41Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}

cc @liggitt @tallclair @jessfraz

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 4, 2017
@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 4, 2017
@k8s-github-robot
Copy link

@jhorwit2
There are no sig labels on this issue. Please add a sig label by:

  1. mentioning a sig: @kubernetes/sig-<group-name>-<group-suffix>
    e.g., @kubernetes/sig-contributor-experience-<group-suffix> to notify the contributor experience sig, OR

  2. specifying the label manually: /sig <label>
    e.g., /sig scalability to apply the sig/scalability label

Note: Method 1 will trigger an email to the group. You can find the group list here and label list here.
The <group-suffix> in the method 1 has to be replaced with one of these: bugs, feature-requests, pr-reviews, test-failures, proposals

@jhorwit2
Copy link
Contributor Author

jhorwit2 commented Oct 4, 2017

/sig auth

@k8s-ci-robot k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Oct 4, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 4, 2017
@jhorwit2
Copy link
Contributor Author

jhorwit2 commented Oct 4, 2017

semi-unrelated: the error message on that event is hideous 👼

@liggitt
Copy link
Member

liggitt commented Oct 4, 2017

@jessfraz @tallclair existing PSPs should continue to work as they did prior to the introduction of this field. doesn't that mean DefaultAllowPrivilegeEscalation has to default to true in PSPs if unspecified?

@liggitt
Copy link
Member

liggitt commented Oct 4, 2017

alternatively, psp.Spec.AllowPrivilegeEscalation could have been a *bool, defaulting to nil/no-opinion

edit: thinking through this more, I think we have to make AllowPrivilegeEscalation a *bool, omitempty... 1.8.0 clients/servers do the wrong thing, dropping an explicit allowPrivilegeEscalation: false and treating it the same as a legacy PSP object that should have defaulted to allowing privilege escalation for backward compatibility

@liggitt liggitt self-assigned this Oct 4, 2017
@liggitt liggitt added this to the v1.8 milestone Oct 4, 2017
@liggitt liggitt added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Oct 4, 2017
@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Labels Complete

@jessfraz @jhorwit2 @liggitt @tallclair

Issue label settings:

  • sig/auth: Issue will be escalated to these SIGs if needed.
  • priority/critical-urgent: Never automatically move out of a release milestone; continually escalate to contributor and SIG through all available channels.
  • kind/bug: Fixes a bug discovered during the current release.
Additional instructions available here The commands available for adding these labels are documented here

@liggitt liggitt added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API milestone-labels-incomplete and removed milestone-labels-complete labels Oct 4, 2017
k8s-github-robot pushed a commit that referenced this issue Oct 5, 2017
Automatic merge from submit-queue (batch tested with PRs 53454, 53446, 52935, 53443, 52917). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use pointer for PSP allow escalation

Fixes #53437 

The `AllowPrivilegeEscalation` field was added to PodSpec and PodSecurityPolicySpec in 1.8.0.

In order to remain compatible with pre-1.8.0 behavior, PodSecurityPolicy objects created against a previous release must not restrict this field, which means the field must default to true in PodSecurityPolicySpec. However, the field was added as a `bool`, not a `*bool`, which means that no defaulting is possible.

We have two options:
1. Require all pre-existing PodSecurityPolicy objects that intend to allow privileged permissions to update to set this new field to true
2. Change the field to a `*bool` and default it to true.

This PR does the latter. With this change, we have the following behavior:

A 1.8.1+ client/server now has three ways to serialize:
* `nil` values are dropped from serialization (because `omitempty`), which is interpreted correctly by other 1.8.1+ clients/servers, and is interpreted as false by 1.8.0
* `false` values are serialized and interpreted correctly by all clients/servers
* `true` values are serialized and interpreted correctly by all clients/servers

A 1.8.0 client/server has two ways to serialize:
* `false` values are dropped from serialization (because `omitempty`), which is interpreted as `false` by other 1.8.0 clients/servers, but as `nil` (and therefore defaulting to true) by 1.8.1+ clients/servers
* `true` values are serialized and interpreted correctly by all clients/servers

The primary concern is the 1.8.0 server dropping the `false` value from serialization, but I consider the compatibility break with pre-1.8 behavior to be more severe, especially if we can resolve the regression in an immediate point release.

```release-note
PodSecurityPolicy: Fixes a compatibility issue that caused policies that previously allowed privileged pods to start forbidding them, due to an incorrect default value for `allowPrivilegeEscalation`. PodSecurityPolicy objects defined using a 1.8.0 client or server that intended to set `allowPrivilegeEscalation` to `false` must be reapplied after upgrading to 1.8.1.
```
@liggitt
Copy link
Member

liggitt commented Oct 12, 2017

fixed in 1.8.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants