http/2(h2, h2c) and gRPC support

[mumoshu]

@brancz Hi, thanks for sharing a very inspiring project 👍

I'm POCing to add support for gRPC but I'm not really sure if it is really feasible.
I'd greatly appreciate if you could leave comments on the state of my POC.
Thanks!

https://github.com/mumoshu/kube-rbac-proxy/tree/grpc-support

In nutshell:

• It runs a gRPC proxy instead of the http(s) one when a specific flag is passed.
• In the notion of gRPC we don't have request path/resource name but do have "method name" instead.
  • Given that, my idea is that I could take a gRPC method name as a nonResourceURL in K8S' RBAC policy, so that we could authorize the method call w/ RBAC
• I'm relying on a bearer token passed via the authorization header(=metadata in gRPC?) to be used for authentication

My end-goal/intended use-case of adding a gRPC support to kube-rbac-proxy is achieve authn/authz for Helm/Tiller. I don't like to implemented a yet another RBAC system inside Helm/Tiller but rather reuse K8S RBAC instead.

helm/helm#1918 (comment)

Ideally, kube-rbac-proxy could authenticate the client and authorize the rpc call w/ RBAC. Once authorized, rbac-proxy could add the authn result to metadata to be used by tiller(upstream of kube-rbac-proxy) to "impersonate" the user. Tiller could then CRUD k8s resources as the user.

WDYT?

[brancz]

Thanks for the praise! Happy it's useful to people 🙂.

I think native gRPC support is a very cool idea. I think we need to find the right abstractions. As gRPC builds on top of HTTP2, maybe we can hop on that. A gRPC method has a specific HTTP2 path which is in the form of /ServiceName/MethodName, I would suggest that we maybe just verify access to that non-resource-url. If we wanted to, we could then add abstractions on top of that, as in a gRPCServiceRole, but not really necessary for a start I think.

Augmenting a request with obtained userdata seems reasonable, on plain HTTP that would be a REMOTE_USER header, that is added by an authentication proxy. Does an equivalent standard exist for gRPC (if not I'd suggest it to be configurable)?

[mumoshu]

@brancz Hi, thanks for your comment, and the direction!

Seems like I've managed to finish it.

Changes so far:

mumoshu/kube-rbac-proxy@master...grpc-and-h2c-support

Gotchas addressed:

• golang's reverse-proxy, http server, client impls don't support h2c(insecure variant of http/2 a.k.a http/2 over plain-tcp)
• There are various types of http servers. One could support both h2 and h2c, only h2c, http1.1+h2(c), http1.1 only.
  • For example tiller in insecure mode supports only grpc over h2c. So trying to "upgrade" a connection from helm to tiller from http1.1 to h2(c) just fails because tiller is unable to respond a HTTP Upgrade request.

Augmenting a request with obtained userdata seems reasonable, on plain HTTP that would be a REMOTE_USER header, that is added by an authentication proxy. Does an equivalent standard exist for gRPC (if not I'd suggest it to be configurable)?

Thanks for the confirmation here 👍

I've ended up with x-forwarded-user plus x-forwarded-groups for now but it seems like x-remote-user and x-remote-groups are also well-known according to my goolge search results.

Probably all I need to do towards my original goal are:

• Add a command-line flag for helm to set a bearer token to be passed to kube-rbac-proxy
• Make tiller reads x-remote-user to impersonate the user
• Change the tiller deployment so that kube-rbac-proxy is in front of tiller as a sidecar container. Make tiller inaccessible from other pods.
• Ensure only cluster-admins can kubectl-exec into tiller to protect tiller.

Sorry for a lengthy comment but things are getting more and more interesting ☺️
As my last comment, I'd appreciate any comment on this.
Thanks!

[brancz]

No worries, your comment is not too long, it covers everything is great detail, and I think this is starting to take shape! Also nice job on the code comments, they guided my through your thought process very well.

Regarding the headers, I'd suggest to go with x-remote-user and x-remote-groups as a default, but make it configurable, same with the delimiter joining the x-remote-groups header, I'd imagine | is not exactly a standard. If all those are configurable I'd say we can take it for a more thorough code review!

Thanks a lot for putting this together, I love it!

[mumoshu]

@brancz Thanks for your comment again!

Regarding the headers, I'd suggest to go with x-remote-user and x-remote-groups as a default, but make it configurable, same with the delimiter joining the x-remote-groups header, I'd imagine | is not exactly a standard. If all those are configurable I'd say we can take it for a more thorough code review!

Sounds great 👍
I'll try to address the points and will submit a PR relatively soon.

[mumoshu]

For anyone interested: The PR is in-progress at #2