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

Support forwarding client certificate to service in header #3756

Closed
NeilMadden opened this issue Nov 24, 2019 · 18 comments
Closed

Support forwarding client certificate to service in header #3756

NeilMadden opened this issue Nov 24, 2019 · 18 comments

Comments

@NeilMadden
Copy link

Feature Request

What problem are you trying to solve?

I would like to be able to use Linkerd client certificate authentication to provide increased security for service authentication within my cluster. In particular, I would like to be able to bind OAuth 2 access tokens to the service mesh client certificates as a defence in depth against access token compromise.

How should the problem be solved?

Add an option to forward the client certificate that was used in establishing the mTLS connection to the backend pod as a HTTP header - for example as a URL-encoded PEM. The header name should ideally be configurable and the linkerd proxy should strip that header from any incoming requests to prevent clients being able to spoof it.

Any alternatives you've considered?

The only other solution I'm aware of is to manually configure TLS, bypassing the service mesh.

How would users interact with this feature?

In Kubernetes use probably a new annotation - something like config.linkerd.io/forward-client-cert-header: X-Forwarded-Cert.

@NeilMadden
Copy link
Author

Full disclosure: I work for an OAuth 2 vendor (https://forgerock.com) that already implements the OAuth 2 mTLS spec, so I would like to be able recommend Linkerd to customers who want to use this. I'm also writing a book about API security that will cover both service mesh (using Linkerd) and OAuth mTLS, so I'd like to be able to demonstrate this.

@grampelberg
Copy link
Contributor

We'd love help with this if you're up to it @NeilMadden. Jump into #contributors on slack and we can chat through what it'll take and get some issues opened up.

@stale
Copy link

stale bot commented Mar 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 1, 2020
@stale stale bot closed this as completed Mar 15, 2020
Help Wanted automation moved this from To do to Done Mar 15, 2020
@jroper
Copy link
Contributor

jroper commented Apr 7, 2021

Is this really wontfix? Isn't the point of mTLS that you can ensure that only clients you know connect? If you're not ensuring that only clients you know connect, then there's no point using mTLS right? So, if you can't read a header to see if mTLS was used for a particular connection.... then what has mTLS achieved? An attacker can just make a plaintext connection, Linkerd will accept it with no problems because it doesn't currently support enforcing mTLS, and to the application, it will look identical to an mTLS connection... so what have we achieved by enabling mTLS? Nothing?

@wmorgan wmorgan added pinned and removed wontfix labels Apr 13, 2021
@wmorgan
Copy link
Member

wmorgan commented Apr 13, 2021

Thanks @jroper . It's not wontfix; that was just stalebot being overzealous.

Generally speaking, we're open to the idea of passing identity info to the application, though whether "passing the cert" is the right implementation of that idea is not clear. Is this something you're interested in working on?

@wmorgan wmorgan reopened this Apr 13, 2021
Help Wanted automation moved this from Done to In progress Apr 13, 2021
@jroper
Copy link
Contributor

jroper commented Apr 15, 2021

Of course, passing identity info is the important thing. It does appear though that there is some level of support across implementations for an x-forwarded-client-cert header, it appears that this header originates from envoy, and has been adopted from a number of platforms, I can even see some level of HTTP server support for reading this in frameworks (eg grpc). This header appears to be able to contain as much or as little of the cert that you want to put in, the most important part I think being the subject. There would be value I think in providing similar support (plus, linkerd version 1 did support this very header).

Do you have a vague idea of the amount of effort for someone unfamiliar with linkerd but very experienced with kubernetes and HTTP client/server implementations to implement this? Do you have a pointer (eg, a source file) for where to start looking? I have already floated the idea of my team implementing this to the person that prioritises resources in my team as it's a feature that will save us from having to implement our own authentication mechanism, so could be quite valuable to us, I just need to know what I'm in for if I do assign someone to do it.

@olix0r
Copy link
Member

olix0r commented Apr 15, 2021

@jroper

Isn't the point of mTLS that you can ensure that only clients you know connect? If you're not ensuring that only clients you know connect, then there's no point using mTLS right? So, if you can't read a header to see if mTLS was used for a particular connection.... then what has mTLS achieved? An attacker can just make a plaintext connection, Linkerd will accept it with no problems because it doesn't currently support enforcing mTLS, and to the application, it will look identical to an mTLS connection... so what have we achieved by enabling mTLS? Nothing?

The solution to this is, in my opinion, to provide policy controls that allow the proxy to enforce mTLS requirements. This is something we're actively working on for stable-2.11.0 (the design is still in-flight but we should have some more concrete progress/plans on that soon). With this, we could also having the proxy reliably share metadata about these connections to the application.

With regard to x-forwarded-client-cert: we're not opposed to supporting, but it would be best to clear on what problems we think it solves. If it's simply to prove that the connection was authenticated, would a proxy-implemented policy enforcement mechanism satisfy this use case? If not, it would be helpful to get clearer on concrete use cases.

I can even see some level of HTTP server support for reading this in frameworks (eg grpc)

Does this actually exist? We're open to integrating with other systems that do this sort of thing, but we're probably not likely to implement these features speculatively.

Do you have a vague idea of the amount of effort for someone unfamiliar with linkerd but very experienced with kubernetes and HTTP client/server implementations to implement this? Do you have a pointer (eg, a source file) for where to start looking?

This would have to be implemented here -- we'd have to modify the tls detection layer to include the actual certificates and not just the identities it reads; and then these would have to be wired into the HTTP server to be instrumented onto headers.

@jroper
Copy link
Contributor

jroper commented Apr 15, 2021

Agreed that service mesh level policies solves this. Sharing the metadata to the application is useful for situations where declarative policies are not enough to meet an applications needs.

If it's simply to prove that the connection was authenticated, would a proxy-implemented policy enforcement mechanism satisfy this use case? If not, it would be helpful to get clearer on concrete use cases.

An example use case we have, we have a multi-tenant serverless platform, each tenant has their own namespace. We also have a single service that offers resources to all tenants. Access to these resources needs to be authenticated and authorized, a tenant can only access resources through this service that they have permission to access. So, this service needs to know which tenant has accessed it, so it can apply its authorization policies - these policies by the way are dynamic and domain specific, they require loading data to see who owns a resource, so it's not something the service mesh can easily apply because it doesn't have that knowledge. The most convenient way to do this (and also, a very secure way) would be to look at the identity from linkerd, and extract the namespace from the service account name, to associate the call to a tenant, and then apply the policy. Without this, what we'll need to do is build an alternative authentication mechanism, eg, give each namespace its own secret that this service can verify, which feels like a lot of wasted effort given that the service mesh could give us this for free.

Another use case we have, again the same multi tenant platform, we inject our own sidecars that both proxy the traffic to our users containers, and offer certain functionality. We want to add a backend office API to these sidecars, so that our platform can perform operations. We'll have a certain service deployed to the cluster that performs these backend office calls. We only want that service to be able to invoke that API. This could be done by a service mesh level policy though, as long as that policy offers path based matching and the ability to assert that a particular service account is making the call.

Does this actually exist? We're open to integrating with other systems that do this sort of thing, but we're probably not likely to implement these features speculatively.

I raised this because in my brief search for how wide spread support for this header is, I found this:

https://www.javadoc.io/doc/com.salesforce.servicelibs/grpc-contrib/latest/com/salesforce/grpc/contrib/xfcc/XForwardedClientCert.html

It's a java class for accessing it in gRPC. Though, it's not part of the core Java gRPC library, and that's the only thing I've been able to find.

This would have to be implemented here -- we'd have to modify the tls detection layer to include the actual certificates and not just the identities it reads; and then these would have to be wired into the HTTP server to be instrumented onto headers.

If the identities are already exposed, then maybe we should start with that, and see where we go from there, eg add an l5d-identity header. Of course, just as important as adding the header is stripping the header from the incoming request, to prevent spoofing.

@olix0r
Copy link
Member

olix0r commented Apr 15, 2021

@jroper Thanks for the thoughtful context!

I'm going to add this issue to our 2.11 milestone so that we have a reminder to update it as our policy design/implementation progresses. I hope that we'll at least support surfacing the mTLS status as a part of this...

@olix0r olix0r added this to the stable-2.11.0 milestone Apr 15, 2021
jroper added a commit to jroper/linkerd2-proxy that referenced this issue Apr 20, 2021
Passes the identity as described in
linkerd/linkerd2#3756.
@jroper
Copy link
Contributor

jroper commented Apr 20, 2021

My attempt at a PR for this is here: linkerd/linkerd2-proxy#981

@NeilMadden
Copy link
Author

Awesome, thanks @jroper - now I can feel better about not getting around to opening that copy of Rust in Action sitting on my Kindle... :-)

jroper added a commit to jroper/linkerd2-proxy that referenced this issue Apr 29, 2021
Passes the identity as described in
linkerd/linkerd2#3756.

Signed-off-by: James Roper <james@jazzy.id.au>
jroper added a commit to jroper/linkerd2-proxy that referenced this issue Apr 29, 2021
Passes the identity as described in
linkerd/linkerd2#3756.

Signed-off-by: James Roper <james@jazzy.id.au>
jroper added a commit to jroper/linkerd2-proxy that referenced this issue Apr 30, 2021
Passes the identity as described in
linkerd/linkerd2#3756

Signed-off-by: James Roper <james@jazzy.id.au>
@jroper
Copy link
Contributor

jroper commented Apr 30, 2021

@NeilMadden not sure if you're interested in trying this out, but if you are, add the following annotations to your pod template spec:

config.linkerd.io/proxy-image: jamesroper/linkerd2-proxy
config.linkerd.io/proxy-version: a1e67d48-identity-headers
config.linkerd.io/proxy-await: disabled

I've tested it with the latest build of linkerd2 from main, it works for me, not sure if it'll work with other (released) versions of linkerd2, but you can try.

@jroper
Copy link
Contributor

jroper commented Apr 30, 2021

Note that what I've implemented doesn't pass the certificate, just the identity (ie, the name of the service account of the service making the call) in a header called l5d-identity.

olix0r pushed a commit to linkerd/linkerd2-proxy that referenced this issue May 12, 2021
Applications have no way to know whether a request was transported
via Linkerd's mTLS and, if it was, what the client's identity was.

This change sets the `l5d-client-id` header with the value of the client's
identity name. Furthermore, any prior values for this header are stripped
from inbound requests so that client identities may not be spoofed.

This header is set on all inbound requests over meshed connections. This
behavior may be made configurable in the future. 

Related to linkerd/linkerd2#3342 and linkerd/linkerd2#3756
@olix0r olix0r removed this from the stable-2.11.0 milestone Oct 21, 2021
@ElvinEfendi
Copy link
Contributor

Maybe somewhat tangential to this issue, but it would also be useful to be able to propagate l5d-identity through different proxy hops all the way to the destination. For example in a multi-cluster scenario A -> gateway -> B, B should arguably use A's identity when applying its policies. And as long as we have mTLS between A <-> gateway and gateway <-> B, this can be done safely.

This would be akin to how HTTP reverse proxies pass real client IP through X-Forwarded-For.

@adleong
Copy link
Member

adleong commented Dec 14, 2021

@ElvinEfendi that sounds like #7235 possibly

@OmerKahani
Copy link

Hi,
given linkerd/linkerd2-proxy#981 was merged, should this issue be closed?

@jroper
Copy link
Contributor

jroper commented May 10, 2022

Technically this issue was about forwarding the client certificate, but given forwarding just the identity solves most, if not all use cases, I think this probably can be closed.

@olix0r
Copy link
Member

olix0r commented May 10, 2022

Given the feature added by @jroper 🎉, I'm going to close this issue. We can revisit other enhancements in followup issues.

@olix0r olix0r closed this as completed May 10, 2022
Help Wanted automation moved this from In progress to Done May 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Help Wanted
  
Done
Development

No branches or pull requests

8 participants