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

Support node-level user namespace remapping #127

Open
derekwaynecarr opened this Issue Oct 10, 2016 · 57 comments

Comments

@derekwaynecarr
Copy link
Member

derekwaynecarr commented Oct 10, 2016

Description

  1. As a cluster admin, I want to protect the node from the rogue container process(es) running inside pod containers with root privileges. If such a process is able to break out into the node, it could be a security issue.
  2. As a cluster admin, I want to support all the images irrespective of what user/group that image is using.
  3. As a cluster admin, I want to allow some pods to disable user namespaces if they require elevated privileges.

Scope of work planned for v1.12

  • alpha support in kubelet to support user namespace remapping

Progress Tracker

  • Alpha
    • Write and maintain draft quality doc
      • During development keep a doc up-to-date about the desired experience of the feature and how someone can try the feature in its current state. Think of it as the README of your new feature and a skeleton for the docs to be written before the Kubernetes release. Paste link to Google Doc: DOC-LINK
    • Design Approval
      • Design Proposal. This goes under docs/proposals. Doing a proposal as a PR allows line-by-line commenting from community, and creates the basis for later design documentation. Paste link to merged design proposal here: kubernetes/community#2067 (supercedes kubernetes/community#2042)
      • Decide which repo this feature's code will be checked into. Not everything needs to land in the core kubernetes repo. kubernetes/kubernetes
      • Initial API review (if API). Maybe same PR as design doc. PR-NUMBER
        • Any code that changes an API (/pkg/apis/...)
        • cc @kubernetes/api
      • Identify shepherd (your SIG lead and/or kubernetes-pm@googlegroups.com will be able to help you). My Shepherd is: replace.me@replaceme.com (and/or GH Handle)
        • A shepherd is an individual who will help acquaint you with the process of getting your feature into the repo, identify reviewers and provide feedback on the feature. They are not (necessarily) the code reviewer of the feature, or tech lead for the area.
        • The shepherd is not responsible for showing up to Kubernetes-PM meetings and/or communicating if the feature is on-track to make the release goals. That is still your responsibility.
      • Identify secondary/backup contact point. My Secondary Contact Point is: replace.me@replaceme.com (and/or GH Handle)
    • Write (code + tests + docs) then get them merged. ALL-PR-NUMBERS
      • Code needs to be disabled by default. Verified by code OWNERS
      • Minimal testing
      • Minimal docs
        • cc @kubernetes/docs on docs PR
        • cc @kubernetes/feature-reviewers on this issue to get approval before checking this off
        • New apis: Glossary Section Item in the docs repo: kubernetes/kubernetes.github.io
      • Update release notes
  • Beta
    • Testing is sufficient for beta
    • User docs with tutorials
      • Updated walkthrough / tutorial in the docs repo: kubernetes/kubernetes.github.io
      • cc @kubernetes/docs on docs PR
      • cc @kubernetes/feature-reviewers on this issue to get approval before checking this off
    • Thorough API review
      • cc @kubernetes/api
  • Stable
    • docs/proposals/foo.md moved to docs/design/foo.md
      • cc @kubernetes/feature-reviewers on this issue to get approval before checking this off
    • Soak, load testing
    • detailed user docs and examples
      • cc @kubernetes/docs
      • cc @kubernetes/feature-reviewers on this issue to get approval before checking this off

FEATURE_STATUS is used for feature tracking and to be updated by @kubernetes/feature-reviewers.
FEATURE_STATUS: IN_DEVELOPMENT

More advice:

Design

  • Once you get LGTM from a @kubernetes/feature-reviewers member, you can check this checkbox, and the reviewer will apply the "design-complete" label.

Coding

  • Use as many PRs as you need. Write tests in the same or different PRs, as is convenient for you.
  • As each PR is merged, add a comment to this issue referencing the PRs. Code goes in the http://github.com/kubernetes/kubernetes repository,
    and sometimes http://github.com/kubernetes/contrib, or other repos.
  • When you are done with the code, apply the "code-complete" label.
  • When the feature has user docs, please add a comment mentioning @kubernetes/feature-reviewers and they will
    check that the code matches the proposed feature and design, and that everything is done, and that there is adequate
    testing. They won't do detailed code review: that already happened when your PRs were reviewed.
    When that is done, you can check this box and the reviewer will apply the "code-complete" label.

Docs

  • Write user docs and get them merged in.
  • User docs go into http://github.com/kubernetes/kubernetes.github.io.
  • When the feature has user docs, please add a comment mentioning @kubernetes/docs.
  • When you get LGTM, you can check this checkbox, and the reviewer will apply the "docs-complete" label.
@derekwaynecarr

This comment has been minimized.

Copy link
Member

derekwaynecarr commented Oct 10, 2016

This work is being done by @pweil- and is reviewed by @derekwaynecarr, it is sponsored by @kubernetes/sig-node

@idvoretskyi idvoretskyi modified the milestone: v1.5 Oct 11, 2016

@mdshuai

This comment has been minimized.

Copy link
Contributor

mdshuai commented Oct 26, 2016

@derekwaynecarr Could you help create a user story card for this feature?

@idvoretskyi

This comment has been minimized.

Copy link
Member

idvoretskyi commented Nov 15, 2016

@derekwaynecarr can you confirm that this feature targets alpha for 1.5?

@pweil-

This comment has been minimized.

Copy link
Member

pweil- commented Nov 16, 2016

@derekwaynecarr can you confirm that this feature targets alpha for 1.5?

Yes, this feature is experimental only so it would be considered alpha.

@idvoretskyi

This comment has been minimized.

Copy link
Member

idvoretskyi commented Dec 13, 2016

@derekwaynecarr @pweil- can you confirm that this item targets beta in 1.6?

@adelton

This comment has been minimized.

Copy link

adelton commented Nov 14, 2017

@derekwaynecarr, the proposal kubernetes/kubernetes#34569 was closed by bot due to inactivity.

@pweil-, in kubernetes/kubernetes#34569 (comment) you've proposed the approach pweil-/kubernetes@16f29eb which changes the group of /var/lib/kubelet/pods to the remapped root group. Do I understand it correctly that this is currently not tracked in any pull request?

@adelton

This comment has been minimized.

Copy link

adelton commented Nov 14, 2017

@pweil-, I also wonder if similar to docker's /var/lib/docker/<uid>.<gid> approach when --userns-remap is used, it might make sense to use /var/lib/kubelet/pods-<uid>.<gid> and just chown/chgroup everything in those subdirectories to the remapped <uid>.<gid>. Why did you opt for just the chgrp and not the full chown?

@pweil-

This comment has been minimized.

Copy link
Member

pweil- commented Nov 14, 2017

@adelton in the end, I think having this be transparent to Kubernetes is the right approach. Whether that be something like shiftfs or implementation in the CRI (moby/moby#28593). You are correct that my existing proposal is not currently tracked in an open PR anymore.

The reasoning behind using the chgrp was to follow our fsgroup strategy where we just ensure group access instead of uid access.

@adelton

This comment has been minimized.

Copy link

adelton commented Nov 14, 2017

Thanks @pweil-.

When you say transparent, you mean that nothing should be needed to be added to code or to configuration on Kubernetes' side to allow running under docker with userns-remap?

As for the fsgroup strategy, do you mean https://kubernetes.io/docs/concepts/policy/pod-security-policy/#fsgroup or some generic methodology within Kubernetes?

I have now filed kubernetes/kubernetes#55707 as an alternative approach where I make the remapped uid/gid an explicit option, and use those values to chown/chgrp the necessary directories.

@pweil-

This comment has been minimized.

Copy link
Member

pweil- commented Nov 14, 2017

When you say transparent, you mean that nothing should be needed to be added to code or to configuration on Kubernetes' side to allow running under docker with userns-remap?

that would be ideal. Whether that is feasible (or more likely, feasible in an acceptable time frame) is another question 😄

As for the fsgroup strategy, do you mean https://kubernetes.io/docs/concepts/policy/pod-security-policy/#fsgroup or some generic methodology within Kubernetes?

Yes

I have now filed kubernetes/kubernetes#55707 as an alternative approach where I make the remapped uid/gid an explicit option, and use those values to chown/chgrp the necessary directories.

👍 subscribed

@adelton

This comment has been minimized.

Copy link

adelton commented Nov 14, 2017

When you say transparent, you mean that nothing should be needed to be added to code or to configuration on Kubernetes' side to allow running under docker with userns-remap?

that would be ideal. Whether that is feasible (or more likely, feasible in an acceptable time frame) is another question

Ideally, the pod would specify how many distinct uids/gids it would require / list of uids it wants to see inside of the containers, and docker or different container runtime would setup the user namespace accordingly. But unless docker also changes ownership of the volumes mounted to the containers, Kubernetes will have to do that as part of the setup.

@adelton

This comment has been minimized.

Copy link

adelton commented Dec 7, 2017

@pwel-, what is the best way to get some review and comments on kubernetes/kubernetes#55707, to get it closer to mergeable state?

@kargakis

This comment has been minimized.

Copy link
Member

kargakis commented Dec 7, 2017

@pweil-

This comment has been minimized.

Copy link
Member

pweil- commented Dec 7, 2017

@adelton I would try to engage the sig-node folks either at their Tuesday meeting or on slack: https://github.com/kubernetes/community/tree/master/sig-node

@adelton

This comment has been minimized.

Copy link

adelton commented Dec 18, 2017

@derekwaynecarr, could you please bring kubernetes/kubernetes#55707 to sig-node's radar?

@idvoretskyi

This comment has been minimized.

Copy link
Member

idvoretskyi commented Jan 22, 2018

@pweil- @derekwaynecarr any progress on this feature is expected?

@derekwaynecarr

This comment has been minimized.

Copy link
Member

derekwaynecarr commented Jul 31, 2018

updated description to capture that it slipped 1.11 and is being tracked in 1.12 as alpha.

@zparnold

This comment has been minimized.

Copy link
Member

zparnold commented Aug 20, 2018

Hey there! @derekwaynecarr I'm the wrangler for the Docs this release. Is there any chance I could have you open up a docs PR against the release-1.12 branch as a placeholder? That gives us more confidence in the feature shipping in this release and gives me something to work with when we start doing reviews/edits. Thanks! If this feature does not require docs, could you please update the features tracking spreadsheet to reflect it?

@derekwaynecarr

This comment has been minimized.

Copy link
Member

derekwaynecarr commented Aug 20, 2018

@zparnold

This comment has been minimized.

Copy link
Member

zparnold commented Aug 25, 2018

Thanks! @vikaschoudhary16 let me know when the PR is up please. 😄

@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Sep 5, 2018

@vikaschoudhary16 @derekwaynecarr @mrunalp --
Any update on docs status for this feature? Are we still planning to land it for 1.12?
At this point, code freeze is upon us, and docs are due on 9/7 (2 days).
If we don't here anything back regarding this feature ASAP, we'll need to remove it from the milestone.

cc: @zparnold @jimangel @tfogo

@derekwaynecarr

This comment has been minimized.

Copy link
Member

derekwaynecarr commented Sep 6, 2018

moving to 1.13 milestone.

@kacole2

This comment has been minimized.

Copy link
Contributor

kacole2 commented Oct 8, 2018

@derekwaynecarr do we feel confident this will hit the deadlines for 1.13? This release is targeted to be more ‘stable’ and will have an aggressive timeline. Please only include this enhancement if there is a high level of confidence it will meet the following deadlines:

  • Docs (open placeholder PRs): 11/8
  • Code Slush: 11/9
  • Code Freeze Begins: 11/15
  • Docs Complete and Reviewed: 11/27

Thanks!

@jimangel

This comment has been minimized.

Copy link
Member

jimangel commented Oct 17, 2018

@derekwaynecarr

Just a friendly reminder about docs and status for 1.13. Thanks!

cc: @vikaschoudhary16

@prbinu

This comment has been minimized.

Copy link

prbinu commented Oct 22, 2018

@derekwaynecarr
We operate k8s clusters that allow execution of third-party containers. From a security perspective, this is an an important feature we have been waiting for a while. Please consider this as high-priority security feature and make it available in v1.13 release.

@kacole2

This comment has been minimized.

Copy link
Contributor

kacole2 commented Oct 22, 2018

@derekwaynecarr there has been no communication on the status but I see @spiffxp has attached a current k/k PR. Are we confident this is going to make the v1.13 milestone? Enhancement freeze is tomorrow COB. If there is no communication on this issue or activity on the PR, this is going to be pulled from the milestone as it doesn't fit with our "stability" theme. If there is no communication after COB tomorrow, an exception will be required to add it back to the milestone. Please let me know where we stand. Thanks!

@kacole2

This comment has been minimized.

Copy link
Contributor

kacole2 commented Oct 24, 2018

lack of communication so this is being removed from 1.13 tracking.

/milestone clear

@kacole2 kacole2 removed the tracked/yes label Oct 24, 2018

@kacole2 kacole2 removed this from the v1.13 milestone Oct 24, 2018

@AishSundar

This comment has been minimized.

Copy link

AishSundar commented Oct 24, 2018

@derekwaynecarr this enhancement has been moved out of 1.13 due to lack of clarity on whats pending for this to land . We are officially in Enhancement freeze now. If this is a critical enhancement you need added back, it will require filing an exception with details as outlined there.

@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Nov 20, 2018

(Some automation I'm testing accidentally sent out a comment, which I've deleted to not make things confusing. Sorry!)

@derekwaynecarr

This comment has been minimized.

Copy link
Member

derekwaynecarr commented Nov 30, 2018

@svenefftinge

This comment has been minimized.

Copy link

svenefftinge commented Nov 30, 2018

Is there anything we could help with? We really need this feature in gitpod.io to give our users root privileges.

@vikaschoudhary16

This comment has been minimized.

Copy link
Member

vikaschoudhary16 commented Dec 18, 2018

@svenefftinge Please review implementation PR, kubernetes/kubernetes#64005
Hoping to get it merged in 1.14

@claurence

This comment has been minimized.

Copy link

claurence commented Jan 16, 2019

@derekwaynecarr Hello - I’m the enhancement’s lead for 1.14 and I’m checking in on this issue to see what work (if any) is being planned for the 1.14 release. Enhancements freeze is Jan 29th and I want to remind that all enhancements must have a KEP

@vikaschoudhary16

This comment has been minimized.

Copy link
Member

vikaschoudhary16 commented Jan 18, 2019

Hi @claurence -- KEP(Proposal) for this was already merged, kubernetes/community#2067. Hoping to get following implementation PR merged in 1.14:
kubernetes/kubernetes#64005

Along with updates to the original design proposal:
kubernetes/community#2595

@claurence

This comment has been minimized.

Copy link

claurence commented Jan 18, 2019

Thanks @vikaschoudhary16 - will this be implemented as alpha in 1.14 then? Based on the tagging it still has the alpha label but let me know if that is incorrect.

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