[RFC] PodRestriction proposal #1950

tallclair · 2018-03-21T06:41:53Z

This is a proposal for a new policy object (and associated admission controller) that adds basic restrictions to most fields on a Pod. This is motivated by a desire to unify the various policies we have now (which all behave in slightly different ways), and to unblock expanding the set of restricted fields. Furthermore, it aims to solve some of the key usability problems with PodSecurityPolicies, such as the inability to compose policies and the difficulty in binding policies.

Help Wanted: I don't actually have time to own the implementation of this proposal, so I'm looking for community members who are interested in realizing this. If we decide to move forward with this approach, my hope is that we can consolidate some of the efforts around other new restriction policies.

Question: Should this be rolled up into the overall policy proposal or something like OPA? Alternatively, should this use a path-based OPA-style API?

Fix for kubernetes/kubernetes#60001

/sig auth

k8s-ci-robot · 2018-03-21T06:42:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tallclair

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~contributors/design-proposals/auth/OWNERS~~ [tallclair]

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

davidopp · 2018-03-21T06:47:49Z

/sig scheduling

ravisantoshgudimetla · 2018-03-22T03:46:05Z

contributors/design-proposals/auth/pod-restriction.md

+**Bind to Pod labels**: Pod labels don't have any authorization of usage without something like
+PodRestriction, so binding to labels isn't sufficient no it's own.
+
+**Bind to Namespaces**: The approach I'm recommending. Cluster level objects use a namespace


This is much more intuitive. I am not sure how well it works at scale. One case I could think of is having a lot of namespaces all having different labels based on which selection could happen resulting in delay of policy binding. This could be avoided by labeling all the namespaces with predefined value though.

ravisantoshgudimetla · 2018-03-22T03:53:30Z

contributors/design-proposals/auth/pod-restriction.md

+
+#### Optional Extensions
+
+-   Namespaced PodRestrictions - PodRestriction objects can be created in a namespace, so that


Can you please elaborate on how to avoid conflicts between restriction objects created in namespace vs cluster-scoped ones? or are the restriction objects which are defined in cluster-scoped cannot be touched in namespace?

If we implemented namespaced restrictions, they would be a strict subset of the cluster restrictions. E.g. the admission controller first validates the pod against the cluster-scoped restrictions (and rejects it immediately if it's invalid). Only if it's valid under the cluster restrictions is it then checked against the namespaced restrictions.

ravisantoshgudimetla · 2018-03-22T04:05:28Z

contributors/design-proposals/auth/pod-restriction.md

+#### Monolith vs. Modular
+
+Our current approach to policies has been "modular", i.e. a different controller and API for each
+"area" of features. This has led to some inconsistencies and gaps in our policies, and makes adding


I think we have gone past that stage of "area" of features as well. While I am not completely against monolithic approach after your point, I think, having a delegation model to area would helpful and might be a good step. For example, podrestriction controller would delegate to scheduling admission controller which can add restrictions specific to scheduling. But a careful grouping of controllers to areas is very much needed and some controllers may be in 2 or more groups.

ravisantoshgudimetla · 2018-03-22T04:17:37Z

contributors/design-proposals/auth/pod-restriction.md

+There are 2 problems with mutating Pod fields: 1) it becomes hard to compose policies (what happens
+when the defaults conflict?) and 2) you need a policy ordering mechanism for when a Pod matches
+multiple policies. Instead, PodRestriction opts out of mutating defaults entirely, and leaves that
+for a different component to solve ([PodPreset?](../service-catalog/pod-preset.md)).


What if mutation is needed via webhooks, won't they be allowed as well? If yes, it may be too restrictive.

I'm not sure I understand this. I'm suggesting that the PodRestriction doesn't do any mutations, but it has nothing to do with whether other controllers / policies make mutating changes.

ok, thanks for the clarification. After reading this first line the paragraph I got confused.

FWIW, I'm not sure what the future of PodPreset is.

ravisantoshgudimetla · 2018-03-22T04:26:56Z

contributors/design-proposals/auth/pod-restriction.md

+	AutomountServiceAccountToken  *BoolRestriction
+	SecurityContext               *PodSecurityContextRestriction
+
+    // TODO: Scheduling restrictions are deeply nested structures and require


I think grouping them might be helpful further. I could talk from scheduler perspective, how about having a schedulerrestriction which has affinity, schedulerName, tolerations etc. This is similar to composition of restrictions.

jhorwit2 · 2018-03-22T17:21:21Z

contributors/design-proposals/auth/pod-restriction.md

+### Existing Policies
+
+There are multiple policies that constrain pod objects today. PodRestriction would supersede these
+policies, and they would eventually be deprecated.


The admission controllers mentioned here for the most part all perform mutation/defautling so how do you envision deprecating them in favor of this admission controller if it isn't supposed to mutate/default fields?

Yeah, another mechanism for defaulting / mutating would need to be in place before they could be deprecated. E.g. extend PodPreset to default those fields.

That sounds like it may get complex for PSP's since that admission controller has very specific logic around choosing a PSP to apply to a pod.

cc @liggitt

tallclair · 2018-03-22T18:10:14Z

@erictune brought up a great point in another thread: Unifying all policies into a single resource means that you can't subdivide authorization on that resource. For instance, I can't have one admin who's responsible for setting the security constraints, and another responsible for setting the resource constraints.

A better approach might be to stick with the "modular" approach we have now, but define a consistent set of design principles that each policy should adhere to. In other words, all policies should bind, compose, and restrict in the same (or very similar) ways.

If we want to stick with the modular approach, I think we need to survey the scope of pod features and define a clearly delineated list of what policies we want, and what features fall into which policy. For instance, where should label restrictions go? SchedulingPolicy? LabelingPolicy? MetadataPolicy?

tallclair · 2018-03-22T18:40:12Z

contributors/design-proposals/auth/pod-restriction.md

+
+In other words, the Pod is first checked against Accept restrictions, and is accepted if any
+match. Then, it is checked against the Drop restrictions, and rejected if any don't match. The
+default behavior at the end is to accept the pod.


Alternative approach - use a firewall analogy:

Fields in the policy are just pattern-matching, there's no concept of per-field allow vs deny

Policy action is allow or deny. If the field matchers in a policy match the pod, the action is taken.

Policy has a priority specifying the order. Conflicts are resolved as deny (fail closed)

@tallclair - I'd prefer having a policy action which would be allow, deny or default

What does a default action mean? Do you mean setting field defaults?

FYI i've added a #1937(comment) that sums up state of policy-related standards.

yastij · 2018-03-22T20:08:48Z

contributors/design-proposals/auth/pod-restriction.md

+and easier to add exceptions for namespaces, but can lead to a combinatorial explosion of different
+policies that can become unmanageable.
+
+**Proposal**: Why not both? The default behavior is intersection, but you can punch a hole through


@tallclair - what if a Policy specifies intersection and another one union, and both are applicable, how do we resolve this ?

I was proposing that the "union" (accept) policies are checked first. Alternatively, the "firewall" approach I mentioned uses priority and favors deny.

bgrant0607 · 2018-03-26T23:19:38Z

contributors/design-proposals/auth/pod-restriction.md

+[#48041](https://github.com/kubernetes/kubernetes/issues/48041),
+[#61185](https://github.com/kubernetes/kubernetes/issues/61185)).
+
+There have been other proposals for policies restricting fields like labels, and annotations.


kubernetes/kubernetes#15390
https://github.com/kubernetes/community/blob/master/contributors/design-proposals/api-machinery/metadata-policy.md

bgrant0607 · 2018-03-26T23:54:44Z

contributors/design-proposals/auth/pod-restriction.md

+-   Namespaced PodRestrictions - PodRestriction objects can be created in a namespace, so that
+    namespace admins can place additional restrictions within their namespace.
+
+-   Additional Bindings - The NamespaceSelector is not a top-level field so that additional binding


It could be at the top level. It just needs to be optional.

bgrant0607 · 2018-03-27T00:02:41Z

contributors/design-proposals/auth/pod-restriction.md

+#### Justification
+
+From a security perspective, it should be sufficient to apply the restriction to Pod requests, so
+why validate other objects? Fail-fast. By surfacing an error before the controller object is


I'm not sure this can work in the case that some other admission controller mutates pod fields.

bgrant0607 · 2018-03-27T00:03:52Z

contributors/design-proposals/auth/pod-restriction.md

+
+New CRDs that hold a PodTemplate and create Pods should be able to be registered against the
+PodRestriction controller. The registration should include a path (auto-discovery?) to extract the
+PodTemplate from the CRD.


@erictune, who is thinking about Operators that might not explicitly expose pod templates

bgrant0607 · 2018-03-27T00:06:49Z

contributors/design-proposals/auth/pod-restriction.md

+new restrictions needlessly complex. I propose we take a "monolithic" approach, with a single policy
+(PodRestriction) for all Pod fields.
+
+A monolithic restriction policy (i.e. single resource type) also allows the user to express more


It would be useful to add disadvantages of a monolithic policy, such as:

harder to evolve (we still don't have fine-grain field versioning)

no single SIG responsible

increases the likelihood of multiple PodRestriction objects applying to the same pods

bgrant0607 · 2018-03-27T00:19:39Z

contributors/design-proposals/auth/pod-restriction.md

+
+-   Pods can only run as privileged if they run with a specific AntiAffinity
+
+-   Pods labeled as staging must not run on production nodes.


The cross product of coordinated policies could be minimized by making up a common classification scheme.

(Without suggesting the classification mechanism at this point.)

Examples:

Pods of class Foo can use volume type Bar and Pods of class Foo must have reduced privileges

Pods of class Privileged can run privileged containers (and permissions of equivalent power) and Pods of class Privileged must run with a specific AntiAffinity

Pods of class Staging must be labeled as env=staging and Pods of class Staging cannot tolerate the Production taint

bgrant0607 · 2018-03-27T00:20:55Z

contributors/design-proposals/auth/pod-restriction.md

+There are 2 problems with mutating Pod fields: 1) it becomes hard to compose policies (what happens
+when the defaults conflict?) and 2) you need a policy ordering mechanism for when a Pod matches
+multiple policies. Instead, PodRestriction opts out of mutating defaults entirely, and leaves that
+for a different component to solve ([PodPreset?](../service-catalog/pod-preset.md)).


FWIW, I'm not sure what the future of PodPreset is.

bgrant0607 · 2018-03-27T00:21:37Z

contributors/design-proposals/auth/pod-restriction.md

+    predictable, and easier to manage (i.e. I want to restrict Pod field X, I know how to do that in
+    the PodRestriction).
+
+2.  Restrictions are self-contained. There isn't complex interdependent logic here. If you want more


I think this pretty much rules out subsuming LimitRange.

bgrant0607 · 2018-03-27T01:05:04Z

contributors/design-proposals/auth/pod-restriction.md

+    vs. Modular](#monolith-vs-modular)). We don't need to build it all at once, but I think over
+    time it will eventually grow into the full version.
+
+4.  Fields are unrestricted by default. This is important due to the size of the API. If I want to


We need to think about API evolution: what should happen when new fields are added?

bgrant0607 · 2018-03-27T01:07:13Z

contributors/design-proposals/auth/pod-restriction.md

+
+const (
+    BindingModeAccept BindingMode = "Accept"
+    BindingModeDrop   BindingMode = "Drop"


I find the term "Drop" confusing. It's Deny, the opposite of Accept? (As opposed to, e.g., elide.)

bgrant0607 · 2018-03-27T01:20:15Z

contributors/design-proposals/auth/pod-restriction.md

+type CapabilitiesRestriction struct {
+	PointerRestriction
+	Add  *StringListRestriction
+	Drop *StringListRestriction


Again, does this mean deny or elide?

bgrant0607 · 2018-03-27T01:27:30Z

contributors/design-proposals/auth/pod-restriction.md

+type StringRestriction struct {
+	PointerRestriction
+
+	Whitelist []string


Does this mean include/exclude, or accept/deny?

What's the rationale for per-field accept/deny rather than top-level accept/deny?

tallclair · 2018-04-04T23:32:53Z

See updates on kubernetes/kubernetes#60001 (comment)

My recommendation is that we focus on the SchedulingPolicy proposal for now (possibly applying some of the ideas from this proposal), and close this specific proposal.

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 21, 2018

k8s-ci-robot requested review from ericchiang and liggitt March 21, 2018 06:41

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 21, 2018

k8s-github-robot added the kind/design Categorizes issue or PR as related to design. label Mar 21, 2018

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Mar 21, 2018

PodRestriction proposal

05d1e3f

tallclair force-pushed the pod-restrict branch from edf874c to 05d1e3f Compare March 21, 2018 06:57

tallclair mentioned this pull request Mar 21, 2018

Common approach for delegated pod admission & policy kubernetes/kubernetes#60001

Open

ravisantoshgudimetla reviewed Mar 22, 2018

View reviewed changes

jhorwit2 reviewed Mar 22, 2018

View reviewed changes

tallclair commented Mar 22, 2018

View reviewed changes

yastij reviewed Mar 22, 2018

View reviewed changes

bgrant0607 mentioned this pull request Mar 26, 2018

Add initial design proposal for Scheduling Policy #1937

Closed

bgrant0607 reviewed Mar 26, 2018

View reviewed changes

bgrant0607 reviewed Mar 27, 2018

View reviewed changes

tallclair closed this Apr 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] PodRestriction proposal #1950

[RFC] PodRestriction proposal #1950

tallclair commented Mar 21, 2018 •

edited

Loading

k8s-ci-robot commented Mar 21, 2018

davidopp commented Mar 21, 2018

ravisantoshgudimetla Mar 22, 2018

ravisantoshgudimetla Mar 22, 2018

tallclair Mar 22, 2018

ravisantoshgudimetla Mar 22, 2018 •

edited

Loading

ravisantoshgudimetla Mar 22, 2018

tallclair Mar 22, 2018

ravisantoshgudimetla Mar 23, 2018

bgrant0607 Mar 27, 2018

ravisantoshgudimetla Mar 22, 2018

jhorwit2 Mar 22, 2018

tallclair Mar 22, 2018

jhorwit2 Mar 22, 2018

tallclair commented Mar 22, 2018 •

edited

Loading

tallclair Mar 22, 2018

yastij Mar 22, 2018

tallclair Mar 22, 2018

yastij Mar 26, 2018

yastij Mar 22, 2018

tallclair Mar 22, 2018

bgrant0607 Mar 26, 2018

bgrant0607 Mar 26, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

bgrant0607 Mar 27, 2018

tallclair commented Apr 4, 2018


		#### Optional Extensions

		- Namespaced PodRestrictions - PodRestriction objects can be created in a namespace, so that


		- Pods can only run as privileged if they run with a specific AntiAffinity

		- Pods labeled as staging must not run on production nodes.

[RFC] PodRestriction proposal #1950

[RFC] PodRestriction proposal #1950

Conversation

tallclair commented Mar 21, 2018 • edited Loading

k8s-ci-robot commented Mar 21, 2018

davidopp commented Mar 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravisantoshgudimetla Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tallclair commented Mar 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tallclair commented Apr 4, 2018

tallclair commented Mar 21, 2018 •

edited

Loading

ravisantoshgudimetla Mar 22, 2018 •

edited

Loading

tallclair commented Mar 22, 2018 •

edited

Loading