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

KEP: Security Profile Design Proposal #2052

Closed
wants to merge 1 commit into from

Conversation

easeway
Copy link

@easeway easeway commented Apr 18, 2018

This is the detailed design proposal for Security Profile.

Security Profile helps managing a secured Kubernetes cluster by solving problems of:

  • Management complexity

    • Complicated configuration of control plane components' security-relevant command line flags, correctness of cert/key/config file permissions;
    • Requires deep understanding of Kubernetes policy objects like RBAC, PodSecurityPolicy, NetworkPolicy, ResourceQuota etc;
    • No support for automatic provisioning of namespaced policy objects upon namespace creation;
    • Requires of domain knowledge about security;
    • Requires keeping up with fast evolving Kubernetes security/isolation features, and update configurations accordingly;
  • Extra efforts building in-house solutions

    • No standard/common ways to solve the problems;
    • It’s very difficult to make in-house solutions sharable/reusable, as a result, efforts are duplicated solving same/similar problems.

by defining curated settings of:

  • Cluster bootstrapping configuration, like command line flags, file permissions, etc
  • Cluster-scoped policy objects, like PodSecurityPolicy
  • Namespace-scoped policy objects, like NetworkPolicy

And simplify the cluster management by specifying the name of security profile.

This proposal is looking for sponsorship from sig-auth,
to create a public repo as https://github.com/kubernetes-sigs/security-profiles,
that community can help curate commonly used security profiles.
Details are described in Release section in the proposal.

/sig auth

/cc @davidopp
/cc @tallclair
/cc @ericchiang
/cc @liggitt

@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 18, 2018
@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. kind/design Categorizes issue or PR as related to design. labels Apr 18, 2018
@easeway
Copy link
Author

easeway commented Apr 18, 2018

/assign @tallclair

@easeway easeway force-pushed the securityprofile-proposal branch from b9aedc7 to b628c64 Compare April 19, 2018 23:34
@k8s-ci-robot k8s-ci-robot added the sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. label Apr 19, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: jbeda

Assign the PR to them by writing /assign @jbeda in a comment when ready.

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:

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

@easeway easeway changed the title Security Profile Design Proposal KEP: Security Profile Design Proposal Apr 19, 2018
@ericchiang
Copy link
Contributor

I'm concerned with the lack of scoping in this proposal. Autopopulating namespaces, config as code, bootstrapping config, conformance testing, meta-policies… there are at least three or four unique sub-projects in this document.

Before we get into the nitty gritty of the proposal, does it make sense to keep all of these concepts as one framework? Naively I could see conformance profiles and namespace templating being separate. Conformance profiles are a way to express the capabilities a cluster provides. Namespace templates are a tool for admins to quickly provision namespaces for users with sane security defaults.

@easeway
Copy link
Author

easeway commented Apr 23, 2018

@ericchiang you are right. The unique pieces covered in the proposal are relatively independent/separate. They can be used individually. The reason putting all things together is for a really simple user experience and also the possibility to share the experience across Kubernetes clusters in different environments.

I agree it will be better to break up and focus on individual pieces. I will send out individual proposals, covering:

And later update this KEP as a complete solution to demonstrate the ultimate user experience we want to achieve after things are integrated.

- TLS required for communication
- Explicitly specify `--wal-dir`, and `--max-wals=0`
- kube-apiserver
- TLS required for communication

Choose a reason for hiding this comment

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

how is this enforced/ verified?

Copy link
Author

Choose a reason for hiding this comment

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

They are defined in bootstrapping rules, the enforcement of bootstrapping rules are implementation specific, thus out of scope in this proposal.
Verification is out of scope.
I will call out these explicitly.

- Audit logging must be explicitly configured
- No anonymous auth, basic auth (unset `--anonymous-auth`, `--basic-auth-file`)
- Required authorization-mode: Node, RBAC
- Excluded authorization-mode: ABAC, AlwaysAllow

Choose a reason for hiding this comment

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

please add: automountServiceAccountToken: false

Copy link
Author

Choose a reason for hiding this comment

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

automountServiceAccountToken is a field in ServiceAccount or Pod spec. Currently, it's difficult to define that in security profile as it's can't be enforced by any component (though security profile can introduce a admission webhook to mutate that field, however, the intention of security profile is to leveraging existing mechanism, not introduce something new). So I think this will be a good feature in PodSecurityPolicy, then it can be defined in security profile.

Choose a reason for hiding this comment

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

ack - that makes sense

- file-permissions
- ... # correct setup of cert/key/config file permissions
- PodSecurityPolicy
- No priviledged

Choose a reason for hiding this comment

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

can you specify? I'm thinking this is: privileged: false, allowPrivilegeEscalation: false, MustRunAsNonRoot

Or is this stronger than what you were thinking for 'default'?

Copy link
Author

Choose a reason for hiding this comment

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

I think it's stronger than what the default is proposed. I'm even thinking of removing No privileged in this profile. The default profile is designed to give the maximum compatibility with existing workloads. Add restrictions to block privileged will definitely break some workloads. So I think I need rephrase the goal for each security profiles, and add one more between default and saas-multitenancy.

Enforcement: in addition to `default`

- PodSecurityPolicy
- No privileged pods

Choose a reason for hiding this comment

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

Isn't this already in 'default'?

Copy link
Author

Choose a reason for hiding this comment

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

I should remove that from default.

Choose a reason for hiding this comment

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

so 'default' would allow privileged pods?
Obviously I'm coming at this from a security angle, but if I turn on security profiles, I'm probably OK with it warning me that I shouldn't be running pods as privileged and then fix that - or are you saying that there isn't a lower level so they're stuck with that issue?

Copy link
Author

Choose a reason for hiding this comment

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

This is my personal opinion (I'd like to hear other thoughts, as I'm probably wrong): the default security profile is provided to bring in the lowest level (there's no lower levels) of security (eliminate some really bad insecure configurations) without breaking compatibility of how people are using Kubernetes today. So we can use default to help Kubernetes users smoothly transit from insecure to secure. I believe privileged containers are allowed in most Kubernetes clusters with trusted users (like inside the same corp or team) and sometimes useful. That's the reason I removed "no-privileged containers" from default profile.

Choose a reason for hiding this comment

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

got it, I see you defined default as "to be compatible with most workloads". That makes sense then.

- persistentVolumeClaim
- projected
- secret
- NetworkPolicy

Choose a reason for hiding this comment

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

Why wouldn't we include this in 'default'?

Copy link
Author

Choose a reason for hiding this comment

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

Adding NetworkPolicy (blocking all traffic by default) will break a lot of existing workloads which doesn't assume the existence of NetworkPolicy. So I think it doesn't belong to default.

Choose a reason for hiding this comment

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

got it, agreed.


#### Release
Security profiles are maintained as an individual project similarly to other Kubernetes open source projects, like heapster, dashboard etc.
It has its own release lifecycle.

Choose a reason for hiding this comment

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

What is the goal? To roughly release in line with K8s major releases, for example?

Copy link
Author

Choose a reason for hiding this comment

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

I will add more details.

Uninstall is easy as resources from a bundle are labeled with bundle name.
With multiple bundles installed, it's possible multiple admission controls, controllers are running side-by-side.
With help of labeled bundle name, the admission controls, controllers will stop enforcement if the currently selected security profile is not from the same bundle.
With this mechanism, it's very simple and lightweight to switch back to the previous security profile if something unexpected happens with a new security profile.

Choose a reason for hiding this comment

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

to the previous security profile version?

Copy link
Author

Choose a reason for hiding this comment

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

not exactly the version. The user may upgrade from default to saas, and find out breaking workloads and decided to switch back to default.

Choose a reason for hiding this comment

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

Makes sense

With help of labeled bundle name, the admission controls, controllers will stop enforcement if the currently selected security profile is not from the same bundle.
With this mechanism, it's very simple and lightweight to switch back to the previous security profile if something unexpected happens with a new security profile.

When upgrading a cluster, it's possible the current security profile may not work on the new cluster version.

Choose a reason for hiding this comment

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

may not work, or may not be supported? (i.e., 'deprecated')

Copy link
Author

Choose a reason for hiding this comment

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

to be accurate: may not be supported. (it's not 'deprecated' which means the profile may still work, but may not guarantee the security level it's promised to provide because of imperfection of rules, bugs in implementations, etc.)


- Any bug found in enforcement logic (admission control, controllers etc)
- Improper/Incomplete rules in the profile
- Other security concerns

Choose a reason for hiding this comment

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

would we ever deprecate a security profile when it no longer ad some critical security functionality, even though it was the best practice for that K8s version?

For example, would we deprecate a security profile for K8s 1.5, because it does not have the ability to enforce RBAC? Is there some minimum K8s version (or recency) we require for security profiles?

Copy link
Author

Choose a reason for hiding this comment

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

In this case, a security profile won't be deprecated for the supported K8S version. In the example mentioned, the profile will not be deprecated for K8s 1.5, however it's not supported in K8s 1.6 or above with RBAC. So for users upgrading K8s from 1.5 to 1.6 they will have to upgrade the security profile to a new version which supports both 1.5 and 1.6.

The alternative is put test suite together with Kubernetes E2E tests in the official Kubernetes repository,
It will be a little difficult to synchronize the development work in two repos.

The details is out of scope of this doc. A separate doc will be focus on conformance tests.

Choose a reason for hiding this comment

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

please share a link to the KEP

Copy link
Author

Choose a reason for hiding this comment

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

I will add that.

@easeway easeway force-pushed the securityprofile-proposal branch from b628c64 to a8cb206 Compare June 13, 2018 18:36
@easeway
Copy link
Author

easeway commented Jun 13, 2018

Removed Conformance tests which is on a different layer and should be separate from security profile.

@idealhack
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 4, 2018
@k8s-ci-robot
Copy link
Contributor

@easeway: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-community-verify a8cb206 link /test pull-community-verify

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 2, 2018
@justaugustus
Copy link
Member

/kind kep
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added kind/kep and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 13, 2018
@mattfarina
Copy link
Contributor

@mikedanese @enj @tallclair This KEP is listed under sig-auth. Have y'all reviewed it yet?

@justaugustus
Copy link
Member

REMINDER: KEPs are moving to k/enhancements on November 30. Please attempt to merge this KEP before then to signal consensus.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.

@justaugustus
Copy link
Member

KEPs have moved to k/enhancements.
This PR will be closed and any additional changes to this KEP should be submitted to k/enhancements.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.
/close

@k8s-ci-robot
Copy link
Contributor

@justaugustus: Closed this PR.

In response to this:

KEPs have moved to k/enhancements.
This PR will be closed and any additional changes to this KEP should be submitted to k/enhancements.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.
/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.

@surajssd
Copy link
Member

What is the new location of this PR? I don't see it in k/enhancements?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/design Categorizes issue or PR as related to design. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants