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

[CRI] Add authentication to streaming server #36666

Open
timstclair opened this issue Nov 11, 2016 · 19 comments

Comments

@timstclair
Copy link

commented Nov 11, 2016

Address TODO here

Proposal here

/cc @kubernetes/sig-node @liggitt

@liggitt

This comment has been minimized.

Copy link
Member

commented Nov 12, 2016

cc @kubernetes/sig-auth

@yujuhong yujuhong added this to the v1.6 milestone Nov 18, 2016

@yujuhong yujuhong modified the milestones: next-candidate, v1.6 Jan 19, 2017

@yujuhong yujuhong modified the milestones: v1.7, next-candidate May 4, 2017

@liggitt

This comment has been minimized.

Copy link
Member

commented Jun 8, 2017

@timstclair can you target the milestone for this appropriately?

@liggitt liggitt added the sig/auth label Jun 8, 2017

@timstclair timstclair modified the milestones: next-candidate, v1.7, v1.8 Jun 8, 2017

@timstclair

This comment has been minimized.

Copy link
Author

commented Jun 10, 2017

Proposal

The current request flow for an exec request looks like this:

  1. apiserver --> kubelet: exec request
  2. kubelet --> CRI shim: get exec request
  3. CRI shim --> kubelet: get exec response (URL of shim server)
  4. kubelet --> apiserver: 302 redirect to URL of shim server
  5. apiserver --> CRI shim: exec
  6. pod <---> CRI shim (proxy) <---> apiserver

I propose as a medium-term auth solution for CRI exec we move the intermediate proxy server from the shim into the Kubelet. I.e. the request flow now looks like:

  1. apiserver --> kubelet: exec request
  2. kubelet --> CRI shim: get exec request
  3. CRI shim --> kubelet: get exec response with (e.g.) local named pipe unix socket
  4. kubelet proxies local socket to the apiserver connection:
    pod <---> kubelet (proxy) <---> apiserver

I don't think there is a big downside to this compared with running the proxy server in the shim process, and it means that the connection gets to use the Kubelet authentication. Eventually we still want to be able to serve from a separate process, but I don't think this blocks that (and that might be easier once we have a better pod identity story).

As for API changes, we could add a field to the GetExec response, e.g. ProxyLocal bool. Alternatively, the kubelet could just use use protocol of the response URI to decide whether to do a redirect or proxy directly (e.g. if the protocol is file:// or unix:// proxy directly, otherwise redirect).

WDYT?

/cc @yujuhong @mrunalp

@sjpotter

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2017

I'm wondering what this would mean for VM oriented pod containers. Currently able to run the server within the VM and give that as a URL, makes exec simple. which seems to be against the initial premise of the proposal

@timstclair

This comment has been minimized.

Copy link
Author

commented Jun 12, 2017

My proposal is to augment what we already have. So, if you're taking advantage of the per-pod server, you can continue using the redirect mechanism we have today. But if you're just running a single server, you might as well have the kubelet serve the proxy and handle the auth.

This means that you still need to handle auth in the per-pod server though. If we create per-pod identity certs and give them the server role, those could be used to authenticate the pod to the apiserver in the TLS handshake, but that's not coming for a while. That means the pod-server will need to do an initial certificate signing request (or is there a better way?). For authenticating the apiserver to the pod, I think we can rely on the unguessable single-use URL.

@yujuhong yujuhong referenced this issue Jun 21, 2017

Closed

CRI: support non-builtin container runtimes #47872

3 of 4 tasks complete
@timstclair

This comment has been minimized.

Copy link
Author

commented Jun 23, 2017

A problem with my proposal above is it assumes the Kubelet understands the protocol used between the CRI shim & the pod. If that's not the case, the kubelet would still need to talk to the CRI shim for the streams, which adds an extra proxy hop. This might be an acceptible temporary solution, but we should still look into what it would take to get the CRI streaming server a proper signed serving cert (can it use the Kubelets cert, and deal with rotation? If not, can it use a CSR? Does it need rotation then?)

@yujuhong

This comment has been minimized.

Copy link
Member

commented Aug 29, 2017

Moving this to 1.9.

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2017

[MILESTONENOTIFIER] Milestone Removed

@timstclair

Important: This issue was missing labels required for the v1.9 milestone for more than 3 days:

kind: Must specify exactly one of kind/bug, kind/cleanup or kind/feature.
priority: Must specify exactly one of priority/critical-urgent, priority/important-longterm or priority/important-soon.

Help
@Random-Liu

This comment has been minimized.

Copy link
Member

commented Apr 4, 2018

An update about this issue.

Here are 2 requirements we have related to this issue:

  • Requirement 1: Make crictl exec, attach, portforward work with dockershim in Q2.
    • Today crictl exec, attach, portforward doesn't work with dockershim. The reason is that dockershim streaming server reuses kubelet server. Kubelet server validates client certificate, and crictl doesn't have a usable one.
    • For other container runtimes (e.g. containerd, cri-o)
      • If we decide streaming server should do client certificate validation, they'll have the same issue.
      • If not, they are fine.
  • Requirement 2: Make the connection between apiserver and CRI container runtime streaming server secure.
    • [Is this true?] CRI container runtime streaming server doesn't necessarily need to validate client certificate, because:
      • The URL for container streaming is randomly generated by the streaming server, and is only for one time use, e.g. http://127.0.0.1:10250/cri/exec/Gct2CA0T
      • The URL is only known by local CRI caller (kubelet, crictl) and apiserver, and it is passed by kubelet to apiserver via secure connection.
    • CRI container runtime needs to provide server certificate for apiserver to know it is not talking to someone else.

We want to come with up solution for requirement 1, and make sure it aligns with requirement 2.

Here are 3 options to address requirement 1:

  • Option 1: Start another streaming server in container runtime for debug on local socket which doesn't need auth/authz, e.g. /run/dockershim.sock.
    • Add insecure or debug option in all streaming request in CRI, and container runtime returns insecure local socket when insecure=true or debug=true.
      • Kubelet should use insecure=false for production use.
      • crictl should use insecure=true for debugging.
    • Cons:
      • This becomes a CRI change only for dockershim, if we don't need client certificate validation in streaming server in the future.
      • Extra overhead from the debug streaming server. For containerd:
        • Without extra debug http server: VmRSS 28.280mb, RssAnon 5.340mb.
        • With extra debug http server: VmRSS 29.048mb, RssAnon 5.716mb.
        • The memory overhead doesn’t seem significant to me. And we can always add an option to disable the debug server.
  • Option 2: Container runtime always serves streaming request on unix domain socket.
    • Kubelet runs a streaming server to serve the real requests from apiserver; container runtime runs a local streaming server to serve local requests from kubelet. Kubelet does the redirection.
    • Pros: Smaller attack surface. Don’t need to handle auth/authz in container runtime. All requests go through kubelet, and authenticated/authorized by kubelet.
    • Cons: Extra redirection overhead in kubelet.
      • Disagreement from @tallclair:
        • These are supposed to be debug requests, not serving high-volume production requests. If the latter is desired, add a server directly to the pod/container (on the user).
        • Requests are already proxied through the apiserver, which is much more likely to be a bottleneck if scalability is an issue.
  • Option 3: Serve dockershim streaming requests over a different port, rather than through the kubelet server, and use the same auth mechanism with other CRI container runtimes.
    • Pros: No CRI changes, no changes for non-dockershim implementations
    • Cons: Still need to handle auth for all container runtimes. We can provide a self signed server cert in short term, but we need to handle server cert and even validate client cert when requirement 2 needs to be addressed.

Several discuss points to help us make the decision:

  • Do we need to validate client cert in container runtime streaming server?
    • No. Option 1 is not an option, because it will be a CRI change only for dockershim.
    • Yes. Option 1 or 2 is required to make crictl be able to talk with streaming server without cert.
  • Do we care about the streaming redirection overhead charged to kubelet?
    • No. Option 2 is the best option, because it makes auth/authz much simpler.
    • Yes. Option 2 is not an option.
  • Option 3 become the option when other options don't work.

/cc @tallclair @yujuhong @dchen1107
/cc @kubernetes/sig-auth-api-reviews @kubernetes/sig-node-api-reviews

@Random-Liu

This comment has been minimized.

Copy link
Member

commented Apr 10, 2018

To summarize the discussion in sig-node today:

  • The overhead in kubelet and resource charge back is important, but less important than security today. There is a bottleneck in apiserver anyway.
  • Option 3 is still our long term plan, but option 2 is a better solution for now, because we don't have a good auth/authz solution for option 3 today.
  • We decide to do option 2 for now, but make it possible to easily switch to option 3 in the future.

In details, we will:

  • Keep today's logic in apiserver and kubelet, and CRI unchanged, so that we can switch to option 3 easily in the future.
  • Add direct streaming support in kubelet, and feature gate it.
  • Serve container streaming on unix domain socket in container runtime.

To switch to option 3 in the future, we should firstly:

  • Have a good solution for auth/authz;
  • Remove the bottleneck in apiserver.

@yujuhong @tallclair @dchen1107 @mikebrow @feiskyer @mrunalp
/cc @kubernetes/sig-node-proposals

@feiskyer

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

+1 to option 2

Add direct streaming support in kubelet, and feature gate it.

If a feature gate is added to kubelet, should all runtimes also add such a feature gate? e.g. runtimes may need deciding to serve streaming APIs on tcp port (as today) or unix domain socket.

@mikebrow

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

Agreed let's switch to a socket for streaming.

@yujuhong yujuhong added this to the next-candidate milestone Apr 23, 2018

@Random-Liu

This comment has been minimized.

Copy link
Member

commented Apr 27, 2018

@mikebrow Volunteered to help on this. Thanks a lot! :D

@Random-Liu

This comment has been minimized.

Copy link
Member

commented May 17, 2018

Discussion for long term solution: #62747

k82cn pushed a commit to k82cn/kubernetes that referenced this issue Jun 1, 2018

Kubernetes Submit Queue
Merge pull request kubernetes#64006 from Random-Liu/streaming-auth
Automatic merge from submit-queue. 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>.

Add proxy for container streaming in kubelet for streaming auth.

For kubernetes#36666, option 2 of kubernetes#36666 (comment).

This PR:
1. Removed the `DirectStreamingRuntime`, and changed `IndirectStreamingRuntime` to `StreamingRuntime`. All `DirectStreamingRuntime`s, `dockertools` and `rkt`, were removed.
2. Proxy container streaming in kubelet instead of returning redirect to apiserver. This solves the container runtime authentication issue, which is what we agreed on in kubernetes#36666.

Please note that, this PR replaced the redirect with proxy directly instead of adding a knob to switch between the 2 behaviors. For existing CRI runtimes like containerd and cri-o, they should change to serve container streaming on localhost, so as to make the whole container streaming connection secure.

 If a general authentication mechanism proposed in kubernetes#62747 is ready, we can switch back to redirect, and all code can be found in github history.

Please also note that this added some overhead in kubelet when there are container streaming connections. However, the actual bottleneck is in the apiserver anyway, because it does proxy for all container streaming happens in the cluster. So it seems fine to get security and simplicity with this overhead. @derekwaynecarr @mrunalp Are you ok with this? Or do you prefer a knob?

@yujuhong @timstclair @dchen1107 @mikebrow @feiskyer 
/cc @kubernetes/sig-node-pr-reviews 
**Release note**:

```release-note
Kubelet now proxies container streaming between apiserver and container runtime. The connection between kubelet and apiserver is authenticated. Container runtime should change streaming server to serve on localhost, to make the connection between kubelet and container runtime local.

In this way, the whole container streaming connection is secure. To switch back to the old behavior, set `--redirect-container-streaming=true` flag.
```
@fejta-bot

This comment has been minimized.

Copy link

commented Aug 15, 2018

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

@liggitt

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

@tallclair is this still needed with the current plans for the streaming server?

@dims

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

@tallclair @mikebrow Any updates? Following up on @liggitt 's query, what is the "current plan" for the streaming server?

@tallclair

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

I should probably write this up as a KEP, but here are the bullet points for now. The solution I would like to see:

  • introduce streaming gRPC calls to the CRI API
  • streaming requests are proxied through the API server & Kubelet
    • client calls apiserver, which does an upgrade request to the kubelet (as today), kubelet calls streaming CRI functions, and proxies the output to the apiserver
  • remove the streaming redirect handling from the apiserver & kubelet
  • rate limit streaming requests everywhere
    • optional: parametrize rate limiting (with max), allowing admission controllers to implement streaming quota or more strict rate limiting
  • for more advanced cases, recommend a containerized daemon approach, possibly with a reference implementation.

This isn't currently staffed, and we don't have a roadmap.

Given these plans, we don't need authorization since we would remove this code path.

@dims

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

long-term-issue (note to self)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.